On 13 Jul 2022, at 20:27, Harry van Haaren wrote:
> From: Emma Finn <[email protected]>
>
> This commit introduces the initial infrastructure required to allow
> different implementations for OvS actions. The patch introduces action
> function pointers which allows user to switch between different action
> implementations available. This will allow for more performance and
> flexibility
> so the user can choose the action implementation to best suite their use case.
>
> Signed-off-by: Emma Finn <[email protected]>
> Acked-by: Harry van Haaren <[email protected]>
>
> ---
<SNIP>
> +/* 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.
> + */
The comment ending should be on the same line. Please fix this in all places of
this patch.
> +static struct odp_execute_action_impl *actions_active_impl;
> +
> +static int
> +odp_actions_impl_set(const char *name)
> +{
> + struct odp_execute_action_impl *active;
> + active = odp_execute_action_set(name);
> + if (!active) {
> + VLOG_ERR("Failed setting action implementation to %s", name);
> + return 1;
> + }
> +
> + actions_active_impl = active;
This should be an atomic store.
> + return 0;
> +
> +}
> +
<SNIP>
> @@ -879,8 +913,20 @@ odp_execute_actions(void *dp, struct dp_packet_batch
> *batch, bool steal,
> continue;
> }
>
> - switch ((enum ovs_action_attr) type) {
> + /* If type is set in the active actions implementation, call the
> + * function-pointer and continue to the next action.
> + */
> + if (actions_active_impl->funcs[attr_type] &&
The actions_active_impl pointer should have been read atomically.
Looks like I also missed this in the previous patches...
> + attr_type <= OVS_ACTION_ATTR_MAX) {
> + actions_active_impl->funcs[attr_type](batch, a);
> + continue;
> + }
Those are the only comments I have on this patch, the rest looks good.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
