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

[Jakub Slepecki <jakub.slepecki@intel.com>]

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!