[PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch

Jakub Slepecki posted 8 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch
Posted by Jakub Slepecki 2 months, 1 week ago
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;

Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>

---
Dropping reviewed-by from Michał due to changes.

Changes in v2:
  - Use FIELD_GET et al. when handling fi.lb_en and fi.lan_en.
  - Rename /LB_LAN/s/_MASK/_M/ because one of uses would need to break
    line.
---
 drivers/net/ethernet/intel/ice/ice_switch.c | 21 +++++++++++++--------
 drivers/net/ethernet/intel/ice/ice_switch.h |  8 ++++++++
 2 files changed, 21 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..b3f5cda1571e 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 (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lb_en))
+		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lb_en, lb_en);
+	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lan_en))
+		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &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 (FIELD_GET(ICE_FLTR_INFO_LB_LAN_VALUE_M, f_info->lb_en))
 		act |= ICE_SINGLE_ACT_LB_ENABLE;
-	if (f_info->lan_en)
+	if (FIELD_GET(ICE_FLTR_INFO_LB_LAN_VALUE_M, f_info->lan_en))
 		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..b694c131ad58 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.h
+++ b/drivers/net/ethernet/intel/ice/ice_switch.h
@@ -72,6 +72,14 @@ enum ice_src_id {
 	ICE_SRC_ID_LPORT,
 };
 
+#define ICE_FLTR_INFO_LB_LAN_VALUE_M BIT(0)
+#define ICE_FLTR_INFO_LB_LAN_FORCE_M BIT(1)
+#define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED			 \
+	(FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_VALUE_M, true) |  \
+	 FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, true))
+#define ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED			 \
+	(FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, true))
+
 struct ice_fltr_info {
 	/* Look up information: how to look up packet */
 	enum ice_sw_lkup_type lkup_type;
-- 
2.43.0

RE: [PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch
Posted by Loktionov, Aleksandr 2 months, 1 week ago

> -----Original Message-----
> From: Slepecki, Jakub <jakub.slepecki@intel.com>
> Sent: Tuesday, November 25, 2025 9:35 AM
> 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>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>
> Subject: [PATCH iwl-next v2 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;
> 
> Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
> 
> ---
> Dropping reviewed-by from Michał due to changes.
> 
> Changes in v2:
>   - Use FIELD_GET et al. when handling fi.lb_en and fi.lan_en.
>   - Rename /LB_LAN/s/_MASK/_M/ because one of uses would need to break
>     line.
> ---

...
 
>  	if (fi->flag & ICE_FLTR_TX_ONLY)
> -		fi->lan_en = false;
> +		lan_en = false;
> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lb_en))
> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lb_en,
> lb_en);
> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lan_en))
> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lan_en,
fi->lb_en and fi->lan_en are declared as bool in struct ice_fltr_info, but you are now treating them as bitfields using FIELD_GET and FIELD_MODIFY.

I realize it could be something like:
struct ice_fltr_info {
    ...
    u8 lb_lan_flags; /* bitfield: value + force */
    ...
};
#define ICE_FLTR_INFO_LB_LAN_VALUE_M    BIT(0)
#define ICE_FLTR_INFO_LB_LAN_FORCE_M    BIT(1)
#define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED \
    (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_VALUE_M, 1) | \
     FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1))
#define ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED \
    (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1))

...
Re: [PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch
Posted by Jakub Slepecki 2 months, 1 week ago
On 2025-11-25 9:59, Loktionov, Aleksandr wrote:
>>   	if (fi->flag & ICE_FLTR_TX_ONLY)
>> -		fi->lan_en = false;
>> +		lan_en = false;
>> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lb_en))
>> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lb_en, lb_en);
>> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lan_en))
>> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lan_en, lan_en);
> fi->lb_en and fi->lan_en are declared as bool in struct ice_fltr_info,
> but you are now treating them as bitfields using FIELD_GET and
> FIELD_MODIFY.

I don't see what you mean here.  Both members are u8 without a bit-field
declaration.  Or do you mean they are used as bool or maybe the _en
suffix?

> I realize it could be something like:
> struct ice_fltr_info {
>      ...
>      u8 lb_lan_flags; /* bitfield: value + force */
>      ...
> };

What I see from this sample is that you want me to: pack them, change
their name, and change their description.  Is this correct?

I fully agree about the description.  It's my mistake I left it as-is.
I'll update it according to the overall changes.

