diff mbox series

[ovs-dev,v5,3/9] dp-packet: Refactor offloading API.

Message ID 20190218160025.29904-4-i.maximets@samsung.com
State Superseded
Headers show
Series dpif-netdev: Partial HWOL fixes/refactoring/unit-tests. | expand

Commit Message

Ilya Maximets Feb. 18, 2019, 4 p.m. UTC
1. No reason to have mbuf related APIs in a generic code.
2. Not only RSS/checksums should be invalidated in case of tunnel
   decapsulation or sending to 'ring' ports.

In order to fix two above issues, new function
'dp_packet_reset_offload' introduced. In order to clean up/unify
the code and simplify addition of new offloading features to non-DPDK
version of dp_packet, introduced 'ol_flags' bitmask. Additionally
reduced code complexity in 'dp_packet_clone_with_headroom' by using
already existent generic APIs.

Unfortunately, we still need to have a special case for mbuf
initialization inside 'dp_packet_init__()'.
'dp_packet_init_specific()' introduced for this purpose as a generic
API for initialization of the implementation-specific fields.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/dp-packet.c   | 15 ++++------
 lib/dp-packet.h   | 75 ++++++++++++++++++++---------------------------
 lib/netdev-dpdk.c |  6 ++--
 lib/netdev.c      |  4 +--
 4 files changed, 41 insertions(+), 59 deletions(-)

Comments

Flavio Leitner Feb. 19, 2019, 9:02 p.m. UTC | #1
On Mon, Feb 18, 2019 at 07:00:19PM +0300, Ilya Maximets wrote:
> 1. No reason to have mbuf related APIs in a generic code.
> 2. Not only RSS/checksums should be invalidated in case of tunnel
>    decapsulation or sending to 'ring' ports.
> 
> In order to fix two above issues, new function
> 'dp_packet_reset_offload' introduced. In order to clean up/unify
> the code and simplify addition of new offloading features to non-DPDK
> version of dp_packet, introduced 'ol_flags' bitmask. Additionally
> reduced code complexity in 'dp_packet_clone_with_headroom' by using
> already existent generic APIs.
> 
> Unfortunately, we still need to have a special case for mbuf
> initialization inside 'dp_packet_init__()'.
> 'dp_packet_init_specific()' introduced for this purpose as a generic
> API for initialization of the implementation-specific fields.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---

Acked-by: Flavio Leitner <fbl@sysclose.org>
diff mbox series

Patch

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 93b0e9c84..f8207ffc2 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -30,9 +30,10 @@  dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so
     b->source = source;
     dp_packet_reset_offsets(b);
     pkt_metadata_init(&b->md, 0);
-    dp_packet_rss_invalidate(b);
-    dp_packet_mbuf_init(b);
     dp_packet_reset_cutlen(b);
+    dp_packet_reset_offload(b);
+    /* Initialize implementation-specific fields of dp_packet. */
+    dp_packet_init_specific(b);
     /* By default assume the packet type to be Ethernet. */
     b->packet_type = htonl(PT_ETH);
 }
@@ -173,16 +174,10 @@  dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom)
 
 #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
+    if (dp_packet_rss_valid(buffer)) {
+        dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
     }
 
     return new_buffer;
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index c6672f6be..b34dada78 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -46,6 +46,13 @@  enum OVS_PACKED_ENUM dp_packet_source {
 
 #define DP_PACKET_CONTEXT_SIZE 64
 
+#ifndef DPDK_NETDEV
+/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
+enum dp_packet_offload_mask {
+    DP_PACKET_OL_RSS_HASH_MASK  = 0x1, /* Is the 'rss_hash' valid? */
+};
+#endif
+
 /* Buffer for holding packet data.  A dp_packet is automatically reallocated
  * as necessary if it grows too large for the available memory.
  * By default the packet type is set to Ethernet (PT_ETH).
@@ -58,8 +65,8 @@  struct dp_packet {
     uint16_t allocated_;        /* Number of bytes allocated. */
     uint16_t data_ofs;          /* First byte actually in use. */
     uint32_t size_;             /* Number of bytes in use. */
+    uint32_t ol_flags;          /* Offloading flags. */
     uint32_t rss_hash;          /* Packet hash. */
-    bool rss_hash_valid;        /* Is the 'rss_hash' valid? */
 #endif
     enum dp_packet_source source;  /* Source of memory allocated as 'base'. */
 
@@ -421,6 +428,16 @@  dp_packet_get_nd_payload(const struct dp_packet *b)
 #ifdef DPDK_NETDEV
 BUILD_ASSERT_DECL(offsetof(struct dp_packet, mbuf) == 0);
 
+static inline void
+dp_packet_init_specific(struct dp_packet *p)
+{
+    /* This initialization is needed for packets that do not come from DPDK
+     * interfaces, when vswitchd is built with --with-dpdk. */
+    p->mbuf.tx_offload = p->mbuf.packet_type = 0;
+    p->mbuf.nb_segs = 1;
+    p->mbuf.next = NULL;
+}
+
 static inline void *
 dp_packet_base(const struct dp_packet *b)
 {
@@ -501,24 +518,9 @@  dp_packet_rss_valid(const struct dp_packet *p)
 }
 
 static inline void
-dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
-{
-}
-
-static inline void
-dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
-{
-    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
-}
-
-/* This initialization is needed for packets that do not come from DPDK
- * interfaces, when vswitchd is built with --with-dpdk. */
-static inline void
-dp_packet_mbuf_init(struct dp_packet *p)
+dp_packet_reset_offload(struct dp_packet *p)
 {
-    p->mbuf.ol_flags = p->mbuf.tx_offload = p->mbuf.packet_type = 0;
-    p->mbuf.nb_segs = 1;
-    p->mbuf.next = NULL;
+    p->mbuf.ol_flags = 0;
 }
 
 static inline bool
@@ -549,13 +551,6 @@  dp_packet_l4_checksum_bad(const struct dp_packet *p)
             PKT_RX_L4_CKSUM_BAD;
 }
 
