On Fri, May 29, 2026 at 04:36:05PM +0100, Anatoly Burakov wrote:
> Currently, each driver has their own code for action parsing, which results
> in a lot of duplication and subtle mismatches in behavior between drivers.
>
> Add common infrastructure, based on the following assumptions:
>
> - All drivers support at most 32 actions at once, but usually far less
> - Not every action is supported by all drivers
> - We can check a few common things to filter out obviously wrong actions
> - Driver performs semantic checks on all valid actions
>
> So, the intention is to reject everything we can reasonably reject at the
> outset without knowing anything about the drivers, parametrize what is
> trivial to parametrize, and leave the rest for the driver to implement.
>
> While we're at it, also add logging infrastructure for Intel common code,
> using the new component name defines that are automatically passed to each
> DPDK driver as it is being built.
>
> Signed-off-by: Anatoly Burakov <[email protected]>
> ---
> drivers/net/intel/common/flow_check.h | 279 ++++++++++++++++++++++++++
> drivers/net/intel/common/log.h | 40 ++++
> 2 files changed, 319 insertions(+)
> create mode 100644 drivers/net/intel/common/flow_check.h
> create mode 100644 drivers/net/intel/common/log.h
>
<snip>
> +/**
> + * Validate and parse a list of rte_flow_action into a parsed action list.
> + *
> + * @param actions pointer to array of rte_flow_action, terminated by
> RTE_FLOW_ACTION_TYPE_END
> + * @param param pointer to ci_flow_actions_check_param structure (can be
> NULL)
> + * @param parsed_actions pointer to ci_flow_actions structure to store
> parsed actions
> + * @param error pointer to rte_flow_error structure for error reporting
> + *
> + * @return 0 on success, negative errno on failure.
> + */
> +static inline int
> +ci_flow_check_actions(const struct rte_flow_action *actions,
> + const struct ci_flow_actions_check_param *param,
> + struct ci_flow_actions *parsed_actions,
> + struct rte_flow_error *error)
> +{
> + size_t i = 0;
> + int ret;
> +
> + if (actions == NULL) {
> + return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION,
> + NULL, "Missing actions");
> + }
> +
> + /* reset the list */
> + *parsed_actions = (struct ci_flow_actions){0};
> +
> + while (actions[i].type != RTE_FLOW_ACTION_TYPE_END) {
> + const struct rte_flow_action *action = &actions[i++];
> +
> + /* skip VOID actions */
> + if (action->type == RTE_FLOW_ACTION_TYPE_VOID)
> + continue;
> +
> + /* generic validation for actions - this will check against
> param as well */
> + ret = __flow_action_check_generic(action, param, error);
> + if (ret < 0)
> + return ret;
> +
> + /* check against global maximum number of actions */
> + if (parsed_actions->count >= RTE_DIM(parsed_actions->actions)) {
> + return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION,
> + action, "Too many
> actions");
> + }
> + /* user may have specified a maximum number of actions */
> + if (param != NULL && param->max_actions != 0 &&
> + parsed_actions->count >= param->max_actions) {
> + return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION,
> + action, "Too many
> actions");
> + }
> + /* add action to the list */
> + CI_DRV_LOG(DEBUG, "Parsed action %u: type=%s",
> parsed_actions->count,
> + ci_flow_action_type_to_str(action->type));
> + parsed_actions->actions[parsed_actions->count++] = action;
> + }
> +
> + /* now, call into user validation if specified */
> + if (param != NULL && param->check != NULL) {
> + ret = param->check(parsed_actions, param, error);
> + if (ret < 0)
> + return ret;
> + }
Running an AI review on this code myself, it highlights the fact that we
are missing a check for an empty parsed_actions array here, and not all
check handlers verify the length as being >0 before dereferencing. For
example, in patch 10, the callbacks don't explicitly check for empty lists
I think it would be reasonable to have the return -EINVAL before this
callback check, right?
> + /* if we didn't parse anything, valid action list is empty */
> + return parsed_actions->count == 0 ? -EINVAL : 0;
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
<snip>
