[PATCH net v5 1/2] vxlan: reject adding a remote to a nexthop fdb entry

[PATCH net v5 1/2] vxlan: reject adding a remote to a nexthop fdb entry

[Henry Martin]

FDB entries backed by a nexthop group must not carry a remote
destination list.  vxlan_fdb_update_existing() currently appends a
remote via vxlan_fdb_append() on an NLM_F_APPEND request with a
multicast or zero MAC even when the entry is backed by a nexthop,
breaking that invariant through three different callers:

- "bridge fdb append" with a dst on a nexthop entry;
- "ip link set dev vx0 type vxlan remote <ip>" (vxlan_changelink());
- "bridge vni add" on a vnifilter device
  (vxlan_update_default_fdb_entry()); this includes a plain
  "bridge vni add vni N" without a group, which falls back to the
  device's default remote, and a unicast group.
All three reach the append block with nhid == 0 and a PERMANENT
state, so they pass the earlier nexthop checks.

There is no crash and no leak: the appended remote is freed with the
entry, and the transmit path ignores it (vxlan_xmit() follows the
nexthop whenever f->nh is set).  But the stray remote changes
deletion semantics: __vxlan_fdb_delete() destroys the whole fdb entry
when the matched remote is the only one on the list, so removing it
via "bridge fdb del ... dst <remote>" or "ip link set ... remote
0.0.0.0" tears down the nexthop entry, not just the stray remote.
The entry state is also misreported: vxlan_fdb_notify(), RTM_GETNEIGH
dumps and the switchdev FDB programming all prefer the remote list
over the nexthop, so user space sees the dead remote while the
datapath follows the nexthop group.

Reject the combination at the top of vxlan_fdb_update_existing(),
next to the existing nexthop guards, so a refused request has no side
effects.

Also move the non-unicast NLM_F_REPLACE rejection to the same place;
it previously returned -EOPNOTSUPP only after the entry state and
flags had been rewritten, skipping both the notification pass and
the err_notify rollback.

Note the resulting behavior change for the rtnetlink callers:
operations that previously succeeded while silently attaching a
remote to a nexthop entry now fail with extack
"Cannot add a remote to a nexthop fdb entry".

