[PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback

[PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Second patch in this series removes IPv6 requirement about the netns
loopback device being the last device being dismantled.

This was needed because rt6_uncached_list_flush_dev()
and ip6_dst_ifdown() had to switch dst dev to a known
device (loopback).

Instead of loopback, we can use the (hidden) blackhole_netdev
which is also always there.

This will allow future simplfications of netdev_run_to()
and other parts of the stack like default_device_exit_batch().

Last two patches are optimizations for both IP families.

Eric Dumazet (4):
  ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache
  ipv6: give an IPv6 dev to blackhole_netdev
  ipv6: add (struct uncached_list)->quarantine list
  ipv4: add (struct uncached_list)->quarantine list

 include/net/ip6_fib.h   |  3 +-
 net/ipv4/route.c        | 12 +++++--
 net/ipv6/addrconf.c     | 78 +++++++++++++++++------------------------
 net/ipv6/route.c        | 42 +++++++++++-----------
 net/ipv6/xfrm6_policy.c |  1 -
 5 files changed, 64 insertions(+), 72 deletions(-)

-- 
2.35.1.265.g69c8d7142f-goog

[PATCH net-next 1/4] ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This counter has never been visible, there is little point
trying to maintain it.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ip6_fib.h   | 3 +--
 net/ipv6/route.c        | 4 ----
 net/ipv6/xfrm6_policy.c | 1 -
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 40ae8f1b18e502cece9f11b3a60c9cc30c2e1a5e..32c83bb688798b5c12f8aeea1c66debe866689d8 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -367,9 +367,8 @@ struct rt6_statistics {
 	__u32		fib_rt_cache;		/* cached rt entries in exception table */
 	__u32		fib_discarded_routes;	/* total number of routes delete */
 
-	/* The following stats are not protected by any lock */
+	/* The following stat is not protected by any lock */
 	atomic_t	fib_rt_alloc;		/* total number of routes alloced */
-	atomic_t	fib_rt_uncache;		/* rt entries in uncached list */
 };
 
 #define RTN_TL_ROOT	0x0001
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f4884cda13b92e72d041680cbabfee2e07ec0f10..53acd1568ebcceb1a1697579ed505cefc7a35a65 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -149,11 +149,9 @@ void rt6_uncached_list_del(struct rt6_info *rt)
 {
 	if (!list_empty(&rt->rt6i_uncached)) {
 		struct uncached_list *ul = rt->rt6i_uncached_list;
-		struct net *net = dev_net(rt->dst.dev);
 
 		spin_lock_bh(&ul->lock);
 		list_del(&rt->rt6i_uncached);
-		atomic_dec(&net->ipv6.rt6_stats->fib_rt_uncache);
 		spin_unlock_bh(&ul->lock);
 	}
 }
@@ -2244,7 +2242,6 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 			 * if caller sets RT6_LOOKUP_F_DST_NOREF flag.
 			 */
 			rt6_uncached_list_add(rt);
-			atomic_inc(&net->ipv6.rt6_stats->fib_rt_uncache);
 			rcu_read_unlock();
 
 			return rt;
@@ -3287,7 +3284,6 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	 * do proper release of the net_device
 	 */
 	rt6_uncached_list_add(rt);
-	atomic_inc(&net->ipv6.rt6_stats->fib_rt_uncache);
 
 	dst = xfrm_lookup(net, &rt->dst, flowi6_to_flowi(fl6), NULL, 0);
 
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index fad687ee6dd81af9ee3591eb2333cfed8ceae8ce..55bb2cbae13ddc236e420fae37cd42bbad14d1b2 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -92,7 +92,6 @@ static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
 	xdst->u.rt6.rt6i_src = rt->rt6i_src;
 	INIT_LIST_HEAD(&xdst->u.rt6.rt6i_uncached);
 	rt6_uncached_list_add(&xdst->u.rt6);
-	atomic_inc(&dev_net(dev)->ipv6.rt6_stats->fib_rt_uncache);
 
 	return 0;
 }
-- 
2.35.1.265.g69c8d7142f-goog

[PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

IPv6 addrconf notifiers wants the loopback device to
be the last device being dismantled at netns deletion.

This caused many limitations and work arounds.

Back in linux-5.3, Mahesh added a per host blackhole_netdev
that can be used whenever we need to make sure objects no longer
refer to a disappearing device.

If we attach to blackhole_netdev an ip6_ptr (allocate an idev),
then we can use this special device (which is never freed)
in place of the loopback_dev (which can be freed).

This will permit improvements in netdev_run_todo() and other parts
of the stack where had steps to make sure loopback_dev was
the last device to disappear.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>
---
 net/ipv6/addrconf.c | 78 +++++++++++++++++++--------------------------
 net/ipv6/route.c    | 21 +++++-------
 2 files changed, 40 insertions(+), 59 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4f402bc38f056e08f3761e63a7bc7a51e54e9384..02d31d4fcab3b3d529c4fe3260216ecee1108e82 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -372,7 +372,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 
 	ASSERT_RTNL();
 
-	if (dev->mtu < IPV6_MIN_MTU)
+	if (dev->mtu < IPV6_MIN_MTU && dev != blackhole_netdev)
 		return ERR_PTR(-EINVAL);
 
 	ndev = kzalloc(sizeof(struct inet6_dev), GFP_KERNEL);
@@ -400,21 +400,22 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 	/* We refer to the device */
 	dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
 
-	if (snmp6_alloc_dev(ndev) < 0) {
-		netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
-			   __func__);
-		neigh_parms_release(&nd_tbl, ndev->nd_parms);
-		dev_put_track(dev, &ndev->dev_tracker);
-		kfree(ndev);
-		return ERR_PTR(err);
-	}
+	if (dev != blackhole_netdev) {
+		if (snmp6_alloc_dev(ndev) < 0) {
+			netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
+				   __func__);
+			neigh_parms_release(&nd_tbl, ndev->nd_parms);
+			dev_put_track(dev, &ndev->dev_tracker);
+			kfree(ndev);
+			return ERR_PTR(err);
+		}
 
-	if (snmp6_register_dev(ndev) < 0) {
-		netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
-			   __func__, dev->name);
-		goto err_release;
+		if (snmp6_register_dev(ndev) < 0) {
+			netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
+				   __func__, dev->name);
+			goto err_release;
+		}
 	}
