| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <IA3PR11MB89860CA2EBCCEDDF0C9E05D9E5D5A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Fri, 21 Nov 2025 09:21:11 +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 4/8] ice: allow overriding
lan_en, lb_en in switch
> -----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 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@...ux.intel.com>
> Signed-off-by: Jakub Slepecki <jakub.slepecki@...el.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
Powered by blists - more mailing lists