> -----Original Message-----
> From: Finn, Emma <[email protected]>
> Sent: Wednesday, January 5, 2022 4:54 PM
> To: [email protected]; Van Haaren, Harry <[email protected]>;
> Amber, Kumar <[email protected]>
> Cc: Finn, Emma <[email protected]>
> Subject: [PATCH v4 1/9] odp-execute: Add function pointers to odp-execute for
> different action implementations.
> 
> 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]>

Some explaining snippets below for future reviewer's reference, for the patch

Acked-by: Harry van Haaren <[email protected]>

<snip>

> +/* Structure represents an implementation of the odp actions. */
> +struct odp_execute_action_impl {
> +    /* When set, the CPU ISA required for this implementation is available
> +     * and the implementation can be used.
> +     */
> +    bool available;
> +
> +    /* Name of the implementation. */
> +    const char *name;
> +
> +    /* Probe function is used to detect if this CPU has the ISA required
> +     * to run the optimized miniflow implementation. It is optional and
> +     * if it is not used, then it must be null.
> +     */
> +    odp_execute_action_probe probe;
> +
> +    /* Called to check requirements and if usable, initializes the
> +     * implementation for use.
> +     */
> +    odp_execute_action_init_func init_func;
> +
> +    /* An array of callback functions, one for each action. */
> +    ATOMIC(odp_execute_cb) funcs[__OVS_KEY_ATTR_MAX];
> +};
> +
> +/* Order of Actions implementations. */
> +enum odp_execute_action_impl_idx {
> +    ACTION_IMPL_SCALAR,
> +    /* See ACTION_IMPL_BEGIN below, for "first to-be-validated" impl.
> +     * Do not change the autovalidator position in this list without updating
> +     * the define below.
> +     */
> +
> +    ACTION_IMPL_MAX,
> +};

The actions struct and enum list of implementations is taken from our best
practice as per MFEX reviews, which allows some nice usages of enum values
such as ACTION_IMPL_MAX to bounds-check arrays later. Thanks Eelco for
review of MFEX patchset and working with Amber to that final design.

<snip>

> -        switch ((enum ovs_action_attr) type) {
> +        /* If type is set in the active actions implementation, call the
> +         * function-pointer and an continue to the next action.
> +         */
> +        enum ovs_action_attr attr_type = (enum ovs_action_attr) type;
> +        if (actions_active_impl.funcs[attr_type]) {
> +            actions_active_impl.funcs[attr_type](NULL, batch, a, 
> should_steal);
> +            continue;
> +        }
> +
> +        /* If the action was not handled by the active function pointers 
> above,
> +         * process them by switching on the type below.
> +         */
> 
> +        switch (attr_type) {
>          case OVS_ACTION_ATTR_HASH: {
>              const struct ovs_action_hash *hash_act = nl_attr_get(a);

This is the main "plug in" point for optimized Actions implementations,
where a function pointer is optionally set for a specific action. Actions that
are not optimized (yet) continue to use the existing switch() code path.

<snip>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to