diff mbox series

[net-next,v6,1/2] net/ipv6: Remove expired routes with a separated list of routes.

Message ID 20230808180309.542341-2-thinker.li@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Remove expired routes with a separated list of routes. | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1831 this patch: 1831
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 1361 this patch: 1361
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1857 this patch: 1857
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kui-Feng Lee Aug. 8, 2023, 6:03 p.m. UTC 
From: Kui-Feng Lee <thinker.li@gmail.com>

FIB6 GC walks trees of fib6_tables to remove expired routes. Walking a tree
can be expensive if the number of routes in a table is big, even if most of
them are permanent. Checking routes in a separated list of routes having
expiration will avoid this potential issue.

Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
 include/net/ip6_fib.h | 65 ++++++++++++++++++++++++++++++++++---------
 net/ipv6/ip6_fib.c    | 57 +++++++++++++++++++++++++++++++++----
 net/ipv6/route.c      |  6 ++--
 3 files changed, 106 insertions(+), 22 deletions(-)

Comments

David Ahern Aug. 11, 2023, 3:09 p.m. UTC | #1 
On 8/8/23 12:03 PM, thinker.li@gmail.com wrote:
> @@ -504,6 +500,49 @@ void fib6_gc_cleanup(void);
>  
>  int fib6_init(void);
>  
> +/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
> + * NULL.
> + */
> +static inline void fib6_set_expires_locked(struct fib6_info *f6i, unsigned long expires)
> +{
> +	struct fib6_table *tb6;
> +
> +	tb6 = f6i->fib6_table;
> +	f6i->expires = expires;
> +	if (tb6 && !fib6_has_expires(f6i))
> +		hlist_add_head(&f6i->gc_link, &tb6->tb6_gc_hlist);
> +	f6i->fib6_flags |= RTF_EXPIRES;
> +}
> +
> +/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
> + * NULL.  If fib6_table is NULL, the fib6_info will no be inserted into the
> + * list of GC candidates until it is inserted into a table.
> + */
> +static inline void fib6_set_expires(struct fib6_info *f6i, unsigned long expires)
> +{
> +	spin_lock_bh(&f6i->fib6_table->tb6_lock);
> +	fib6_set_expires_locked(f6i, expires);
> +	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
> +}
> +
> +static inline void fib6_clean_expires_locked(struct fib6_info *f6i)
> +{
> +	struct fib6_table *tb6;
> +
> +	tb6 = f6i->fib6_table;
> +	if (tb6 && fib6_has_expires(f6i))
> +		hlist_del_init(&f6i->gc_link);

The tb6 check is not needed; if the fib6_info is on a gc list it should
be removed here.

> +	f6i->fib6_flags &= ~RTF_EXPIRES;
> +	f6i->expires = 0;
> +}
> +
> +static inline void fib6_clean_expires(struct fib6_info *f6i)
> +{
> +	spin_lock_bh(&f6i->fib6_table->tb6_lock);
> +	fib6_clean_expires_locked(f6i);
> +	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
> +}
> +
>  struct ipv6_route_iter {
>  	struct seq_net_private p;
>  	struct fib6_walker w;


> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index bac768d36cc1..8e86b67fe5ef 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1057,6 +1060,11 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
>  				    lockdep_is_held(&table->tb6_lock));
>  		}
>  	}
> +
> +	if (fib6_has_expires(rt)) {
> +		hlist_del_init(&rt->gc_link);
> +		rt->fib6_flags &= ~RTF_EXPIRES;
> +	}

Use fib6_clean_expires_locked here.

With those 2 changes:
Reviewed-by: David Ahern <dsahern@kernel.org>


