diff mbox series

[iwl-net] ice: do not configure destination override for switchdev

Message ID 20241209140856.277801-1-larysa.zaremba@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [iwl-net] ice: do not configure destination override for switchdev | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 2 this patch: 2
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 58 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 78 this patch: 78
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-09--15-00 (tests: 762)

Commit Message

Larysa Zaremba Dec. 9, 2024, 2:08 p.m. UTC
After switchdev is enabled and disabled later, LLDP packets sending stops,
despite working perfectly fine before and during switchdev state.
To reproduce (creating/destroying VF is what triggers the reconfiguration):

devlink dev eswitch set pci/<address> mode switchdev
echo '2' > /sys/class/net/<ifname>/device/sriov_numvfs
echo '0' > /sys/class/net/<ifname>/device/sriov_numvfs

This happens because LLDP relies on the destination override functionality.
It needs to 1) set a flag in the descriptor, 2) set the VSI permission to
make it valid. The permissions are set when the PF VSI is first configured,
but switchdev then enables it for the uplink VSI (which is always the PF)
once more when configured and disables when deconfigured, which leads to
software-generated LLDP packets being blocked.

Do not modify the destination override permissions when configuring
switchdev, as the enabled state is the default configuration that is never
modified.

Fixes: 1a1c40df2e80 ("ice: set and release switchdev environment")
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_eswitch.c |  6 ------
 drivers/net/ethernet/intel/ice/ice_lib.c     | 18 ------------------
 drivers/net/ethernet/intel/ice/ice_lib.h     |  4 ----
 3 files changed, 28 deletions(-)

Comments

Simon Horman Dec. 10, 2024, 5:08 p.m. UTC | #1
On Mon, Dec 09, 2024 at 03:08:53PM +0100, Larysa Zaremba wrote:
> After switchdev is enabled and disabled later, LLDP packets sending stops,
> despite working perfectly fine before and during switchdev state.
> To reproduce (creating/destroying VF is what triggers the reconfiguration):
> 
> devlink dev eswitch set pci/<address> mode switchdev
> echo '2' > /sys/class/net/<ifname>/device/sriov_numvfs
> echo '0' > /sys/class/net/<ifname>/device/sriov_numvfs
> 
> This happens because LLDP relies on the destination override functionality.
> It needs to 1) set a flag in the descriptor, 2) set the VSI permission to
> make it valid. The permissions are set when the PF VSI is first configured,
> but switchdev then enables it for the uplink VSI (which is always the PF)
> once more when configured and disables when deconfigured, which leads to
> software-generated LLDP packets being blocked.
> 
> Do not modify the destination override permissions when configuring
> switchdev, as the enabled state is the default configuration that is never
> modified.
> 
> Fixes: 1a1c40df2e80 ("ice: set and release switchdev environment")
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>
Buvaneswaran, Sujai Jan. 6, 2025, 7:51 a.m. UTC | #2
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Larysa Zaremba
> Sent: Monday, December 9, 2024 7:39 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> Eric Dumazet <edumazet@google.com>; Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: do not configure destination
> override for switchdev
> 
> After switchdev is enabled and disabled later, LLDP packets sending stops,
> despite working perfectly fine before and during switchdev state.
> To reproduce (creating/destroying VF is what triggers the reconfiguration):
> 
> devlink dev eswitch set pci/<address> mode switchdev echo '2' >
> /sys/class/net/<ifname>/device/sriov_numvfs
> echo '0' > /sys/class/net/<ifname>/device/sriov_numvfs
> 
> This happens because LLDP relies on the destination override functionality.
> It needs to 1) set a flag in the descriptor, 2) set the VSI permission to make it
> valid. The permissions are set when the PF VSI is first configured, but
> switchdev then enables it for the uplink VSI (which is always the PF) once
> more when configured and disables when deconfigured, which leads to
> software-generated LLDP packets being blocked.
> 
> Do not modify the destination override permissions when configuring
> switchdev, as the enabled state is the default configuration that is never
> modified.
> 
> Fixes: 1a1c40df2e80 ("ice: set and release switchdev environment")
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_eswitch.c |  6 ------
>  drivers/net/ethernet/intel/ice/ice_lib.c     | 18 ------------------
>  drivers/net/ethernet/intel/ice/ice_lib.h     |  4 ----
>  3 files changed, 28 deletions(-)
> 
Hi,

Observing below call trace while creating VFs in Switchdev mode with this patch in net-queue.

