From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f43.google.com (mail-wg0-f43.google.com [74.125.82.43]) by dpdk.org (Postfix) with ESMTP id 014A2C7BC for ; Wed, 29 Apr 2015 12:45:17 +0200 (CEST) Received: by wgin8 with SMTP id n8so23575855wgi.0 for ; Wed, 29 Apr 2015 03:45:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=+NKEiRdSThjCM+BHErup2gJZUjKGl6ICf67xUv+ryn0=; b=YIHJq06wH9z8p5Yj8iFfLpBGuxvtXCmGPwXYPzr0+Doj6/UIBbpI2dw0gAY3OsPiR7 FUoTGFCF8TztTunLP0DfVKHqGgHIMGU3r3PnY2UtXy0wrdcL2RpMZD9lT0UyQH7EI4q+ taX819cAcDE7TiMgdBSaszibDUbA4J8UlYPMHJPFYH1opQI2yJnxsXUtqxvSNysKfJOC OJMDkr/q8EDthciOBQHPcYoO1q54NY9eLPdhtdRWCKECHC+fSXCV8t99yyuFmf6844Y0 JoDG6Gr8a37E2+r8T8EyGRuSh45R3b504WIpvrqsBuhRc+foxcFq6u9vjj3pS4Wn4EuB C2Rg== X-Gm-Message-State: ALoCoQmGJUD+zvbQKJjDXp/g52OuUwA1vtvNgJsShT1cTLCtLVvRoYEwEMh7mMIERoUFzUQk3a69 X-Received: by 10.194.175.70 with SMTP id by6mr41648586wjc.42.1430304316841; Wed, 29 Apr 2015 03:45:16 -0700 (PDT) Received: from [10.16.0.195] (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id dl2sm20633416wib.2.2015.04.29.03.45.14 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 29 Apr 2015 03:45:16 -0700 (PDT) Message-ID: <5540B637.7000808@6wind.com> Date: Wed, 29 Apr 2015 12:45:11 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" , "dev@dpdk.org" References: <1430226150-30057-1-git-send-email-konstantin.ananyev@intel.com> <5540AA67.5050809@6wind.com> <2601191342CEEE43887BDE71AB977258214225EB@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258214225EB@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Apr 2015 10:45:17 -0000 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 >>> --- >>> 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 >> >> Regards, >> Olivier >