-
 	/* One reference from device. */
 	refcount_set(&ndev->refcnt, 1);
 
@@ -445,25 +446,28 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
 
 	ipv6_mc_init_dev(ndev);
 	ndev->tstamp = jiffies;
-	err = addrconf_sysctl_register(ndev);
-	if (err) {
-		ipv6_mc_destroy_dev(ndev);
-		snmp6_unregister_dev(ndev);
-		goto err_release;
+	if (dev != blackhole_netdev) {
+		err = addrconf_sysctl_register(ndev);
+		if (err) {
+			ipv6_mc_destroy_dev(ndev);
+			snmp6_unregister_dev(ndev);
+			goto err_release;
+		}
 	}
 	/* protected by rtnl_lock */
 	rcu_assign_pointer(dev->ip6_ptr, ndev);
 
-	/* Join interface-local all-node multicast group */
-	ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
+	if (dev != blackhole_netdev) {
+		/* Join interface-local all-node multicast group */
+		ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
 
-	/* Join all-node multicast group */
-	ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
-
-	/* Join all-router multicast group if forwarding is set */
-	if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
-		ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
+		/* Join all-node multicast group */
+		ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
 
+		/* Join all-router multicast group if forwarding is set */
+		if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
+			ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
+	}
 	return ndev;
 
 err_release:
@@ -7233,26 +7237,8 @@ int __init addrconf_init(void)
 		goto out_nowq;
 	}
 
-	/* The addrconf netdev notifier requires that loopback_dev
-	 * has it's ipv6 private information allocated and setup
-	 * before it can bring up and give link-local addresses
-	 * to other devices which are up.
-	 *
-	 * Unfortunately, loopback_dev is not necessarily the first
-	 * entry in the global dev_base list of net devices.  In fact,
-	 * it is likely to be the very last entry on that list.
-	 * So this causes the notifier registry below to try and
-	 * give link-local addresses to all devices besides loopback_dev
-	 * first, then loopback_dev, which cases all the non-loopback_dev
-	 * devices to fail to get a link-local address.
-	 *
-	 * So, as a temporary fix, allocate the ipv6 structure for
-	 * loopback_dev first by hand.
-	 * Longer term, all of the dependencies ipv6 has upon the loopback
-	 * device and it being up should be removed.
-	 */
 	rtnl_lock();
