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 <michal.swiatkowski@linux.intel.com>
Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
---
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;
}
/**
@@ -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
> -----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 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 <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
> ---
> 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
On 2025-11-21 10:21, Loktionov, Aleksandr wrote:
>> -----Original Message-----
>> 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 ?
I agree this can look strange. lb_en and lan_en are both u8 in
ice_switch.h:/^struct ice_fltr_info/ and we assign them from bool.
Before, even though we had the same implicit conversion bool -> u8 we
did not use either of u8s to hold anything else.
> 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?
The latter. Original intention, one of, was to avoid implying this
can be extended, because it should not: for better customization we
have "advanced" rules, and "simple" rules shouldn't try to chase them.
Instead, porting everything to "advanced" rules would be more reasonable.
I make an exception here, because cost of any other option is way higher.
That being said, I don't see any reason to not use
FIELD_{GET,PREP,MODIFY}. I will modify this accordingly across the
series.
Thanks!
>> }
>>
>> /**
© 2016 - 2025 Red Hat, Inc.