|SUCCESS| net/mlx5: add validation for indirect actions

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163278

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-24

Reviewing patch: net/mlx5: add validation for indirect actions

Commit: 20260416001052.2624-1-rboussanni at gmail.com
Author: Rayane Boussanni <rboussanni at gmail.com>
Subject: [PATCH] net/mlx5: add validation for indirect actions

This patch adds validation logic for RSS and ConnTrack indirect actions
in the Hardware Steering flow engine by wiring up existing validation
functions.

> diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
> index bca5b2769e..fc5d59be98 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> @@ -346,6 +346,21 @@ mlx5_flow_ct_init(struct rte_eth_dev *dev,
>  		  uint32_t nb_conn_tracks,
>  		  uint16_t nb_queue);
>  
> +static int
> +mlx5_hw_validate_action_rss(struct rte_eth_dev *dev,
> +				       const struct rte_flow_action *action,
> +				       const struct rte_flow_action *mask,
> +				       const struct rte_flow_actions_template_attr *attr,
> +				       uint64_t action_flags,
> +				       struct rte_flow_error *error);
> +static int
> +mlx5_hw_validate_action_conntrack(struct rte_eth_dev *dev,
> +					     const struct rte_flow_action *action,
> +					     const struct rte_flow_action *mask,
> +					     const struct rte_flow_actions_template_attr *attr,
> +					     uint64_t action_flags,
> +					     struct rte_flow_error *error);

Does this code use tabs for indentation? The coding style guide mandates
that tabs should be used for indentation with 8-character width. The
parameter alignment here appears to use spaces rather than tabs. Please
verify proper indentation using tabs.

[ ... ]

> @@ -6595,6 +6610,7 @@ flow_hw_validate_action_meter_mark(struct rte_eth_dev *dev,
>  	return 0;
>  }
>  
> +

Does this code add an unnecessary blank line? Empty lines should only be
used where they improve readability or separate logical blocks.

> @@ -6604,6 +6620,8 @@ flow_hw_validate_action_meter_mark(struct rte_eth_dev *dev,
>   *   Pointer to the indirect action.
>   * @param[in] mask
>   *   Pointer to the indirect action mask.
> + * @param[in] attr
> + *   Pointer to the action template attributes.
>   * @param[in, out] action_flags
>   *   Holds the actions detected until now.
>   * @param[in, out] fixed_cnt

[ ... ]

> @@ -6637,11 +6656,16 @@ flow_hw_validate_action_indirect(struct rte_eth_dev *dev,
>  		*action_flags |= MLX5_FLOW_ACTION_METER;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_RSS:
> -		/* TODO: Validation logic (same as flow_hw_actions_validate) */
> +		ret = mlx5_hw_validate_action_rss(dev, action, mask, attr, *action_flags, error);
> +		if (ret < 0)
> +			return ret;

Does this code exceed 80 characters on the function call line? The coding
style guide specifies a line length limit of 80 characters.

>  		*action_flags |= MLX5_FLOW_ACTION_RSS;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> -		/* TODO: Validation logic (same as flow_hw_actions_validate) */
> +		ret = mlx5_hw_validate_action_conntrack(dev, action, mask, attr,
> +							 *action_flags, error);

Does this code use spaces instead of tabs for the continuation line
alignment? Continuation lines should be aligned to opening parenthesis or
start position using tabs, and spaces only for fine alignment within the
same indentation level.

> +		if (ret < 0)
> +			return ret;
>  		*action_flags |= MLX5_FLOW_ACTION_CT;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_COUNT:

[ ... ]

> @@ -7352,6 +7376,7 @@ mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_INDIRECT:
>  			ret = flow_hw_validate_action_indirect(dev, action,
>  							       mask,
> +								   attr,

Does this code use spaces for indentation? The new parameter 'attr'
appears to be indented with spaces instead of tabs. Please verify that
tabs are used for indentation to match DPDK coding standards.

>  							       &action_flags,
>  							       &fixed_cnt,
>  							       error);