[  +0.000188] ice 0000:b1:00.0: Enabling 1 VFs with 17 vectors and 16 queues per VF
[  +0.000062] list_add corruption. next->prev should be prev (ff1d7c830300c6f0), but was ff282828ff282828. (next=ff1d7c5367d61330).
[  +0.000015] ------------[ cut here ]------------
[  +0.000001] kernel BUG at lib/list_debug.c:29!
[  +0.000007] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[  +0.000004] CPU: 81 UID: 0 PID: 2758 Comm: bash Kdump: loaded Not tainted 6.13.0-rc3+ #1
[  +0.000003] Hardware name: Dell Inc. PowerEdge R750/06V45N, BIOS 1.3.8 08/31/2021
[  +0.000002] RIP: 0010:__list_add_valid_or_report+0x61/0xa0
[  +0.000008] Code: c7 c7 a8 97 b2 8f e8 7e e4 af ff 0f 0b 48 c7 c7 d0 97 b2 8f e8 70 e4 af ff 0f 0b 4c 89 c1 48 c7 c7 f8 97 b2 8f e8 5f e4 af ff <0f> 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 50 98 b2 8f e8 48 e4 af
[  +0.000002] RSP: 0018:ff5ebf3d22093d20 EFLAGS: 00010246
[  +0.000003] RAX: 0000000000000075 RBX: ff1d7c54143a1330 RCX: 0000000000000000
[  +0.000002] RDX: 0000000000000000 RSI: ff1d7c81f06a0bc0 RDI: ff1d7c81f06a0bc0
[  +0.000001] RBP: ff1d7c83030097d8 R08: 0000000000000000 R09: ff5ebf3d22093bd8
[  +0.000002] R10: ff5ebf3d22093bd0 R11: ffffffff901debc8 R12: ff1d7c5367d61330
[  +0.000001] R13: ff1d7c830300c6f0 R14: 0000000000000000 R15: 0000000000000000
[  +0.000002] FS:  00007fea5e4e4740(0000) GS:ff1d7c81f0680000(0000) knlGS:0000000000000000
[  +0.000002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  +0.000001] CR2: 0000562ef57c7608 CR3: 000000019037c002 CR4: 0000000000773ef0
[  +0.000002] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  +0.000001] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  +0.000001] PKRU: 55555554
[  +0.000002] Call Trace:
[  +0.000003]  <TASK>
[  +0.000002]  ? die+0x37/0x90
[  +0.000007]  ? do_trap+0xdd/0x100
[  +0.000004]  ? __list_add_valid_or_report+0x61/0xa0
[  +0.000003]  ? do_error_trap+0x65/0x80
[  +0.000002]  ? __list_add_valid_or_report+0x61/0xa0
[  +0.000003]  ? exc_invalid_op+0x52/0x70
[  +0.000005]  ? __list_add_valid_or_report+0x61/0xa0
[  +0.000002]  ? asm_exc_invalid_op+0x1a/0x20
[  +0.000007]  ? __list_add_valid_or_report+0x61/0xa0
[  +0.000005]  ice_mbx_init_vf_info+0x3c/0x60 [ice]
[  +0.000076]  ice_initialize_vf_entry+0x99/0xa0 [ice]

