[net] i40e: Fix handling changed priv flags

Commit Message

Tony Nguyen Nov. 13, 2024, 9:07 p.m. UTC

From: Peter Große <[e-mail]>

After assembling the new private flags on a PF, the operation to determine
the changed flags uses the wrong bitmaps. Instead of xor-ing orig_flags
with new_flags, it uses the still unchanged pf->flags, thus changed_flags
is always 0.

Fix it by using the correct bitmaps.

The issue was discovered while debugging why disabling source pruning
stopped working with release 6.7. Although the new flags will be copied to
pf->flags later on in that function, disabling source pruning requires
a reset of the PF, which was skipped due to this bug.

Disabling source pruning:
$ sudo ethtool --set-priv-flags eno1 disable-source-pruning on
$ sudo ethtool --show-priv-flags eno1
Private flags for eno1:
MFP                   : off
total-port-shutdown   : off
LinkPolling           : off
flow-director-atr     : on
veb-stats             : off
hw-atr-eviction       : off
link-down-on-close    : off
legacy-rx             : off
disable-source-pruning: on
disable-fw-lldp       : off
rs-fec                : off
base-r-fec            : off
vf-vlan-pruning       : off

Regarding reproducing:

I observed the issue with a rather complicated lab setup, where
 * two VLAN interfaces are created on eno1
 * each with a different MAC address assigned
 * each moved into a separate namespace
 * both VLANs are bridged externally, so they form a single layer 2 network

The external bridge is done via a channel emulator adding packet loss and
delay and the application in the namespaces tries to send/receive traffic
and measure the performance. Sender and receiver are separated by
namespaces, yet the network card "sees its own traffic" send back to it.
To make that work, source pruning has to be disabled.

Cc: [e-mail]
Fixes: 70756d0a4727 ("i40e: Use DECLARE_BITMAP for flags and hw_features fields in i40e_pf")
Signed-off-by: Peter Große <[e-mail]>
Reviewed-by: Paul Menzel <[e-mail]>
Reviewed-by: Przemek Kitszel <[e-mail]>
Tested-by: Pucha Himasekhar Reddy <[e-mail]> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <[e-mail]>
---
iwl: https://lore.kernel.org/intel-wired-lan/[e-mail]/

 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

[e-mail] Nov. 15, 2024, 3:40 a.m. UTC | #1

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[e-mail]>:

On Wed, 13 Nov 2024 13:07:04 -0800 you wrote:
> From: Peter Große <[e-mail]>
> 
> After assembling the new private flags on a PF, the operation to determine
> the changed flags uses the wrong bitmaps. Instead of xor-ing orig_flags
> with new_flags, it uses the still unchanged pf->flags, thus changed_flags
> is always 0.
> 
> [...]

Here is the summary with links:
  - [net] i40e: Fix handling changed priv flags
    https://git.kernel.org/netdev/net/c/ea301aec8bb7

You are awesome, thank you!

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index f2506511bbff..bce5b76f1e7a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -5299,7 +5299,7 @@  static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 	}
 
 flags_complete:
-	bitmap_xor(changed_flags, pf->flags, orig_flags, I40E_PF_FLAGS_NBITS);
+	bitmap_xor(changed_flags, new_flags, orig_flags, I40E_PF_FLAGS_NBITS);
 
 	if (test_bit(I40E_FLAG_FW_LLDP_DIS, changed_flags))
 		reset_needed = I40E_PF_RESET_AND_REBUILD_FLAG;