Hi Konstantin, On 04/29/2015 12:39 PM, Ananyev, Konstantin wrote: > Hi Olivier, > >> -----Original Message----- >> From: Olivier MATZ [mailto:olivier.matz at 6wind.com] >> Sent: Wednesday, April 29, 2015 10:55 AM >> To: Ananyev, Konstantin; dev at dpdk.org >> Subject: Re: [dpdk-dev] [PATCH] test-pmd fix default mbuf size >> >> Hi Konstantin, >> >> On 04/28/2015 03:02 PM, Konstantin Ananyev wrote: >>> Latest mbuf changes (priv_size addition and related fixes) >>> exposed small problem with testpmd default config: >>> testpmd default mbuf size is exaclty 2KB, that causes >>> ixgbe PMD to select scattered RX even for configs with 'normal' >>> max packet length (max_rx_pkt_len == ETHER_MAX_LEN). >>> >>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev at intel.com> >>> --- >>> app/test-pmd/testpmd.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>> index 389fc24..037e7ec 100644 >>> --- a/app/test-pmd/testpmd.h >>> +++ b/app/test-pmd/testpmd.h >>> @@ -48,7 +48,8 @@ >>> * Default size of the mbuf data buffer to receive standard 1518-byte >>> * Ethernet frames in a mono-segment memory buffer. >>> */ >>> -#define DEFAULT_MBUF_DATA_SIZE 2048 /**< Default size of mbuf data buffer. >>> */ >>> +#define DEFAULT_MBUF_DATA_SIZE (2048 + RTE_PKTMBUF_HEADROOM) >>> +/**< Default size of mbuf data buffer. */ >>> >>> /* >>> * The maximum number of segments per packet is used when creating >>> >> >> Indeed, this regression is introduced by one of my recent >> patch: >> http://dpdk.org/browse/dpdk/commit/?id=dfb03bbe2b39156f0e42e7f29e09c1e6b6def265 >> >> Before, m->buf_len was initialized to RTE_PKTMBUF_HEADROOM + 2048. >> Now it is set to 2048. >> >> Maybe a line like this should be added in the commit log: >> Fixes: dfb03bbe2b ("app/testpmd: use standard functions to initialize >> mbufs and mbuf pool") >> >> Just one question Konstantin: could you just confirm that the >> reason of the bug? From what I understand: >> - buf_len is 2048 >> - the rx data size when receiving a packet is 2048 - hdroom = 1920 >> - at init, the ixgbe driver configures the hw to set the max rx >> data size, but it has to be a power of 2, so 1024 is chosen > > At ixgbe_dev_rx_init(), we need to setup SRRCTL[rqx_id]. BSIZEPACKET value: > > "BSIZEPACKET 4:0 0x2 Receive Buffer Size for Packet Buffer. > The value is in 1 KB resolution. Value can be from 1 KB to 16 KB. Default > buffer size is > 2 KB. This field should not be set to 0x0. This field should be greater or > equal to 0x2 > in queues where RSC is enabled." > > As it is it in 1KB units, our 1920 B are rounded down to 1K, and we have to > enable scatter RX mode. > > As I understand, same story for igb devices. > I40e seems doesn't have such limitation. > >> - then the initialization code check if a packet of ETHER_MAX_LEN >> fits in max rx size, and if not, it selects the scatter mode. >> >> It makes me wondering 2 things: >> - should we add a comment in the test-pmd to explain that? (maybe >> not, as it is driver-specific, and it is just an optimization) > > Might be, or probably somewhere to the docs? > >> - should we check the other examples to see if the same problem >> exists? > > Good point. > I did a quick check, and yes it seems few other sample apps are also affected: > > examples/qos_sched/main.h:#define MBUF_DATA_SIZE (1528 + RTE_PKTMBUF_HEADROOM) > examples/skeleton/basicfwd.c:#define MBUF_DATA_SIZE (1600 + > RTE_PKTMBUF_HEADROOM) > examples/packet_ordering/main.c:#define MBUF_DATA_SIZE (1600 + > RTE_PKTMBUF_HEADROOM) > examples/rxtx_callbacks/main.c:#define MBUF_DATA_SIZE (1600 + > RTE_PKTMBUF_HEADROOM) > > I suppose, have to make v2 to include all of the above... > What probably is more beneficial - inside rte_mbuf.h: > > #define RTE_MBUF_DEFAULT_DATA_SIZE (2048 + RTE_PKTMBUF_HEADROOM) > > With some good comments for it, and make all appropriate examples to use it. > Again, then it would appear in the API reference automatically. > Konstantin
Yes, good idea, it would solve the doc issue and it factorizes the default mbuf data size somewhere. Regards, Olivier > >> >> If my understanding is correct, >> Acked-by: Olivier Matz <olivier.matz at 6wind.com> >> >> Regards, >> Olivier >