--
pw-bot: cr
Kui-Feng Lee Aug. 11, 2023, 11:37 p.m. UTC | #2 
On 8/11/23 08:09, David Ahern wrote:
> On 8/8/23 12:03 PM, thinker.li@gmail.com wrote:
>> @@ -504,6 +500,49 @@ void fib6_gc_cleanup(void);
>>   
>>   int fib6_init(void);
>>   
>> +/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
>> + * NULL.
>> + */
>> +static inline void fib6_set_expires_locked(struct fib6_info *f6i, unsigned long expires)
>> +{
>> +	struct fib6_table *tb6;
>> +
>> +	tb6 = f6i->fib6_table;
>> +	f6i->expires = expires;
>> +	if (tb6 && !fib6_has_expires(f6i))
>> +		hlist_add_head(&f6i->gc_link, &tb6->tb6_gc_hlist);
>> +	f6i->fib6_flags |= RTF_EXPIRES;
>> +}
>> +
>> +/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
>> + * NULL.  If fib6_table is NULL, the fib6_info will no be inserted into the
>> + * list of GC candidates until it is inserted into a table.
>> + */
>> +static inline void fib6_set_expires(struct fib6_info *f6i, unsigned long expires)
>> +{
>> +	spin_lock_bh(&f6i->fib6_table->tb6_lock);
>> +	fib6_set_expires_locked(f6i, expires);
>> +	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
>> +}
>> +
>> +static inline void fib6_clean_expires_locked(struct fib6_info *f6i)
>> +{
>> +	struct fib6_table *tb6;
>> +
>> +	tb6 = f6i->fib6_table;
>> +	if (tb6 && fib6_has_expires(f6i))
>> +		hlist_del_init(&f6i->gc_link);
> 
> The tb6 check is not needed; if the fib6_info is on a gc list it should
> be removed here.

I will fix it.

> 
>> +	f6i->fib6_flags &= ~RTF_EXPIRES;
>> +	f6i->expires = 0;
>> +}
>> +
>> +static inline void fib6_clean_expires(struct fib6_info *f6i)
>> +{
>> +	spin_lock_bh(&f6i->fib6_table->tb6_lock);
>> +	fib6_clean_expires_locked(f6i);
>> +	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
>> +}
>> +
>>   struct ipv6_route_iter {
>>   	struct seq_net_private p;
>>   	struct fib6_walker w;
> 
> 
>> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>> index bac768d36cc1..8e86b67fe5ef 100644
>> --- a/net/ipv6/ip6_fib.c
>> +++ b/net/ipv6/ip6_fib.c
>> @@ -1057,6 +1060,11 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
>>   				    lockdep_is_held(&table->tb6_lock));
>>   		}
>>   	}
>> +
>> +	if (fib6_has_expires(rt)) {
>> +		hlist_del_init(&rt->gc_link);
>> +		rt->fib6_flags &= ~RTF_EXPIRES;
>> +	}
> 
> Use fib6_clean_expires_locked here.

Got it!

> 
> With those 2 changes:
> Reviewed-by: David Ahern <dsahern@kernel.org>
> 
> 
> --
> pw-bot: cr
diff mbox series

Patch

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 05e6f756feaf..e6f4d986fb63 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -179,6 +179,9 @@  struct fib6_info {
 
 	refcount_t			fib6_ref;
 	unsigned long			expires;
+
+	struct hlist_node		gc_link;
+
 	struct dst_metrics		*fib6_metrics;
 #define fib6_pmtu		fib6_metrics->metrics[RTAX_MTU-1]
 
@@ -247,19 +250,6 @@  static inline bool fib6_requires_src(const struct fib6_info *rt)
 	return rt->fib6_src.plen > 0;
 }
 
-static inline void fib6_clean_expires(struct fib6_info *f6i)
-{
-	f6i->fib6_flags &= ~RTF_EXPIRES;
-	f6i->expires = 0;
-}
-
-static inline void fib6_set_expires(struct fib6_info *f6i,
-				    unsigned long expires)
-{
-	f6i->expires = expires;
-	f6i->fib6_flags |= RTF_EXPIRES;
-}
-
 static inline bool fib6_check_expired(const struct fib6_info *f6i)
 {
 	if (f6i->fib6_flags & RTF_EXPIRES)
@@ -267,6 +257,11 @@  static inline bool fib6_check_expired(const struct fib6_info *f6i)
 	return false;
 }
 
