mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Pavlu <petr.pavlu@suse.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	David Ahern <dsahern@kernel.org>
Cc: Kui-Feng Lee <thinker.li@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Petr Pavlu <petr.pavlu@suse.com>
Subject: [PATCH net] net/ipv6: Fix the RT cache flush via sysctl using a previous delay
Date: Wed, 29 May 2024 15:52:51 +0200	[thread overview]
Message-ID: <20240529135251.4074-1-petr.pavlu@suse.com> (raw)

The net.ipv6.route.flush system parameter takes a value which specifies
a delay used during the flush operation for aging exception routes. The
written value is however not used in the currently requested flush and
instead utilized only in the next one.

A problem is that ipv6_sysctl_rtcache_flush() first reads the old value
of net->ipv6.sysctl.flush_delay into a local delay variable and then
calls proc_dointvec() which actually updates the sysctl based on the
provided input.

Fix the problem by removing net->ipv6.sysctl.flush_delay because the
value is never actually used after the flush operation and instead use
a temporary ctl_table in ipv6_sysctl_rtcache_flush() pointing directly
to the local delay variable.

Fixes: 4990509f19e8 ("[NETNS][IPV6]: Make sysctls route per namespace.")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---

Note that when testing this fix, I noticed that an aging exception route
(created via ICMP redirect) was not getting removed when triggering the
flush operation unless the associated fib6_info was an expiring route.
It looks the logic introduced in 5eb902b8e719 ("net/ipv6: Remove expired
routes with a separated list of routes.") otherwise missed registering
the fib6_info with the GC. That is potentially a separate issue, just
adding it here in case someone decides to test this patch and possibly
run into this problem too.

 include/net/netns/ipv6.h |  1 -
 net/ipv6/route.c         | 13 ++++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 5f2cfd84570a..2ed7659013a4 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -20,7 +20,6 @@ struct netns_sysctl_ipv6 {
 	struct ctl_table_header *frags_hdr;
 	struct ctl_table_header *xfrm6_hdr;
 #endif
-	int flush_delay;
 	int ip6_rt_max_size;
 	int ip6_rt_gc_min_interval;
 	int ip6_rt_gc_timeout;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bbc2a0dd9314..f07f050003c3 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6335,15 +6335,17 @@ static int rt6_stats_seq_show(struct seq_file *seq, void *v)
 static int ipv6_sysctl_rtcache_flush(struct ctl_table *ctl, int write,
 			      void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct net *net;
+	struct net *net = ctl->extra1;
+	struct ctl_table lctl;
 	int delay;
 	int ret;
+
 	if (!write)
 		return -EINVAL;
 
-	net = (struct net *)ctl->extra1;
-	delay = net->ipv6.sysctl.flush_delay;
-	ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
+	lctl = *ctl;
+	lctl.data = &delay;
+	ret = proc_dointvec(&lctl, write, buffer, lenp, ppos);
 	if (ret)
 		return ret;
 
@@ -6368,7 +6370,6 @@ static struct ctl_table ipv6_route_table_template[] = {
 	},
 	{
 		.procname	=	"flush",
-		.data		=	&init_net.ipv6.sysctl.flush_delay,
 		.maxlen		=	sizeof(int),
 		.mode		=	0200,
 		.proc_handler	=	ipv6_sysctl_rtcache_flush
@@ -6444,7 +6445,6 @@ struct ctl_table * __net_init ipv6_route_sysctl_init(struct net *net)
 	if (table) {
 		table[0].data = &net->ipv6.sysctl.ip6_rt_max_size;
 		table[1].data = &net->ipv6.ip6_dst_ops.gc_thresh;
-		table[2].data = &net->ipv6.sysctl.flush_delay;
 		table[2].extra1 = net;
 		table[3].data = &net->ipv6.sysctl.ip6_rt_gc_min_interval;
 		table[4].data = &net->ipv6.sysctl.ip6_rt_gc_timeout;
@@ -6521,7 +6521,6 @@ static int __net_init ip6_route_net_init(struct net *net)
 #endif
 #endif
 
-	net->ipv6.sysctl.flush_delay = 0;
 	net->ipv6.sysctl.ip6_rt_max_size = INT_MAX;
 	net->ipv6.sysctl.ip6_rt_gc_min_interval = HZ / 2;
 	net->ipv6.sysctl.ip6_rt_gc_timeout = 60*HZ;

base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
-- 
2.35.3


             reply	other threads:[~2024-05-29 13:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29 13:52 Petr Pavlu [this message]
     [not found] ` <CAHE2DV1S4oKved063WaYzqsoiEe1hY=ZoRxjFfPX1m0-N0MsdQ@mail.gmail.com>
2024-05-31  8:53   ` Petr Pavlu
2024-06-04  8:30     ` Paolo Abeni
2024-06-07  8:33       ` Petr Pavlu

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=20240529135251.4074-1-petr.pavlu@suse.com \
    --to=petr.pavlu@suse.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=thinker.li@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®