Skip to content

Commit

Permalink
Fix lack of timestamps in Netmap packets
Browse files Browse the repository at this point in the history
There were two bugs which prevented Netmap kernel
timestamps being set to FastClick packets.

1. Netmap ts was being taken from nm_desc->hdr.ts.
This field is updated by netmap_user.h helper functions.
FastClick, however, access Netmap rings directly, bypassing
helper functions. So this field was not being updated to
current time.

2. In case of HAVE_NETMAP_PACKET_POOL && HAVE_BATCH, only
batch_head (the first packets of batch) was provided with ts
taken from Netmap. Rest of packets had uninitialized timestamps
(with zero values).

Also fix signed/unsigned comparison warning.
  • Loading branch information
piotrjurkiewicz authored and tbarbette committed Nov 26, 2018
1 parent 58dfc60 commit 6b10967
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 20 deletions.
28 changes: 12 additions & 16 deletions elements/userlevel/fromnetmapdevice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

CLICK_DECLS

FromNetmapDevice::FromNetmapDevice() : _device(NULL),_keephand(false)
FromNetmapDevice::FromNetmapDevice() : _device(NULL), _keephand(false)
{
#if HAVE_BATCH
in_batch_mode = BATCH_MODE_YES;
Expand Down Expand Up @@ -69,6 +69,7 @@ FromNetmapDevice::configure(Vector<String> &conf, ErrorHandler *errh)
.read("KEEPHAND",_keephand)
.complete() < 0)
return -1;

#if HAVE_NUMA
if (_use_numa) {
const char* device = ifname.c_str();
Expand Down Expand Up @@ -100,7 +101,6 @@ FromNetmapDevice::configure(Vector<String> &conf, ErrorHandler *errh)
r = configure_rx(thisnode,n_queues,n_queues,errh);
}


if (r != 0) return r;

return 0;
Expand Down Expand Up @@ -184,12 +184,14 @@ FromNetmapDevice::initialize(ErrorHandler *errh)
}

inline bool
FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask) {
FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
{
unsigned nr_pending = 0;

int sent = 0;

for (int i = begin; i <= end; i++) {

lock();

struct nm_desc* nmd = _device->nmds[i];
Expand All @@ -201,8 +203,8 @@ FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
cur = rxring->cur;

n = nm_ring_space(rxring);
if (_burst > 0 && n > (int)_burst) {
nr_pending += n - (int)_burst;
if (_burst > 0 && (int) n > _burst) {
nr_pending += n - _burst;
n = _burst;
}

Expand All @@ -211,20 +213,21 @@ FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
continue;
}

Timestamp ts = Timestamp::make_usec(nmd->hdr.ts.tv_sec, nmd->hdr.ts.tv_usec);

sent+=n;

#if HAVE_NETMAP_PACKET_POOL && HAVE_BATCH
PacketBatch *batch_head = NetmapDevice::make_netmap_batch(n, rxring, cur);
if (!batch_head) goto error;
#else

Timestamp ts = Timestamp::make_usec(rxring->ts.tv_sec, rxring->ts.tv_usec);

#if HAVE_BATCH
PacketBatch *batch_head = NULL;
Packet* last = NULL;
unsigned int count = n;
#endif

while (n > 0) {

struct netmap_slot* slot = &rxring->slot[cur];
Expand Down Expand Up @@ -261,7 +264,7 @@ FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
#endif
p->set_packet_type_anno(Packet::HOST);
p->set_mac_header(p->data());

p->set_timestamp_anno(ts);
#if HAVE_BATCH
if (batch_head == NULL) {
batch_head = PacketBatch::start_head(p);
Expand All @@ -270,7 +273,6 @@ FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
}
last = p;
#else
p->set_timestamp_anno(ts);
output(0).push(p);
#endif
cur = nm_ring_next(rxring, cur);
Expand All @@ -286,30 +288,26 @@ FromNetmapDevice::receive_packets(Task* task, int begin, int end, bool fromtask)
rxring->head = rxring->cur = cur;
unlock();
#if HAVE_BATCH
batch_head->set_timestamp_anno(ts);
output_push_batch(0,batch_head);
#endif

}

if ((int) nr_pending > _burst) { //TODO size/4 or something
if (fromtask) {
task->fast_reschedule();
} else {

task->reschedule();
}
}

add_count(sent);
return sent;

error: //No more buffer

click_chatter("No more buffers !");
router()->master()->kill_router(router());
return 0;


}

void
Expand Down Expand Up @@ -339,8 +337,6 @@ void FromNetmapDevice::add_handlers()
add_write_handler("reset_counts", reset_count_handler, 0, Handler::BUTTON);
}



CLICK_ENDDECLS
ELEMENT_REQUIRES(userlevel netmap QueueDevice)
EXPORT_ELEMENT(FromNetmapDevice)
Expand Down
2 changes: 0 additions & 2 deletions elements/userlevel/fromnetmapdevice.hh
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ CLICK_DECLS
*
*/



class FromNetmapDevice: public RXQueueDevice {

public:
Expand Down
2 changes: 1 addition & 1 deletion include/click/netmapdevice.hh
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class NetmapDevice {
#if HAVE_NETMAP_PACKET_POOL
static WritablePacket* make_netmap(unsigned char* data, struct netmap_ring* rxring, struct netmap_slot* slot);
# if HAVE_BATCH
static PacketBatch* make_netmap_batch(unsigned int n, struct netmap_ring* rxring,unsigned int &cur);
static PacketBatch* make_netmap_batch(unsigned int n, struct netmap_ring* rxring, unsigned int &cur);
# endif
#endif
static NetmapDevice* open(String ifname);
Expand Down
7 changes: 6 additions & 1 deletion lib/netmapdevice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,13 @@ WritablePacket* NetmapDevice::make_netmap(unsigned char* data, struct netmap_rin
/**
* Creates a batch of packet directly from a netmap ring.
*/
PacketBatch* NetmapDevice::make_netmap_batch(unsigned int n, struct netmap_ring* rxring,unsigned int &cur) {
PacketBatch* NetmapDevice::make_netmap_batch(unsigned int n, struct netmap_ring* rxring, unsigned int &cur) {
if (n <= 0) return NULL;
struct netmap_slot* slot;
WritablePacket* last = 0;

Timestamp ts = Timestamp::make_usec(rxring->ts.tv_sec, rxring->ts.tv_usec);

PacketPool& packet_pool = *WritablePacket::make_local_packet_pool();

WritablePacket*& _head = packet_pool.pd;
Expand Down Expand Up @@ -262,6 +264,9 @@ PacketBatch* NetmapDevice::make_netmap_batch(unsigned int n, struct netmap_ring*
}
last->set_next(next);

last->set_packet_type_anno(Packet::HOST);
last->set_mac_header(last->data());
last->set_timestamp_anno(ts);
}

_head = static_cast<WritablePacket*>(next);
Expand Down

0 comments on commit 6b10967

Please sign in to comment.