+static inline bool fib6_has_expires(const struct fib6_info *f6i)
+{
+	return f6i->fib6_flags & RTF_EXPIRES;
+}
+
 /* Function to safely get fn->fn_sernum for passed in rt
  * and store result in passed in cookie.
  * Return true if we can get cookie safely
@@ -388,6 +383,7 @@  struct fib6_table {
 	struct inet_peer_base	tb6_peers;
 	unsigned int		flags;
 	unsigned int		fib_seq;
+	struct hlist_head       tb6_gc_hlist;	/* GC candidates */
 #define RT6_TABLE_HAS_DFLT_ROUTER	BIT(0)
 };
 
@@ -504,6 +500,49 @@  void fib6_gc_cleanup(void);
 
 int fib6_init(void);
 
+/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
+ * NULL.
+ */
+static inline void fib6_set_expires_locked(struct fib6_info *f6i, unsigned long expires)
+{
+	struct fib6_table *tb6;
+
+	tb6 = f6i->fib6_table;
+	f6i->expires = expires;
+	if (tb6 && !fib6_has_expires(f6i))
+		hlist_add_head(&f6i->gc_link, &tb6->tb6_gc_hlist);
+	f6i->fib6_flags |= RTF_EXPIRES;
+}
+
+/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
+ * NULL.  If fib6_table is NULL, the fib6_info will no be inserted into the
+ * list of GC candidates until it is inserted into a table.
+ */
+static inline void fib6_set_expires(struct fib6_info *f6i, unsigned long expires)
+{
+	spin_lock_bh(&f6i->fib6_table->tb6_lock);
+	fib6_set_expires_locked(f6i, expires);
+	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
+}
+
+static inline void fib6_clean_expires_locked(struct fib6_info *f6i)
+{
+	struct fib6_table *tb6;
+
+	tb6 = f6i->fib6_table;
+	if (tb6 && fib6_has_expires(f6i))
+		hlist_del_init(&f6i->gc_link);
+	f6i->fib6_flags &= ~RTF_EXPIRES;
+	f6i->expires = 0;
+}
+
+static inline void fib6_clean_expires(struct fib6_info *f6i)
+{
+	spin_lock_bh(&f6i->fib6_table->tb6_lock);
+	fib6_clean_expires_locked(f6i);
+	spin_unlock_bh(&f6i->fib6_table->tb6_lock);
+}
+
 struct ipv6_route_iter {
 	struct seq_net_private p;
 	struct fib6_walker w;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index bac768d36cc1..8e86b67fe5ef 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -160,6 +160,8 @@  struct fib6_info *fib6_info_alloc(gfp_t gfp_flags, bool with_fib6_nh)
 	INIT_LIST_HEAD(&f6i->fib6_siblings);
 	refcount_set(&f6i->fib6_ref, 1);
 
+	INIT_HLIST_NODE(&f6i->gc_link);
+
 	return f6i;
 }
 
@@ -246,6 +248,7 @@  static struct fib6_table *fib6_alloc_table(struct net *net, u32 id)
 				   net->ipv6.fib6_null_entry);
 		table->tb6_root.fn_flags = RTN_ROOT | RTN_TL_ROOT | RTN_RTINFO;
 		inet_peer_base_init(&table->tb6_peers);
+		INIT_HLIST_HEAD(&table->tb6_gc_hlist);
 	}
 
 	return table;
@@ -1057,6 +1060,11 @@  static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
 				    lockdep_is_held(&table->tb6_lock));
 		}
 	}
