Re: [PATCH net-next 5/9] vxlan: move VXLAN_F_MDB to struct vxlan_dev flags

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Moving VXLAN_F_MDB out of `struct vxlan_config.flags` into the
  new `vxlan->flags` bitmap silently loosens mlxsw's VXLAN offload…

--

vxlan: move VXLAN_F_MDB to struct vxlan_dev flags

VXLAN_F_MDB is moved out of struct vxlan_config.flags into a new
unsigned long flags field in struct vxlan_dev as VXLAN_DEV_F_MDB,
manipulated with set_bit()/clear_bit()/test_bit().  This is preparation
for turning vxlan->cfg into an RCU-protected pointer.

> diff --git a/drivers/net/vxlan/vxlan_mdb.c b/drivers/net/vxlan/vxlan_mdb.c
> index 56ca92832833..cf606256d092 100644
> --- a/drivers/net/vxlan/vxlan_mdb.c
> +++ b/drivers/net/vxlan/vxlan_mdb.c
> @@ -1219,7 +1219,7 @@ vxlan_mdb_entry_get(struct vxlan_dev *vxlan,
>  		goto err_free_entry;
>  
>  	if (hlist_is_singular_node(&mdb_entry->mdb_node, &vxlan->mdb_list))
> -		vxlan->cfg.flags |= VXLAN_F_MDB;
> +		set_bit(VXLAN_DEV_F_MDB, &vxlan->flags);
>  
>  	return mdb_entry;
>  

[ ... ]

> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index f41db72e9229..f4f519a365f5 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h

[ ... ]

> @@ -332,7 +337,6 @@ struct vxlan_dev {
>  #define VXLAN_F_IPV6_LINKLOCAL		0x8000
>  #define VXLAN_F_TTL_INHERIT		0x10000
>  #define VXLAN_F_VNIFILTER               0x20000
> -#define VXLAN_F_MDB			0x40000

[Severity: Medium]
Does removing this bit from the vxlan_config.flags space also change mlxsw
offload admission?  vxlan_config.flags is read outside the vxlan driver by
mlxsw, which rejects any flag it does not know about:

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 catch-all exists in mlxsw_sp_nve_vxlan_ipv6_flags_check(), and both
are reached from mlxsw_sp_nve_vxlan_can_offload(), which takes its cfg
straight from netdev_priv(params->dev):

  mlxsw_sp_netdevice_vxlan_event() -> mlxsw_sp_bridge_vxlan_join() ->
      mlxsw_sp_nve_fid_enable() -> ops->can_offload() -> flags_check()

Before this patch, "bridge mdb add dev vxlan0 ..." made
vxlan_mdb_entry_get() set VXLAN_F_MDB (0x40000) in vxlan->cfg.flags, so a
subsequent "ip link set vxlan0 master br0" on an otherwise offloadable
device failed with the "VxLAN: Unsupported flag" extack.  After the move the
bit lives in vxlan->flags, which mlxsw never inspects, so that enslavement
now succeeds.

mlxsw has no VXLAN MDB handling (vxlan_mdb.c emits no switchdev
notification), so in that ordering the MDB entries would be silently not
honoured in hardware and multicast would follow FDB-based hardware flooding
instead.

The pre-patch rejection was ordering dependent (MDB entries added after
enslavement were never revalidated, so the same unoffloaded state was
already reachable), and commit bc6c6b013ffe ("vxlan: mdb: Add an internal
flag to indicate MDB usage") describes VXLAN_F_MDB purely as an internal
data path flag, so the mlxsw behaviour looks accidental.  Still, the
changelog presents this as pure preparation for the RCU cfg conversion.

Could the cross-driver effect be mentioned in the changelog, and could the
mlxsw side confirm it?  Checking the end of the series (9c4524e8ff6c), the
bit stays in vxlan->flags and the mlxsw supported-flags masks are unchanged,
so no later patch addresses this.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903120840.1024153-1-edumazet%40google.com