[ovs-dev,v15,05/15] dp-packet: copy data from multi-seg. DPDK mbuf
diff mbox series

Message ID 20190911080828.2087-6-michalx.obrembski@intel.com
State Superseded
Headers show
Series
  • Support multi-segment mbufs
Related show

Commit Message

Michal Obrembski Sept. 11, 2019, 8:08 a.m. UTC
From: Michael Qiu <qiudayu@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.kavanagh@intel.com>
Co-authored-by: Tiago Lam <tiago.lam@intel.com>

Signed-off-by: Michael Qiu <qiudayu@chinac.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Signed-off-by: Tiago Lam <tiago.lam@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Michal Obrembski <michalx.obrembski@intel.com>
---
 lib/dp-packet.c   | 54 ++++++++++++++++++++++++++++++++++++++++++------------
 lib/dp-packet.h   | 29 +++++++++++++++++++++++++++++
 lib/netdev-dpdk.c |  1 +
 3 files changed, 72 insertions(+), 12 deletions(-)

Comments

0-day Robot Sept. 11, 2019, 9:15 a.m. UTC | #1
Bleep bloop.  Greetings Obrembski, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Michal Obrembski <michalx.obrembski@intel.com>
Lines checked: 186, Warnings: 1, Errors: 0


build:
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/dummy.lo -MD -MP -MF lib/.deps/dummy.Tpo -c lib/dummy.c -o lib/dummy.o
depbase=`echo lib/dpctl.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/dpctl.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpctl.lo lib/dpctl.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/dpctl.lo -MD -MP -MF lib/.deps/dpctl.Tpo -c lib/dpctl.c -o lib/dpctl.o
depbase=`echo lib/dp-packet.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/dp-packet.lo -MD -MP -MF $depbase.Tpo -c -o lib/dp-packet.lo lib/dp-packet.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/dp-packet.lo -MD -MP -MF lib/.deps/dp-packet.Tpo -c lib/dp-packet.c -o lib/dp-packet.o
lib/dp-packet.c: In function 'dp_packet_clone_with_headroom':
lib/dp-packet.c:230:5: error: implicit declaration of function 'dp_packet_copy_comon_members' [-Werror=implicit-function-declaration]
     dp_packet_copy_comon_members(new_buffer, buffer);
     ^
lib/dp-packet.c:232:15: error: 'struct dp_packet' has no member named 'rss_hash_valid'
     new_buffer->rss_hash_valid = buffer->rss_hash_valid;
               ^
lib/dp-packet.c:232:40: error: 'const struct dp_packet' has no member named 'rss_hash_valid'
     new_buffer->rss_hash_valid = buffer->rss_hash_valid;
                                        ^
cc1: all warnings being treated as errors
make[2]: *** [lib/dp-packet.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Patch
diff mbox series

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index c7675fd..bc31a04 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -179,6 +179,41 @@  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 *b, size_t headroom) {
+    struct dp_packet *new_buffer;
+    uint32_t pkt_len = dp_packet_size(b);
+
+    /* Copy multi-seg data. */
+    if (b->source == DPBUF_DPDK && !rte_pktmbuf_is_contiguous(&b->mbuf)) {
+        void *dst = NULL;
+        struct rte_mbuf *mbuf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
+
+        new_buffer = dp_packet_new_with_headroom(pkt_len, headroom);
+        dst = dp_packet_data(new_buffer);
+        dp_packet_set_size(new_buffer, pkt_len);
+
+        if (!rte_pktmbuf_read(mbuf, 0, pkt_len, dst)) {
+            dp_packet_delete(new_buffer);
+            return NULL;
+        }
+    } else {
+        new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(b),
+                                                        dp_packet_size(b),
+                                                        headroom);
+    }
+
+    dp_packet_copy_common_members(new_buffer, b);
+
+    dp_packet_copy_mbuf_flags(new_buffer, b);
+    if (dp_packet_rss_valid(new_buffer)) {
+        new_buffer->mbuf.hash.rss = b->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. */
@@ -187,22 +222,16 @@  dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom)
 {
     struct dp_packet *new_buffer;
     uint32_t mark;
+    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);
-    /* 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));
+                                                    pkt_len, headroom);
 
-#ifdef DPDK_NETDEV
-    new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
-#endif
+    dp_packet_copy_comon_members(new_buffer, buffer);
 
-    if (dp_packet_rss_valid(buffer)) {
-        dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
+    new_buffer->rss_hash_valid = buffer->rss_hash_valid;
+    if (dp_packet_rss_valid(new_buffer)) {
+        new_buffer->rss_hash = buffer->rss_hash;
     }
     if (dp_packet_has_flow_mark(buffer, &mark)) {
         dp_packet_set_flow_mark(new_buffer, mark);
@@ -210,6 +239,7 @@  dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom)
 
     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 7b2c4a0..3efa9d6 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -149,6 +149,10 @@  struct dp_packet *dp_packet_clone_data_with_headroom(const void *, size_t,
                                                      size_t headroom);
 static inline void dp_packet_delete(struct dp_packet *);
 
+static inline void
+dp_packet_copy_common_members(struct dp_packet *new_b,
+                              const struct dp_packet *b);
+
 static inline void *dp_packet_at(const struct dp_packet *, size_t offset,
                                  size_t size);
 static inline void *dp_packet_at_assert(const struct dp_packet *,
@@ -159,6 +163,8 @@  dp_packet_mbuf_from_offset(const struct dp_packet *b, size_t *offset);
 void
 dp_packet_mbuf_write(struct rte_mbuf *mbuf, int16_t ofs, uint32_t len,
                      const void *data);
+static inline void
+dp_packet_copy_mbuf_flags(struct dp_packet *dst, const struct dp_packet *src);
 #endif
 static inline void *dp_packet_tail(const struct dp_packet *);
 static inline void *dp_packet_end(const struct dp_packet *);
@@ -212,6 +218,17 @@  dp_packet_delete(struct dp_packet *b)
     }
 }
 
+/* Copies the following fields into the 'new_b', which represent the common
+ * fields between DPDK and non-DPDK packets: l2_pad_size, l2_5_ofs, l3_ofs,
+ * l4_ofs, cutlen, packet_type and md. */
+static inline void
+dp_packet_copy_common_members(struct dp_packet *new_b,
+                              const struct dp_packet *b) {
+    memcpy(&new_b->l2_pad_size, &b->l2_pad_size,
+           sizeof(struct dp_packet) -
+           offsetof(struct dp_packet, l2_pad_size));
+}
+
 /* If 'b' contains at least 'offset + size' bytes of data, returns a pointer to
  * byte 'offset'.  Otherwise, returns a null pointer. For DPDK packets, this
  * means the 'offset' + 'size' must fall within the same mbuf (not necessarily
@@ -715,6 +732,18 @@  dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
     b->mbuf.buf_len = s;
 }
 
+static inline 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;
+    const struct rte_mbuf *buf_src = &src->mbuf;
+
+    buf_dst->ol_flags = buf_src->ol_flags;
+    buf_dst->packet_type = buf_src->packet_type;
+    buf_dst->tx_offload = buf_src->tx_offload;
+}
+
 /* Returns the RSS hash of the packet 'p'.  Note that the returned value is
  * correct only if 'dp_packet_rss_valid(p)' returns true */
 static inline uint32_t
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 09fd72d..6fef910 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2472,6 +2472,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++;
     }