-	idev = ipv6_add_dev(init_net.loopback_dev);
+	idev = ipv6_add_dev(blackhole_netdev);
 	rtnl_unlock();
 	if (IS_ERR(idev)) {
 		err = PTR_ERR(idev);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 53acd1568ebcceb1a1697579ed505cefc7a35a65..5fc1a1de9481c859adc332746ccfcf237db6541f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -156,14 +156,10 @@ void rt6_uncached_list_del(struct rt6_info *rt)
 	}
 }
 
-static void rt6_uncached_list_flush_dev(struct net *net, struct net_device *dev)
+static void rt6_uncached_list_flush_dev(struct net_device *dev)
 {
-	struct net_device *loopback_dev = net->loopback_dev;
 	int cpu;
 
-	if (dev == loopback_dev)
-		return;
-
 	for_each_possible_cpu(cpu) {
 		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
 		struct rt6_info *rt;
@@ -174,7 +170,7 @@ static void rt6_uncached_list_flush_dev(struct net *net, struct net_device *dev)
 			struct net_device *rt_dev = rt->dst.dev;
 
 			if (rt_idev->dev == dev) {
-				rt->rt6i_idev = in6_dev_get(loopback_dev);
+				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
 				in6_dev_put(rt_idev);
 			}
 
@@ -371,13 +367,12 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 {
 	struct rt6_info *rt = (struct rt6_info *)dst;
 	struct inet6_dev *idev = rt->rt6i_idev;
-	struct net_device *loopback_dev =
-		dev_net(dev)->loopback_dev;
 
-	if (idev && idev->dev != loopback_dev) {
-		struct inet6_dev *loopback_idev = in6_dev_get(loopback_dev);
-		if (loopback_idev) {
-			rt->rt6i_idev = loopback_idev;
+	if (idev && idev->dev != blackhole_netdev) {
+		struct inet6_dev *blackhole_idev = in6_dev_get(blackhole_netdev);
+
+		if (blackhole_idev) {
+			rt->rt6i_idev = blackhole_idev;
 			in6_dev_put(idev);
 		}
 	}
@@ -4892,7 +4887,7 @@ void rt6_sync_down_dev(struct net_device *dev, unsigned long event)
 void rt6_disable_ip(struct net_device *dev, unsigned long event)
 {
 	rt6_sync_down_dev(dev, event);
-	rt6_uncached_list_flush_dev(dev_net(dev), dev);
+	rt6_uncached_list_flush_dev(dev);
 	neigh_ifdown(&nd_tbl, dev);
 }
 
-- 
2.35.1.265.g69c8d7142f-goog

[PATCH net-next 3/4] ipv6: add (struct uncached_list)->quarantine list

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This is an optimization to keep the per-cpu lists as short as possible:

Whenever rt6_uncached_list_flush_dev() changes one rt6_info
matching the disappearing device, it can can transfer the object
to a quarantine list, waiting for a final rt6_uncached_list_del().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/route.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5fc1a1de9481c859adc332746ccfcf237db6541f..6690666c9b0e32e7e801ac481876ea4aa31e4ead 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -130,6 +130,7 @@ static struct fib6_info *rt6_get_route_info(struct net *net,
 struct uncached_list {
 	spinlock_t		lock;
 	struct list_head	head;
+	struct list_head	quarantine;
 };
 
 static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt6_uncached_list);
@@ -151,7 +152,7 @@ void rt6_uncached_list_del(struct rt6_info *rt)
 		struct uncached_list *ul = rt->rt6i_uncached_list;
 
 		spin_lock_bh(&ul->lock);
-		list_del(&rt->rt6i_uncached);
+		list_del_init(&rt->rt6i_uncached);
 		spin_unlock_bh(&ul->lock);
 	}
 }
@@ -162,16 +163,21 @@ static void rt6_uncached_list_flush_dev(struct net_device *dev)
 
 	for_each_possible_cpu(cpu) {
 		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
-		struct rt6_info *rt;
+		struct rt6_info *rt, *safe;
+
+		if (list_empty(&ul->head))
+			continue;
 
 		spin_lock_bh(&ul->lock);
-		list_for_each_entry(rt, &ul->head, rt6i_uncached) {
+		list_for_each_entry_safe(rt, safe, &ul->head, rt6i_uncached) {
 			struct inet6_dev *rt_idev = rt->rt6i_idev;
 			struct net_device *rt_dev = rt->dst.dev;
+			bool handled = false;
 
 			if (rt_idev->dev == dev) {
 				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
 				in6_dev_put(rt_idev);
+				handled = true;
 			}
 
 			if (rt_dev == dev) {
@@ -179,7 +185,11 @@ static void rt6_uncached_list_flush_dev(struct net_device *dev)
 				dev_replace_track(rt_dev, blackhole_netdev,
 						  &rt->dst.dev_tracker,
 						  GFP_ATOMIC);
+				handled = true;
 			}
+			if (handled)
+				list_move(&rt->rt6i_uncached,
+					  &ul->quarantine);
 		}
 		spin_unlock_bh(&ul->lock);
 	}
@@ -6721,6 +6731,7 @@ int __init ip6_route_init(void)
 		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
 
 		INIT_LIST_HEAD(&ul->head);
+		INIT_LIST_HEAD(&ul->quarantine);
 		spin_lock_init(&ul->lock);
 	}
 
-- 
2.35.1.265.g69c8d7142f-goog

[PATCH net-next 4/4] ipv4: add (struct uncached_list)->quarantine list

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

This is an optimization to keep the per-cpu lists as short as possible:

Whenever rt_flush_dev() changes one rtable dst.dev
matching the disappearing device, it can can transfer the object
to a quarantine list, waiting for a final rt_del_uncached_list().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/route.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 634766e6c7cc8a472e912f7d4e99bb6be0397bb6..202d6b1fff43fb095427720ec36fe3744aeb7149 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1485,6 +1485,7 @@ static bool rt_cache_route(struct fib_nh_common *nhc, struct rtable *rt)
 struct uncached_list {
 	spinlock_t		lock;
 	struct list_head	head;
+	struct list_head	quarantine;
 };
 
 static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt_uncached_list);
@@ -1506,7 +1507,7 @@ void rt_del_uncached_list(struct rtable *rt)
 		struct uncached_list *ul = rt->rt_uncached_list;
 
 		spin_lock_bh(&ul->lock);
-		list_del(&rt->rt_uncached);
+		list_del_init(&rt->rt_uncached);
 		spin_unlock_bh(&ul->lock);
 	}
 }
