Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 Jan 2023 15:07:59 -0700
From:   dann frazier <dann.frazier@canonical.com>
To:     Sasha Levin <sashal@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Roi Dayan <roid@nvidia.com>, Maor Dickman <maord@nvidia.com>,
        Saeed Mahameed <saeedm@nvidia.com>,
        Ran Drori <rdrori@nvidia.com>,
        Frode Nordahl <frode.nordahl@canonical.com>
Subject: Re: [PATCH 5.15 049/230] net/mlx5e: Check action fwd/drop flag
 exists also for nic flows
Message-ID: <Y8CEv90mCZkmuFAq@xps13.dannf>
References: <20220711090604.055883544@linuxfoundation.org>
 <20220711090605.473699898@linuxfoundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220711090605.473699898@linuxfoundation.org>
Precedence: bulk

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");