[ovs-dev] [PATCH v8 07/13] dp-packet: Handle multi-seg mbufs in put_uninit().

Lam, Tiago tiago.lam at intel.com
Fri Jun 22 19:04:30 UTC 2018


On 18/06/2018 12:52, Eelco Chaudron wrote:
> 
> 
> On 11 Jun 2018, at 18:21, Tiago Lam wrote:
> 
>> The dp_packet_put_uninit() function is, in its current implementation,
>> operating on the data buffer of a dp_packet as if it were contiguous,
>> which in the case of multi-segment mbufs means they operate on the 
>> first
>> mbuf in the chain. However, when making use of multi-segment mbufs, it
>> is the data length of the last mbuf in the mbuf chain that should be
>> adjusted. This function has thus been modified to support 
>> multi-segment
>> mbufs.
>>
>> Co-authored-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>> Signed-off-by: Tiago Lam <tiago.lam at intel.com>
>> ---
>>  lib/dp-packet.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
>> index 2aaeaae..9f8503e 100644
>> --- a/lib/dp-packet.c
>> +++ b/lib/dp-packet.c
>> @@ -321,7 +321,19 @@ dp_packet_put_uninit(struct dp_packet *b, size_t 
>> size)
>>      void *p;
>>      dp_packet_prealloc_tailroom(b, size);
>>      p = dp_packet_tail(b);
>> +#ifdef DPDK_NETDEV
>> +    struct rte_mbuf *mbuf;
>> +
>> +    if (b->source == DPBUF_DPDK) {
>> +        mbuf = rte_pktmbuf_lastseg(&b->mbuf);
>> +    } else {
>> +        mbuf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
>> +    }
>> +
>> +    mbuf->data_len += size;
> 
> How can we be sure there is room in the (last) mbuf->data? The 
> dp_packet_set_size() is not allocating more room (but it's truncating).

dp_packet_prealloc_tailroom() called at the top is responsible for
arranging memory and make sure it returns with enough memory. Otherwise,
it keeps the same behavior of hard asserting.

> The mbuf->data_len should be handled in dp_packet_set_size(), and not 
> here so it will work for all these cases.
> 
This was a leftover, thanks for spotting it! I've fixed it and will
include with the next iteration.

Tiago.


More information about the dev mailing list