@@ -1521,20 +1522,24 @@ static void ipv4_dst_destroy(struct dst_entry *dst)
 
 void rt_flush_dev(struct net_device *dev)
 {
-	struct rtable *rt;
+	struct rtable *rt, *safe;
 	int cpu;
 
 	for_each_possible_cpu(cpu) {
 		struct uncached_list *ul = &per_cpu(rt_uncached_list, cpu);
 
+		if (list_empty(&ul->head))
+			continue;
+
 		spin_lock_bh(&ul->lock);
-		list_for_each_entry(rt, &ul->head, rt_uncached) {
+		list_for_each_entry_safe(rt, safe, &ul->head, rt_uncached) {
 			if (rt->dst.dev != dev)
 				continue;
 			rt->dst.dev = blackhole_netdev;
 			dev_replace_track(dev, blackhole_netdev,
 					  &rt->dst.dev_tracker,
 					  GFP_ATOMIC);
+			list_move(&rt->rt_uncached, &ul->quarantine);
 		}
 		spin_unlock_bh(&ul->lock);
 	}
@@ -3706,6 +3711,7 @@ int __init ip_rt_init(void)
 		struct uncached_list *ul = &per_cpu(rt_uncached_list, cpu);
 
 		INIT_LIST_HEAD(&ul->head);
+		INIT_LIST_HEAD(&ul->quarantine);
 		spin_lock_init(&ul->lock);
 	}
 #ifdef CONFIG_IP_ROUTE_CLASSID
-- 
2.35.1.265.g69c8d7142f-goog

Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 10 Feb 2022 13:42:27 -0800 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Second patch in this series removes IPv6 requirement about the netns
> loopback device being the last device being dismantled.
> 
> This was needed because rt6_uncached_list_flush_dev()
> and ip6_dst_ifdown() had to switch dst dev to a known
> device (loopback).
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache
    https://git.kernel.org/netdev/net-next/c/2d4feb2c1ba7
  - [net-next,2/4] ipv6: give an IPv6 dev to blackhole_netdev
    https://git.kernel.org/netdev/net-next/c/e5f80fcf869a
  - [net-next,3/4] ipv6: add (struct uncached_list)->quarantine list
    https://git.kernel.org/netdev/net-next/c/ba55ef81637c
  - [net-next,4/4] ipv4: add (struct uncached_list)->quarantine list
    https://git.kernel.org/netdev/net-next/c/29e5375d7fcb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback

[Jakub Kicinski]

On Thu, 10 Feb 2022 13:42:27 -0800 Eric Dumazet wrote:
> Second patch in this series removes IPv6 requirement about the netns
> loopback device being the last device being dismantled.

Great!

> This was needed because rt6_uncached_list_flush_dev()
> and ip6_dst_ifdown() had to switch dst dev to a known
> device (loopback).
> 
> Instead of loopback, we can use the (hidden) blackhole_netdev
> which is also always there.
> 
> This will allow future simplfications of netdev_run_to()

Should I take a stab at it, or is it on your todo list, anyway?

> and other parts of the stack like default_device_exit_batch().
> 
> Last two patches are optimizations for both IP families.

Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback

[Eric Dumazet]

On Fri, Feb 11, 2022 at 3:57 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 10 Feb 2022 13:42:27 -0800 Eric Dumazet wrote:
> > Second patch in this series removes IPv6 requirement about the netns
> > loopback device being the last device being dismantled.
>
> Great!
>
> > This was needed because rt6_uncached_list_flush_dev()
> > and ip6_dst_ifdown() had to switch dst dev to a known
> > device (loopback).
> >
> > Instead of loopback, we can use the (hidden) blackhole_netdev
> > which is also always there.
> >
> > This will allow future simplfications of netdev_run_to()
>
> Should I take a stab at it, or is it on your todo list, anyway?

I have not started this yet, I will be happy to review your patches, thanks !

Re: [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev

[Ido Schimmel]

On Thu, Feb 10, 2022 at 01:42:29PM -0800, Eric Dumazet wrote:
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 4f402bc38f056e08f3761e63a7bc7a51e54e9384..02d31d4fcab3b3d529c4fe3260216ecee1108e82 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -372,7 +372,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
>  
>  	ASSERT_RTNL();
>  
> -	if (dev->mtu < IPV6_MIN_MTU)
> +	if (dev->mtu < IPV6_MIN_MTU && dev != blackhole_netdev)
>  		return ERR_PTR(-EINVAL);
>  
>  	ndev = kzalloc(sizeof(struct inet6_dev), GFP_KERNEL);
> @@ -400,21 +400,22 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
>  	/* We refer to the device */
>  	dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
>  
> -	if (snmp6_alloc_dev(ndev) < 0) {
> -		netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> -			   __func__);
> -		neigh_parms_release(&nd_tbl, ndev->nd_parms);
> -		dev_put_track(dev, &ndev->dev_tracker);
> -		kfree(ndev);
> -		return ERR_PTR(err);
> -	}
> +	if (dev != blackhole_netdev) {
> +		if (snmp6_alloc_dev(ndev) < 0) {
> +			netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> +				   __func__);
> +			neigh_parms_release(&nd_tbl, ndev->nd_parms);
> +			dev_put_track(dev, &ndev->dev_tracker);
> +			kfree(ndev);
> +			return ERR_PTR(err);
> +		}

Hi,

Our regression machines crashed with the following splat [1]. Can be
reproduced with:

# ./fib_nexthops.sh -t ipv6_torture

From tools/testing/selftests/net

Only had a couple of minutes (need to leave), but looks like the
following patch helps [2]. I can continue working on it tomorrow
morning, but wanted to mention it now in case you have a better idea /
someone else bumps into the same issue.

Thanks

[1]
[  210.313674] ==================================================================
[  210.314672] BUG: KASAN: null-ptr-deref in __ip6_make_skb+0x17fb/0x20a0
[  210.315410] Write of size 8 at addr 0000000000000c00 by task ping/290
[  210.316183]
[  210.316359] CPU: 2 PID: 290 Comm: ping Not tainted 5.17.0-rc3-custom-02030-g17b65b552bb5 #1215
[  210.317335] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1.fc35 04/01/2014
[  210.318318] Call Trace:
[  210.318618]  <TASK>
[  210.318876]  dump_stack_lvl+0x8b/0xb3
[  210.319803]  kasan_report.cold+0x116/0x11b
[  210.320876]  kasan_check_range+0xf5/0x1d0
[  210.321348]  __ip6_make_skb+0x17fb/0x20a0
[  210.323985]  ip6_push_pending_frames+0xcd/0x110
[  210.324628]  rawv6_sendmsg+0x29f5/0x37f0
[  210.329150]  inet_sendmsg+0x9e/0xe0
[  210.329596]  __sys_sendto+0x23d/0x360
[  210.333029]  __x64_sys_sendto+0xe1/0x1b0
[  210.334542]  do_syscall_64+0x35/0x80
[  210.334975]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[  210.335632] RIP: 0033:0x7f2d8cd2f3aa
[  210.336041] Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 7e c3 0f 1f 44 00 00 41 54 48 83 ec 30 44 89
[  210.338299] RSP: 002b:00007ffe1a5ec768 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[  210.339345] RAX: ffffffffffffffda RBX: 0000000000000038 RCX: 00007f2d8cd2f3aa
[  210.340207] RDX: 0000000000000040 RSI: 00007f2d8cfbb320 RDI: 0000000000000004
[  210.341284] RBP: 00007ffe1a5ec7f0 R08: 00007ffe1a5efee4 R09: 000000000000001c
[  210.342212] R10: 0000000000000800 R11: 0000000000000246 R12: 00007f2d8cebb480
[  210.343008] R13: 00007f2d8cea7f3d R14: 00007f2d8cea0c00 R15: 00007f2d8cebaa70
[  210.343785]  </TASK>
[  210.344344] ==================================================================

(gdb) l *(__ip6_make_skb+0x17fb)
0xffffffff8389fb5b is in __ip6_make_skb (./arch/x86/include/asm/atomic64_64.h:88).
83       *
84       * Atomically increments @v by 1.
85       */
86      static __always_inline void arch_atomic64_inc(atomic64_t *v)
87      {
88              asm volatile(LOCK_PREFIX "incq %0"
89                           : "=m" (v->counter)
90                           : "m" (v->counter) : "memory");
91      }
92      #define arch_atomic64_inc arch_atomic64_inc

[2]
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 02d31d4fcab3..57fbd6f03ff8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -400,16 +400,16 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
        /* We refer to the device */
        dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
 
-       if (dev != blackhole_netdev) {
-               if (snmp6_alloc_dev(ndev) < 0) {
-                       netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
-                                  __func__);
-                       neigh_parms_release(&nd_tbl, ndev->nd_parms);
-                       dev_put_track(dev, &ndev->dev_tracker);
-                       kfree(ndev);
-                       return ERR_PTR(err);
-               }
+       if (snmp6_alloc_dev(ndev) < 0) {
+               netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
+                          __func__);
+               neigh_parms_release(&nd_tbl, ndev->nd_parms);
+               dev_put_track(dev, &ndev->dev_tracker);
+               kfree(ndev);
+               return ERR_PTR(err);
+       }
 
