On 22 Jun 2018, at 21:03, Lam, Tiago wrote:
On 18/06/2018 12:23, Eelco Chaudron wrote:
On 11 Jun 2018, at 18:21, Tiago Lam wrote:
From: Mark Kavanagh <[email protected]>
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 <[email protected]>:
https://patchwork.ozlabs.org/patch/777570/
Co-authored-by: Tiago Lam <[email protected]>
Signed-off-by: Mark Kavanagh <[email protected]>
Signed-off-by: Tiago Lam <[email protected]>
---
lib/dp-packet.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 596cfe6..82e45ad 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -626,13 +626,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. */
+ * 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->tx_offload = mbuf->packet_type = 0;
+ mbuf->nb_segs = 1;
+ mbuf->next = NULL;
Are we sure this is all we need? Asking as rte_pktmbuf_reset() cleans
of
some more field, so want to make sure we hit all the corner cases.
Unfortunately, we can not use it here due to the sanity check.
In this case it doesn't seem to me that we need to replicate what DPDK
would do. Eventually, before sending this packet out (because this is
a
DPBUF_MALLOC dp_packet), the values will be copied to an mbuf properly
initialised by DPDK, and we just need to make sure we are not copying
random values as it was happening before with `ol_falgs` and
`tx_offload`. For the use of DPDK's mbuf manipulation functions, such
as
rte_pktmbuf_tailroom(), these flags are enough.
Thanks for the clarification. Guess it would also mess up the fields
just setup by dp_packet_use__() which I missed during review.
On a similar note, I don't think the comment above, "The DPDK rte
library will still otherwise manage the mbuf.", is relevant anymore
since DPDK won't manage this packet at all. I'll remove it for next
iteration.
Tiago.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
