Jumbo Frames in DPDK devices about pcapplusplus HOT 7 CLOSED

I think it's possible to support a configurable packet size and have a default of RTE_MBUF_DEFAULT_BUF_SIZE.

Please note we probably also need to change this:

Would you consider opening a PR with this change?

from pcapplusplus.

As seen here, both MBUF_DATA_SIZE_DEFINE and rte_pktmbuf_pool_create depend on RTE_MBUF_DEFAULT_DATAROOM. Do we modify the values in MBUF_DATA_SIZE_DEFINE and rte_pktmbuf_pool_create when enabling Jumbo Frame using macros? For example, like this.

MBufRawPacket.cpp

#ifndef MBUF_DATA_SIZE_DEFINE                                                                                                                                                        
#ifdef JUMBO_FRAME_SIZE_DEFINE                                                                                                                                                       
#   define MBUF_DATA_SIZE_DEFINE (JUMBO_FRAME_SIZE_DEFINE + RTE_PKTMBUF_HEADROOM)                                                                                                    
#else                                                                                                                                                                                
#   define MBUF_DATA_SIZE_DEFINE RTE_MBUF_DEFAULT_DATAROOM                                                                                                                           
#endif                                                                                                                                                                               
#endi

const int MBufRawPacket::MBUF_DATA_SIZE = MBUF_DATA_SIZE_DEFINE;

DpdkDevice.cpp and KniDevice.cpp

#ifdef JUMBO_FRAME_SIZE_DEFINE
#   define MBUF_DATA_SIZE_DEFINE (JUMBO_FRAME_SIZE_DEFINE + RTE_PKTMBUF_HEADROOM)
#else
#   define MBUF_DATA_SIZE_DEFINE RTE_MBUF_DEFAULT_DATAROOM
#endif
    // create mbuf pool                                                                                                                                                              
    memPool = rte_pktmbuf_pool_create(mempoolName, mBufPoolSize, MEMPOOL_CACHE_SIZE, 0, MBUF_DATA_SIZE_DEFINE, rte_socket_id());  

Such modifications appear quite messy. Perhaps we could redefine MBUF_DATA_SIZE_DEFINE in one place and then use it directly in other places.

from pcapplusplus.

If only three places, it should be fine. Personally, I consider it is more appropriate to define the value within the header, yet I'm struggling to identify the suitable header for this purpose.

from pcapplusplus.

If only three places, it should be fine. Personally, I consider it is more appropriate to define the value within the header, yet I'm struggling to identify the suitable header for this purpose.

The beginning of a bad smell indeed. Yes, we should define it in a header file.

from pcapplusplus.

I think that instead of a const value, we can add it to the DpdkDevice constructor as an optional argument (with a default size of RTE_MBUF_DEFAULT_BUF_SIZE) so users can set whatever value they want.

MBufRawPacket::init() takes a DpdkDevice as an argument so we can extract this value from there.

Regarding KniDevice - I wouldn't change anything unless you really need it. KNI has been deprecated in DPDK and we'll soon remove it from PcapPlusPlus code-base

from pcapplusplus.

I've noticed that we try to minimize the inclusion of DPDK headers in our header files. If we modify the function signature in the header file to add a default value for the function parameter, RTE_MBUF_DEFAULT_BUF_SIZE, it will inevitably require including DPDK headers. I'm not sure if this is the right approach.

from pcapplusplus.

I've noticed that we try to minimize the inclusion of DPDK headers in our header files. If we modify the function signature in the header file to add a default value for the function parameter, RTE_MBUF_DEFAULT_BUF_SIZE, it will inevitably require including DPDK headers. I'm not sure if this is the right approach.

Yes, you're right. Maybe the default value can be some magic number (like -1 or something else) which will translate to RTE_MBUF_DEFAULT_BUF_SIZE. We can mention it in the doxygen documentation

from pcapplusplus.