+       if (dev != blackhole_netdev) {
                if (snmp6_register_dev(ndev) < 0) {
                        netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
                                   __func__, dev->name);

>  
> -	if (snmp6_register_dev(ndev) < 0) {
> -		netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
> -			   __func__, dev->name);
> -		goto err_release;
> +		if (snmp6_register_dev(ndev) < 0) {
> +			netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
> +				   __func__, dev->name);
> +			goto err_release;
> +		}
>  	}
> -
>  	/* One reference from device. */
>  	refcount_set(&ndev->refcnt, 1);
>  
> @@ -445,25 +446,28 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
>  
>  	ipv6_mc_init_dev(ndev);
>  	ndev->tstamp = jiffies;
> -	err = addrconf_sysctl_register(ndev);
> -	if (err) {
> -		ipv6_mc_destroy_dev(ndev);
> -		snmp6_unregister_dev(ndev);
> -		goto err_release;
> +	if (dev != blackhole_netdev) {
> +		err = addrconf_sysctl_register(ndev);
> +		if (err) {
> +			ipv6_mc_destroy_dev(ndev);
> +			snmp6_unregister_dev(ndev);
> +			goto err_release;
> +		}
>  	}
>  	/* protected by rtnl_lock */
>  	rcu_assign_pointer(dev->ip6_ptr, ndev);
>  
> -	/* Join interface-local all-node multicast group */
> -	ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
> +	if (dev != blackhole_netdev) {
> +		/* Join interface-local all-node multicast group */
> +		ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
>  
> -	/* Join all-node multicast group */
> -	ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
> -
> -	/* Join all-router multicast group if forwarding is set */
> -	if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
> -		ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
> +		/* Join all-node multicast group */
> +		ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
>  
> +		/* Join all-router multicast group if forwarding is set */
> +		if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
> +			ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
> +	}
>  	return ndev;
>  
>  err_release:

Re: [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev

[Eric Dumazet]

On Sun, Feb 13, 2022 at 10:05 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Thu, Feb 10, 2022 at 01:42:29PM -0800, Eric Dumazet wrote:
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 4f402bc38f056e08f3761e63a7bc7a51e54e9384..02d31d4fcab3b3d529c4fe3260216ecee1108e82 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -372,7 +372,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
> >
> >       ASSERT_RTNL();
> >
> > -     if (dev->mtu < IPV6_MIN_MTU)
> > +     if (dev->mtu < IPV6_MIN_MTU && dev != blackhole_netdev)
> >               return ERR_PTR(-EINVAL);
> >
> >       ndev = kzalloc(sizeof(struct inet6_dev), GFP_KERNEL);
> > @@ -400,21 +400,22 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
> >       /* We refer to the device */
> >       dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
> >
> > -     if (snmp6_alloc_dev(ndev) < 0) {
> > -             netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> > -                        __func__);
> > -             neigh_parms_release(&nd_tbl, ndev->nd_parms);
> > -             dev_put_track(dev, &ndev->dev_tracker);
> > -             kfree(ndev);
> > -             return ERR_PTR(err);
> > -     }
> > +     if (dev != blackhole_netdev) {
> > +             if (snmp6_alloc_dev(ndev) < 0) {
> > +                     netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> > +                                __func__);
> > +                     neigh_parms_release(&nd_tbl, ndev->nd_parms);
> > +                     dev_put_track(dev, &ndev->dev_tracker);
> > +                     kfree(ndev);
> > +                     return ERR_PTR(err);
> > +             }
>
> Hi,
>
> Our regression machines crashed with the following splat [1]. Can be
> reproduced with:
>
> # ./fib_nexthops.sh -t ipv6_torture
>
> From tools/testing/selftests/net
>
> Only had a couple of minutes (need to leave), but looks like the
> following patch helps [2]. I can continue working on it tomorrow
> morning, but wanted to mention it now in case you have a better idea /
> someone else bumps into the same issue.
>
> Thanks
>
> [1]
> [  210.313674] ==================================================================
> [  210.314672] BUG: KASAN: null-ptr-deref in __ip6_make_skb+0x17fb/0x20a0
> [  210.315410] Write of size 8 at addr 0000000000000c00 by task ping/290
> [  210.316183]
> [  210.316359] CPU: 2 PID: 290 Comm: ping Not tainted 5.17.0-rc3-custom-02030-g17b65b552bb5 #1215
> [  210.317335] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1.fc35 04/01/2014
> [  210.318318] Call Trace:
> [  210.318618]  <TASK>
> [  210.318876]  dump_stack_lvl+0x8b/0xb3
> [  210.319803]  kasan_report.cold+0x116/0x11b
> [  210.320876]  kasan_check_range+0xf5/0x1d0
> [  210.321348]  __ip6_make_skb+0x17fb/0x20a0
> [  210.323985]  ip6_push_pending_frames+0xcd/0x110
> [  210.324628]  rawv6_sendmsg+0x29f5/0x37f0
> [  210.329150]  inet_sendmsg+0x9e/0xe0
> [  210.329596]  __sys_sendto+0x23d/0x360
> [  210.333029]  __x64_sys_sendto+0xe1/0x1b0
> [  210.334542]  do_syscall_64+0x35/0x80
> [  210.334975]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  210.335632] RIP: 0033:0x7f2d8cd2f3aa
> [  210.336041] Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 7e c3 0f 1f 44 00 00 41 54 48 83 ec 30 44 89
> [  210.338299] RSP: 002b:00007ffe1a5ec768 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
> [  210.339345] RAX: ffffffffffffffda RBX: 0000000000000038 RCX: 00007f2d8cd2f3aa
> [  210.340207] RDX: 0000000000000040 RSI: 00007f2d8cfbb320 RDI: 0000000000000004
> [  210.341284] RBP: 00007ffe1a5ec7f0 R08: 00007ffe1a5efee4 R09: 000000000000001c
> [  210.342212] R10: 0000000000000800 R11: 0000000000000246 R12: 00007f2d8cebb480
> [  210.343008] R13: 00007f2d8cea7f3d R14: 00007f2d8cea0c00 R15: 00007f2d8cebaa70
> [  210.343785]  </TASK>
> [  210.344344] ==================================================================
>
> (gdb) l *(__ip6_make_skb+0x17fb)
> 0xffffffff8389fb5b is in __ip6_make_skb (./arch/x86/include/asm/atomic64_64.h:88).
> 83       *
> 84       * Atomically increments @v by 1.
> 85       */
> 86      static __always_inline void arch_atomic64_inc(atomic64_t *v)
> 87      {
> 88              asm volatile(LOCK_PREFIX "incq %0"
> 89                           : "=m" (v->counter)
> 90                           : "m" (v->counter) : "memory");
> 91      }
> 92      #define arch_atomic64_inc arch_atomic64_inc
>
> [2]
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 02d31d4fcab3..57fbd6f03ff8 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -400,16 +400,16 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
>         /* We refer to the device */
>         dev_hold_track(dev, &ndev->dev_tracker, GFP_KERNEL);
>
> -       if (dev != blackhole_netdev) {
> -               if (snmp6_alloc_dev(ndev) < 0) {
> -                       netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> -                                  __func__);
> -                       neigh_parms_release(&nd_tbl, ndev->nd_parms);
> -                       dev_put_track(dev, &ndev->dev_tracker);
> -                       kfree(ndev);
> -                       return ERR_PTR(err);
> -               }
> +       if (snmp6_alloc_dev(ndev) < 0) {
> +               netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
> +                          __func__);
> +               neigh_parms_release(&nd_tbl, ndev->nd_parms);
> +               dev_put_track(dev, &ndev->dev_tracker);
> +               kfree(ndev);
> +               return ERR_PTR(err);
> +       }
>
> +       if (dev != blackhole_netdev) {
>                 if (snmp6_register_dev(ndev) < 0) {
>                         netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
>                                    __func__, dev->name);
>
> >
> > -     if (snmp6_register_dev(ndev) < 0) {
> > -             netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
> > -                        __func__, dev->name);
> > -             goto err_release;
> > +             if (snmp6_register_dev(ndev) < 0) {
> > +                     netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
> > +                                __func__, dev->name);
> > +                     goto err_release;
> > +             }
> >       }
> > -
> >       /* One reference from device. */
> >       refcount_set(&ndev->refcnt, 1);
> >
> > @@ -445,25 +446,28 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
> >
> >       ipv6_mc_init_dev(ndev);
> >       ndev->tstamp = jiffies;
> > -     err = addrconf_sysctl_register(ndev);
> > -     if (err) {
> > -             ipv6_mc_destroy_dev(ndev);
> > -             snmp6_unregister_dev(ndev);
> > -             goto err_release;
> > +     if (dev != blackhole_netdev) {
> > +             err = addrconf_sysctl_register(ndev);
> > +             if (err) {
> > +                     ipv6_mc_destroy_dev(ndev);
> > +                     snmp6_unregister_dev(ndev);
> > +                     goto err_release;
> > +             }
> >       }
> >       /* protected by rtnl_lock */
> >       rcu_assign_pointer(dev->ip6_ptr, ndev);
> >
> > -     /* Join interface-local all-node multicast group */
> > -     ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
> > +     if (dev != blackhole_netdev) {
> > +             /* Join interface-local all-node multicast group */
> > +             ipv6_dev_mc_inc(dev, &in6addr_interfacelocal_allnodes);
> >
> > -     /* Join all-node multicast group */
> > -     ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
> > -
> > -     /* Join all-router multicast group if forwarding is set */
> > -     if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
> > -             ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
> > +             /* Join all-node multicast group */
> > +             ipv6_dev_mc_inc(dev, &in6addr_linklocal_allnodes);
> >
> > +             /* Join all-router multicast group if forwarding is set */
> > +             if (ndev->cnf.forwarding && (dev->flags & IFF_MULTICAST))
> > +                     ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
> > +     }
> >       return ndev;
> >
> >  err_release:

Hi Ido

I was coming to a similar conclusion, with an internal syzbot report.

Thanks.