[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA3PR11MB89862D16B3E71FCB8DF2113EE5D5A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Fri, 21 Nov 2025 08:54:32 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: "Slepecki, Jakub" <jakub.slepecki@...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>, "Slepecki, Jakub"
<jakub.slepecki@...el.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-next 5/8] ice: update mac, vlan
rules when toggling between VEB and VEPA
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf
> Of Jakub Slepecki
> Sent: Thursday, November 20, 2025 5:28 PM
> To: intel-wired-lan@...ts.osuosl.org
> Cc: linux-kernel@...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;
> Slepecki, Jakub <jakub.slepecki@...el.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@...ux.intel.com>
> Signed-off-by: Jakub Slepecki <jakub.slepecki@...el.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
Powered by blists - more mailing lists