netdev - Re: [Intel-wired-lan] [PATCH iwl-next 5/8] ice: update mac, vlan rules when toggling between VEB and VEPA

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: <0776633b-7e0a-46f6-b4d9-4bcf459bae62@intel.com>
Date: Fri, 21 Nov 2025 10:25:16 +0100
From: Jakub Slepecki <jakub.slepecki@...el.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "michal.swiatkowski@...ux.intel.com"
	<michal.swiatkowski@...ux.intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next 5/8] ice: update mac, vlan
 rules when toggling between VEB and VEPA

On 2025-11-21 9:54, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> When changing into VEPA mode MAC rules are modified to forward all
>> traffic to the wire instead of allowing some packets to go into the
>> loopback.
>> MAC,VLAN rules may and will also be used to forward loopback traffic
>> in VEB, so when we switch to VEPA, we want them to behave similarly to
>> MAC-only rules.
> Is it possible to verify from shell? Could be nice to add exact steps to reproduce/verify.

It's not straightforward.  The easiest way is to observe traffic on the
wire (or lack of thereof).  For my testing, I have a patch that:

# cat /sys/kernel/debug/ice/0000:45:00.0/switch_rules
lkup=0x0, id=8207, flag=0x0001, action=0x0, lan=no, lb=no, count=1, mac=00:00:00:00:00:00, ethertype=0x88cc
lkup=0x0, id=20509, flag=0x0002, action=0x4, lan=no, lb=no, count=1, mac=00:00:00:00:00:00, ethertype=0x88cc
lkup=0x0, id=24593, flag=0x0002, action=0x4, lan=no, lb=no, count=1, mac=00:00:00:00:00:00, ethertype=0x8808
lkup=0x1, id=14353, flag=0x0002, action=0x0, lan=yes, lb=yes, count=1, mac=33:33:ff:0b:64:f2
lkup=0x1, id=18456, flag=0x0002, action=0x0, lan=yes, lb=yes, count=1, mac=33:33:ff:df:a9:13
lkup=0x1, id=24594, flag=0x0002, action=0x0, lan=yes, lb=yes, count=1, mac=33:33:ff:f0:75:00
lkup=0x1, id=4108, flag=0x0002, action=0x1, lan=yes, lb=yes, count=3, mac=01:00:5e:00:00:01
lkup=0x1, id=6156, flag=0x0002, action=0x1, lan=yes, lb=yes, count=3, mac=33:33:00:00:00:01
lkup=0x1, id=8208, flag=0x0002, action=0x0, lan=no, lb=yes, count=1, mac=22:0a:5b:f0:75:00
lkup=0x1, id=22538, flag=0x0002, action=0x0, lan=no, lb=yes, count=1, mac=ba:d1:81:0b:64:f2
lkup=0x1, id=18455, flag=0x0002, action=0x0, lan=no, lb=yes, count=1, mac=d6:3b:b5:df:a9:13
lkup=0x1, id=2056, flag=0x0002, action=0x1, lan=yes, lb=yes, count=3, mac=ff:ff:ff:ff:ff:ff
lkup=0x4, id=26632, flag=0x0002, action=0x1, lan=yes, lb=no, count=3, tpid=0x8100, valid=yes, vlan=0
lkup=0x4, id=9, flag=0x0002, action=0x1, lan=yes, lb=no, count=3, tpid=0x0000, valid=yes, vlan=0

I could RFC it here or on e1000 if it seems useful.  Otherwise, one
could enable and pay (very close) attention to 0x02A[01] commands.

I'll try to write something to clear it up in the commit message.

>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
>> b/drivers/net/ethernet/intel/ice/ice_main.c
>> index 0b6175ade40d..661af039bf4f 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -8147,17 +8148,38 @@ ice_bridge_setlink(struct net_device *dev,
>> struct nlmsghdr *nlh,
>>   		/* Update the unicast switch filter rules for the
>> corresponding
>>   		 * switch of the netdev
>>   		 */
>> -		err = ice_update_sw_rule_bridge_mode(hw);
>> +		err = ice_update_sw_rule_bridge_mode(hw,
>> ICE_SW_LKUP_MAC);
>>   		if (err) {
>> -			netdev_err(dev, "switch rule update failed, mode
>> = %d err %d aq_err %s\n",
>> -				   mode, err,
>> +			/* evb_veb is expected to be already reverted in
>> error
>> +			 * path because of the potential rollback.
>> +			 */
>> +			hw->evb_veb = old_evb_veb;
>> +			goto err_without_rollback;
>> +		}
>> +		err = ice_update_sw_rule_bridge_mode(hw,
>> ICE_SW_LKUP_MAC_VLAN);
>> +		if (err) {
>> +			/* ice_update_sw_rule_bridge_mode looks this up,
>> so we
>> +			 * must revert it before attempting a rollback.
>> +			 */
>> +			hw->evb_veb = old_evb_veb;
>> +			goto err_rollback_mac;
>> +		}
>> +		pf_sw->bridge_mode = mode;
>> +		continue;
>> +
>> +err_rollback_mac:
>> +		rb_err = ice_update_sw_rule_bridge_mode(hw,
>> ICE_SW_LKUP_MAC);
>> +		if (rb_err) {
>> +			netdev_err(dev, "switch rule update failed, mode
>> = %d err %d; rollback failed, err %d aq_err %s\n",
>> +				   mode, err, rb_err,
>>   				   libie_aq_str(hw-
>>> adminq.sq_last_status));
>> -			/* revert hw->evb_veb */
>> -			hw->evb_veb = (pf_sw->bridge_mode ==
>> BRIDGE_MODE_VEB);
>> -			return err;
>> +			return rb_err;
> On rollback failure you now return `rb_err` instead of the original `err`.
> This is a visible semantic change.
> Please justify it in the commit message (and confirm callers expect rollback status rather than the original failure).

Agreed.  I'll see if function documentation would need a refresh for
this, too.

Thanks!