I don't think packing them is worth it.  The memory gain is negligible
and the cost is primarily in readability and consistency: we've always
had two fields for these with clear responsibility for each, names
match with datasheet (both "lan en" and "lb en" will hit Table 7-12.),
and packing them would require twice as many constants.

Would the clarification in the description be enough to address your
concerns?  Something like (please ignore bad line breaks):

struct ice_fltr_info {
	...
	/* Rule creation will populate VALUE bit of these members based on switch
	 * type if their FORCE bit is not set.
	 */
	u8 lb_en;	/* VALUE bit: packet can be looped back */
	u8 lan_en;	/* VALUE bit: packet can be forwarded to uplink */
};

> #define ICE_FLTR_INFO_LB_LAN_VALUE_M    BIT(0)
> #define ICE_FLTR_INFO_LB_LAN_FORCE_M    BIT(1)
> #define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED \
>      (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_VALUE_M, 1) | \
>       FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1))
> #define ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED \
>      (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1))

Does this mean you want me to use {1,0} instead of {true,false}?

In ice_fill_sw_info() I'd prefer to keep them as boolean because they are
semantically correct: we're calculating defaults and then we apply them if
specific values are not forced elsewhere.  Maybe a comment or docs change
would be more in place?  In ICE_FLTR_INFO_LB_LAN_FORCE_{ENABLED,DISABLED},
I used boolean to stay consistent with the ice_fill_sw_info().

But it's not a strong preference.  If it's preferable, I'll change it
to {1,0} across the patch.

Thanks!
RE: [PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch
Posted by Loktionov, Aleksandr 2 months, 1 week ago

> -----Original Message-----
> From: Slepecki, Jakub <jakub.slepecki@intel.com>
> Sent: Friday, November 28, 2025 12:56 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; 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
> Subject: Re: [PATCH iwl-next v2 4/8] ice: allow overriding lan_en,
> lb_en in switch
> 
> On 2025-11-25 9:59, Loktionov, Aleksandr wrote:
> >>   	if (fi->flag & ICE_FLTR_TX_ONLY)
> >> -		fi->lan_en = false;
> >> +		lan_en = false;
> >> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lb_en))
> >> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lb_en,
> lb_en);
> >> +	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lan_en))
> >> +		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lan_en,
> lan_en);
> > fi->lb_en and fi->lan_en are declared as bool in struct
> ice_fltr_info,
> > but you are now treating them as bitfields using FIELD_GET and
> > FIELD_MODIFY.
> 
> I don't see what you mean here.  Both members are u8 without a bit-
> field declaration.  Or do you mean they are used as bool or maybe the
> _en suffix?
> 
> > I realize it could be something like:
> > struct ice_fltr_info {
> >      ...
> >      u8 lb_lan_flags; /* bitfield: value + force */
> >      ...
> > };
> 
> What I see from this sample is that you want me to: pack them, change
> their name, and change their description.  Is this correct?
> 
> I fully agree about the description.  It's my mistake I left it as-is.
> I'll update it according to the overall changes.
> 
> I don't think packing them is worth it.  The memory gain is negligible
> and the cost is primarily in readability and consistency: we've always
> had two fields for these with clear responsibility for each, names
> match with datasheet (both "lan en" and "lb en" will hit Table 7-12.),
> and packing them would require twice as many constants.
> 
> Would the clarification in the description be enough to address your
> concerns?  Something like (please ignore bad line breaks):
> 
> struct ice_fltr_info {
> 	...
> 	/* Rule creation will populate VALUE bit of these members based
> on switch
> 	 * type if their FORCE bit is not set.
> 	 */
> 	u8 lb_en;	/* VALUE bit: packet can be looped back */
> 	u8 lan_en;	/* VALUE bit: packet can be forwarded to uplink
> */
> };
> 
> > #define ICE_FLTR_INFO_LB_LAN_VALUE_M    BIT(0)
> > #define ICE_FLTR_INFO_LB_LAN_FORCE_M    BIT(1)
> > #define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED \
> >      (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_VALUE_M, 1) | \
> >       FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1)) #define
> > ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED \
> >      (FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, 1))
> 
> Does this mean you want me to use {1,0} instead of {true,false}?
> 
> In ice_fill_sw_info() I'd prefer to keep them as boolean because they
> are semantically correct: we're calculating defaults and then we apply
> them if specific values are not forced elsewhere.  Maybe a comment or
> docs change would be more in place?  In
> ICE_FLTR_INFO_LB_LAN_FORCE_{ENABLED,DISABLED},
> I used boolean to stay consistent with the ice_fill_sw_info().
> 
> But it's not a strong preference.  If it's preferable, I'll change it
> to {1,0} across the patch.
> 
> Thanks!

