> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of > Brett Creeley > Sent: Thursday, March 14, 2024 4:54 PM > To: ivecera <[email protected]>; [email protected] > Cc: moderated list:INTEL ETHERNET DRIVERS <intel-wired- > [email protected]>; open list <[email protected]>; Loktionov, > Aleksandr <[email protected]>; Eric Dumazet > <[email protected]>; Nguyen, Anthony L > <[email protected]>; [email protected]; Jakub Kicinski > <[email protected]>; Paolo Abeni <[email protected]>; David S. Miller > <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal > > On 3/13/2024 6:56 AM, Ivan Vecera wrote: > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > > > > Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove > > administratively set MAC") fixed an issue where untrusted VF was > > allowed to remove its own MAC address although this was assigned > > administratively from PF. Unfortunately the introduced check is wrong > > because it causes that MAC filters for other MAC addresses including > > multi-cast ones are not removed. > > > > <snip> > > if (ether_addr_equal(addr, vf->default_lan_addr.addr) && > > i40e_can_vf_change_mac(vf)) > > was_unimac_deleted = true; > > else > > continue; > > > > if (i40e_del_mac_filter(vsi, al->list[i].addr)) { > > ... > > </snip> > > > > The else path with `continue` effectively skips any MAC filter removal > > except one for primary MAC addr when VF is allowed to do so. > > Fix the check condition so the `continue` is only done for primary MAC > > address. > > > > Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove > > administratively set MAC") > > Signed-off-by: Ivan Vecera <[email protected]> > > --- > > drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > index b34c71770887..10267a300770 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c > > @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct > i40e_vf *vf, u8 *msg) > > /* Allow to delete VF primary MAC only if it was not set > > * administratively by PF or if VF is trusted. > > */ > > - if (ether_addr_equal(addr, vf->default_lan_addr.addr) && > > - i40e_can_vf_change_mac(vf)) > > - was_unimac_deleted = true; > > - else > > - continue; > > + if (ether_addr_equal(addr, vf->default_lan_addr.addr)) { > > + if (i40e_can_vf_change_mac(vf)) > > + was_unimac_deleted = true; > > + else > > + continue; > > + } > > Seems okay to me. > > Reviewed-by: Brett Creeley <[email protected]> > > > > > if (i40e_del_mac_filter(vsi, al->list[i].addr)) { > > ret = -EINVAL; > > -- > > 2.43.0 > > > >
Tested-by: Rafal Romanowski <[email protected]>
