netdev - Re: [iwl-net v2] iavf: allow changing VLAN state without calling PF

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] [day] [month] [year] [list]

Message-ID: <6c32336c-09ef-429f-8bf8-f591e90ae32b@intel.com>
Date: Thu, 5 Sep 2024 12:59:18 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
	<mschmidt@...hat.com>
CC: <netdev@...r.kernel.org>, <wojciech.drewek@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
Subject: Re: [iwl-net v2] iavf: allow changing VLAN state without calling PF

On 9/5/24 11:14, Michal Swiatkowski wrote:
> First case:
>> ip l a l $VF name vlanx type vlan id 100
>> ip l d vlanx
>> ip l a l $VF name vlanx type vlan id 100
> 
> As workqueue can be execute after sometime, there is a window to have
> call trace like that:
> - iavf_del_vlan
> - iavf_add_vlan
> - iavf_del_vlans (wq)
> 
> It means that our VLAN 100 will change the state from IAVF_VLAN_ACTIVE
> to IAVF_VLAN_REMOVE (iavf_del_vlan). After that in iavf_add_vlan state
> won't be changed because VLAN 100 is on the filter list. The final
> result is that the VLAN 100 filter isn't added in hardware (no
> iavf_add_vlans call).
> 
> To fix that change the state if the filter wasn't removed yet directly
> to active. It is save as IAVF_VLAN_REMOVE means that virtchnl message
> wasn't sent yet.
> 
> Second case:
>> ip l a l $VF name vlanx type vlan id 100
> Any type of VF reset ex. change trust
>> ip l s $PF vf $VF_NUM trust on
>> ip l d vlanx
>> ip l a l $VF name vlanx type vlan id 100
> 
> In case of reset iavf driver is responsible for readding all filters
> that are being used. To do that all VLAN filters state are changed to
> IAVF_VLAN_ADD. Here is even longer window for changing VLAN state from
> kernel side, as workqueue isn't called immediately. We can have call
> trace like that:
> 
> - changing to IAVF_VLAN_ADD (after reset)
> - iavf_del_vlan (called from kernel ops)
> - iavf_del_vlans (wq)
> 
> Not exsisitng VLAN filters will be removed from hardware. It isn't a
> bug, ice driver will handle it fine. However, we can have call trace
> like that:
> 
> - changing to IAVF_VLAN_ADD (after reset)
> - iavf_del_vlan (called from kernel ops)
> - iavf_add_vlan (called from kernel ops)
> - iavf_del_vlans (wq)
> 
> With fix for previous case we end up with no VLAN filters in hardware.
> We have to remove VLAN filters if the state is IAVF_VLAN_ADD and delete
> VLAN was called. It is save as IAVF_VLAN_ADD means that virtchnl message
> wasn't sent yet.
> 
> Fixes: 0c0da0e95105 ("iavf: refactor VLAN filter states")
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> ---
> v2: https://lore.kernel.org/netdev/20240904120052.24561-1-michal.swiatkowski@linux.intel.com/
>   * add missing state in case of delete
> ---
>   drivers/net/ethernet/intel/iavf/iavf_main.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>