[PATCH net-next v5 6/8] vxlan: vnifilter: roll back a VNI add that fails

[Ali Firas <alishmery18@gmail.com>]

vxlan_vni_add() publishes a new VNI before the steps that can still
fail: the node goes into the VNI rhashtable, the VNI list and, when the
device is up, the per-socket VNI hash. Only then does
vxlan_vni_update_group() install the VNI's default FDB entry and join
its multicast group. If either fails, nothing is undone and
RTM_NEWTUNNEL is sent anyway, so a request that returns an error leaves
behind a VNI that is listed, notified and accepts unicast VXLAN frames,
but has no default entry or no membership. A retry of a VNI that
inherits the device remote, or of one whose join failed, does not repair
it: vxlan_vni_add() then takes the update path, which finds the stored
remote equal to the requested one and returns 0.

Both failures are reachable without memory pressure. Installing the
entry fails with -ENOSPC once the device holds maxaddress FDB entries.
The join fails with -ENOBUFS once the socket holds
net.ipv4.igmp_max_memberships groups (20 by default).

Make installing the entry the last step that can fail, then undo the
publication when it does. On create, vxlan_vni_update_group() now joins
the group first and leaves it again if the entry cannot be installed.
The membership is given back only if this join took it, not when
-EADDRINUSE says the socket already held it. Installing the entry
either succeeds or leaves no FDB state behind: vxlan_fdb_create() frees
what it allocated, and "vxlan: leave FDB state and flags unchanged
when an update fails" covers an append to an existing entry. That leaves
vxlan_vni_add() only the VNI itself to unpublish. It does so with the
helpers vxlan_vni_del() uses, in reverse order of publication. It frees
the node through the same call_rcu(), so an RX or TX lookup that still
holds it under RCU does not see it freed. RTM_NEWTUNNEL is now sent
only on success. Commit aa6ca1c5c338 ("vxlan: vnifilter: send
notification on VNI add") sent it on failure too, because the VNI then
stayed installed; a VNI added without a group or remote still gets it,
as that commit intended.

vxlan_vni_delete_group() is not reused here. Its guard also fires when
only the device has a remote, so on such a device it deletes a default
entry even when installing that entry is what failed. Since
vxlan_fdb_parse() accepts any NDA_SRC_VNI, that entry can be one
installed out of band.

Ido raised the missing rollback in review of a vxlan_igmp_join() fix
(Links below), and the automated review of v4 of this series raised it
again. Both failures were triggered in QEMU on net-next with the limits
above, without modifying the kernel or injecting errors, and showed each
symptom described above.

Tested in QEMU: test_vxlan_vnifiltering.sh gains a case for each failure
later in this series; both fail on net-next and pass on this series,
with and without KASAN and lockdep.

Changing the group of an existing VNI is left as it is. It leaves the
old group before joining the new one, which lets a swap succeed at the
membership limit, so a join that fails there still has the new group
recorded. Fixing that needs a transaction.

This is ordered before "vxlan: vnifilter: account the default FDB entry
to memcg", which makes installing the entry fail routinely at a memory
cgroup limit. It targets net-next, as Jakub suggested in review of the
earlier posting.

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Link: https://lore.kernel.org/netdev/20260323095544.3311285-4-bestswngs@gmail.com/
Link: https://lore.kernel.org/netdev/20260324133449.GA460138@shredder/
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Cc: Weiming Shi <bestswngs@gmail.com>
Cc: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v5: new patch, ordered before the FDB accounting patch that makes the
failure routine (review of v4 6/7). The missing rollback was noted in
Weiming Shi's vxlan_igmp_join() fix and raised by Ido in its review
(Links); the changelog says what it did in QEMU (sinfo bot). It
supersedes the separately posted rollback series:
https://lore.kernel.org/netdev/20260911215203.3054653-1-alishmery18@gmail.com/
Unlike that posting it carries a Fixes tag, which maintainer-netdev.rst
requires for fixes regardless of the tree.

 drivers/net/vxlan/vxlan_vnifilter.c | 50 +++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 07aa641f16e8..6e896ddf658c 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -606,6 +606,7 @@ static int vxlan_vni_update_group(struct vxlan_dev *vxlan,
 	const struct vxlan_config *cfg = rtnl_dereference(vxlan->cfg);
 	const union vxlan_addr *newrip = NULL, *oldrip = NULL;
 	union vxlan_addr old_remote_ip;
+	bool joined = false;
 	int ret = 0;
 
 	memcpy(&old_remote_ip, &vninode->remote_ip, sizeof(old_remote_ip));
@@ -633,13 +634,31 @@ static int vxlan_vni_update_group(struct vxlan_dev *vxlan,
 	if (!create && oldrip && newrip && vxlan_addr_equal(oldrip, newrip))
 		return 0;
 
+	/* A new VNI joins its group before its default FDB entry is installed,
+	 * so that installing the entry is the last step that can fail, and a
+	 * failure leaves vxlan_vni_add() only the VNI itself to undo.
+	 */
+	if (create && group && (vxlan->dev->flags & IFF_UP) &&
+	    vxlan_addr_multicast(group)) {
+		ret = vxlan_igmp_join(vxlan, group, 0);
+		if (ret == -EADDRINUSE)
+			ret = 0;
+		else if (!ret)
+			joined = true;
+		if (ret)
+			goto out;
+	}
+
 	ret = vxlan_update_default_fdb_entry(vxlan, vninode->vni,
 					     oldrip, newrip,
 					     cfg->remote_ifindex,
 					     cfg->remote_ifindex,
 					     extack);
-	if (ret)
+	if (ret) {
+		if (joined)
+			vxlan_igmp_leave(vxlan, group, 0);
 		goto out;
+	}
 
 	if (group) {
 		memcpy(&vninode->remote_ip, group, sizeof(vninode->remote_ip));
@@ -647,7 +666,7 @@ static int vxlan_vni_update_group(struct vxlan_dev *vxlan,
 			vxlan_vnifilter_seq_inc(dev_net(vxlan->dev));
 	}
 
-	if (vxlan->dev->flags & IFF_UP) {
+	if (!create && (vxlan->dev->flags & IFF_UP)) {
 		if (vxlan_addr_multicast(&old_remote_ip) &&
 		    !vxlan_group_used(vn, vxlan, vninode->vni,
 				      &old_remote_ip,
@@ -850,6 +869,14 @@ static void vxlan_vni_free(struct vxlan_vni_node *vninode)
 	kfree(vninode);
 }
 
+static void vxlan_vni_node_rcu_free(struct rcu_head *rcu)
+{
+	struct vxlan_vni_node *v;
+
+	v = container_of(rcu, struct vxlan_vni_node, rcu);
+	vxlan_vni_free(v);
+}
+
 static int vxlan_vni_add(struct vxlan_dev *vxlan,
 			 struct vxlan_vni_group *vg,
 			 u32 vni, union vxlan_addr *group,
@@ -889,18 +916,21 @@ static int vxlan_vni_add(struct vxlan_dev *vxlan,
 
 	err = vxlan_vni_update_group(vxlan, vninode, group, true, &changed,
 				     extack);
+	if (err)
+		goto err_unpublish;
 
 	vxlan_vnifilter_notify(vxlan, vninode, RTM_NEWTUNNEL);
 
-	return err;
-}
-
-static void vxlan_vni_node_rcu_free(struct rcu_head *rcu)
-{
-	struct vxlan_vni_node *v;
+	return 0;
 
-	v = container_of(rcu, struct vxlan_vni_node, rcu);
-	vxlan_vni_free(v);
+err_unpublish:
+	if (vxlan->dev->flags & IFF_UP)
+		vxlan_vs_add_del_vninode(vxlan, vninode, true);
+	__vxlan_vni_del_list(vxlan, vg, vninode);
+	rhashtable_remove_fast(&vg->vni_hash, &vninode->vnode,
+			       vxlan_vni_rht_params);
+	call_rcu(&vninode->rcu, vxlan_vni_node_rcu_free);
+	return err;
 }
 
 static int vxlan_vni_del(struct vxlan_dev *vxlan,
-- 
2.53.0