[ovs-dev] [RFC PATCH V4 5/9] dp-packet: init specific mbuf fields to 0

[Kavanagh, Mark B]

>From: Chandran, Sugesh
>Sent: Friday, December 8, 2017 6:00 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 5/9] dp-packet: init specific mbuf fields
>to 0
>
>
>
>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 5/9] dp-packet: init specific mbuf fields to
>0
>>
>> dp_packets are created using xmalloc(); in the case of OvS-DPDK, it's
>possible
>> the the resultant mbuf portion of the dp_packet contains random data. For
>> some mbuf fields, specifically those related to multi-segment mbufs and/or
>> offload features, random values may cause unexpected behaviour, should the
>> dp_packet's contents be later copied to a DPDK mbuf. It is critical
>therefore, that
>> these fields should be initialized to 0.
>>
>> This patch ensures that the following mbuf fields are initialized to 0, on
>creation
>> of a new dp_packet:
>>    - ol_flags
>>    - nb_segs
>>    - tx_offload
>>    - packet_type
>>
>> Adapted from an idea by Michael Qiu <qiudayu at chinac.com>:
>> https://patchwork.ozlabs.org/patch/777570/
>>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>> ---
>>  lib/dp-packet.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 7ac0404..d5e68e2 100644
>> --- a/lib/dp-packet.h
>> +++ b/lib/dp-packet.h
>> @@ -624,13 +624,13 @@ dp_packet_mbuf_rss_flag_reset(struct dp_packet *p
>> OVS_UNUSED)
>>
>>  /* This initialization is needed for packets that do not come
>>   * from DPDK interfaces, when vswitchd is built with --with-dpdk.
>> - * The DPDK rte library will still otherwise manage the mbuf.
>> - * We only need to initialize the mbuf ol_flags. */
>> + * The DPDK rte library will still otherwise manage the mbuf. */
>>  static inline void
>>  dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)  {  #ifdef
>> DPDK_NETDEV
>> -    p->mbuf.ol_flags = 0;
>> +    struct rte_mbuf *mbuf = &(p->mbuf);
>> +    mbuf->ol_flags = mbuf->nb_segs = mbuf->tx_offload =
>> + mbuf->packet_type = 0;
>[Sugesh] Just to understand bit more, the nb_segs and the packet_type are set
>by the NIC on a mbuf before reporting to the OVS. Can you please provide the

That's true - but what if packets originate in the guest?

>details on what issues you are exactly facing when it not set.

On the Tx path, the NIC driver interprets the invalid values as packet offloads requested by the application, and handles the packet accordingly.

Thanks,
Mark 

>>  #endif
>>  }
>>
>> --
>> 1.9.3