Message ID | 1430226150-30057-1-git-send-email-konstantin.ananyev@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 952ECC5D2; Tue, 28 Apr 2015 15:02:37 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 1D45AC5D0 for <dev@dpdk.org>; Tue, 28 Apr 2015 15:02:35 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 28 Apr 2015 06:02:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,663,1422950400"; d="scan'208";a="686844974" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by orsmga001.jf.intel.com with ESMTP; 28 Apr 2015 06:02:33 -0700 Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com [10.237.217.46]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id t3SD2Wl8005621; Tue, 28 Apr 2015 14:02:32 +0100 Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1]) by sivswdev02.ir.intel.com with ESMTP id t3SD2WI0030762; Tue, 28 Apr 2015 14:02:32 +0100 Received: (from kananye1@localhost) by sivswdev02.ir.intel.com with id t3SD2WqI030758; Tue, 28 Apr 2015 14:02:32 +0100 From: Konstantin Ananyev <konstantin.ananyev@intel.com> To: dev@dpdk.org Date: Tue, 28 Apr 2015 14:02:30 +0100 Message-Id: <1430226150-30057-1-git-send-email-konstantin.ananyev@intel.com> X-Mailer: git-send-email 1.7.4.1 Subject: [dpdk-dev] [PATCH] test-pmd fix default mbuf size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Ananyev, Konstantin
April 28, 2015, 1:02 p.m. UTC
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@intel.com>
---
app/test-pmd/testpmd.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
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@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 - 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) - should we check the other examples to see if the same problem exists? If my understanding is correct, Acked-by: Olivier Matz <olivier.matz@6wind.com> Regards, Olivier
Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Wednesday, April 29, 2015 10:55 AM > To: Ananyev, Konstantin; dev@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@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 > > If my understanding is correct, > Acked-by: Olivier Matz <olivier.matz@6wind.com> > > Regards, > Olivier
Hi Konstantin, On 04/29/2015 12:39 PM, Ananyev, Konstantin wrote: > Hi Olivier, > >> -----Original Message----- >> From: Olivier MATZ [mailto:olivier.matz@6wind.com] >> Sent: Wednesday, April 29, 2015 10:55 AM >> To: Ananyev, Konstantin; dev@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@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@6wind.com> >> >> Regards, >> Olivier >
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