Re: [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart

On 6/19/26 11:58 AM, Nicolas Dichtel wrote:

Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :

quoted

When handling proxy_ndp, if rtnl_net_trylock() fails, the operation is
retried but as the value was already modified by the initial
proc_dointvec() call, the restarted syscall will read the newly modified
value as the 'old' state.

Fix this by restoring the original value and position pointer before
restarting the syscall.

Is it not better to call rtnl_net_trylock() at the beginning of the function?
It avoids flapping the sysctl value.

IMHO it is not better if we want to reduce the time we are holding RTNL 
lock. I think the idea is that if the user introduces a invalid value, 
we don't need to take the lock at all.

That is the general pattern I see around the sysctl code (IPv4 and 
IPv6). Given the current efforts to reduce the usage of RTNL I think 
this approach would be better.

In any case, it is not a blocker for me so if we all agree that your 
suggestion is better I don't mind taking that path.

Thanks for all the reviews!

quoted

Fixes: c92d5491a6d9 ("netconf: add support for IPv6 proxy_ndp")
Signed-off-by: Fernando Fernandez Mancera <redacted>
---
  net/ipv6/addrconf.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8ff015975e27..1cfb223476bd 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c

@@ -6483,8 +6483,9 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
  		void *buffer, size_t *lenp, loff_t *ppos)
  {
  	int *valp = ctl->data;
-	int ret;
+	loff_t pos = *ppos;
  	int old, new;
+	int ret;
  
  	old = *valp;
  	ret = proc_dointvec(ctl, write, buffer, lenp, ppos);

@@ -6493,8 +6494,12 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
  	if (write && old != new) {
  		struct net *net = ctl->extra2;
  
-		if (!rtnl_net_trylock(net))
+		if (!rtnl_net_trylock(net)) {
+			/* Restore the original values before restarting */
+			*valp = old;
+			*ppos = pos;
  			return restart_syscall();
+		}
  
  		if (valp == &net->ipv6.devconf_dflt->proxy_ndp) {
  			inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,