[ovs-dev] [PATCH v11 01/14] netdev-dpdk: fix mbuf sizing

[Lam, Tiago]

On 16/10/2018 14:46, Stokes, Ian wrote:
>> From: Mark Kavanagh <mark.b.kavanagh at intel.com>
>>
>> There are numerous factors that must be considered when calculating the
>> size of an mbuf:
>> - the data portion of the mbuf must be sized in accordance With Rx
>>   buffer alignment (typically 1024B). So, for example, in order to
>>   successfully receive and capture a 1500B packet, mbufs with a
>>   data portion of size 2048B must be used.
>> - in OvS, the elements that comprise an mbuf are:
>>   * the dp packet, which includes a struct rte mbuf (704B)
>>   * RTE_PKTMBUF_HEADROOM (128B)
>>   * packet data (aligned to 1k, as previously described)
>>   * RTE_PKTMBUF_TAILROOM (typically 0)
>>
>> Some PMDs require that the total mbuf size (i.e. the total sum of all of
>> the above-listed components' lengths) is cache-aligned. To satisfy this
>> requirement, it may be necessary to round up the total mbuf size with
>> respect to cacheline size. In doing so, it's possible that the dp_packet's
>> data portion is inadvertently increased in size, such that it no longer
>> adheres to Rx buffer alignment. Consequently, the following property of
>> the mbuf no longer holds true:
>>
>>     mbuf.data_len == mbuf.buf_len - mbuf.data_off
>>
>> This creates a problem in the case of multi-segment mbufs, where that
>> assumption is assumed to be true for all but the final segment in an mbuf
>> chain. Resolve this issue by adjusting the size of the mbuf's private data
>> portion, as opposed to the packet data portion when aligning mbuf size to
>> cachelines.
>>
>> Fixes: 4be4d22 ("netdev-dpdk: clean up mbuf initialization")
>> Fixes: 31b88c9 ("netdev-dpdk: round up mbuf_size to cache_line_size")
>> CC: Santosh Shukla <santosh.shukla at caviumnetworks.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>> Acked-by: Santosh Shukla <santosh.shukla at caviumnetworks.com>
>> Acked-by: Eelco Chaudron <echaudro at redhat.com>
> 
> Thanks for this Tiago, I think this would make sense to apply independently of the multi seg series. Thoughts? It could also be backported to 2.10 and 2.9 at least.
> 
> One other issue is that I'd like to see the documentation regarding the memory model examples updated as part of this patch.
> 
> For example prior to this patch the memory requested for an MTU of 1500 would be 3008 Bytes per mbuf (Header size + element size + trailer size). After this patch however it would be 2880. You could be updating the examples in a later path in the series but I would prefer to keep the docs in sync with the code changes in the same patch.

Thanks for looking into this, Ian.

This makes sense to me as there's nothing in these patches specific to
multi-segment mbufs. Just some cleanups and fixes.

I'll add the documentation fixes and your comments to 02/14 as part of
the next iteration.

Tiago.