* [PATCH net-next 1/4] ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
@ 2022-02-10 21:42 ` Eric Dumazet
2022-02-10 21:42 ` [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev Eric Dumazet
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-02-10 21:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Mahesh Bandewar, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
2022-02-10 21:42 ` [PATCH net-next 1/4] ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache Eric Dumazet
@ 2022-02-10 21:42 ` Eric Dumazet
2022-02-13 18:05 ` Ido Schimmel
2022-02-10 21:42 ` [PATCH net-next 3/4] ipv6: add (struct uncached_list)->quarantine list Eric Dumazet
` (3 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2022-02-10 21:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Mahesh Bandewar, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev
2022-02-10 21:42 ` [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev Eric Dumazet
@ 2022-02-13 18:05 ` Ido Schimmel
2022-02-13 18:12 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2022-02-13 18:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, netdev, Eric Dumazet, Mahesh Bandewar
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:
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev
2022-02-13 18:05 ` Ido Schimmel
@ 2022-02-13 18:12 ` Eric Dumazet
0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-02-13 18:12 UTC (permalink / raw)
To: Ido Schimmel
Cc: Eric Dumazet, David S . Miller, Jakub Kicinski, netdev, Mahesh Bandewar
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next 3/4] ipv6: add (struct uncached_list)->quarantine list
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
2022-02-10 21:42 ` [PATCH net-next 1/4] ipv6: get rid of net->ipv6.rt6_stats->fib_rt_uncache Eric Dumazet
2022-02-10 21:42 ` [PATCH net-next 2/4] ipv6: give an IPv6 dev to blackhole_netdev Eric Dumazet
@ 2022-02-10 21:42 ` Eric Dumazet
2022-02-10 21:42 ` [PATCH net-next 4/4] ipv4: " Eric Dumazet
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-02-10 21:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Mahesh Bandewar, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 4/4] ipv4: add (struct uncached_list)->quarantine list
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
` (2 preceding siblings ...)
2022-02-10 21:42 ` [PATCH net-next 3/4] ipv6: add (struct uncached_list)->quarantine list Eric Dumazet
@ 2022-02-10 21:42 ` Eric Dumazet
2022-02-11 12:00 ` [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback patchwork-bot+netdevbpf
2022-02-11 23:57 ` Jakub Kicinski
5 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-02-10 21:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Mahesh Bandewar, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
` (3 preceding siblings ...)
2022-02-10 21:42 ` [PATCH net-next 4/4] ipv4: " Eric Dumazet
@ 2022-02-11 12:00 ` patchwork-bot+netdevbpf
2022-02-11 23:57 ` Jakub Kicinski
5 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-02-11 12:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, kuba, netdev, edumazet, maheshb
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback
2022-02-10 21:42 [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback Eric Dumazet
` (4 preceding siblings ...)
2022-02-11 12:00 ` [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback patchwork-bot+netdevbpf
@ 2022-02-11 23:57 ` Jakub Kicinski
2022-02-12 2:38 ` Eric Dumazet
5 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2022-02-11 23:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, Mahesh Bandewar
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next 0/4] ipv6: remove addrconf reliance on loopback
2022-02-11 23:57 ` Jakub Kicinski
@ 2022-02-12 2:38 ` Eric Dumazet
0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2022-02-12 2:38 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Eric Dumazet, David S . Miller, netdev, Mahesh Bandewar
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 !
^ permalink raw reply [flat|nested] 10+ messages in thread