[dpdk-dev] [PATCH] test-pmd fix default mbuf size

[dpdk-dev] [PATCH] test-pmd fix default mbuf size

[Konstantin Ananyev]

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
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] test-pmd fix default mbuf size

[Olivier MATZ]

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

Re: [dpdk-dev] [PATCH] test-pmd fix default mbuf size

[Ananyev, Konstantin]

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

Re: [dpdk-dev] [PATCH] test-pmd fix default mbuf size

[Olivier MATZ]

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
>