tbarbette / fastclick

FastClick - A faster version of the Click Modular Router featuring batching, advanced multi-processing and improved Netmap and DPDK support (ANCS'15). Check the metron branch for Metron specificities (NSDI'18). PacketMill modifications (ASPLOS'21) as well as MiddleClick(ToN, 2021) are merged in main.
Other
279 stars 81 forks source link

--with-netmap does not compile #353

Closed larsbro closed 2 years ago

larsbro commented 2 years ago

When configue--with-netmap: ../elements/userlevel/fromnetmapdevice.cc:189:1: error: no declaration matches ‘PacketBatch* FromNetmapDevice::pull_batch(int, unsigned int)’ FromNetmapDevice::pull_batch(int port, unsigned max) {

If HAVE_NETMAP_PACKET_POOL is not set, pull_batch() is declared as a noop in fromnetmapdevice.cc line 189 So it eems to me that --enable-netmap-pool is the only viable solution to get batch processing

--with-netmap --enable-netmap-pool ../include/click/packet.hh:3153:21: error: ‘NetmapBufQ’ has not been declared if (NetmapBufQ::is_valid_netmap_buffer(head))

In packet.hh, there is a reference to NetmapBufQ, declared in include/click/netmapdevice.hh. On one hand, one might say that packet.hh should not depend on elements, on the other hand, it might be fair enough if the user specifies "--with-netmap".

Unfortunately, netmapdevice.hh also includes packet.hh, so packet.hh cannot just include netmapdevice.hh.

I suggest that NetmapBufQ is moved to a header file of its own, eg.: netmapbufq.hh, and the interfaces: is_netmap_packet() and is_valid_netmap_packet() are changed to take pointers (char and buffer_destructor) instead pf Packet*, so that netmapbufq.hh does not depend on packet.hh

Finally, there seems to be something with NetmapDevice::make_netmap(): p->initialize() should take an arg, and WritablePacket::make_local_packet_pool() does not seem to exist (probably not to be used, since initialize_local_packet_pool() has been called previouslu by the thread itself. I suppose, local_packet_pool() is to be called instead. but that only works if HAVE_MULTITHREAD is set.

So, the mandatory config for netmap is --with-netmap --enable-netmap-pool --enable-user-multithread as I can see it.

tbarbette commented 2 years ago

Hi @larsbro ! Yes I can believe it broke through time as I'm not using Netmap much anymore, more DPDK. The continuous integration verifies only a specific set of configuration option. I'll take a look asap :)

tbarbette commented 2 years ago

Everything should be fixed in https://github.com/tbarbette/fastclick/tree/fixnetmap. The CI was actually not compiling for Netmap since I had to move to GitHub actions. I'm waiting for the CI to finish then I'll merge and we'll close this :) Thanks for reporting the problem !

tbarbette commented 2 years ago

@larsbro please try and tell me if everything is smooth now :)