Received-SPF: pass (google.com: domain of linux-kernel+bounces-118026-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 25 Mar 2024 22:19:35 +0100
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
To: Petr Oros <poros@redhat.com>
Cc: netdev@vger.kernel.org, jesse.brandeburg@intel.com,
	anthony.l.nguyen@intel.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, wojciech.drewek@intel.com,
	intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] ice: fix enabling RX VLAN filtering
Message-ID: <ZgHqZ5ebaO9VkvYx@mev-dev>
References: <20240325201901.39365-1-poros@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240325201901.39365-1-poros@redhat.com>

On Mon, Mar 25, 2024 at 09:19:01PM +0100, Petr Oros wrote:
> ice_port_vlan_on/off() was introduced in commit 2946204b3fa8 ("ice:
> implement bridge port vlan"). But ice_port_vlan_on() incorrectly assigns
> ena_rx_filtering to inner_vlan_ops in DVM mode.
> This causes an error when rx_filtering cannot be enabled in legacy mode.
> 
> Reproducer:
>  echo 1 > /sys/class/net/$PF/device/sriov_numvfs
>  ip link set $PF vf 0 spoofchk off trust on vlan 3
> dmesg:
>  ice 0000:41:00.0: failed to enable Rx VLAN filtering for VF 0 VSI 9 during VF rebuild, error -95
> 
> Fixes: 2946204b3fa8 ("ice: implement bridge port vlan")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
>  .../ethernet/intel/ice/ice_vf_vsi_vlan_ops.c   | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
> index 80dc4bcdd3a41c..b3e1bdcb80f84d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_vsi_vlan_ops.c
> @@ -26,24 +26,22 @@ static void ice_port_vlan_on(struct ice_vsi *vsi)
>  	struct ice_vsi_vlan_ops *vlan_ops;
>  	struct ice_pf *pf = vsi->back;
>  
> -	if (ice_is_dvm_ena(&pf->hw)) {
> -		vlan_ops = &vsi->outer_vlan_ops;
> -
> -		/* setup outer VLAN ops */
> -		vlan_ops->set_port_vlan = ice_vsi_set_outer_port_vlan;
> -		vlan_ops->clear_port_vlan = ice_vsi_clear_outer_port_vlan;
> +	/* setup inner VLAN ops */
> +	vlan_ops = &vsi->inner_vlan_ops;
>  
> -		/* setup inner VLAN ops */
> -		vlan_ops = &vsi->inner_vlan_ops;
> +	if (ice_is_dvm_ena(&pf->hw)) {
>  		vlan_ops->add_vlan = noop_vlan_arg;
>  		vlan_ops->del_vlan = noop_vlan_arg;
>  		vlan_ops->ena_stripping = ice_vsi_ena_inner_stripping;
>  		vlan_ops->dis_stripping = ice_vsi_dis_inner_stripping;
>  		vlan_ops->ena_insertion = ice_vsi_ena_inner_insertion;
>  		vlan_ops->dis_insertion = ice_vsi_dis_inner_insertion;
> -	} else {
> -		vlan_ops = &vsi->inner_vlan_ops;
>  
> +		/* setup outer VLAN ops */
> +		vlan_ops = &vsi->outer_vlan_ops;
> +		vlan_ops->set_port_vlan = ice_vsi_set_outer_port_vlan;
> +		vlan_ops->clear_port_vlan = ice_vsi_clear_outer_port_vlan;
> +	} else {
>  		vlan_ops->set_port_vlan = ice_vsi_set_inner_port_vlan;
>  		vlan_ops->clear_port_vlan = ice_vsi_clear_inner_port_vlan;
>  	}

Thanks for fixing it.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> -- 
> 2.43.2
>