Regards,
Sujai B
Larysa Zaremba Feb. 19, 2025, 11:58 a.m. UTC | #3
On Mon, Jan 06, 2025 at 08:51:40AM +0100, Buvaneswaran, Sujai wrote:
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> > Larysa Zaremba
> > Sent: Monday, December 9, 2024 7:39 PM
> > To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; intel-wired-
> > lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> > Eric Dumazet <edumazet@google.com>; Michal Swiatkowski
> > <michal.swiatkowski@linux.intel.com>; Jakub Kicinski <kuba@kernel.org>;
> > Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>
> > Subject: [Intel-wired-lan] [PATCH iwl-net] ice: do not configure destination
> > override for switchdev
> > 
> > After switchdev is enabled and disabled later, LLDP packets sending stops,
> > despite working perfectly fine before and during switchdev state.
> > To reproduce (creating/destroying VF is what triggers the reconfiguration):
> > 
> > devlink dev eswitch set pci/<address> mode switchdev echo '2' >
> > /sys/class/net/<ifname>/device/sriov_numvfs
> > echo '0' > /sys/class/net/<ifname>/device/sriov_numvfs
> > 
> > This happens because LLDP relies on the destination override functionality.
> > It needs to 1) set a flag in the descriptor, 2) set the VSI permission to make it
> > valid. The permissions are set when the PF VSI is first configured, but
> > switchdev then enables it for the uplink VSI (which is always the PF) once
> > more when configured and disables when deconfigured, which leads to
> > software-generated LLDP packets being blocked.
> > 
> > Do not modify the destination override permissions when configuring
> > switchdev, as the enabled state is the default configuration that is never
> > modified.
> > 
> > Fixes: 1a1c40df2e80 ("ice: set and release switchdev environment")
> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_eswitch.c |  6 ------
> >  drivers/net/ethernet/intel/ice/ice_lib.c     | 18 ------------------
> >  drivers/net/ethernet/intel/ice/ice_lib.h     |  4 ----
> >  3 files changed, 28 deletions(-)
> > 
> Hi,
> 
> Observing below call trace while creating VFs in Switchdev mode with this patch in net-queue.
> 
> [  +0.000188] ice 0000:b1:00.0: Enabling 1 VFs with 17 vectors and 16 queues per VF
> [  +0.000062] list_add corruption. next->prev should be prev (ff1d7c830300c6f0), but was ff282828ff282828. (next=ff1d7c5367d61330).
> [  +0.000015] ------------[ cut here ]------------
> [  +0.000001] kernel BUG at lib/list_debug.c:29!
> [  +0.000007] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> [  +0.000004] CPU: 81 UID: 0 PID: 2758 Comm: bash Kdump: loaded Not tainted 6.13.0-rc3+ #1
> [  +0.000003] Hardware name: Dell Inc. PowerEdge R750/06V45N, BIOS 1.3.8 08/31/2021
> [  +0.000002] RIP: 0010:__list_add_valid_or_report+0x61/0xa0
> [  +0.000008] Code: c7 c7 a8 97 b2 8f e8 7e e4 af ff 0f 0b 48 c7 c7 d0 97 b2 8f e8 70 e4 af ff 0f 0b 4c 89 c1 48 c7 c7 f8 97 b2 8f e8 5f e4 af ff <0f> 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 50 98 b2 8f e8 48 e4 af
> [  +0.000002] RSP: 0018:ff5ebf3d22093d20 EFLAGS: 00010246
> [  +0.000003] RAX: 0000000000000075 RBX: ff1d7c54143a1330 RCX: 0000000000000000
> [  +0.000002] RDX: 0000000000000000 RSI: ff1d7c81f06a0bc0 RDI: ff1d7c81f06a0bc0
> [  +0.000001] RBP: ff1d7c83030097d8 R08: 0000000000000000 R09: ff5ebf3d22093bd8
> [  +0.000002] R10: ff5ebf3d22093bd0 R11: ffffffff901debc8 R12: ff1d7c5367d61330
> [  +0.000001] R13: ff1d7c830300c6f0 R14: 0000000000000000 R15: 0000000000000000
> [  +0.000002] FS:  00007fea5e4e4740(0000) GS:ff1d7c81f0680000(0000) knlGS:0000000000000000
> [  +0.000002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  +0.000001] CR2: 0000562ef57c7608 CR3: 000000019037c002 CR4: 0000000000773ef0
> [  +0.000002] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  +0.000001] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  +0.000001] PKRU: 55555554
> [  +0.000002] Call Trace:
> [  +0.000003]  <TASK>
> [  +0.000002]  ? die+0x37/0x90
> [  +0.000007]  ? do_trap+0xdd/0x100
> [  +0.000004]  ? __list_add_valid_or_report+0x61/0xa0
> [  +0.000003]  ? do_error_trap+0x65/0x80
> [  +0.000002]  ? __list_add_valid_or_report+0x61/0xa0
> [  +0.000003]  ? exc_invalid_op+0x52/0x70
> [  +0.000005]  ? __list_add_valid_or_report+0x61/0xa0
> [  +0.000002]  ? asm_exc_invalid_op+0x1a/0x20
> [  +0.000007]  ? __list_add_valid_or_report+0x61/0xa0
> [  +0.000005]  ice_mbx_init_vf_info+0x3c/0x60 [ice]
> [  +0.000076]  ice_initialize_vf_entry+0x99/0xa0 [ice]
> 
> Regards,
> Sujai B
> 

Hello,
the problem was addressed by another patch [0] and is not related to this
change.

