> -----Original Message-----
> From: Finn, Emma <[email protected]>
> Sent: Wednesday, January 12, 2022 9:43 AM
> To: [email protected]; Van Haaren, Harry <[email protected]>;
> Amber, Kumar <[email protected]>; Stokes, Ian <[email protected]>;
> [email protected]
> Cc: Finn, Emma <[email protected]>
> 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 <[email protected]>
> Acked-by: Harry van Haaren <[email protected]>
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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
