netdev - Re: [Intel-wired-lan] [iwl-net v1] ice: clear port vlan config during reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ebe2e5d-3243-4fb6-8798-a331f1ce3539@intel.com>
Date: Fri, 6 Sep 2024 15:11:55 +0200
From: Wojciech Drewek <wojciech.drewek@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [iwl-net v1] ice: clear port vlan config during
 reset



On 06.09.2024 14:57, Michal Swiatkowski wrote:
> Since commit 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with
> ice_vf_reconfig_vsi()") VF VSI is only reconfigured instead of
> recreated. The context configuration from previous setting is still the
> same. If any of the config needs to be cleared it needs to be cleared
> explicitly.
> 
> Previously there was assumption that port vlan will be cleared
> automatically. Now, when VSI is only reconfigured we have to do it in the
> code.
> 
> Not clearing port vlan configuration leads to situation when the driver
> VSI config is different than the VSI config in HW. Traffic can't be
> passed after setting and clearing port vlan, because of invalid VSI
> config in HW.
> 
> Example reproduction:
>> ip a a dev $(VF) $(VF_IP_ADDRESS)
>> ip l s dev $(VF) up
>> ping $(VF_IP_ADDRESS)
> ping is working fine here
>> ip link set eth5 vf 0 vlan 100
>> ip link set eth5 vf 0 vlan 0
>> ping $(VF_IP_ADDRESS)
> ping isn't working
> 
> Fixes: 2a2cb4c6c181 ("ice: replace ice_vf_recreate_vsi() with ice_vf_reconfig_vsi()")
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>

>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   |  7 +++
>  .../net/ethernet/intel/ice/ice_vsi_vlan_lib.c | 57 +++++++++++++++++++
>  .../net/ethernet/intel/ice/ice_vsi_vlan_lib.h |  1 +
>  3 files changed, 65 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index 5635e9da2212..9fe2a309c5ff 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> @@ -335,6 +335,13 @@ static int ice_vf_rebuild_host_vlan_cfg(struct ice_vf *vf, struct ice_vsi *vsi)
>  
>  		err = vlan_ops->add_vlan(vsi, &vf->port_vlan_info);
>  	} else {
> +		/* clear possible previous port vlan config */
> +		err = ice_vsi_clear_port_vlan(vsi);
> +		if (err) {
> +			dev_err(dev, "failed to clear port VLAN via VSI parameters for VF %u, error %d\n",
> +				vf->vf_id, err);
> +			return err;
> +		}
>  		err = ice_vsi_add_vlan_zero(vsi);
>  	}
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
> index 6e8f2aab6080..5291f2888ef8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.c
> @@ -787,3 +787,60 @@ int ice_vsi_clear_outer_port_vlan(struct ice_vsi *vsi)
>  	kfree(ctxt);
>  	return err;
>  }
> +
> +int ice_vsi_clear_port_vlan(struct ice_vsi *vsi)
> +{
> +	struct ice_hw *hw = &vsi->back->hw;
> +	struct ice_vsi_ctx *ctxt;
> +	int err;
> +
> +	ctxt = kzalloc(sizeof(*ctxt), GFP_KERNEL);
> +	if (!ctxt)
> +		return -ENOMEM;
> +
> +	ctxt->info = vsi->info;
> +
> +	ctxt->info.port_based_outer_vlan = 0;
> +	ctxt->info.port_based_inner_vlan = 0;
> +
> +	ctxt->info.inner_vlan_flags =
> +		FIELD_PREP(ICE_AQ_VSI_INNER_VLAN_TX_MODE_M,
> +			   ICE_AQ_VSI_INNER_VLAN_TX_MODE_ALL);
> +	if (ice_is_dvm_ena(hw)) {
> +		ctxt->info.inner_vlan_flags |=
> +			FIELD_PREP(ICE_AQ_VSI_INNER_VLAN_EMODE_M,
> +				   ICE_AQ_VSI_INNER_VLAN_EMODE_NOTHING);
> +		ctxt->info.outer_vlan_flags =
> +			FIELD_PREP(ICE_AQ_VSI_OUTER_VLAN_TX_MODE_M,
> +				   ICE_AQ_VSI_OUTER_VLAN_TX_MODE_ALL);
> +		ctxt->info.outer_vlan_flags |=
> +			FIELD_PREP(ICE_AQ_VSI_OUTER_TAG_TYPE_M,
> +				   ICE_AQ_VSI_OUTER_TAG_VLAN_8100);
> +		ctxt->info.outer_vlan_flags |=
> +			ICE_AQ_VSI_OUTER_VLAN_EMODE_NOTHING <<
> +			ICE_AQ_VSI_OUTER_VLAN_EMODE_S;
> +	}
> +
> +	ctxt->info.sw_flags2 &= ~ICE_AQ_VSI_SW_FLAG_RX_VLAN_PRUNE_ENA;
> +	ctxt->info.valid_sections =
> +		cpu_to_le16(ICE_AQ_VSI_PROP_OUTER_TAG_VALID |
> +			    ICE_AQ_VSI_PROP_VLAN_VALID |
> +			    ICE_AQ_VSI_PROP_SW_VALID);
> +
> +	err = ice_update_vsi(hw, vsi->idx, ctxt, NULL);
> +	if (err) {
> +		dev_err(ice_pf_to_dev(vsi->back), "update VSI for clearing port based VLAN failed, err %d aq_err %s\n",
> +			err, ice_aq_str(hw->adminq.sq_last_status));
> +	} else {
> +		vsi->info.port_based_outer_vlan =
> +			ctxt->info.port_based_outer_vlan;
> +		vsi->info.port_based_inner_vlan =
> +			ctxt->info.port_based_inner_vlan;
> +		vsi->info.outer_vlan_flags = ctxt->info.outer_vlan_flags;
> +		vsi->info.inner_vlan_flags = ctxt->info.inner_vlan_flags;
> +		vsi->info.sw_flags2 = ctxt->info.sw_flags2;
> +	}
> +
> +	kfree(ctxt);
> +	return err;
> +}
> diff --git a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.h b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.h
> index f0d84d11bd5b..12b227621a7d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_vsi_vlan_lib.h
> @@ -36,5 +36,6 @@ int ice_vsi_ena_outer_insertion(struct ice_vsi *vsi, u16 tpid);
>  int ice_vsi_dis_outer_insertion(struct ice_vsi *vsi);
>  int ice_vsi_set_outer_port_vlan(struct ice_vsi *vsi, struct ice_vlan *vlan);
>  int ice_vsi_clear_outer_port_vlan(struct ice_vsi *vsi);
> +int ice_vsi_clear_port_vlan(struct ice_vsi *vsi);
>  
>  #endif /* _ICE_VSI_VLAN_LIB_H_ */