Add test cases to fib_nexthops.sh in the same fashion as the sibling
fix, commit 98fc57d16744 ("vxlan: reject dynamic fdb entries that
reference a nexthop id").

This issue was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v5: no changes to this patch.

v4: Also move the non-unicast REPLACE rejection before the entry is
    modified (the same half-update class, pre-existing), describe the
    stray remote's effect on __vxlan_fdb_delete() instead of calling
    it "unused", and note that a plain "bridge vni add" or a unicast
    group hits the check too (sashiko review of v2).

v3: Describe the actual consequence and add fib_nexthops.sh test
    cases as requested by Ilya Maximets.

v2: Move the check before the entry state/flags update, next to the
    other nexthop guards (reported by sashiko, confirmed by Jakub).

 drivers/net/vxlan/vxlan_core.c              | 26 +++++++++++++--------------
 tools/testing/selftests/net/fib_nexthops.sh | 18 ++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 3390e341d1e58..13143085065f6 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -1002,6 +1016,22 @@ static int vxlan_fdb_update_existing(struct vxlan_dev *vxlan,
 		return -EOPNOTSUPP;
 	}
 
+	if (rcu_access_pointer(f->nh) &&
+	    (flags & NLM_F_APPEND) &&
+	    (is_multicast_ether_addr(f->key.eth_addr) ||
+	     is_zero_ether_addr(f->key.eth_addr))) {
+		NL_SET_ERR_MSG(extack,
+			       "Cannot add a remote to a nexthop fdb entry");
+		return -EOPNOTSUPP;
+	}
+
+	if ((flags & NLM_F_REPLACE) &&
+	    (is_multicast_ether_addr(f->key.eth_addr) ||
+	     is_zero_ether_addr(f->key.eth_addr))) {
+		NL_SET_ERR_MSG(extack, "Cannot replace non-unicast fdb entries");
+		return -EOPNOTSUPP;
+	}
+
 	/* Do not allow an externally learned entry to take over an entry added
 	 * by the user.
 	 */
@@ -1018,22 +1048,18 @@ static int vxlan_fdb_update_existing(struct vxlan_dev *vxlan,
 	}
 
 	if ((flags & NLM_F_REPLACE)) {
-		/* Only change unicasts */
-		if (!(is_multicast_ether_addr(f->key.eth_addr) ||
-		      is_zero_ether_addr(f->key.eth_addr))) {
-			if (nhid) {
-				rc = vxlan_fdb_nh_update(vxlan, f, nhid, extack);
-				if (rc < 0)
-					return rc;
-			} else {
-				rc = vxlan_fdb_replace(f, ip, port, vni,
-						       ifindex, &oldrd);
-			}
-			notify |= rc;
+		/* Only change unicasts; non-unicast replace is rejected
+		 * before any state is modified.
+		 */
+		if (nhid) {
+			rc = vxlan_fdb_nh_update(vxlan, f, nhid, extack);
+			if (rc < 0)
+				return rc;
 		} else {
-			NL_SET_ERR_MSG(extack, "Cannot replace non-unicast fdb entries");
-			return -EOPNOTSUPP;
+			rc = vxlan_fdb_replace(f, ip, port, vni,
+					       ifindex, &oldrd);
 		}
+		notify |= rc;
 	}
 	if ((flags & NLM_F_APPEND) &&
 	    (is_multicast_ether_addr(f->key.eth_addr) ||

diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index 431d7bed76224..bdc23205bde4c 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -547,6 +547,15 @@ ipv6_fdb_grp_fcnal()
 	run_cmd "$BRIDGE fdb append 02:02:00:00:00:17 dev vx10 dst 2001:db8:91::11 self dynamic"
 	log_test $? 255 "Fdb mac append with nexthop group and dynamic state"
 
+	# fdb entries with a nexthop group cannot take a remote append
+	run_cmd "$BRIDGE fdb add 00:00:00:00:00:00 dev vx10 nhid 102 self static"
+	run_cmd "$BRIDGE fdb append 00:00:00:00:00:00 dev vx10 dst 2001:db8:91::11 self"
+	log_test $? 255 "Fdb append with nexthop group"
+
+	# fdb entries with a nexthop group cannot be replaced with a non-unicast MAC
+	run_cmd "$BRIDGE fdb replace 00:00:00:00:00:00 dev vx10 dst 2001:db8:91::11 self permanent"
+	log_test $? 255 "Fdb replace with nexthop group"
+
 	run_cmd "$IP -6 ro add 2001:db8:101::1/128 nhid 66"
 	log_test $? 2 "Route add with fdb nexthop"
 
@@ -697,6 +706,15 @@ ipv4_fdb_grp_fcnal()
 	run_cmd "$BRIDGE fdb append 02:02:00:00:00:17 dev vx10 dst 10.0.0.3 self dynamic"
 	log_test $? 255 "Fdb mac append with nexthop group and dynamic state"
 
+	# fdb entries with a nexthop group cannot take a remote append
+	run_cmd "$BRIDGE fdb add 00:00:00:00:00:00 dev vx10 nhid 102 self static"
+	run_cmd "$BRIDGE fdb append 00:00:00:00:00:00 dev vx10 dst 10.0.0.3 self"
+	log_test $? 255 "Fdb append with nexthop group"
+
+	# fdb entries with a nexthop group cannot be replaced with a non-unicast MAC
+	run_cmd "$BRIDGE fdb replace 00:00:00:00:00:00 dev vx10 dst 10.0.0.3 self permanent"
+	log_test $? 255 "Fdb replace with nexthop group"
+
 	run_cmd "$IP ro add 172.16.0.0/22 nhid 16"
 	log_test $? 2 "Route add with fdb nexthop"
 
-- 
2.43.7

[PATCH net v5 2/2] vxlan: reject VNI add when the default fdb entry is nexthop-backed

[Henry Martin]

vxlan_vni_add() installs the new VNI - hash table insert, group list,
socket attach and the RTM_NEWTUNNEL notification - before calling
vxlan_vni_update_group(), which in turn calls
vxlan_update_default_fdb_entry() to store the group (or the device's
default remote) into the all-zeros fdb entry of that VNI.

Now that vxlan_fdb_update_existing() rejects appending a remote to a
nexthop-backed default entry, the group store can fail with
-EOPNOTSUPP after the VNI has already been installed and announced,
leaving it half-installed: present in the tables and announced, but
with no remote stored and no multicast join.

Check whether the default fdb entry for the VNI is backed by a
nexthop before installing the VNI, and reject the add up front.

Note: the missing unwind in vxlan_vni_add() predates this series;
this check avoids giving user space a deterministic way to reach it.

Cc: stable@vger.kernel.org
Fixes: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---

v5: fix a build error on current net/main: vxlan->cfg became an
    RCU-managed pointer, so reference the function-local cfg
    (reported by kernel test robot).

v4: new patch; the failure path was made reachable by patch 1, so
    guard it before the VNI is installed (sashiko review of v2).

 drivers/net/vxlan/vxlan_core.c      | 14 ++++++++++++++
 drivers/net/vxlan/vxlan_private.h   |  2 ++
 drivers/net/vxlan/vxlan_vnifilter.c | 14 ++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 3390e341d1e58..13143085065f6 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -422,6 +422,20 @@ static struct vxlan_fdb *vxlan_find_mac(struct vxlan_dev *vxlan,
 	return f;
 }
 
+bool vxlan_fdb_entry_nh_backed(struct vxlan_dev *vxlan, const u8 *mac,
+			       __be32 vni)
+{
+	struct vxlan_fdb *f;
+	bool ret;
+
+	spin_lock_bh(&vxlan->hash_lock);
+	f = vxlan_find_mac(vxlan, mac, vni);
+	ret = f && rcu_access_pointer(f->nh);
+	spin_unlock_bh(&vxlan->hash_lock);
+
+	return ret;
+}
+
 /* caller should hold vxlan->hash_lock */
 static struct vxlan_rdst *vxlan_fdb_find_rdst(struct vxlan_fdb *f,
 					      union vxlan_addr *ip, __be16 port,

diff --git a/drivers/net/vxlan/vxlan_private.h b/drivers/net/vxlan/vxlan_private.h
index b1eec22163608..0454689965aeb 100644
--- a/drivers/net/vxlan/vxlan_private.h
+++ b/drivers/net/vxlan/vxlan_private.h
@@ -11,6 +11,8 @@
 
 extern unsigned int vxlan_net_id;
 extern const u8 all_zeros_mac[ETH_ALEN + 2];
+bool vxlan_fdb_entry_nh_backed(struct vxlan_dev *vxlan,
+			       const u8 *mac, __be32 vni);
 extern const struct rhashtable_params vxlan_vni_rht_params;
 
 #define PORT_HASH_BITS	8

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index dd94085e08865..f07ae2a6a59ed 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -737,6 +737,20 @@ static int vxlan_vni_add(struct vxlan_dev *vxlan,
 		return err;
 	}
 
+	/* A default fdb entry that is backed by a nexthop group can
+	 * never take a remote; reject before the VNI is installed,
+	 * otherwise a failure in vxlan_vni_update_group() would leave
+	 * the VNI half-installed.
+	 */
+	if ((group && !vxlan_addr_any(group)) ||
+	    !vxlan_addr_any(&cfg->remote_ip)) {
+		if (vxlan_fdb_entry_nh_backed(vxlan, all_zeros_mac, v)) {
+			NL_SET_ERR_MSG(extack,
+				       "Cannot add a remote to a nexthop fdb entry");
+			return -EOPNOTSUPP;
+		}
+	}
+
 	vninode = vxlan_vni_alloc(vxlan, v);
 	if (!vninode)
 		return -ENOMEM;
-- 
2.43.7

Re: [PATCH net v5 2/2] vxlan: reject VNI add when the default fdb entry is nexthop-backed

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The series order leaves a bisect window, and nothing protects the
  stable backport.
- [Low] The commit message gets the order of operations in
  vxlan_vni_add() wrong.

--

vxlan: reject VNI add when the default fdb entry is nexthop-backed

This commit adds vxlan_fdb_entry_nh_backed() and calls it from
vxlan_vni_add(). A VNI add with a group, or with a device default remote,
is now rejected when the all-zeros fdb entry for that VNI is backed by a
nexthop. The check runs before the VNI is installed, so a later
-EOPNOTSUPP from vxlan_fdb_update_existing() can no longer leave the VNI
half-installed.

> vxlan_vni_add() installs the new VNI - hash table insert, group list,
> socket attach and the RTM_NEWTUNNEL notification - before calling
> vxlan_vni_update_group(), which in turn calls
> vxlan_update_default_fdb_entry() to store the group (or the device's
> default remote) into the all-zeros fdb entry of that VNI.

[Severity: Low]
Does this ordering match vxlan_vni_add()? In the code, the notification
goes out after vxlan_vni_update_group() returns, and it goes out whatever
the return value is:

	err = vxlan_vni_update_group(vxlan, vninode, group, true, &changed,
				     extack);

	vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);

	return err;

The end state described later in the message is accurate: the VNI is
installed and announced despite the error. But it is announced after the
failed update, not before it.

Could this paragraph be reworded to match?

> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index 124ba47758e91..07e17f4b93530 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -837,6 +837,20 @@ static int vxlan_vni_add(struct vxlan_dev *vxlan,
>  		return err;
>  	}
>  
> +	/* A default fdb entry that is backed by a nexthop group can
> +	 * never take a remote; reject before the VNI is installed,
> +	 * otherwise a failure in vxlan_vni_update_group() would leave
> +	 * the VNI half-installed.
> +	 */
> +	if ((group && !vxlan_addr_any(group)) ||
> +	    !vxlan_addr_any(&cfg->remote_ip)) {
> +		if (vxlan_fdb_entry_nh_backed(vxlan, all_zeros_mac, v)) {
> +			NL_SET_ERR_MSG(extack,
> +				       "Cannot add a remote to a nexthop fdb entry");
> +			return -EOPNOTSUPP;
> +		}
> +	}

[Severity: Low]
Does the series order leave a bisect window between this patch and the
earlier "vxlan: reject adding a remote to a nexthop fdb entry"?

That earlier patch adds this check to vxlan_fdb_update_existing():

	if (rcu_access_pointer(f->nh) &&
	    (flags & NLM_F_APPEND) &&
	    (is_multicast_ether_addr(f->key.eth_addr) ||
	     is_zero_ether_addr(f->key.eth_addr))) {
		NL_SET_ERR_MSG(extack,
			       "Cannot add a remote to a nexthop fdb entry");
		return -EOPNOTSUPP;
	}

With only that patch applied, a sequence like this:

  bridge fdb add 00:00:00:00:00:00 dev vx0 nhid X self permanent src_vni N
  bridge vni add dev vx0 vni N group G

goes through this path:

vxlan_vni_add()
  vxlan_vni_update_group()
    vxlan_update_default_fdb_entry()
      vxlan_fdb_update()
        vxlan_fdb_update_existing() returns -EOPNOTSUPP

By then the VNI is already in the hash table and the group list, and
RTM_NEWTUNNEL is still sent. That is the half-installed state this patch
is meant to prevent. Leaving out the group has the same effect when the
device has a default remote. Before the earlier patch, the same request
succeeded and appended a stray remote.

Both patches carry Cc: stable and the same Fixes: tag, but neither one
records the dependency.

The earlier patch only touches vxlan_fdb_update_existing(), so it will
probably apply cleanly to older trees. This patch was reworked in v5 for
the RCU conversion of vxlan->cfg (the local cfg from
rtnl_dereference(vxlan->cfg)), so its context is unlikely to apply there.

Could stable end up taking the earlier patch on its own? This guard does
not depend on the earlier patch, so would it make sense to put it first
in the series, or to squash the two?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028111626.3460759-1-bsdhenrymartin%40gmail.com

Re: [PATCH net v5 1/2] vxlan: reject adding a remote to a nexthop fdb entry

[Ido Schimmel]

Thank you for giving me two minutes to reply before posting a new
version [1][2].

[1] https://lore.kernel.org/netdev/CAEnQdOoQRuH8A=HnJqruN_kp5exzTwV7bK-LZUVo=LGuSMmgNA@mail.gmail.com/
[2] https://lore.kernel.org/netdev/20261027104713.328028-1-bsdhenrymartin@gmail.com/

On Wed, Oct 28, 2026 at 07:16:25PM +0800, Henry Martin wrote:
> FDB entries backed by a nexthop group must not carry a remote
> destination list.  vxlan_fdb_update_existing() currently appends a
> remote via vxlan_fdb_append() on an NLM_F_APPEND request with a
> multicast or zero MAC even when the entry is backed by a nexthop,
> breaking that invariant through three different callers:

BUM traffic should always be flooded. It makes no sense to load balance
it between different VTEPs. Simply forbid the creation of a multicast /
all-zeroes FDB that points to an FDB nexthop in
vxlan_fdb_update_create(). This is net-next material.

Re: [PATCH net v5 1/2] vxlan: reject adding a remote to a nexthop fdb entry

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new fib_nexthops.sh cases never create a nexthop-backed
  00:00:00:00:00:00 entry.
- [Low] Baseline decision: false (resolved later in the series).
- [Low] The commit message says that "vxlan_fdb_notify(), RTM_GETNEIGH
  dumps and the switchdev FDB programming all prefer the remote list…
- [Low] The new "Fdb replace with nexthop group" test (fib_nexthops.sh
  lines 555-557 for IPv6 and 714-716 for IPv4) passes with or without…

--

vxlan: reject adding a remote to a nexthop fdb entry

This patch makes vxlan_fdb_update_existing() reject NLM_F_APPEND on a
multicast or all-zeros fdb entry that is backed by a nexthop group, and
does the check before any entry state is modified. It also moves the
non-unicast NLM_F_REPLACE rejection ahead of the state/flags update. New
fib_nexthops.sh cases are added for both.

> The entry state is also misreported: vxlan_fdb_notify(), RTM_GETNEIGH
> dumps and the switchdev FDB programming all prefer the remote list
> over the nexthop, so user space sees the dead remote while the
> datapath follows the nexthop group.

[Severity: Low]
Is this accurate for the netlink paths? vxlan_fdb_info() checks the
nexthop before the remote:

drivers/net/vxlan/vxlan_core.c:vxlan_fdb_info() {
    ...
	if (nh) {
		if (nla_put_u32(skb, NDA_NH_ID, nh_id))
			goto nla_put_failure;
	} else if (rdst) {
		if (send_ip && vxlan_nla_put_addr(skb, NDA_DST,
						  &rdst->remote_ip))
    ...
}

So the RTM_NEWNEIGH notification sent through
vxlan_fdb_notify()->__vxlan_fdb_notify() and the vxlan_fdb_get() reply
both carry NDA_NH_ID, not the stray remote. vxlan_fdb_dump() handles
f->nh entries before it walks the remote list, and it passes a NULL rdst.

As far as I can tell, only the switchdev notifier part of
vxlan_fdb_notify() (vxlan_fdb_switchdev_call_notifiers() with rd) uses
the stray remote. Could the commit message be narrowed to that path?

> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 278d41f58d895..3aead3c7f65f0 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1011,6 +1011,22 @@ static int vxlan_fdb_update_existing(struct vxlan_dev *vxlan,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (rcu_access_pointer(f->nh) &&
> +	    (flags & NLM_F_APPEND) &&
> +	    (is_multicast_ether_addr(f->key.eth_addr) ||
> +	     is_zero_ether_addr(f->key.eth_addr))) {
> +		NL_SET_ERR_MSG(extack,
> +			       "Cannot add a remote to a nexthop fdb entry");
> +		return -EOPNOTSUPP;
> +	}

[Severity: Low]
With only this patch applied, can this new rejection leave a VNI
half-installed on a vnifilter device? The commit message says "so a
refused request has no side effects". However, vxlan_vni_add() publishes
the vninode before it makes the call that now fails:

drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vni_add() {
    ...
	__vxlan_vni_add_list(vxlan, vg, vninode);

	if (vxlan->dev->flags & IFF_UP)
		vxlan_vs_add_del_vninode(vxlan, vninode, false);

	err = vxlan_vni_update_group(vxlan, vninode, group, true, &changed,
				     extack);

	vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);

	return err;
}

Suppose the per-VNI all-zeros entry is nexthop-backed, and either a group
is given or cfg->remote_ip is not any. Then the call chain is:

vxlan_vni_add()
  vxlan_vni_update_group()
    vxlan_update_default_fdb_entry()
      vxlan_fdb_update()
        vxlan_fdb_update_existing()   <- -EOPNOTSUPP

After that failure, vxlan_vni_update_group() jumps to out and skips both
the remote_ip copy and the IGMP join. vxlan_vni_add() still sends
RTM_NEWTUNNEL. It returns the error without removing the vninode from
vg->vni_hash or from the list.

The next patch in this series, "vxlan: reject VNI add when the default
fdb entry is nexthop-backed", adds a check in vxlan_vni_add() before
vxlan_vni_alloc(). That check rejects this case before anything is
installed, so the problem only exists at the bisect point between the two
patches. Would it make sense to reorder or fold the two patches so that
each one is self-consistent on its own?

[ ... ]

> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index 431d7bed76224..bdc23205bde4c 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -547,6 +547,15 @@ ipv6_fdb_grp_fcnal()
>  	run_cmd "$BRIDGE fdb append 02:02:00:00:00:17 dev vx10 dst 2001:db8:91::11 self dynamic"
>  	log_test $? 255 "Fdb mac append with nexthop group and dynamic state"
>  
> +	# fdb entries with a nexthop group cannot take a remote append
> +	run_cmd "$BRIDGE fdb add 00:00:00:00:00:00 dev vx10 nhid 102 self static"
> +	run_cmd "$BRIDGE fdb append 00:00:00:00:00:00 dev vx10 dst 2001:db8:91::11 self"
> +	log_test $? 255 "Fdb append with nexthop group"

[Severity: Medium]
Does this fdb add actually create a nexthop-backed all-zeros entry?
Earlier in ipv6_fdb_grp_fcnal(), vx10 is created with a remote:

	run_cmd "$IP link add name vx10 type vxlan id 1010 local 2001:db8:91::9 remote 2001:db8:91::10 dstport 4789 nolearning noudpcsum tos inherit ttl 100"

Because cfg->remote_ip is set, __vxlan_dev_create() has already installed
a plain, non-nexthop all_zeros_mac entry:

drivers/net/vxlan/vxlan_core.c:__vxlan_dev_create() {
    ...
	/* create an fdb entry for a valid default destination */
	if (!vxlan_addr_any(&cfg->remote_ip)) {
		union vxlan_addr rip = cfg->remote_ip;

		spin_lock_bh(&vxlan->hash_lock);
		err = vxlan_fdb_update(vxlan, all_zeros_mac,
				       &rip,
				       NUD_REACHABLE | NUD_PERMANENT,
				       NLM_F_EXCL | NLM_F_CREATE,
    ...
}

bridge fdb add sends NLM_F_CREATE | NLM_F_EXCL. vxlan_fdb_update() finds
the existing entry and returns -EEXIST:

	f = vxlan_find_mac(vxlan, mac, src_vni);
	if (f) {
		if (flags & NLM_F_EXCL) {
			netdev_dbg(vxlan->dev,
				   "lost race to create %pM\n", mac);
			return -EEXIST;
		}

Nothing checks the result of that run_cmd. The append that follows then
reaches vxlan_fdb_update_existing() with f->nh == NULL. The new guard does
not fire, vxlan_fdb_append() adds the remote, and bridge exits 0.

If so, log_test $? 255 would report FAIL on a fixed kernel, and the test
would never reach the new f->nh check. ipv4_fdb_grp_fcnal() has the same
problem, because vx10 is created there with remote 10.0.0.2.

Any of these would avoid the collision: delete the default entry first,
create vx10 without a remote, or use a multicast MAC for the
nexthop-backed entry.

> +
> +	# fdb entries with a nexthop group cannot be replaced with a non-unicast MAC
> +	run_cmd "$BRIDGE fdb replace 00:00:00:00:00:00 dev vx10 dst 2001:db8:91::11 self permanent"
> +	log_test $? 255 "Fdb replace with nexthop group"

[Severity: Low]
Can this test distinguish a fixed kernel from an unfixed one? The check
it hits is the moved "Cannot replace non-unicast fdb entries" rejection
in vxlan_fdb_update_existing(), and that check does not look at f->nh.

Before this patch the same request also returned -EOPNOTSUPP. The only
difference was that it did so after f->state and f->flags had been
rewritten. bridge exits with 255 in both cases.

To cover the half-update this patch fixes, the test could check in bridge
fdb show that state and flags are unchanged after the failed replace.

Because of the setup problem above, the entry being replaced is also the
plain default entry, not a nexthop-backed one.

The comment "fdb entries with a nexthop group cannot be replaced with a
non-unicast MAC" also looks inaccurate. The check is not specific to
nexthops, and the MAC is the lookup key, not the thing being replaced.
The copy in ipv4_fdb_grp_fcnal() has the same issues.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028111626.3460759-1-bsdhenrymartin%40gmail.com