[ovs-dev] [PATCH v11 02/14] dp-packet: Init specific mbuf fields.
Stokes, Ian
ian.stokes at intel.com
Tue Oct 16 14:28:56 UTC 2018
> 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.
Thanks
Ian
> + mbuf->next = NULL;
> #endif
> }
>
> --
> 2.7.4
More information about the dev
mailing list