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

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

git sha: <patch does not include git commit sha>
Author: Rayane Boussanni <rboussanni at gmail.com>
Subject: [PATCH v2] net/mlx5: add validation for indirect actions

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..ec1cdbe1fa 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);

The forward declarations show inconsistent indentation. The first
function has the parameters aligned with tabs (likely 8-space indents),
while the second uses spaces. Does the codebase consistently use tabs
for indentation in function declarations? According to DPDK coding
standards, tabs should be used for indentation.

> +
>  static __rte_always_inline uint32_t flow_hw_tx_tag_regc_mask(struct rte_eth_dev *dev);
>  static __rte_always_inline uint32_t flow_hw_tx_tag_regc_value(struct rte_eth_dev *dev);
>  

[ ... ]

> @@ -6604,6 +6619,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 +6655,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, error);
> +		if (ret < 0)
> +			return ret;
>  		*action_flags |= MLX5_FLOW_ACTION_RSS;
>  		break;

In flow_hw_validate_action_indirect()->RTE_FLOW_ACTION_TYPE_RSS, does
this code correctly handle the action_flags update? If
mlx5_hw_validate_action_rss() returns a non-zero success value (>0), the
code still sets MLX5_FLOW_ACTION_RSS. Should action_flags be updated only
on success (ret == 0)?

>  	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;
>  		*action_flags |= MLX5_FLOW_ACTION_CT;
>  		break;

Same question for RTE_FLOW_ACTION_TYPE_CONNTRACK: if
mlx5_hw_validate_action_conntrack() returns a positive value, does this
code correctly handle the action_flags update?

[ ... ]

> @@ -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,
>  							       &action_flags,
>  							       &fixed_cnt,
>  							       error);