diff mbox series

[ovs-dev,v11,02/10] odp-execute: Add function pointer for pop_vlan action.

Message ID 20220714175158.3709150-3-emma.finn@intel.com
State Superseded
Headers show
Series Actions Infrastructure + Optimizations | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Emma Finn July 14, 2022, 5:51 p.m. UTC
This commit removes the pop_vlan action from the large switch
and creates a separate function for batched processing. A function
pointer is also added to call the new batched function for the pop_vlan
action.

Signed-off-by: Emma Finn <emma.finn@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/odp-execute-private.c | 16 +++++++++++++++-
 lib/odp-execute-private.h |  4 ++++
 lib/odp-execute.c         | 31 +++++++++++++++++++++++++------
 3 files changed, 44 insertions(+), 7 deletions(-)

Comments

Eelco Chaudron July 15, 2022, 8:06 a.m. UTC | #1
On 14 Jul 2022, at 19:51, Emma Finn wrote:

> This commit removes the pop_vlan action from the large switch
> and creates a separate function for batched processing. A function
> pointer is also added to call the new batched function for the pop_vlan
> action.
>
> Signed-off-by: Emma Finn <emma.finn@intel.com>
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
> ---

Thanks for fixing all the comments!

Acked-by: Eelco Chaudron <echaudro@redhat.com>

//Eelco
diff mbox series

Patch

diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
index ba4aee09b..47cc1b4bc 100644
--- a/lib/odp-execute-private.c
+++ b/lib/odp-execute-private.c
@@ -33,7 +33,7 @@  static struct odp_execute_action_impl action_impls[] = {
     [ACTION_IMPL_SCALAR] = {
         .available = false,
         .name = "scalar",
-        .init_func = NULL,
+        .init_func = odp_action_scalar_init,
     },
 };
 
@@ -87,5 +87,19 @@  odp_execute_action_init(void)
 
         VLOG_INFO("Action implementation %s (available: %s)",
                   action_impls[i].name, avail ? "Yes" : "No");
+
+        /* The following is a run-time check to make sure a scalar
+         * implementation exists for the given ISA implementation. This is to
+         * make sure the autovalidator works as expected. */
+        if (avail && i != ACTION_IMPL_SCALAR) {
+            for (int j = 0; j < __OVS_ACTION_ATTR_MAX; j++) {
+                /* No ovs_assert(), as it can be compiled out. */
+                if (action_impls[ACTION_IMPL_SCALAR].funcs[j] == NULL
+                    && action_impls[i].funcs[j] != NULL) {
+                    ovs_assert_failure(OVS_SOURCE_LOCATOR, __func__,
+                                       "Missing scalar action function!");
+                }
+            }
+        }
     }
 }
diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
index 25a003e3d..f890e0cf1 100644
--- a/lib/odp-execute-private.h
+++ b/lib/odp-execute-private.h
@@ -66,6 +66,10 @@  BUILD_ASSERT_DECL(ACTION_IMPL_SCALAR == 0);
  * initializing the scalar/generic codepath. */
 void odp_execute_action_init(void);
 
+/* Init functions for the action implementations. Initializes the function
+ * pointers for optimized action types. */
+int odp_action_scalar_init(struct odp_execute_action_impl *self);
+
 struct odp_execute_action_impl * odp_execute_action_set(const char *name);
 
 #endif /* ODP_EXTRACT_PRIVATE */
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 8a120223e..4ef82da76 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -834,6 +834,29 @@  requires_datapath_assistance(const struct nlattr *a)
     return false;
 }
 
+static void
+action_pop_vlan(struct dp_packet_batch *batch,
+                const struct nlattr *a OVS_UNUSED)
+{
+    struct dp_packet *packet;
+
+    DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+        eth_pop_vlan(packet);
+    }
+}
+
+/* Implementation of the scalar actions impl init function. Build up the
+ * array of func ptrs here. */
+int
+odp_action_scalar_init(struct odp_execute_action_impl *self)
+{
+    /* Set function pointers for actions that can be applied directly, these
+     * are identified by OVS_ACTION_ATTR_*. */
+    self->funcs[OVS_ACTION_ATTR_POP_VLAN] = action_pop_vlan;
+
+    return 0;
+}
+
 /* The active function pointers on the datapath. ISA optimized implementations
  * are enabled by plugging them into this static arary, which is consulted when
  * applying actions on the datapath. */
@@ -989,12 +1012,6 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
             break;
         }
 
-        case OVS_ACTION_ATTR_POP_VLAN:
-            DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
-                eth_pop_vlan(packet);
-            }
-            break;
-
         case OVS_ACTION_ATTR_PUSH_MPLS: {
             const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
 
@@ -1145,6 +1162,8 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
         case OVS_ACTION_ATTR_CT:
         case OVS_ACTION_ATTR_UNSPEC:
         case __OVS_ACTION_ATTR_MAX:
+        /* The following actions are handled by the scalar implementation. */
+        case OVS_ACTION_ATTR_POP_VLAN:
             OVS_NOT_REACHED();
         }