On 11 Jun 2018, at 18:21, Tiago Lam wrote:
From: Michael Qiu <qiud...@chinac.com>
When doing packet clone, if packet source is from DPDK driver,
multi-segment must be considered, and copy the segment's data one by
one.
Also, lots of DPDK mbuf's info is missed during a copy, like packet
type, ol_flags, etc. That information is very important for DPDK to
do
packets processing.
Co-authored-by: Mark Kavanagh <mark.b.kavan...@intel.com>
Signed-off-by: Michael Qiu <qiud...@chinac.com>
Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com>
---
lib/dp-packet.c | 76
+++++++++++++++++++++++++++++++++++++++++++++++--------
lib/dp-packet.h | 3 +++
lib/netdev-dpdk.c | 1 +
3 files changed, 69 insertions(+), 11 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index d0fab94..2e65b82 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -48,6 +48,23 @@ dp_packet_use__(struct dp_packet *b, void *base,
size_t allocated,
dp_packet_set_size(b, 0);
}
+#ifdef DPDK_NETDEV
+void
+dp_packet_copy_mbuf_flags(struct dp_packet *dst, const struct
dp_packet *src)
+{
+ ovs_assert(dst != NULL && src != NULL);
+ struct rte_mbuf *buf_dst = &(dst->mbuf);
+ struct rte_mbuf buf_src = src->mbuf;
+
+ buf_dst->nb_segs = buf_src.nb_segs;
+ buf_dst->ol_flags = buf_src.ol_flags;
+ buf_dst->packet_type = buf_src.packet_type;
+ buf_dst->tx_offload = buf_src.tx_offload;
+}
+#else
+#define dp_packet_copy_mbuf_flags(arg1, arg2)
+#endif
+
/* Initializes 'b' as an empty dp_packet that contains the
'allocated' bytes of
* memory starting at 'base'. 'base' should be the first byte of a
region
* obtained from malloc(). It will be freed (with free()) if 'b' is
resized or
@@ -158,6 +175,50 @@ dp_packet_clone(const struct dp_packet *buffer)
return dp_packet_clone_with_headroom(buffer, 0);
}
+#ifdef DPDK_NETDEV
+struct dp_packet *
+dp_packet_clone_with_headroom(const struct dp_packet *buffer,
+ size_t headroom) {
+ struct dp_packet *new_buffer;
+ uint32_t pkt_len = dp_packet_size(buffer);
+
+ /* copy multi-seg data */
+ if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) {
+ uint32_t offset = 0;
+ void *dst = NULL;
+ struct rte_mbuf *tmbuf = CONST_CAST(struct rte_mbuf *,
+ &(buffer->mbuf));
+
+ new_buffer = dp_packet_new_with_headroom(pkt_len, headroom);
+ dp_packet_set_size(new_buffer, pkt_len + headroom);
+ dst = dp_packet_tail(new_buffer);
+
The dst is not pointing to the first data, but to the end of the data,
as dp_packet_set_size() has been called. Why not just call dst =
dp_packet_put_uninit().
+ while (tmbuf) {
+ rte_memcpy((char *)dst + offset,
+ rte_pktmbuf_mtod(tmbuf, void *),
tmbuf->data_len);
+ offset += tmbuf->data_len;
+ tmbuf = tmbuf->next;
Why not using rte_pktmbuf_read() here with a check its not a contiguous
read?
+ }
+ } else {
+ new_buffer =
dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
+
dp_packet_size(buffer),
+ headroom);
+ }
+
+ /* Copy the following fields into the returned buffer:
l2_pad_size,
+ * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
+ memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
+ sizeof(struct dp_packet) -
+ offsetof(struct dp_packet, l2_pad_size));
+
+ dp_packet_copy_mbuf_flags(new_buffer, buffer);
+ if (dp_packet_rss_valid(new_buffer)) {
+ new_buffer->mbuf.hash.rss = buffer->mbuf.hash.rss;
+ }
+
+ return new_buffer;
+}
+#else
/* Creates and returns a new dp_packet whose data are copied from
'buffer'.
* The returned dp_packet will additionally have 'headroom' bytes of
* headroom. */
@@ -165,32 +226,25 @@ struct dp_packet *
dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t
headroom)
{
struct dp_packet *new_buffer;
+ uint32_t pkt_len = dp_packet_size(buffer);
new_buffer =
dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
-
dp_packet_size(buffer),
- headroom);
+ pkt_len, headroom);
+
/* Copy the following fields into the returned buffer:
l2_pad_size,
* l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
sizeof(struct dp_packet) -
offsetof(struct dp_packet, l2_pad_size));
-#ifdef DPDK_NETDEV
- new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
-#else
new_buffer->rss_hash_valid = buffer->rss_hash_valid;
-#endif
-
if (dp_packet_rss_valid(new_buffer)) {
-#ifdef DPDK_NETDEV
- new_buffer->mbuf.hash.rss = buffer->mbuf.hash.rss;
-#else
new_buffer->rss_hash = buffer->rss_hash;
-#endif
}
return new_buffer;
}
+#endif
/* Creates and returns a new dp_packet that initially contains a copy
of the
* 'size' bytes of data starting at 'data' with no headroom or
tailroom. */
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 4946fa3..3852756 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -124,6 +124,9 @@ void dp_packet_init_dpdk(struct dp_packet *);
void dp_packet_init(struct dp_packet *, size_t);
void dp_packet_uninit(struct dp_packet *);
+void dp_packet_copy_mbuf_flags(struct dp_packet *dst,
+ const struct dp_packet *src);
+
struct dp_packet *dp_packet_new(size_t);
struct dp_packet *dp_packet_new_with_headroom(size_t, size_t
headroom);
struct dp_packet *dp_packet_clone(const struct dp_packet *);
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index efd7c20..9b1fb9a 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2215,6 +2215,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
struct dp_packet_batch *batch)
memcpy(rte_pktmbuf_mtod(pkts[txcnt], void *),
dp_packet_data(packet), size);
dp_packet_set_size((struct dp_packet *)pkts[txcnt], size);
+ dp_packet_copy_mbuf_flags((struct dp_packet *)pkts[txcnt],
packet);
txcnt++;
}
--
2.7.4
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