For u8 fields if they are used as u8 value (not bit fields) using FIELD_ macros not good.
What about compromise:
 - Keep lan_en and lb_en as bool or u8 with clear comments.
 - Do NOT use FIELD macros unless these fields are truly packed bitfields.
 - If FORCE/ VALUE semantics are needed, either:
   +Introduce a dedicated flags field with proper bitmask macros, OR
   +Keep separate fields and handle FORCE logic explicitly in code without FIELD macros.

And handle FORCE logic explicitly:

if (!force_lb)
    fi->lb_en = lb_en;
if (!force_lan)
    fi->lan_en = lan_en;

?


Re: [PATCH iwl-next v2 4/8] ice: allow overriding lan_en, lb_en in switch
Posted by Jakub Slepecki 2 months ago
On 2025-12-01 8:37, Loktionov, Aleksandr wrote:
> For u8 fields if they are used as u8 value (not bit fields) using FIELD_ macros not good.
> What about compromise:
>   - Keep lan_en and lb_en as bool or u8 with clear comments.
>   - Do NOT use FIELD macros unless these fields are truly packed bitfields.

After patch, lan_en and lb_en are always dealt with via FIELD_ macros.
The confusing part could be in ice_fill_sw_info():

+	bool lan_en = false;
+	bool lb_en = false;

Where throughout the function we decide on VALUE for each (stored as bool
lan_en and bool lb_en), and only then we apply it:

+	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lb_en))
+		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lb_en, lb_en);
+	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, fi->lan_en))
+		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &fi->lan_en, lan_en);

I could tweak names of the variables or maybe hold them as u8:

static void ice_fill_sw_info(struct ice_hw *hw, struct ice_fltr_info *fi)
{
	u8 lan_en = fi->lan_en;
	u8 lb_en = fi->lb_en;
	...
		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &lb_en, true);
	...
	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, lb_en))
		fi->lb_en = lb_en;
}

Or s/true/1/g per previous discussion.

This seems better at expressing the "tmp -> decide -> commit" than the
current version.  See draft patch at the bottom.

>   - If FORCE/ VALUE semantics are needed, either:
>     +Introduce a dedicated flags field with proper bitmask macros, OR

I addressed this option in my previous response.  Let's not exclude
it yet, but since there are still some alternatives I would prefer to
avoid it.

>     +Keep separate fields and handle FORCE logic explicitly in code without FIELD macros.
> 
> And handle FORCE logic explicitly:
> 
> if (!force_lb)
>      fi->lb_en = lb_en;
> if (!force_lan)
>      fi->lan_en = lan_en;
> 
> ?

I intend to force values in 8/8 in ice_fltr.c in
ice_fltr_add_macs_to_list().  Decision is made based on:

1. whether MAC is multicast or unicast,
2. whether VSI has PVID, and
3. whether VSI has VLAN 0.

There are also implicit conditions, because ice_fltr_add_macs_to_list()
is only called in particular cases after some expected changes in overall
driver state; compared to ice_fill_sw_info():

4. filter is being created,
5. filter is MAC or MAC,VLAN,
6. target is guaranteed to be a single VSI, and
7. VLAN filters are guaranteed to be already created.

I'm reluctant to move decision-making to ice_fill_sw_info(), because
of these implicit conditions (and possibly more; each would need to
be proven).  Overall, I see two meaningful options (with some variants):

a. In ice_fill_sw_info(), reconstruct all needed information (1-3 from
    *hw + *fi) and make the decision,
b. Before ice_fill_sw_info(), make the decision and store it, then use
    the result in ice_fill_sw_info().

FORCE is (b).  I chose it because it seemed to be overall cheapest
(LOC, memory use, number of operations) and (subjective) semantically
correct (ice_fltr.c is responsible for requesting filters for a VSI;
ice_fill_sw_info() is responsible for populating defaults).

We could also go all the way in the other direction:

struct ice_fltr_info {
	...
	bool lan_en;
	bool force_lan_en;
	bool lb_en;
	bool force_lb_en;
};

? (or s/bool/u8/)

