Re: [PATCH v3 net-next 3/6] vxlan: move VXLAN_F_MDB to struct vxlan_dev flags

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
vxlan: move VXLAN_F_MDB to struct vxlan_dev flags

This moves the internal VXLAN_F_MDB runtime state out of struct
vxlan_config.flags into a new unsigned long flags field in struct
vxlan_dev, accessed with set_bit()/clear_bit()/test_bit(), in
preparation for making vxlan->cfg an RCU-protected pointer.

One question about an out-of-tree-of-this-patch consumer of
cfg->flags below.

[Severity: Medium]
Does this change relax the mlxsw NVE offload validation?

mlxsw checks cfg->flags with a deny-by-default mask, so the 0x40000 bit
used to land in the "unsupported flag" test:

drivers/net/ethernet/mellanox/mlxsw/spectrum_nve_vxlan.c:
#define MLXSW_SP_NVE_VXLAN_IPV4_SUPPORTED_FLAGS (VXLAN_F_UDP_ZERO_CSUM_TX | \
						 VXLAN_F_LEARN | \
						 VXLAN_F_LOCALBYPASS)

static bool mlxsw_sp_nve_vxlan_ipv4_flags_check(const struct vxlan_config *cfg,
						struct netlink_ext_ack *extack)
{
	...
	if (cfg->flags & ~MLXSW_SP_NVE_VXLAN_IPV4_SUPPORTED_FLAGS) {
		NL_SET_ERR_MSG_MOD(extack, "VxLAN: Unsupported flag");
		return false;
	}

The same mask exists for the IPv6 underlay in
mlxsw_sp_nve_vxlan_ipv6_flags_check(), and both are reached from
mlxsw_sp_nve_vxlan_can_offload(), which only looks at &vxlan->cfg.

So for a device that already has an MDB entry:

  bridge mdb add dev vxlan0 port vxlan0 grp 239.1.1.1 permanent \
      dst 198.51.100.1
    -> vxlan_mdb_add() -> __vxlan_mdb_add() -> vxlan_mdb_entry_get()

before this patch that set VXLAN_F_MDB in cfg->flags, and a later
enslavement to an mlxsw-offloaded bridge was refused with "VxLAN:
Unsupported flag". After the move, mlxsw_sp_nve_vxlan_can_offload() has
no way to observe the MDB state, since nothing outside vxlan_core.c and
vxlan_mdb.c tests VXLAN_DEV_F_MDB, and the device is accepted for
offload while the configured MDB entries are not honoured by the
hardware forwarding plane.

Note the previous rejection was already order-dependent: MDB entries
added after the FID was enabled were never re-validated, because vxlan
MDB emits no switchdev notification at all (git grep switchdev
drivers/net/vxlan/ matches only vxlan_core.c).

Would it make sense to add a test_bit(VXLAN_DEV_F_MDB, &vxlan->flags)
check in mlxsw_sp_nve_vxlan_can_offload(), or to state in the commit
message that MDB presence is intentionally no longer a bar to offload?

 
 	return mdb_entry;
 

[ ... ]

[ ... ]