+
+	if (fib6_has_expires(rt)) {
+		hlist_del_init(&rt->gc_link);
+		rt->fib6_flags &= ~RTF_EXPIRES;
+	}
 }
 
 /*
@@ -1118,9 +1126,9 @@  static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
 				if (!(iter->fib6_flags & RTF_EXPIRES))
 					return -EEXIST;
 				if (!(rt->fib6_flags & RTF_EXPIRES))
-					fib6_clean_expires(iter);
+					fib6_clean_expires_locked(iter);
 				else
-					fib6_set_expires(iter, rt->expires);
+					fib6_set_expires_locked(iter, rt->expires);
 
 				if (rt->fib6_pmtu)
 					fib6_metric_set(iter, RTAX_MTU,
@@ -1479,6 +1487,10 @@  int fib6_add(struct fib6_node *root, struct fib6_info *rt,
 		if (rt->nh)
 			list_add(&rt->nh_list, &rt->nh->f6i_list);
 		__fib6_update_sernum_upto_root(rt, fib6_new_sernum(info->nl_net));
+
+		if (fib6_has_expires(rt))
+			hlist_add_head(&rt->gc_link, &table->tb6_gc_hlist);
+
 		fib6_start_gc(info->nl_net, rt);
 	}
 
@@ -2285,9 +2297,8 @@  static void fib6_flush_trees(struct net *net)
  *	Garbage collection
  */
 
-static int fib6_age(struct fib6_info *rt, void *arg)
+static int fib6_age(struct fib6_info *rt, struct fib6_gc_args *gc_args)
 {
-	struct fib6_gc_args *gc_args = arg;
 	unsigned long now = jiffies;
 
 	/*
@@ -2295,7 +2306,7 @@  static int fib6_age(struct fib6_info *rt, void *arg)
 	 *	Routes are expired even if they are in use.
 	 */
 
-	if (rt->fib6_flags & RTF_EXPIRES && rt->expires) {
+	if (fib6_has_expires(rt) && rt->expires) {
 		if (time_after(now, rt->expires)) {
 			RT6_TRACE("expiring %p\n", rt);
 			return -1;
@@ -2312,6 +2323,40 @@  static int fib6_age(struct fib6_info *rt, void *arg)
 	return 0;
 }
 
+static void fib6_gc_table(struct net *net,
+			  struct fib6_table *tb6,
+			  struct fib6_gc_args *gc_args)
+{
+	struct fib6_info *rt;
+	struct hlist_node *n;
+	struct nl_info info = {
+		.nl_net = net,
+		.skip_notify = false,
+	};
+
+	hlist_for_each_entry_safe(rt, n, &tb6->tb6_gc_hlist, gc_link)
+		if (fib6_age(rt, gc_args) == -1)
+			fib6_del(rt, &info);
+}
+
+static void fib6_gc_all(struct net *net, struct fib6_gc_args *gc_args)
+{
+	struct fib6_table *table;
+	struct hlist_head *head;
+	unsigned int h;
+
+	rcu_read_lock();
+	for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
+		head = &net->ipv6.fib_table_hash[h];
+		hlist_for_each_entry_rcu(table, head, tb6_hlist) {
+			spin_lock_bh(&table->tb6_lock);
+			fib6_gc_table(net, table, gc_args);
+			spin_unlock_bh(&table->tb6_lock);
+		}
+	}
+	rcu_read_unlock();
+}
+
 void fib6_run_gc(unsigned long expires, struct net *net, bool force)
 {
 	struct fib6_gc_args gc_args;
@@ -2327,7 +2372,7 @@  void fib6_run_gc(unsigned long expires, struct net *net, bool force)
 			  net->ipv6.sysctl.ip6_rt_gc_interval;
 	gc_args.more = 0;
 
-	fib6_clean_all(net, fib6_age, &gc_args);
+	fib6_gc_all(net, &gc_args);
 	now = jiffies;
 	net->ipv6.ip6_rt_last_gc = now;
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 10751df16dab..db10c36f34bb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3761,10 +3761,10 @@  static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
 		rt->dst_nocount = true;
 
 	if (cfg->fc_flags & RTF_EXPIRES)
-		fib6_set_expires(rt, jiffies +
-				clock_t_to_jiffies(cfg->fc_expires));
+		fib6_set_expires_locked(rt, jiffies +
+					clock_t_to_jiffies(cfg->fc_expires));
 	else
-		fib6_clean_expires(rt);
+		fib6_clean_expires_locked(rt);
 
 	if (cfg->fc_protocol == RTPROT_UNSPEC)
 		cfg->fc_protocol = RTPROT_BOOT;