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?
