[ovs-dev] [PATCH] netdev-dpdk: Add mbuf HEADROOM after alignment.

[Lam, Tiago]

On 27/11/2018 13:57, Stokes, Ian wrote:
>> Commit dfaf00e started using the result of dpdk_buf_size() to calculate
>> the available size on each mbuf, as opposed to using the previous
>> MBUF_SIZE macro. However, this was calculating the mbuf size by adding up
>> the MTU with RTE_PKTMBUF_HEADROOM and only then aligning to
>> NETDEV_DPDK_MBUF_ALIGN. Instead, the accounting for the
>> RTE_PKTMBUF_HEADROOM should only happen after alignment, as per below.
>>
>> Before alignment:
>> ROUNDUP(MTU(1500) + RTE_PKTMBUF_HEADROOM(128), 1024) = 2048
>>
>> After aligment:
>> ROUNDUP(MTU(1500), 1024) + 128 = 2176
>>
>> This might seem insignificant, however, it might have performance
>> implications in DPDK, where each mbuf is expected to have 2k +
>> RTE_PKTMBUF_HEADROOM of available space. This is because not only some
>> NICs have course grained alignments of 1k, they will also take
>> RTE_PKTMBUF_HEADROOM bytes from the overall available space in an mbuf
>> when setting up their Rx requirements. Thus, only the "After alignment"
>> case above would guarantee a 2k of available room, as the "Before
>> alignment" would report only 1920B.
>>
>> Some extra information can be found at:
>> https://mails.dpdk.org/archives/dev/2018-November/119219.html
>>
>> Note: This has been found by Ian Stokes while going through some af_packet
>> checks.
>>
> 
> Hi Tiago,, thanks for this, just a query below.
> 
>> Reported-by: Ian Stokes <ian.stokes at intel.com>
>> Fixes: dfaf00e ("netdev-dpdk: fix mbuf sizing")
>> Signed-off-by: Tiago Lam <tiago.lam at intel.com>
>> ---
>>  Documentation/topics/dpdk/memory.rst | 20 ++++++++++----------
>>  lib/netdev-dpdk.c                    |  6 ++++--
>>  2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/topics/dpdk/memory.rst
>> b/Documentation/topics/dpdk/memory.rst
>> index c9b739f..3c4ee17 100644
>> --- a/Documentation/topics/dpdk/memory.rst
>> +++ b/Documentation/topics/dpdk/memory.rst
>> @@ -107,8 +107,8 @@ Example 1
>>
>>   MTU = 1500 Bytes
>>   Number of mbufs = 262144
>> - Mbuf size = 2752 Bytes
>> - Memory required = 262144 * 2752 = 721 MB
>> + Mbuf size = 2880 Bytes
>> + Memory required = 262144 * 2880 = 755 MB
> 
> These measurements don't deem to take the header and trailer into account for total size when calculating the memory requirements? Is there any particular reason? 
> 
> total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
> 
> I would expect above to be 262144 * 3008 = 788 MB.

Thanks for looking into it, Ian.

I hadn't considered that.

But I guess you're right, technically, since that will end up being the
total size of each mempool element. However, the size of each mbuf is
still the 2880B above.

Looking into commit 31154f95 (before the first change to this file was
applied), I can see that the size of each mbuf would be 2944B (for a
1500B MTU), where each mempool element would have a size of 3008B (the
extra 64B is the mp->header_size above). And we are using the 3008B
instead of the 2944B as the mbuf size. Was this on purpose to reflect
the total memory required?

Just wondering though, I can add the same logic anyway, which would
change the values to 2880B + 64B = 2944B.

Tiago.