Re: [PATCH 5.15 049/230] net/mlx5e: Check action fwd/drop flag exists also for nic flows

dann frazier
2023-01-12T22:14:47Z

On Mon, Jul 11, 2022 at 11:05:05AM +0200, Greg Kroah-Hartman wrote:
> From: Roi Dayan <roid@nvidia.com>
>
> [ Upstream commit 6b50cf45b6a0e99f3cab848a72ecca8da56b7460 ]
>
> The driver should add offloaded rules with either a fwd or drop action.
> The check existed in parsing fdb flows but not when parsing nic flows.
> Move the test into actions_match_supported() which is called for
> checking nic flows and fdb flows.
>
> Signed-off-by: Roi Dayan <roid@nvidia.com>
> Reviewed-by: Maor Dickman <maord@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

hey Sasha,

A contact at Nvidia tells me that this has caused a regression w/
OVN HW offload. To fix that, commit 7f8770c7 ("net/mlx5e: Set action
fwd flag when parsing tc action goto") is also required.

I'm not really sure what flagged this patch for stable, so I don't
know whether to suggest it be reverted, or that additonal patch be
applied. Roi - what's your thought?

-dann

> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 3aa8d0b83d10..fe52db591121 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -3305,6 +3305,12 @@ static bool actions_match_supported(struct mlx5e_priv *priv,
> ct_flow = flow_flag_test(flow, CT) && !ct_clear;
> actions = flow->attr->action;
>
> + if (!(actions &
> + (MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | MLX5_FLOW_CONTEXT_ACTION_DROP))) {
> + NL_SET_ERR_MSG_MOD(extack, "Rule must have at least one forward/drop action");
> + return false;
> + }
> +
> if (mlx5e_is_eswitch_flow(flow)) {
> if (flow->attr->esw_attr->split_count && ct_flow &&
> !MLX5_CAP_GEN(flow->attr->esw_attr->in_mdev, reg_c_preserve)) {
> @@ -4207,13 +4213,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> attr->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> }
>
> - if (!(attr->action &
> - (MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | MLX5_FLOW_CONTEXT_ACTION_DROP))) {
> - NL_SET_ERR_MSG_MOD(extack,
> - "Rule must have at least one forward/drop action");
> - return -EOPNOTSUPP;
> - }
> -
> if (esw_attr->split_count > 0 && !mlx5_esw_has_fwd_fdb(priv->mdev)) {
> NL_SET_ERR_MSG_MOD(extack,
> "current firmware doesn't support split rule for port mirroring");