[ovs-dev] [PATCH v11 02/14] dp-packet: Init specific mbuf fields.

Flavio Leitner fbl at sysclose.org
Thu Nov 1 19:19:59 UTC 2018


Hi Tiago, Ian,

On Tue, Oct 16, 2018 at 02:28:56PM +0000, Stokes, Ian wrote:
> > From: Mark Kavanagh <mark.b.kavanagh at intel.com>
> > 
> > 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
> > appropriate values on creation of a new dp_packet:
> >    - ol_flags=0
> >    - nb_segs=1
> >    - tx_offload=0
> >    - packet_type=0
> >    - next=NULL
> > 
> > Adapted from an idea by Michael Qiu <qiudayu at chinac.com>:
> > https://patchwork.ozlabs.org/patch/777570/
> > 
> > Co-authored-by: Tiago Lam <tiago.lam at intel.com>
> > 
> > Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> > Signed-off-by: Tiago Lam <tiago.lam at intel.com>
> > Acked-by: Eelco Chaudron <echaudro at redhat.com>
> > Acked-by: Flavio Leitner <fbl at sysclose.org>
> 
> Thanks Tiago,
> 
> This will need a rebase. One minor comment below.
> > ---
> >  lib/dp-packet.h | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/dp-packet.h b/lib/dp-packet.h index ba91e58..b948fe1
> > 100644
> > --- a/lib/dp-packet.h
> > +++ b/lib/dp-packet.h
> > @@ -625,14 +625,15 @@ 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. */
> > + * from DPDK interfaces, when vswitchd is built with --with-dpdk. */
> >  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->tx_offload = mbuf->packet_type = 0;
> > +    mbuf->nb_segs = 1;
> 
> Is it just for ease of access that you cast a pointer to mbuf?
> 
> Just looking at the other rte specific functions implemented in the
> file, they access mbuf via p->mbuf rather than the cast. I would
> like to keep implementation uniform across functions if possible.

This seems to be needed but not related to the multi seg, so I wonder
if this patch could be posted as a standalone fix.  The goal is to
apply it sooner and reduce the patchset size.

What do you think?

Thanks,
fbl


> 
> Thanks
> Ian
> 
> > +    mbuf->next = NULL;
> >  #endif
> >  }
> > 
> > --
> > 2.7.4



More information about the dev mailing list