All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Arges <carges@cloudflare.com>
To: Ido Schimmel <idosch@nvidia.com>
Cc: David Ahern <dsahern@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>, Shuah Khan <shuah@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, kernel-team@cloudflare.com
Subject: Re: [PATCH net-next v2 2/3] ipv6: hash uncached routes by device
Date: Thu, 17 Sep 2026 14:41:54 -0500	[thread overview]
Message-ID: <aqxCgm7lV4FruQho@20HS2G4> (raw)
In-Reply-To: <20260917101405.GA1202580@shredder>

On 2026-09-17 13:14:05, Ido Schimmel wrote:
> On Mon, Sep 14, 2026 at 09:03:36PM -0500, Chris J Arges wrote:
> > rt6_uncached_list_flush_dev() currently walks every per-CPU uncached route
> > list for each device being removed. Hash uncached routes by their inet6
> > device so ordinary device teardown only visits the matching bucket on each
> > CPU.
> 
> The code is doing something else and hashing using dst_dev():
> 
> 	struct net_device *rt_dev = dst_dev(&rt->dst);
> [...]
> 		ul = &table->buckets[hash_ptr(rt_dev,
> 					      CONFIG_IPV6_UNCACHED_ROUTE_HASH_BITS)];
> 
> > 
> > ip6_rt_get_dev_rcu() can return loopback or an L3 master while rt6i_idev
> > still refers to the original interface, so such a route must be reachable
> > from either device. Place those routes on a separate per-CPU list that is
> > always visited in addition to the keyed bucket.
> > 
> > This avoids growing struct rt6_info while filtering most unrelated routes
> > from ordinary device teardown.
> > 
> > The table has 2^CONFIG_IPV6_UNCACHED_ROUTE_HASH_BITS buckets and defaults
> > to 64. Larger values shorten each bucket, but every additional bit doubles
> > the per-CPU memory used by the table. The default costs approximately
> > 1.5 KiB per possible CPU on x86-64.
> > 
> > Signed-off-by: Chris J Arges <carges@cloudflare.com>
> > ---
> >  net/ipv6/Kconfig |  13 +++++++
> 
> Same comment as in patch 1 about the Kconfig.
> 
> >  net/ipv6/route.c | 102 +++++++++++++++++++++++++++++++++++++------------------
> >  2 files changed, 82 insertions(+), 33 deletions(-)
> > 
> > diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
> > index c3806c6ac96f..0253178668bd 100644
> > --- a/net/ipv6/Kconfig
> > +++ b/net/ipv6/Kconfig
> > @@ -18,6 +18,19 @@ menuconfig IPV6
> >  
> >  if IPV6
> >  
> > +config IPV6_UNCACHED_ROUTE_HASH_BITS
> > +	int "IPv6 uncached route hash bits"
> > +	range 1 10
> > +	default 6
> > +	help
> > +	  This option sets the number of buckets used in the IPv6 uncached
> > +	  route hash table to 2^IPV6_UNCACHED_ROUTE_HASH_BITS buckets. The
> > +	  allowed values select between 2 and 1024 buckets. Larger values
> > +	  reduce collisions, but each additional bit doubles the per-CPU
> > +	  memory used by the table.
> > +
> > +	  If unsure, use the default of 6 bits (64 buckets).
> > +
> >  config IPV6_ROUTER_PREF
> >  	bool "IPv6: Router Preference (RFC 4191) support"
> >  	help
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 7535b09068a0..080dce329168 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -40,6 +40,7 @@
> >  #include <linux/seq_file.h>
> >  #include <linux/nsproxy.h>
> >  #include <linux/slab.h>
> > +#include <linux/hash.h>
> >  #include <linux/jhash.h>
> >  #include <linux/siphash.h>
> >  #include <net/net_namespace.h>
> > @@ -133,11 +134,27 @@ struct uncached_list {
> >  	struct list_head	head;
> >  };
> >  
> > -static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt6_uncached_list);
> > +#define RT6_UNCACHED_HASH_SIZE	BIT(CONFIG_IPV6_UNCACHED_ROUTE_HASH_BITS)
> > +
> > +struct rt6_uncached_table {
> > +	struct uncached_list buckets[RT6_UNCACHED_HASH_SIZE];
> > +	/* Routes that must be discoverable through two different devices. */
> > +	struct uncached_list mismatch;
> > +};
> > +
> > +static DEFINE_PER_CPU_ALIGNED(struct rt6_uncached_table, rt6_uncached_table);
> >  
> >  void rt6_uncached_list_add(struct rt6_info *rt)
> >  {
> > -	struct uncached_list *ul = raw_cpu_ptr(&rt6_uncached_list);
> > +	struct rt6_uncached_table *table = raw_cpu_ptr(&rt6_uncached_table);
> > +	struct net_device *rt_dev = dst_dev(&rt->dst);
> > +	struct uncached_list *ul;
> > +
> > +	if (rt->rt6i_idev && rt->rt6i_idev->dev != rt_dev)
> > +		ul = &table->mismatch;
> > +	else
> > +		ul = &table->buckets[hash_ptr(rt_dev,
> > +					      CONFIG_IPV6_UNCACHED_ROUTE_HASH_BITS)];
> >  
> >  	rt->dst.rt_uncached_list = ul;
> >  
> > @@ -157,40 +174,51 @@ void rt6_uncached_list_del(struct rt6_info *rt)
> >  	}
> >  }
> >  
> > +static void rt6_uncached_list_flush(struct uncached_list *ul,
> > +				    struct net_device *dev)
> > +{
> > +	struct rt6_info *rt, *safe;
> > +
> > +	if (list_empty(&ul->head))
> > +		return;
> > +
> > +	spin_lock_bh(&ul->lock);
> > +	list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
> > +		struct inet6_dev *rt_idev = rt->rt6i_idev;
> > +		struct net_device *rt_dev = dst_dev(&rt->dst);
> > +		bool handled = false;
> 
> https://docs.kernel.org/next/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
> 
> > +
> > +		if (rt_idev && rt_idev->dev == dev) {
> > +			rt->rt6i_idev = in6_dev_get(blackhole_netdev);
> > +			in6_dev_put(rt_idev);
> > +			handled = true;
> > +		}
> > +
> > +		if (rt_dev == dev) {
> > +			rt->dst.dev = blackhole_netdev;
> 
> Please use:
> 
> rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
> 
> See commit 1469773b246a ("ipv4: use rcu_assign_pointer() in
> rt_flush_dev()")
> 
> > +			netdev_ref_replace(rt_dev, blackhole_netdev,
> > +					   &rt->dst.dev_tracker, GFP_ATOMIC);
> > +			handled = true;
> > +		}
> > +		if (handled)
> > +			list_del_init(&rt->dst.rt_uncached);
> > +	}
> > +	spin_unlock_bh(&ul->lock);
> > +}
> > +
> >  static void rt6_uncached_list_flush_dev(struct net_device *dev)
> >  {
> >  	int cpu;
> >  
> >  	for_each_possible_cpu(cpu) {
> > -		struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
> > -		struct rt6_info *rt, *safe;
> > +		struct rt6_uncached_table *table;
> > +		struct uncached_list *ul;
> >  
> > -		if (list_empty(&ul->head))
> > -			continue;
> > -
> > -		spin_lock_bh(&ul->lock);
> > -		list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
> > -			struct inet6_dev *rt_idev = rt->rt6i_idev;
> > -			struct net_device *rt_dev = rt->dst.dev;
> > -			bool handled = false;
> > -
> > -			if (rt_idev && rt_idev->dev == dev) {
> > -				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
> > -				in6_dev_put(rt_idev);
> > -				handled = true;
> > -			}
> > -
> > -			if (rt_dev == dev) {
> > -				rt->dst.dev = blackhole_netdev;
> > -				netdev_ref_replace(rt_dev, blackhole_netdev,
> > -						   &rt->dst.dev_tracker,
> > -						   GFP_ATOMIC);
> > -				handled = true;
> > -			}
> > -			if (handled)
> > -				list_del_init(&rt->dst.rt_uncached);
> > -		}
> > -		spin_unlock_bh(&ul->lock);
> > +		table = per_cpu_ptr(&rt6_uncached_table, cpu);
> > +		ul = &table->buckets[hash_ptr(dev,
> > +					      CONFIG_IPV6_UNCACHED_ROUTE_HASH_BITS)];
> > +		rt6_uncached_list_flush(ul, dev);
> > +		rt6_uncached_list_flush(&table->mismatch, dev);
> 
> The mismatch list can be quite long depending on the workload and every
> device needs to walk it for every CPU.
> 
> AFAICT, when there is a mismatch, dst_dev() is either loopback or a VRF
> device. Can you instead hash based on rt6i_idev->dev (fallback to
> dst_dev() when not available) and only iterate over all the buckets when
> the device that is going away is loopback / VRF?
> 
> That way, in the common case, you only need to walk one list per-CPU.

Ido,
Yea I like this approach much better. Thank you for the reviews.
I've re-tested and implemented your feedback into v3:
https://lore.kernel.org/all/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
--chris

  reply	other threads:[~2026-09-17 19:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  2:03 [PATCH net-next v2 0/3] net: hash uncached route lists by device Chris J Arges
2026-09-15  2:03 ` [PATCH net-next v2 1/3] ipv4: hash uncached routes " Chris J Arges
2026-09-16 16:49   ` Ido Schimmel
2026-09-19  2:48   ` netdev-bot+sashiko
2026-09-15  2:03 ` [PATCH net-next v2 2/3] ipv6: " Chris J Arges
2026-09-17 10:14   ` Ido Schimmel
2026-09-17 19:41     ` Chris Arges [this message]
2026-09-19  2:48   ` netdev-bot+sashiko
2026-09-15  2:03 ` [PATCH net-next v2 3/3] selftests: net: cover IPv6 uncached route device mismatch Chris J Arges
2026-09-19  2:48   ` netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqxCgm7lV4FruQho@20HS2G4 \
    --to=carges@cloudflare.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kernel-team@cloudflare.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.