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

[Chandran, Sugesh]

Regards
_Sugesh


> -----Original Message-----
> From: Kavanagh, Mark B
> Sent: Tuesday, December 12, 2017 12:13 PM
> To: Chandran, Sugesh <sugesh.chandran 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
> 
> >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.
[Sugesh] yes that's right , thank you for the clarification.

> 
> Thanks,
> Mark
> 
> >>  #endif
> >>  }
> >>
> >> --
> >> 1.9.3