mbox series

[net-next,0/6] xfrm & bonding: Correct use of xso.real_dev

Message ID 20250407133542.2668491-1-cratiu@nvidia.com
Headers show
Series xfrm & bonding: Correct use of xso.real_dev | expand

Message

Cosmin Ratiu April 7, 2025, 1:35 p.m. UTC 
This patch series was motivated by fixing a few bugs in the bonding
driver related to xfrm state migration on device failover.

struct xfrm_dev_offload has two net_device pointers: dev and real_dev.
The first one is the device the xfrm_state is offloaded on and the
second one is used by the bonding driver to manage the underlying device
xfrm_states are actually offloaded on. When bonding isn't used, the two
pointers are the same.

This causes confusion in drivers: Which device pointer should they use?
If they want to support bonding, they need to only use real_dev and
never look at dev.

Furthermore, real_dev is used without proper locking from multiple code
paths and changing it is dangerous. See commit [1] for example.

This patch series clears things out by removing all uses of real_dev
from outside the bonding driver.
Then, the bonding driver is refactored to fix a couple of long standing
races and the original bug which motivated this patch series.

[1] commit f8cde9805981 ("bonding: fix xfrm real_dev null pointer
dereference")

Cosmin Ratiu (6):
Cleaning up unnecessary uses of xso.real_dev:
  net/mlx5: Avoid using xso.real_dev unnecessarily
  xfrm: Use xdo.dev instead of xdo.real_dev
  xfrm: Remove unneeded device check from validate_xmit_xfrm
Refactoring device operations to get an explicit device pointer:
  xfrm: Add explicit dev to .xdo_dev_state_{add,delete,free}
Fixing a bonding xfrm state migration bug:
  bonding: Mark active offloaded xfrm_states
Fixing long standing races in bonding:
  bonding: Fix multiple long standing offload races

 Documentation/networking/xfrm_device.rst      | 10 +-
 drivers/net/bonding/bond_main.c               | 93 +++++++++++--------
 .../net/ethernet/chelsio/cxgb4/cxgb4_main.c   | 20 ++--
 .../inline_crypto/ch_ipsec/chcr_ipsec.c       | 18 ++--
 .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c    | 40 ++++----
 drivers/net/ethernet/intel/ixgbevf/ipsec.c    | 20 ++--
 .../marvell/octeontx2/nic/cn10k_ipsec.c       | 18 ++--
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 28 +++---
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  1 +
 .../net/ethernet/netronome/nfp/crypto/ipsec.c | 11 +--
 drivers/net/netdevsim/ipsec.c                 | 15 ++-
 include/linux/netdevice.h                     | 10 +-
 include/net/xfrm.h                            |  8 ++
 net/xfrm/xfrm_device.c                        | 13 +--
 net/xfrm/xfrm_state.c                         | 16 ++--
 15 files changed, 175 insertions(+), 146 deletions(-)

Comments

Tariq Toukan April 8, 2025, 5:06 p.m. UTC | #1 
On 07/04/2025 16:35, Cosmin Ratiu wrote:
> xso.real_dev is the active device of an offloaded xfrm state and is
> managed by bonding. As such, it's subject to change when states are
> migrated to a new device. Using it in places other than
> offloading/unoffloading the states is risky.
> 
> This commit saves the device into the driver-specific struct
> mlx5e_ipsec_sa_entry and switches mlx5e_ipsec_init_macs() and
> mlx5e_ipsec_netevent_event() to make use of it.
> 
> Additionally, mlx5e_xfrm_update_stats() used xso.real_dev to validate
> that correct net locks are held. But in a bonding config, the net of the
> master device is the same as the underlying devices, and the net is
> already a local var, so use that instead.
> 
> The only remaining references to xso.real_dev are now in the
> .xdo_dev_state_add() / .xdo_dev_state_delete() path.
> 
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Acked-by: Tariq Toukan <tariqt@nvidia.com>

Thanks.