[ovs-dev] [PATCH 4/5] lib/dp-packet: copy multi-segments data from DPDK mbuf

Michael Qiu qdy220091330 at gmail.com
Tue Nov 1 03:54:33 UTC 2016



2016/10/28 18:50, Kavanagh, Mark B :
>> When doing packet clone, if packet source is from DPDK driver,
>> multi-segment must be considered, and copy the segment's
>> data one by one.
>>
>> Signed-off-by: Michael Qiu <qiudayu at chinac.com>
>> Signed-off-by: Jijiang Liu <liujijiang at chinac.com>
>> ---
>> lib/dp-packet.c | 25 ++++++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
>> index 619f651..3ca1fde 100644
>> --- a/lib/dp-packet.c
>> +++ b/lib/dp-packet.c
>> @@ -164,9 +164,28 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t
>> headroom)
>> {
>>      struct dp_packet *new_buffer;
>>
>> -    new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
>> -                                                 dp_packet_size(buffer),
>> -                                                 headroom);
>> +    uint32_t size = dp_packet_size(buffer);
>> +
>> +    /* copy multi-seg data */
>> +    #ifdef DPDK_NETDEV
> Align compiler guard to far left margin

OK, I've got your point, will fix it.

>
>> +    if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) {
>> +        struct rte_mbuf *tmbuf = &(buffer->mbuf);
> This assignment introduces a compiler warning:
> 	lib/dp-packet.c:173:34: warning: initialization discards 'const' qualifier from pointer target type [enabled by default]
> This removes the warning:
> +        struct rte_mbuf *tmbuf = CONST_CAST(struct rte_mbuf *, &(buffer->mbuf));

Thanks, you are right, and I will fix it.

>> +        new_buffer = dp_packet_new_with_headroom(size, headroom);
>> +        void *dst = dp_packet_put_uninit(new_buffer, size);
> Why do you need to call dp_packet_put_uninit? This adds an additional (and unneccesary) 'size' bytes to the tail of the dp_packet.

First, dp_packet_put_uninit() will finally be called in before, see call 
path:
dp_packet_clone() ---> dp_packet_clone_with_headroom() ---> 
dp_packet_clone_data_with_headroom() --->
         dp_packet_put() ---> dp_packet_put_uninit()

Second, dp_packet_put_uninit() does not add an additional 'size' bytes 
in this case, because dp_packet_prealloc_tailroom() will check if it has 
'size' bytes in its tailroom, and we have already allocated enought size 
here.

Third, the new allocated buffer hasn't set the pkt_len(packet size).

I use it only becasue it return the data start address here, and we also 
set the packet size here.

>
>> +        uint32_t off_set = 0;
>> +
>> +        while (tmbuf) {
>> +            rte_memcpy((char *)dst + off_set,
>> +                       rte_pktmbuf_mtod(tmbuf, void *), tmbuf->data_len);
>> +            off_set += tmbuf->data_len;
>> +            tmbuf = tmbuf->next;
>> +        }
>> +    }
>> +    else
>> +    #endif
>> +        new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
>> +                                                        size, headroom);
>> +
>>      new_buffer->l2_pad_size = buffer->l2_pad_size;
>>      new_buffer->l2_5_ofs = buffer->l2_5_ofs;
>>      new_buffer->l3_ofs = buffer->l3_ofs;
>> --
>> 1.8.3.1
>


More information about the dev mailing list