-static inline void
-reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
-{
-    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
-                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
-}
-
 static inline bool
 dp_packet_has_flow_mark(const struct dp_packet *p, uint32_t *mark)
 {
@@ -568,6 +563,13 @@  dp_packet_has_flow_mark(const struct dp_packet *p, uint32_t *mark)
 }
 
 #else /* DPDK_NETDEV */
+
+static inline void
+dp_packet_init_specific(struct dp_packet *p OVS_UNUSED)
+{
+    /* There are no implementation-specific fields for initialization. */
+}
+
 static inline void *
 dp_packet_base(const struct dp_packet *b)
 {
@@ -628,29 +630,19 @@  static inline void
 dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
 {
     p->rss_hash = hash;
-    p->rss_hash_valid = true;
+    p->ol_flags |= DP_PACKET_OL_RSS_HASH_MASK;
 }
 
 static inline bool
 dp_packet_rss_valid(const struct dp_packet *p)
 {
-    return p->rss_hash_valid;
-}
-
-static inline void
-dp_packet_rss_invalidate(struct dp_packet *p)
-{
-    p->rss_hash_valid = false;
-}
-
-static inline void
-dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED)
-{
+    return p->ol_flags & DP_PACKET_OL_RSS_HASH_MASK;
 }
 
 static inline void
-dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)
+dp_packet_reset_offload(struct dp_packet *p)
 {
+    p->ol_flags = 0;
 }
 
 static inline bool
@@ -677,11 +669,6 @@  dp_packet_l4_checksum_bad(const struct dp_packet *p OVS_UNUSED)
     return false;
 }
 
-static inline void
-reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED)
-{
-}
-
 static inline bool
 dp_packet_has_flow_mark(const struct dp_packet *p OVS_UNUSED,
                         uint32_t *mark OVS_UNUSED)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f07b10c88..4ef41bf75 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3774,11 +3774,11 @@  netdev_dpdk_ring_send(struct netdev *netdev, int qid,
     struct dp_packet *packet;
 
     /* When using 'dpdkr' and sending to a DPDK ring, we want to ensure that
-     * the rss hash field is clear. This is because the same mbuf may be
+     * the offload fields are clear. This is because the same mbuf may be
      * modified by the consumer of the ring and return into the datapath
-     * without recalculating the RSS hash. */
+     * without recalculating the RSS hash or revalidating the checksums. */
     DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
-        dp_packet_mbuf_rss_flag_reset(packet);
+        dp_packet_reset_offload(packet);
     }
 
     netdev_dpdk_send__(dev, qid, batch, concurrent_txq);
diff --git a/lib/netdev.c b/lib/netdev.c
index 45b50f26c..9d7c24c27 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -814,10 +814,10 @@  netdev_pop_header(struct netdev *netdev, struct dp_packet_batch *batch)
     DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, batch) {
         packet = netdev->netdev_class->pop_header(packet);
         if (packet) {
-            /* Reset the checksum offload flags if present, to avoid wrong
+            /* Reset the offload flags if present, to avoid wrong
              * interpretation in the further packet processing when
              * recirculated.*/
-            reset_dp_packet_checksum_ol_flags(packet);
+            dp_packet_reset_offload(packet);
             dp_packet_batch_refill(batch, packet, i);
         }
     }