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

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


On 18/06/2018 14:06, Eelco Chaudron wrote:
> 
> 
> On 11 Jun 2018, at 18:21, Tiago Lam wrote:
> 
>> When enabled with DPDK OvS relies on mbufs allocated by mempools to
>> receive and output data on DPDK ports. Until now, each OvS dp_packet 
>> has
>> had only one mbuf associated, which is allocated with the maximum
>> possible size, taking the MTU into account. This approach, however,
>> doesn't allow us to increase the allocated size in an mbuf, if needed,
>> since an mbuf is allocated and initialised upon mempool creation. 
>> Thus,
>> in the current implementatin this is dealt with by calling
>> OVS_NOT_REACHED() and terminating OvS.
>>
>> To avoid this, and allow the (already) allocated space to be better
>> used, dp_packet_resize__() now tries to use the available room, both 
>> the
>> tailroom and the headroom, to make enough space for the new data. 
>> Since
>> this happens for packets of source DPBUF_DPDK, the single-segment mbuf
>> case mentioned above is also covered by this new aproach in 
>> resize__().
>>
>> Signed-off-by: Tiago Lam <tiago.lam at intel.com>
>> ---
>>  lib/dp-packet.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
>> index 399fadb..d0fab94 100644
>> --- a/lib/dp-packet.c
>> +++ b/lib/dp-packet.c
>> @@ -237,9 +237,51 @@ dp_packet_resize__(struct dp_packet *b, size_t 
>> new_headroom, size_t new_tailroom
>>      new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;
>>
>>      switch (b->source) {
>> +    /* When resizing mbufs, both a single mbuf and multi-segment 
>> mbufs (where
>> +     * data is not contigously held in memory), both the headroom and 
>> the
>> +     * tailroom available will be used to make more space for where 
>> data needs
>> +     * to be inserted. I.e if there's not enough headroom, data may 
>> be shifted
>> +     * right if there's enough tailroom.
>> +     * However, this is not bulletproof and in some cases the space 
>> available
>> +     * won't be enough - in those cases, an error should be returned 
>> and the
>> +     * packet dropped. */
>>      case DPBUF_DPDK:
>> -        OVS_NOT_REACHED();
>> +    {
>> +        size_t miss_len;
>> +
>> +        if (new_headroom == dp_packet_headroom(b)) {
>> +            /* This is a tailroom adjustment. Since there's no 
>> tailroom space
>> +             * left, try and shift data towards the head to free up 
>> tail space,
>> +             * if there's enough headroom */
>> +
>> +            miss_len = new_tailroom - dp_packet_tailroom(b);
>> +
>> +            if (miss_len <= new_headroom) {
>> +                dp_packet_shift(b, -miss_len);
>> +            } else {
>> +                /* XXX: Handle error case and report error to caller 
>> */
>> +                OVS_NOT_REACHED();
> Should we add another fragment here, asking as dp_packet_set_size() can 
> free buffers?

I think I've covered this in my reply to the cover letter and to patch
06/13, we can continue the discussion there.

>> +            }
>> +        } else {
> 
> Can it also be possible that we need to adjust both tail and head room?

My understanding is that dp_packet_resize__() should be called
internally only, by either dp_packet_prealloc_tailroom() or
dp_packet_prealloc_headroom(), thus it should only change one of those
at a time. The case that handles `DPBUF_MALLOC` makes the same assumption.

Tiago.


More information about the dev mailing list