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.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 38 ++++++++++++++++-----
drivers/net/ethernet/intel/ice/ice_switch.c | 8 +++--
drivers/net/ethernet/intel/ice/ice_switch.h | 3 +-
3 files changed, 37 insertions(+), 12 deletions(-)
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
@@ -8115,8 +8115,8 @@ ice_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
struct ice_pf *pf = ice_netdev_to_pf(dev);
struct nlattr *attr, *br_spec;
struct ice_hw *hw = &pf->hw;
+ int rem, v, rb_err, err = 0;
struct ice_sw *pf_sw;
- int rem, v, err = 0;
pf_sw = pf->first_sw;
/* find the attribute in the netlink message */
@@ -8126,6 +8126,7 @@ ice_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
nla_for_each_nested_type(attr, IFLA_BRIDGE_MODE, br_spec, rem) {
__u16 mode = nla_get_u16(attr);
+ u8 old_evb_veb = hw->evb_veb;
if (mode != BRIDGE_MODE_VEPA && mode != BRIDGE_MODE_VEB)
return -EINVAL;
@@ -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;
}
- pf_sw->bridge_mode = mode;
+err_without_rollback:
+ netdev_err(dev, "switch rule update failed, mode = %d err %d aq_err %s\n",
+ mode, err, libie_aq_str(hw->adminq.sq_last_status));
+ return err;
}
return 0;
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 7b63588948fd..b1445dfb1b64 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -3065,10 +3065,12 @@ ice_update_pkt_fwd_rule(struct ice_hw *hw, struct ice_fltr_info *f_info)
/**
* ice_update_sw_rule_bridge_mode
* @hw: pointer to the HW struct
+ * @lkup: recipe/lookup type to update
*
* Updates unicast switch filter rules based on VEB/VEPA mode
*/
-int ice_update_sw_rule_bridge_mode(struct ice_hw *hw)
+int ice_update_sw_rule_bridge_mode(struct ice_hw *hw,
+ enum ice_sw_lkup_type lkup)
{
struct ice_switch_info *sw = hw->switch_info;
struct ice_fltr_mgmt_list_entry *fm_entry;
@@ -3076,8 +3078,8 @@ int ice_update_sw_rule_bridge_mode(struct ice_hw *hw)
struct mutex *rule_lock; /* Lock to protect filter rule list */
int status = 0;
- rule_lock = &sw->recp_list[ICE_SW_LKUP_MAC].filt_rule_lock;
- rule_head = &sw->recp_list[ICE_SW_LKUP_MAC].filt_rules;
+ rule_lock = &sw->recp_list[lkup].filt_rule_lock;
+ rule_head = &sw->recp_list[lkup].filt_rules;
mutex_lock(rule_lock);
list_for_each_entry(fm_entry, rule_head, list_entry) {
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.h b/drivers/net/ethernet/intel/ice/ice_switch.h
index a7dc4bfec3a0..60527475959b 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.h
+++ b/drivers/net/ethernet/intel/ice/ice_switch.h
@@ -360,7 +360,8 @@ int
ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
u16 lkups_cnt, struct ice_adv_rule_info *rinfo,
struct ice_rule_query_data *added_entry);
-int ice_update_sw_rule_bridge_mode(struct ice_hw *hw);
+int ice_update_sw_rule_bridge_mode(struct ice_hw *hw,
+ enum ice_sw_lkup_type lkup);
int ice_add_vlan(struct ice_hw *hw, struct list_head *m_list);
int ice_remove_vlan(struct ice_hw *hw, struct list_head *v_list);
int ice_add_mac(struct ice_hw *hw, struct list_head *m_lst);
--
2.43.0
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jakub Slepecki
> Sent: Thursday, November 20, 2025 5:28 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; michal.swiatkowski@linux.intel.com;
> Slepecki, Jakub <jakub.slepecki@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next 5/8] ice: update mac, vlan
> rules when toggling between VEB and VEPA
>
> 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.
>
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 38 ++++++++++++++++----
> -
> drivers/net/ethernet/intel/ice/ice_switch.c | 8 +++--
> drivers/net/ethernet/intel/ice/ice_switch.h | 3 +-
> 3 files changed, 37 insertions(+), 12 deletions(-)
>
> 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
> @@ -8115,8 +8115,8 @@ ice_bridge_setlink(struct net_device *dev,
> struct nlmsghdr *nlh,
> struct ice_pf *pf = ice_netdev_to_pf(dev);
> struct nlattr *attr, *br_spec;
> struct ice_hw *hw = &pf->hw;
> + int rem, v, rb_err, err = 0;
> struct ice_sw *pf_sw;
> - int rem, v, err = 0;
>
> pf_sw = pf->first_sw;
> /* find the attribute in the netlink message */ @@ -8126,6
> +8126,7 @@ ice_bridge_setlink(struct net_device *dev, struct nlmsghdr
> *nlh,
>
> nla_for_each_nested_type(attr, IFLA_BRIDGE_MODE, br_spec, rem)
> {
> __u16 mode = nla_get_u16(attr);
> + u8 old_evb_veb = hw->evb_veb;
>
> if (mode != BRIDGE_MODE_VEPA && mode != BRIDGE_MODE_VEB)
> return -EINVAL;
> @@ -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).
> }
>
> - pf_sw->bridge_mode = mode;
> +err_without_rollback:
> + netdev_err(dev, "switch rule update failed, mode = %d
> err %d aq_err %s\n",
> + mode, err, libie_aq_str(hw-
> >adminq.sq_last_status));
> + return err;
> }
>
> return 0;
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c
> b/drivers/net/ethernet/intel/ice/ice_switch.c
> index 7b63588948fd..b1445dfb1b64 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> @@ -3065,10 +3065,12 @@ ice_update_pkt_fwd_rule(struct ice_hw *hw,
> struct ice_fltr_info *f_info)
> /**
> * ice_update_sw_rule_bridge_mode
> * @hw: pointer to the HW struct
> + * @lkup: recipe/lookup type to update
> *
> * Updates unicast switch filter rules based on VEB/VEPA mode
> */
> -int ice_update_sw_rule_bridge_mode(struct ice_hw *hw)
> +int ice_update_sw_rule_bridge_mode(struct ice_hw *hw,
> + enum ice_sw_lkup_type lkup)
> {
> struct ice_switch_info *sw = hw->switch_info;
> struct ice_fltr_mgmt_list_entry *fm_entry; @@ -3076,8 +3078,8
> @@ int ice_update_sw_rule_bridge_mode(struct ice_hw *hw)
> struct mutex *rule_lock; /* Lock to protect filter rule list */
> int status = 0;
>
> - rule_lock = &sw->recp_list[ICE_SW_LKUP_MAC].filt_rule_lock;
> - rule_head = &sw->recp_list[ICE_SW_LKUP_MAC].filt_rules;
> + rule_lock = &sw->recp_list[lkup].filt_rule_lock;
> + rule_head = &sw->recp_list[lkup].filt_rules;
>
> mutex_lock(rule_lock);
> list_for_each_entry(fm_entry, rule_head, list_entry) { diff --
> git a/drivers/net/ethernet/intel/ice/ice_switch.h
> b/drivers/net/ethernet/intel/ice/ice_switch.h
> index a7dc4bfec3a0..60527475959b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.h
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.h
> @@ -360,7 +360,8 @@ int
> ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
> u16 lkups_cnt, struct ice_adv_rule_info *rinfo,
> struct ice_rule_query_data *added_entry); -int
> ice_update_sw_rule_bridge_mode(struct ice_hw *hw);
> +int ice_update_sw_rule_bridge_mode(struct ice_hw *hw,
> + enum ice_sw_lkup_type lkup);
> int ice_add_vlan(struct ice_hw *hw, struct list_head *m_list); int
> ice_remove_vlan(struct ice_hw *hw, struct list_head *v_list); int
> ice_add_mac(struct ice_hw *hw, struct list_head *m_lst);
> --
> 2.43.0
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!
© 2016 - 2025 Red Hat, Inc.