Current working draft:
-- >8 --
Subject: [PATCH iwl-next v2 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;

Signed-off-by: Jakub Slepecki <jakub.slepecki@intel.com>
---
  drivers/net/ethernet/intel/ice/ice_switch.c | 27 ++++++++++++++-------
  drivers/net/ethernet/intel/ice/ice_switch.h | 19 ++++++++++++---
  2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 04e5d653efce..3896edaa8652 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -2534,12 +2534,14 @@ int ice_get_initial_sw_cfg(struct ice_hw *hw)
   *
   * This helper function populates the lb_en and lan_en elements of the provided
   * ice_fltr_info struct using the switch's type and characteristics of the
- * switch rule being configured.
+ * switch rule being configured.  Elements are updated only if their FORCE bit
+ * is not set.
   */
  static void ice_fill_sw_info(struct ice_hw *hw, struct ice_fltr_info *fi)
  {
-	fi->lb_en = false;
-	fi->lan_en = false;
+	u8 lan_en = fi->lan_en;
+	u8 lb_en = fi->lb_en;
+
  	if ((fi->flag & ICE_FLTR_TX) &&
  	    (fi->fltr_act == ICE_FWD_TO_VSI ||
  	     fi->fltr_act == ICE_FWD_TO_VSI_LIST ||
@@ -2549,7 +2551,8 @@ 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;
+			FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &lb_en,
+				     true);
  
  		/* Set lan_en to TRUE if
  		 * 1. The switch is a VEB AND
@@ -2578,14 +2581,20 @@ 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;
+				FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M,
+					     &lan_en, true);
  		} else {
-			fi->lan_en = true;
+			FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &lan_en,
+				     true);
  		}
  	}
  
  	if (fi->flag & ICE_FLTR_TX_ONLY)
-		fi->lan_en = false;
+		FIELD_MODIFY(ICE_FLTR_INFO_LB_LAN_VALUE_M, &lan_en, false);
+	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, lb_en))
+		fi->lb_en = lb_en;
+	if (!FIELD_GET(ICE_FLTR_INFO_LB_LAN_FORCE_M, lan_en))
+		fi->lan_en = lan_en;
  }
  
  /**
@@ -2669,9 +2678,9 @@ ice_fill_sw_rule(struct ice_hw *hw, struct ice_fltr_info *f_info,
  		return;
  	}
  
-	if (f_info->lb_en)
+	if (FIELD_GET(ICE_FLTR_INFO_LB_LAN_VALUE_M, f_info->lb_en))
  		act |= ICE_SINGLE_ACT_LB_ENABLE;
-	if (f_info->lan_en)
+	if (FIELD_GET(ICE_FLTR_INFO_LB_LAN_VALUE_M, f_info->lan_en))
  		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..e421c562626c 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.h
+++ b/drivers/net/ethernet/intel/ice/ice_switch.h
@@ -72,6 +72,14 @@ enum ice_src_id {
  	ICE_SRC_ID_LPORT,
  };
  
+#define ICE_FLTR_INFO_LB_LAN_VALUE_M BIT(0)
+#define ICE_FLTR_INFO_LB_LAN_FORCE_M BIT(1)
+#define ICE_FLTR_INFO_LB_LAN_FORCE_ENABLED			 \
+	(FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_VALUE_M, true) |  \
+	 FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, true))
+#define ICE_FLTR_INFO_LB_LAN_FORCE_DISABLED			 \
+	(FIELD_PREP_CONST(ICE_FLTR_INFO_LB_LAN_FORCE_M, true))
+
  struct ice_fltr_info {
  	/* Look up information: how to look up packet */
  	enum ice_sw_lkup_type lkup_type;
@@ -131,9 +139,14 @@ struct ice_fltr_info {
  	 */
  	u8 qgrp_size;
  
-	/* Rule creations populate these indicators basing on the switch type */
-	u8 lb_en;	/* Indicate if packet can be looped back */
-	u8 lan_en;	/* Indicate if packet can be forwarded to the uplink */
+	/* Following members have two bits: VALUE and FORCE.  Rule creation will
+	 * populate VALUE bit of these members based on switch type, but only if
+	 * their FORCE bit is not set.
+	 *
+	 * See ICE_FLTR_INFO_LB_LAN_VALUE_M and ICE_FLTR_INFO_LB_LAN_FORCE_M.
+	 */
+	u8 lb_en;	/* VALUE bit: packet can be looped back */
+	u8 lan_en;	/* VALUE bit: packet can be forwarded to the uplink */
  };
  
  struct ice_update_recipe_lkup_idx_params {
-- 
2.43.0