|SUCCESS| [v3] net/mlx5: add validation for indirect actions

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-14

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

Author: Rayane Boussanni <rboussanni at gmail.com>

This patch implements missing validation logic for RSS and Connection
Tracking (ConnTrack) indirect actions in the Hardware Steering (HWS)
flow engine.

> diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
> index bca5b2769e..da5eb0bc42 100644
> --- a/drivers/net/mlx5/mlx5_flow_hw.c
> +++ b/drivers/net/mlx5/mlx5_flow_hw.c

[ ... ]

> @@ -6604,6 +6621,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
> @@ -6618,6 +6637,7 @@ static int
>  flow_hw_validate_action_indirect(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, bool *fixed_cnt,
>  				 struct rte_flow_error *error)
>  {
> @@ -6637,11 +6657,17 @@ 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, true, error);
> +		if (ret < 0)
> +			return ret;
>  		*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);
> +		if (ret < 0)
> +			return ret;

Does this code need to check 'ret < 0' here? The function
mlx5_hw_validate_action_conntrack() appears to follow standard DPDK
error handling patterns where non-zero return values indicate errors.
Looking at similar validation code in this file, error checks typically
use 'if (ret)' rather than 'if (ret < 0)'. Can this use the simpler
form for consistency?

>  		*action_flags |= MLX5_FLOW_ACTION_CT;
>  		break;
>  	case RTE_FLOW_ACTION_TYPE_COUNT:
> @@ -7139,6 +7165,7 @@ mlx5_hw_validate_action_rss(struct rte_eth_dev *dev,
>  			      const struct rte_flow_action *template_mask,
>  			      const struct rte_flow_actions_template_attr *template_attr,
>  			      __rte_unused uint64_t action_flags,
> +			      bool is_indirect,
>  			      struct rte_flow_error *error)
>  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
> @@ -7148,6 +7175,8 @@ mlx5_hw_validate_action_rss(struct rte_eth_dev *dev,
>  		return rte_flow_error_set(error, EINVAL,
>  					  RTE_FLOW_ERROR_TYPE_ATTR, NULL,
>  					  "RSS action supported for ingress only");
> +	if (is_indirect)
> +		return 0;

Does this early return for indirect RSS skip validation checks that
should still apply? The function mlx5_hw_validate_action_rss() performs
ingress/egress validation above this new check, but then returns
immediately for indirect actions. Are there other validation steps in
this function (beyond the ingress check) that should also apply to
indirect RSS actions? If the RSS configuration in the indirect action
handle is already validated elsewhere as mentioned in the commit
message, can you clarify what happens if template_attr has conflicting
direction settings?

>  	if (mask != NULL)
>  		return mlx5_validate_action_rss(dev, template_action, error);
>  	else
> @@ -7352,6 +7381,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,
>  							       &action_flags,
>  							       &fixed_cnt,
>  							       error);
> @@ -7407,7 +7437,7 @@ mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_RSS:
>  			ret = mlx5_hw_validate_action_rss(dev, action, mask,
>  							  attr, action_flags,
> -							  error);
> +							  false, error);
>  			if (ret)
>  				return ret;
>  			action_flags |= MLX5_FLOW_ACTION_RSS;