Re: [ovs-dev] [PATCH] netdev-offload-tc: Reserve lower tc prios for ip ethertypes

[Paul Blakey via dev]

On 26/10/2022 17:13, Ilya Maximets wrote:

On 10/25/22 11:59, Roi Dayan wrote:

From: Paul Blakey <pa...@nvidia.com>

Currently ethertype to prio hmap is static and the first ethertype
being used gets a lower priority. Usually there is an arp request
before the ip traffic and the arp ethertype gets a lower tc priority
while the ip traffic proto gets a higher priority.
In this case ip traffic will go through more hops in tc and HW.
Instead, reserve lower priorities for ip ethertypes.

Signed-off-by: Paul Blakey <pa...@nvidia.com>
Reviewed-by: Roi Dayan <r...@nvidia.com>
---
  lib/netdev-offload-tc.c | 31 +++++++++++++++++++++++++------
  lib/tc.h                |  2 ++
  2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index f6f90a741fde..5b3f29d04c2a 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -325,6 +325,26 @@ struct prio_map_data {
      uint16_t prio;
  };

+static uint16_t

+get_next_available_prio(ovs_be16 protocol)
+{
+    static uint16_t last_prio = TC_RESERVED_PRIORITY_MAX;
+
+    if (protocol == htons(ETH_P_IP)) {
+        return TC_RESERVED_PRIORITY_IPV4;
+    } else if (protocol == htons(ETH_P_IPV6)) {
+        return TC_RESERVED_PRIORITY_IPV6;
+    }

Wouldn't this cause problems if multi_mask_per_prio is not supported?
i.e. we should not use the same prio for all IPv4 if they have different
masks in this case.

Or am I missing something?

Best regards, Ilya Maximets.

Good catch will fix.

+
+    /* last_prio can overflow if there will be many different kinds of
+     * flows which shouldn't happen organically. */
+    if (last_prio == UINT16_MAX) {
+        return 0;
+    }
+
+    return ++last_prio;
+}
+
  /* Get free prio for tc flower
   * If prio is already allocated for mask/eth_type combination then return it.
   * If not assign new prio.
@@ -336,11 +356,11 @@ get_prio_for_tc_flower(struct tc_flower *flower)
  {
      static struct hmap prios = HMAP_INITIALIZER(&prios);
      static struct ovs_mutex prios_lock = OVS_MUTEX_INITIALIZER;
-    static uint16_t last_prio = TC_RESERVED_PRIORITY_MAX;
      size_t key_len = sizeof(struct tc_flower_key);
      size_t hash = hash_int((OVS_FORCE uint32_t) flower->key.eth_type, 0);
      struct prio_map_data *data;
      struct prio_map_data *new_data;
+    uint16_t prio;

if (!multi_mask_per_prio) {

          hash = hash_bytes(&flower->mask, key_len, hash);
@@ -359,21 +379,20 @@ get_prio_for_tc_flower(struct tc_flower *flower)
          }
      }

- if (last_prio == UINT16_MAX) {

-        /* last_prio can overflow if there will be many different kinds of
-         * flows which shouldn't happen organically. */
+    prio = get_next_available_prio(flower->key.eth_type);
+    if (!prio) {
          ovs_mutex_unlock(&prios_lock);
          return 0;
      }

new_data = xzalloc(sizeof *new_data);

      memcpy(&new_data->mask, &flower->mask, key_len);
-    new_data->prio = ++last_prio;
+    new_data->prio = prio;
      new_data->protocol = flower->key.eth_type;
      hmap_insert(&prios, &new_data->node, hash);
      ovs_mutex_unlock(&prios_lock);

- return new_data->prio;

+    return prio;
  }

static uint32_t

diff --git a/lib/tc.h b/lib/tc.h
index 2e64ad372592..12753c16d405 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -49,6 +49,8 @@
  enum tc_flower_reserved_prio {
      TC_RESERVED_PRIORITY_NONE,
      TC_RESERVED_PRIORITY_POLICE,
+    TC_RESERVED_PRIORITY_IPV4,
+    TC_RESERVED_PRIORITY_IPV6,
      __TC_RESERVED_PRIORITY_MAX
  };
  #define TC_RESERVED_PRIORITY_MAX (__TC_RESERVED_PRIORITY_MAX -1)

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev