| Message ID | 20220112094244.81402-3-emma.finn@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Actions Infrastructure + Optimizations | expand |
> -----Original Message----- > From: Finn, Emma <emma.finn@intel.com> > Sent: Wednesday, January 12, 2022 9:43 AM > To: dev@openvswitch.org; Van Haaren, Harry <harry.van.haaren@intel.com>; > Amber, Kumar <kumar.amber@intel.com>; Stokes, Ian <ian.stokes@intel.com>; > i.maximets@ovn.org > Cc: Finn, Emma <emma.finn@intel.com> > Subject: [PATCH v5 2/8] odp-execute: Add function pointer for pop_vlan action. > > 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> Thanks for the patch Emma, Minor comment below. > --- > lib/odp-execute-private.c | 19 +++++++++++++++++- > lib/odp-execute.c | 41 +++++++++++++++++++++++++++++++++------ > lib/odp-execute.h | 2 ++ > 3 files changed, 55 insertions(+), 7 deletions(-) > > diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c > index 6441c491c..d88ff4921 100644 > --- a/lib/odp-execute-private.c > +++ b/lib/odp-execute-private.c > @@ -29,13 +29,14 @@ > > int32_t action_autoval_init(struct odp_execute_action_impl *self); > VLOG_DEFINE_THIS_MODULE(odp_execute_private); > +static uint32_t active_action_impl_index; > > static struct odp_execute_action_impl action_impls[] = { > [ACTION_IMPL_SCALAR] = { > .available = 1, > .name = "scalar", > .probe = NULL, > - .init_func = NULL, > + .init_func = odp_action_scalar_init, > }, > }; > > @@ -49,6 +50,22 @@ action_impl_copy_funcs(struct odp_execute_action_impl > *to, > } > } > > +int32_t > +odp_execute_action_set(const char *name, > + struct odp_execute_action_impl *active) > +{ > + uint32_t i; > + for (i = 0; i < ACTION_IMPL_MAX; i++) { > + /* string compare, and set ptrs *atomically*. */ Minor, capitalize String above for comment. Other than that LGTM. Ian > + if (strcmp(action_impls[i].name, name) == 0) { > + action_impl_copy_funcs(active, &action_impls[i]); > + active_action_impl_index = i; > + return 0; > + } > + } > + return -1; > +} > + > void > odp_execute_action_init(void) > { > diff --git a/lib/odp-execute.c b/lib/odp-execute.c > index 49dfa2a74..ab051aecc 100644 > --- a/lib/odp-execute.c > +++ b/lib/odp-execute.c > @@ -831,6 +831,28 @@ requires_datapath_assistance(const struct nlattr *a) > return false; > } > > +static void > +action_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch, > + const struct nlattr *a OVS_UNUSED, > + bool should_steal 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. > + */ > +int32_t > +odp_action_scalar_init(struct odp_execute_action_impl *self) > +{ > + 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. > @@ -843,10 +865,22 @@ odp_execute_init(void) > static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > if (ovsthread_once_start(&once)) { > odp_execute_action_init(); > + odp_actions_impl_set("scalar"); > ovsthread_once_done(&once); > } > } > > +int32_t > +odp_actions_impl_set(const char *name) > +{ > + > + int err = odp_execute_action_set(name, &actions_active_impl); > + if (err) { > + VLOG_ERR("error %d from action set to %s\n", err, name); > + return -1; > + } > + return 0; > +} > > /* Executes all of the 'actions_len' bytes of datapath actions in 'actions' on > * the packets in 'batch'. If 'steal' is true, possibly modifies and > @@ -962,12 +996,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); > > @@ -1100,6 +1128,7 @@ odp_execute_actions(void *dp, struct > dp_packet_batch *batch, bool steal, > } > case OVS_ACTION_ATTR_OUTPUT: > case OVS_ACTION_ATTR_LB_OUTPUT: > + case OVS_ACTION_ATTR_POP_VLAN: > case OVS_ACTION_ATTR_TUNNEL_PUSH: > case OVS_ACTION_ATTR_TUNNEL_POP: > case OVS_ACTION_ATTR_USERSPACE: > diff --git a/lib/odp-execute.h b/lib/odp-execute.h > index c4f5303e7..6441392b9 100644 > --- a/lib/odp-execute.h > +++ b/lib/odp-execute.h > @@ -32,6 +32,8 @@ struct dp_packet_batch; > /* Called once at initialization time. */ > void odp_execute_init(void); > > +int32_t odp_actions_impl_set(const char *name); > + > typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch, > const struct nlattr *action, bool should_steal); > > -- > 2.25.1
diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c index 6441c491c..d88ff4921 100644 --- a/lib/odp-execute-private.c +++ b/lib/odp-execute-private.c @@ -29,13 +29,14 @@ int32_t action_autoval_init(struct odp_execute_action_impl *self); VLOG_DEFINE_THIS_MODULE(odp_execute_private); +static uint32_t active_action_impl_index; static struct odp_execute_action_impl action_impls[] = { [ACTION_IMPL_SCALAR] = { .available = 1, .name = "scalar", .probe = NULL, - .init_func = NULL, + .init_func = odp_action_scalar_init, }, }; @@ -49,6 +50,22 @@ action_impl_copy_funcs(struct odp_execute_action_impl *to, } } +int32_t +odp_execute_action_set(const char *name, + struct odp_execute_action_impl *active) +{ + uint32_t i; + for (i = 0; i < ACTION_IMPL_MAX; i++) { + /* string compare, and set ptrs *atomically*. */ + if (strcmp(action_impls[i].name, name) == 0) { + action_impl_copy_funcs(active, &action_impls[i]); + active_action_impl_index = i; + return 0; + } + } + return -1; +} + void odp_execute_action_init(void) { diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 49dfa2a74..ab051aecc 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -831,6 +831,28 @@ requires_datapath_assistance(const struct nlattr *a) return false; } +static void +action_pop_vlan(void *dp OVS_UNUSED, struct dp_packet_batch *batch, + const struct nlattr *a OVS_UNUSED, + bool should_steal 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. + */ +int32_t +odp_action_scalar_init(struct odp_execute_action_impl *self) +{ + 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. @@ -843,10 +865,22 @@ odp_execute_init(void) static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; if (ovsthread_once_start(&once)) { odp_execute_action_init(); + odp_actions_impl_set("scalar"); ovsthread_once_done(&once); } } +int32_t +odp_actions_impl_set(const char *name) +{ + + int err = odp_execute_action_set(name, &actions_active_impl); + if (err) { + VLOG_ERR("error %d from action set to %s\n", err, name); + return -1; + } + return 0; +} /* Executes all of the 'actions_len' bytes of datapath actions in 'actions' on * the packets in 'batch'. If 'steal' is true, possibly modifies and @@ -962,12 +996,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); @@ -1100,6 +1128,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal, } case OVS_ACTION_ATTR_OUTPUT: case OVS_ACTION_ATTR_LB_OUTPUT: + case OVS_ACTION_ATTR_POP_VLAN: case OVS_ACTION_ATTR_TUNNEL_PUSH: case OVS_ACTION_ATTR_TUNNEL_POP: case OVS_ACTION_ATTR_USERSPACE: diff --git a/lib/odp-execute.h b/lib/odp-execute.h index c4f5303e7..6441392b9 100644 --- a/lib/odp-execute.h +++ b/lib/odp-execute.h @@ -32,6 +32,8 @@ struct dp_packet_batch; /* Called once at initialization time. */ void odp_execute_init(void); +int32_t odp_actions_impl_set(const char *name); + typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch, const struct nlattr *action, bool should_steal);