> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf
> Of Jakub Slepecki
> Sent: Thursday, November 20, 2025 5:28 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; Kitszel,
> Przemyslaw <[email protected]>; Nguyen, Anthony L
> <[email protected]>; [email protected];
> Slepecki, Jakub <[email protected]>
> Subject: [Intel-wired-lan] [PATCH iwl-next 4/8] ice: allow overriding
> lan_en, lb_en in switch
>
> Currently, lan_en and lb_en are determined based on switching mode,
> destination MAC, and the lookup type, action type and flags of the
> rule in question. This gives little to no options for the user (such
> as
> ice_fltr.c) to enforce rules to behave in a specific way.
>
> Such functionality is needed to work with pairs of rules, for example,
> when handling MAC forward to LAN together with MAC,VLAN forward to
> loopback rules pair. This case could not be easily deduced in a
> context of a single filter without adding a specialized flag.
>
> Instead of adding a specialized flag to mark special scenario rules,
> we add a slightly more generic flag to the lan_en and lb_en themselves
> for the ice_fltr.c to request specific destination flags later on, for
> example, to override value:
>
> struct ice_fltr_info fi;
> fi.lb_en = ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED;
> fi.lan_en = ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED;
>
> Reviewed-by: Michal Swiatkowski <[email protected]>
> Signed-off-by: Jakub Slepecki <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_switch.c | 21 +++++++++++++-------
> - drivers/net/ethernet/intel/ice/ice_switch.h | 7 +++++++
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c
> b/drivers/net/ethernet/intel/ice/ice_switch.c
> index 04e5d653efce..7b63588948fd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> @@ -2538,8 +2538,9 @@ int ice_get_initial_sw_cfg(struct ice_hw *hw)
> */
> static void ice_fill_sw_info(struct ice_hw *hw, struct ice_fltr_info
> *fi) {
> - fi->lb_en = false;
> - fi->lan_en = false;
> + bool lan_en = false;
> + bool lb_en = false;
> +
> if ((fi->flag & ICE_FLTR_TX) &&
> (fi->fltr_act == ICE_FWD_TO_VSI ||
> fi->fltr_act == ICE_FWD_TO_VSI_LIST || @@ -2549,7 +2550,7
> @@ static void ice_fill_sw_info(struct ice_hw *hw, struct
> ice_fltr_info *fi)
> * packets to the internal switch that will be dropped.
> */
> if (fi->lkup_type != ICE_SW_LKUP_VLAN)
> - fi->lb_en = true;
> + lb_en = true;
>
> /* Set lan_en to TRUE if
> * 1. The switch is a VEB AND
> @@ -2578,14 +2579,18 @@ static void ice_fill_sw_info(struct ice_hw
> *hw, struct ice_fltr_info *fi)
> !is_unicast_ether_addr(fi-
> >l_data.mac.mac_addr)) ||
> (fi->lkup_type == ICE_SW_LKUP_MAC_VLAN &&
> !is_unicast_ether_addr(fi-
> >l_data.mac.mac_addr)))
> - fi->lan_en = true;
> + lan_en = true;
> } else {
> - fi->lan_en = true;
> + lan_en = true;
> }
> }
>
> if (fi->flag & ICE_FLTR_TX_ONLY)
> - fi->lan_en = false;
> + lan_en = false;
> + if (!(fi->lb_en & ICE_FLTR_INFO_LB_LAN_FORCE_MASK))
> + fi->lb_en = lb_en;
> + if (!(fi->lan_en & ICE_FLTR_INFO_LB_LAN_FORCE_MASK))
> + fi->lan_en = lan_en;
For me it looks strange.
What type the fi->lb_en has?
fi->lb_en declared as bool, and you assign fi->lan_en from bool.
But you check condition by fi->lan_en & ICE_FLTR_INFO_LB_LAN_FORCE_MASK ?
It rases questions if fi->lan_en a bool why use fi->lan_en &
ICE_FLTR_INFO_LB_LAN_FORCE_MASK then?
And if fi->lan_en is a bitmask why not use
FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_MASK, fi->lan_en) and
why not something like:
if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_MASK, fi->lan_en))
FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_MASK, &fi->lan_en, lan_en);
It could preserve unrelated bits (like FORCE) and make the code resilient to
future changes in bit positions?
> }
>
> /**
> @@ -2669,9 +2674,9 @@ ice_fill_sw_rule(struct ice_hw *hw, struct
> ice_fltr_info *f_info,
> return;
> }
>
> - if (f_info->lb_en)
> + if (f_info->lb_en & ICE_FLTR_INFO_LB_LAN_VALUE_MASK)
> act |= ICE_SINGLE_ACT_LB_ENABLE;
> - if (f_info->lan_en)
> + if (f_info->lan_en & ICE_FLTR_INFO_LB_LAN_VALUE_MASK)
> act |= ICE_SINGLE_ACT_LAN_ENABLE;
>
> switch (f_info->lkup_type) {
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.h
> b/drivers/net/ethernet/intel/ice/ice_switch.h
> index 671d7a5f359f..a7dc4bfec3a0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.h
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.h
> @@ -72,6 +72,13 @@ enum ice_src_id {
> ICE_SRC_ID_LPORT,
> };
>
> +#define ICE_FLTR_INFO_LB_LAN_VALUE_MASK BIT(0) #define
> +ICE_FLTR_INFO_LB_LAN_FORCE_MASK BIT(1)
> +#define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED \
> + (ICE_FLTR_INFO_LB_LAN_VALUE_MASK | \
> + ICE_FLTR_INFO_LB_LAN_FORCE_MASK)
> +#define ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED
> +ICE_FLTR_INFO_LB_LAN_FORCE_MASK
> +
> struct ice_fltr_info {
> /* Look up information: how to look up packet */
> enum ice_sw_lkup_type lkup_type;
> --
> 2.43.0
