[ovs-dev] [RFC PATCH V4 2/9] dp-packet: fix reset_packet for multi-seg mbufs

[Kavanagh, Mark B]

>From: Chandran, Sugesh
>Sent: Friday, December 8, 2017 5:55 PM
>To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; dev at openvswitch.org;
>qiudayu at chinac.com
>Cc: Stokes, Ian <ian.stokes at intel.com>; Loftus, Ciara
><ciara.loftus at intel.com>; santosh.shukla at caviumnetworks.com
>Subject: RE: [ovs-dev][RFC PATCH V4 2/9] dp-packet: fix reset_packet for
>multi-seg mbufs
>
>
>
>Regards
>_Sugesh
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Friday, December 8, 2017 12:02 PM
>> To: dev at openvswitch.org; qiudayu at chinac.com
>> Cc: Stokes, Ian <ian.stokes at intel.com>; Loftus, Ciara
><ciara.loftus at intel.com>;
>> santosh.shukla at caviumnetworks.com; Chandran, Sugesh
>> <sugesh.chandran at intel.com>; Kavanagh, Mark B
>> <mark.b.kavanagh at intel.com>
>> Subject: [ovs-dev][RFC PATCH V4 2/9] dp-packet: fix reset_packet for multi-
>seg
>> mbufs
>>
>> When adjusting the size of a dp_packet, dp_packet_set_data() should be
>invoked
>> before dp_packet_set_size(),since for DPDK multi-segment mbufs, the former
>> will use the segments's data_off and buf_len to derive the frame size that
>should
>> be set (this behaviour is introduced in a subsequent commit).
>>
>> Currently, in dp_packet_reset_packet(), that order is reversed.
>> Swap the order of same to resolve.
>>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>> ---
>>  lib/dp-packet.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index b4b721c..47502ad 100644
>> --- a/lib/dp-packet.h
>> +++ b/lib/dp-packet.h
>> @@ -569,8 +569,8 @@ dp_packet_set_data(struct dp_packet *b, void *data)
>> static inline void  dp_packet_reset_packet(struct dp_packet *b, int off)  {
>> -    dp_packet_set_size(b, dp_packet_size(b) - off);
>>      dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
>> +    dp_packet_set_size(b, dp_packet_size(b) - off);
>[Sugesh] With the subsequent commit changes, this going to be bit difficult to
>make sure the packet size is set properly.
>Is is possible to keep these two function to be indepedant as before?
>I can see many places these functions are get called independently. So in the
>packet processing pipeline, its likely that
> set_size function get invoked without set_data that may cause the wrong
>segment size. It would be great if we can mandate in the code that the offsets
>are
>set correctly before setting the packet size each time? What do you think? Do
>you have any suggestion on this? If there are no way, atleast this has to be
>commented in the code.

That's a fair point Sugesh; I'll take a look and see what I can do in the next version.

Thanks,
Mark

>>      dp_packet_reset_offsets(b);
>>  }
>>
>> --
>> 1.9.3