Re: [PATCH] ipv6 addrconf: remove addrconf_sysctl_hop_limit()

[David Miller]

From: Maciej Żenczykowski 
Date: Thu, 29 Sep 2016 00:33:43 -0700

> From: Maciej Żenczykowski 
> 
> This is an effective no-op in terms of user observable behaviour.
> 
> By preventing the overwrite of non-null extra1/extra2 fields
> in addrconf_sysctl() we can enable the use of proc_dointvec_minmax().
> 
> This allows us to eliminate the constant min/max (1..255) trampoline
> function that is addrconf_sysctl_hop_limit().
> 
> This is nice because it simplifies the code, and allows future
> sysctls with constant min/max limits to also not require trampolines.
> 
> We still can't eliminate the trampoline for mtu because it isn't
> actually a constant (it depends on other tunables of the device)
> and thus requires at-write-time logic to enforce range.
> 
> Signed-off-by: Maciej Żenczykowski 

Applied, thanks.

Re: [PATCH] ipv6 addrconf: remove addrconf_sysctl_hop_limit()

[Erik Kline]

Seems fine to me.

Acked-by: Erik Kline 

[PATCH] ipv6 addrconf: remove addrconf_sysctl_hop_limit()

[Maciej Żenczykowski]

From: Maciej Żenczykowski 

This is an effective no-op in terms of user observable behaviour.

By preventing the overwrite of non-null extra1/extra2 fields
in addrconf_sysctl() we can enable the use of proc_dointvec_minmax().

This allows us to eliminate the constant min/max (1..255) trampoline
function that is addrconf_sysctl_hop_limit().

This is nice because it simplifies the code, and allows future
sysctls with constant min/max limits to also not require trampolines.

We still can't eliminate the trampoline for mtu because it isn't
actually a constant (it depends on other tunables of the device)
and thus requires at-write-time logic to enforce range.

Signed-off-by: Maciej Żenczykowski 
---
 net/ipv6/addrconf.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..8bd2d06eefe7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5467,20 +5467,6 @@ int addrconf_sysctl_forward(struct ctl_table *ctl, int 
write,
 }
 
 static
-int addrconf_sysctl_hop_limit(struct ctl_table *ctl, int write,
-  void __user *buffer, size_t *lenp, loff_t *ppos)
-{
-   struct ctl_table lctl;
-   int min_hl = 1, max_hl = 255;
-
-   lctl = *ctl;
-   lctl.extra1 = _hl;
-   lctl.extra2 = _hl;
-
-   return proc_dointvec_minmax(, write, buffer, lenp, ppos);
-}
-
-static
 int addrconf_sysctl_mtu(struct ctl_table *ctl, int write,
void __user *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -5713,6 +5699,9 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct 
ctl_table *ctl,
return ret;
 }
 
+static const int one = 1;
+static const int two_five_five = 255;
+
 static const struct ctl_table addrconf_sysctl[] = {
{
.procname   = "forwarding",
@@ -5726,7 +5715,9 @@ static const struct ctl_table addrconf_sysctl[] = {
.data   = _devconf.hop_limit,
.maxlen = sizeof(int),
.mode   = 0644,
-   .proc_handler   = addrconf_sysctl_hop_limit,
+   .proc_handler   = proc_dointvec_minmax,
+   .extra1 = (void *),
+   .extra2 = (void *)_five_five,
},
{
.procname   = "mtu",
@@ -6044,8 +6035,14 @@ static int __addrconf_sysctl_register(struct net *net, 
char *dev_name,
 
for (i = 0; table[i].data; i++) {
table[i].data += (char *)p - (char *)_devconf;
-   table[i].extra1 = idev; /* embedded; no ref */
-   table[i].extra2 = net;
+   /* If one of these is already set, then it is not safe to
+* overwrite either of them: this makes proc_dointvec_minmax
+* usable.
+*/
+   if (!table[i].extra1 && !table[i].extra2) {
+   table[i].extra1 = idev; /* embedded; no ref */
+   table[i].extra2 = net;
+   }
}
 
snprintf(path, sizeof(path), "net/ipv6/conf/%s", dev_name);
-- 
2.8.0.rc3.226.g39d4020