[0] https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20250211174322.603652-1-marcin.szycik@linux.intel.com/
Buvaneswaran, Sujai Feb. 25, 2025, 6:22 a.m. UTC | #4
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Larysa Zaremba
> Sent: Monday, December 9, 2024 7:39 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Zaremba, Larysa <larysa.zaremba@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> Eric Dumazet <edumazet@google.com>; Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: do not configure destination
> override for switchdev
> 
> After switchdev is enabled and disabled later, LLDP packets sending stops,
> despite working perfectly fine before and during switchdev state.
> To reproduce (creating/destroying VF is what triggers the reconfiguration):
> 
> devlink dev eswitch set pci/<address> mode switchdev echo '2' >
> /sys/class/net/<ifname>/device/sriov_numvfs
> echo '0' > /sys/class/net/<ifname>/device/sriov_numvfs
> 
> This happens because LLDP relies on the destination override functionality.
> It needs to 1) set a flag in the descriptor, 2) set the VSI permission to make it
> valid. The permissions are set when the PF VSI is first configured, but
> switchdev then enables it for the uplink VSI (which is always the PF) once
> more when configured and disables when deconfigured, which leads to
> software-generated LLDP packets being blocked.
> 
> Do not modify the destination override permissions when configuring
> switchdev, as the enabled state is the default configuration that is never
> modified.
> 
> Fixes: 1a1c40df2e80 ("ice: set and release switchdev environment")
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_eswitch.c |  6 ------
>  drivers/net/ethernet/intel/ice/ice_lib.c     | 18 ------------------
>  drivers/net/ethernet/intel/ice/ice_lib.h     |  4 ----
>  3 files changed, 28 deletions(-)
> 
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
index fb527434b58b..b44a375e6365 100644
--- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
+++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
@@ -50,9 +50,6 @@  static int ice_eswitch_setup_env(struct ice_pf *pf)
 	if (vlan_ops->dis_rx_filtering(uplink_vsi))
 		goto err_vlan_filtering;
 
-	if (ice_vsi_update_security(uplink_vsi, ice_vsi_ctx_set_allow_override))
-		goto err_override_uplink;
-
 	if (ice_vsi_update_local_lb(uplink_vsi, true))
 		goto err_override_local_lb;
 
@@ -64,8 +61,6 @@  static int ice_eswitch_setup_env(struct ice_pf *pf)
 err_up:
 	ice_vsi_update_local_lb(uplink_vsi, false);
 err_override_local_lb:
-	ice_vsi_update_security(uplink_vsi, ice_vsi_ctx_clear_allow_override);
-err_override_uplink:
 	vlan_ops->ena_rx_filtering(uplink_vsi);
 err_vlan_filtering:
 	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
@@ -276,7 +271,6 @@  static void ice_eswitch_release_env(struct ice_pf *pf)
 	vlan_ops = ice_get_compat_vsi_vlan_ops(uplink_vsi);
 
 	ice_vsi_update_local_lb(uplink_vsi, false);
-	ice_vsi_update_security(uplink_vsi, ice_vsi_ctx_clear_allow_override);
 	vlan_ops->ena_rx_filtering(uplink_vsi);
 	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
 			 ICE_FLTR_TX);
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index a7d45a8ce7ac..e07fc8851e1d 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3930,24 +3930,6 @@  void ice_vsi_ctx_clear_antispoof(struct ice_vsi_ctx *ctx)
 				 ICE_AQ_VSI_SEC_TX_PRUNE_ENA_S);
 }
 
-/**
- * ice_vsi_ctx_set_allow_override - allow destination override on VSI
- * @ctx: pointer to VSI ctx structure
- */
-void ice_vsi_ctx_set_allow_override(struct ice_vsi_ctx *ctx)
-{
-	ctx->info.sec_flags |= ICE_AQ_VSI_SEC_FLAG_ALLOW_DEST_OVRD;
-}
-
-/**
- * ice_vsi_ctx_clear_allow_override - turn off destination override on VSI
- * @ctx: pointer to VSI ctx structure
- */
-void ice_vsi_ctx_clear_allow_override(struct ice_vsi_ctx *ctx)
-{
-	ctx->info.sec_flags &= ~ICE_AQ_VSI_SEC_FLAG_ALLOW_DEST_OVRD;
-}
-
 /**
  * ice_vsi_update_local_lb - update sw block in VSI with local loopback bit
  * @vsi: pointer to VSI structure
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index 10d6fc479a32..6085039bac95 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -104,10 +104,6 @@  ice_vsi_update_security(struct ice_vsi *vsi, void (*fill)(struct ice_vsi_ctx *))
 void ice_vsi_ctx_set_antispoof(struct ice_vsi_ctx *ctx);
 
 void ice_vsi_ctx_clear_antispoof(struct ice_vsi_ctx *ctx);
-
-void ice_vsi_ctx_set_allow_override(struct ice_vsi_ctx *ctx);
-
-void ice_vsi_ctx_clear_allow_override(struct ice_vsi_ctx *ctx);
 int ice_vsi_update_local_lb(struct ice_vsi *vsi, bool set);
 int ice_vsi_add_vlan_zero(struct ice_vsi *vsi);
 int ice_vsi_del_vlan_zero(struct ice_vsi *vsi);