[ovs-dev] [PATCH v8 01/13] netdev-dpdk: fix mbuf sizing

[Lam, Tiago]

On 12/06/2018 07:51, santosh wrote:
> Hi,
> 
> 
> On Monday 11 June 2018 09:51 PM, Tiago Lam 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>
>> ---
> 
> Change looks good to me.
> Even for default 2k buffer: This patch saves one cacheline_sz(128B) area for arm64
> platform.
> 
> Acked-by: Santosh Shukla <santosh.shukla at caviumnetworks.com>
> Thanks.
> 

Thanks for your Ack, Santosh. I've sent a new iteration and kept your
Ack. There's no functional change, only a DBG log has been updated to
print the cache line size. But let me know if that's not OK with you and
I'll remove the Ack.

Tiago.