Hi Tiago, Ian, On Tue, Oct 16, 2018 at 02:28:56PM +0000, Stokes, Ian wrote: > > From: Mark Kavanagh <mark.b.kavan...@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 <qiud...@chinac.com>: > > https://patchwork.ozlabs.org/patch/777570/ > > > > Co-authored-by: Tiago Lam <tiago....@intel.com> > > > > Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com> > > Signed-off-by: Tiago Lam <tiago....@intel.com> > > Acked-by: Eelco Chaudron <echau...@redhat.com> > > Acked-by: Flavio Leitner <f...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev