netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Implement rfc7559 ipv6 router solicitation backoff
@ 2016-09-24 16:01 Maciej Żenczykowski
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:01 UTC (permalink / raw)
  To: Linux NetDev, David Miller, Erik Kline, Lorenzo Colitti



^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-24 16:01 Implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
@ 2016-09-24 16:05 ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
                     ` (5 more replies)
  0 siblings, 6 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..11fa1a5564d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6044,8 +6044,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 *)&ipv6_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

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

replace with extra1/2 magic

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 11fa1a5564d4..3a835495fb53 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 = &min_hl;
-	lctl.extra2 = &max_hl;
-
-	return proc_dointvec_minmax(&lctl, 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 int one = 1;
+static 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		= &ipv6_devconf.hop_limit,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= addrconf_sysctl_hop_limit,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &one,
+		.extra2		= &two_five_five,
 	},
 	{
 		.procname	= "mtu",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to -1 meaning an unlimited number of retransmits.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3a835495fb53..6c63bf06fbcf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
 	if (idev->if_flags & IF_RA_RCVD)
 		goto out;
 
-	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
+	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
 		write_unlock(&idev->lock);
 		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
 			ndisc_send_rs(dev, &lladdr,
@@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
-		  ifp->idev->cnf.rtr_solicits > 0 &&
+		  ifp->idev->cnf.rtr_solicits != 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
 	read_unlock_bh(&ifp->idev->lock);
 
@@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits <= 0)
+	if (idev->cnf.rtr_solicits == 0)
 		return -EINVAL;
 
 	write_lock_bh(&idev->lock);
@@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static int minus_one = -1;
 static int one = 1;
 static int two_five_five = 255;
 
@@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Accessible via:
  /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval

For now we default it to the same value as the normal interval.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/linux/ipv6.h      |  1 +
 include/net/addrconf.h    |  1 +
 include/uapi/linux/ipv6.h |  1 +
 net/ipv6/addrconf.c       | 11 +++++++++++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c6dbcd84a2c7..7e9a789be5e0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -18,6 +18,7 @@ struct ipv6_devconf {
 	__s32		dad_transmits;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
+	__s32		rtr_solicit_max_interval;
 	__s32		rtr_solicit_delay;
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9826d3a9464c..275e5af4c2f4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,6 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 395876060f50..8c2772340c3f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -177,6 +177,7 @@ enum {
 	DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
 	DEVCONF_DROP_UNSOLICITED_NA,
 	DEVCONF_KEEP_ADDR_ON_DOWN,
+	DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c63bf06fbcf..255be34cdbce 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
 	array[DEVCONF_RTR_SOLICIT_INTERVAL] =
 		jiffies_to_msecs(cnf->rtr_solicit_interval);
+	array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
+		jiffies_to_msecs(cnf->rtr_solicit_max_interval);
 	array[DEVCONF_RTR_SOLICIT_DELAY] =
 		jiffies_to_msecs(cnf->rtr_solicit_delay);
 	array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
@@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "router_solicitation_max_interval",
+		.data		= &ipv6_devconf.rtr_solicit_max_interval,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.procname	= "router_solicitation_delay",
 		.data		= &ipv6_devconf.rtr_solicit_delay,
 		.maxlen		= sizeof(int),
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                     ` (2 preceding siblings ...)
  2016-09-24 16:05   ` [PATCH 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  2016-09-25  2:14     ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  5 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 255be34cdbce..f2147b3352b9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,24 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 initial)
+{
+	s64 r = 900000 + (prandom_u32() % 200001); /* 0.9 .. 1.1 */
+	s32 v = initial * r / 1000000;
+	return v;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
+{
+	s64 r = 1900000 + (prandom_u32() % 200001); /* 1.9 .. 2.1 */
+	s32 v = cur * r / 1000000;
+	if (v > ceiling) {
+		r -= 1000000; /* 0.9 .. 1.1 */
+		v = ceiling * r / 1000000;
+	}
+	return v;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3716,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3993,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5153,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                     ` (3 preceding siblings ...)
  2016-09-24 16:05   ` [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  2016-09-24 16:05   ` [PATCH 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  5 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitation_max_interval
  /proc/sys/net/ipv6/conf/default/router_solicitation_max_interval
from 4 seconds to 1 hour.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 275e5af4c2f4..8f3677269f9a 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,7 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
-#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited)
  2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                     ` (4 preceding siblings ...)
  2016-09-24 16:05   ` [PATCH 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
@ 2016-09-24 16:05   ` Maciej Żenczykowski
  5 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-24 16:05 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitations
  /proc/sys/net/ipv6/conf/default/router_solicitations
from 3 to unlimited.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 8f3677269f9a..f2d072787947 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -1,7 +1,7 @@
 #ifndef _ADDRCONF_H
 #define _ADDRCONF_H
 
-#define MAX_RTR_SOLICITATIONS		3
+#define MAX_RTR_SOLICITATIONS		-1		/* unlimited */
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
 #define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* Re: [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-24 16:05   ` [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-25  2:14     ` Maciej Żenczykowski
  2016-09-25  2:14       ` [PATCH] " Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  2:14 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: Linux NetDev, Erik Kline, Lorenzo Colitti

Ok, so that seems to have all sorts of __divdi3 or __aeabi_ldivmod
undefined errors on 32-bit platforms (ie. arm/m68k/i386).

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25  2:14     ` Maciej Żenczykowski
@ 2016-09-25  2:14       ` Maciej Żenczykowski
  2016-09-25  9:33         ` David Miller
  0 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  2:14 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 255be34cdbce..6384a1cde056 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,24 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 initial)
+{
+	u32 r = (9 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = initial * (u64)r >> 20;       /* ~ multiply by 0.9 .. 1.1 */
+	return v;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
+{
+	u32 r = (19 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = cur * (u64)r >> 20;           /* ~ multiply by 1.9 .. 2.1 */
+	if (v > ceiling) {
+		r -= 1 << 20;
+		v = ceiling * (u64)r >> 20;   /* ~ multiply by 0.9 .. 1.1 */
+	}
+	return v;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3716,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3993,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5153,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* Re: [PATCH] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25  2:14       ` [PATCH] " Maciej Żenczykowski
@ 2016-09-25  9:33         ` David Miller
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: David Miller @ 2016-09-25  9:33 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, netdev, ek, lorenzo


Please do not do this.

When you need to repsin a patch in a patch series to fix
or otherwise resolve something, you must make a fresh
resubmission of the entire patch series.

You also must properly mark the resubmission with a
proper indication that this is a new version of the
patch series by saying, for example, "[PATCH v2 ..."
in the Subject lines.

Thanks.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-25  9:33         ` David Miller
@ 2016-09-25  9:59           ` Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
                               ` (6 more replies)
  0 siblings, 7 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..11fa1a5564d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6044,8 +6044,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 *)&ipv6_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

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
                               ` (5 subsequent siblings)
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

replace with extra1/2 magic

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 11fa1a5564d4..3a835495fb53 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 = &min_hl;
-	lctl.extra2 = &max_hl;
-
-	return proc_dointvec_minmax(&lctl, 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 int one = 1;
+static 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		= &ipv6_devconf.hop_limit,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= addrconf_sysctl_hop_limit,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &one,
+		.extra2		= &two_five_five,
 	},
 	{
 		.procname	= "mtu",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-26 15:34               ` Lorenzo Colitti
  2016-09-25  9:59             ` [PATCH v2 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
                               ` (4 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to -1 meaning an unlimited number of retransmits.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3a835495fb53..6c63bf06fbcf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
 	if (idev->if_flags & IF_RA_RCVD)
 		goto out;
 
-	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
+	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
 		write_unlock(&idev->lock);
 		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
 			ndisc_send_rs(dev, &lladdr,
@@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
-		  ifp->idev->cnf.rtr_solicits > 0 &&
+		  ifp->idev->cnf.rtr_solicits != 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
 	read_unlock_bh(&ifp->idev->lock);
 
@@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits <= 0)
+	if (idev->cnf.rtr_solicits == 0)
 		return -EINVAL;
 
 	write_lock_bh(&idev->lock);
@@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static int minus_one = -1;
 static int one = 1;
 static int two_five_five = 255;
 
@@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
                               ` (3 subsequent siblings)
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Accessible via:
  /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval

For now we default it to the same value as the normal interval.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/linux/ipv6.h      |  1 +
 include/net/addrconf.h    |  1 +
 include/uapi/linux/ipv6.h |  1 +
 net/ipv6/addrconf.c       | 11 +++++++++++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c6dbcd84a2c7..7e9a789be5e0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -18,6 +18,7 @@ struct ipv6_devconf {
 	__s32		dad_transmits;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
+	__s32		rtr_solicit_max_interval;
 	__s32		rtr_solicit_delay;
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9826d3a9464c..275e5af4c2f4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,6 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 395876060f50..8c2772340c3f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -177,6 +177,7 @@ enum {
 	DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
 	DEVCONF_DROP_UNSOLICITED_NA,
 	DEVCONF_KEEP_ADDR_ON_DOWN,
+	DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c63bf06fbcf..255be34cdbce 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
 	array[DEVCONF_RTR_SOLICIT_INTERVAL] =
 		jiffies_to_msecs(cnf->rtr_solicit_interval);
+	array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
+		jiffies_to_msecs(cnf->rtr_solicit_max_interval);
 	array[DEVCONF_RTR_SOLICIT_DELAY] =
 		jiffies_to_msecs(cnf->rtr_solicit_delay);
 	array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
@@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "router_solicitation_max_interval",
+		.data		= &ipv6_devconf.rtr_solicit_max_interval,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.procname	= "router_solicitation_delay",
 		.data		= &ipv6_devconf.rtr_solicit_delay,
 		.maxlen		= sizeof(int),
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                               ` (2 preceding siblings ...)
  2016-09-25  9:59             ` [PATCH v2 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
                               ` (2 subsequent siblings)
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 255be34cdbce..6384a1cde056 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,24 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 initial)
+{
+	u32 r = (9 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = initial * (u64)r >> 20;       /* ~ multiply by 0.9 .. 1.1 */
+	return v;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
+{
+	u32 r = (19 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = cur * (u64)r >> 20;           /* ~ multiply by 1.9 .. 2.1 */
+	if (v > ceiling) {
+		r -= 1 << 20;
+		v = ceiling * (u64)r >> 20;   /* ~ multiply by 0.9 .. 1.1 */
+	}
+	return v;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3716,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3993,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5153,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                               ` (3 preceding siblings ...)
  2016-09-25  9:59             ` [PATCH v2 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-25  9:59             ` [PATCH v2 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  2016-09-25 10:04             ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl David Miller
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitation_max_interval
  /proc/sys/net/ipv6/conf/default/router_solicitation_max_interval
from 4 seconds to 1 hour.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 275e5af4c2f4..8f3677269f9a 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,7 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
-#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v2 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited)
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                               ` (4 preceding siblings ...)
  2016-09-25  9:59             ` [PATCH v2 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
@ 2016-09-25  9:59             ` Maciej Żenczykowski
  2016-09-25 10:04             ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl David Miller
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25  9:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitations
  /proc/sys/net/ipv6/conf/default/router_solicitations
from 3 to unlimited.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 8f3677269f9a..f2d072787947 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -1,7 +1,7 @@
 #ifndef _ADDRCONF_H
 #define _ADDRCONF_H
 
-#define MAX_RTR_SOLICITATIONS		3
+#define MAX_RTR_SOLICITATIONS		-1		/* unlimited */
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
 #define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* Re: [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                               ` (5 preceding siblings ...)
  2016-09-25  9:59             ` [PATCH v2 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
@ 2016-09-25 10:04             ` David Miller
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
  6 siblings, 2 replies; 64+ messages in thread
From: David Miller @ 2016-09-25 10:04 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, netdev, ek, lorenzo


This is missing an appropriate "[PATCH v2 0/7] ..." cover letter that
explains at a high level what this patch series is doing, why it is
doing it, and how it is doing it.

You'll have to submit a v3 with this fixed.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* (unknown), 
  2016-09-25 10:04             ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl David Miller
@ 2016-09-25 10:52               ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                                   ` (7 more replies)
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
  1 sibling, 8 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

Hi,

This patch series implements RFC7559 style backoff of IPv6 router
solicitation requests.

Patches 1 and 2 are minor cleanup and stand on their own.

Patch 3 allows a (potentially) infinite number of RS'es to be sent
when the rtr_solicits sysctl is set to -1 (this depends on patch 1).

Patch 4 is just boilerplate to add a new sysctl for the maximum
backoff period.

Patch 5 implements the backoff algorithm (and depends on the previous
patches).

Patches 6 and 7 switch the defaults over to enable this by default.

[PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in
[PATCH v3 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
[PATCH v3 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
[PATCH v3 4/7] ipv6 addrconf: add new sysctl
[PATCH v3 5/7] ipv6 addrconf: implement RFC7559 router solicitation
[PATCH v3 6/7] ipv6 addrconf: change default
[PATCH v3 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS

Changes v2->v3:
  none

Changes v1->v2:
  avoid 64-bit divisions to fix 32-bit build errors

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
                                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..11fa1a5564d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6044,8 +6044,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 *)&ipv6_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

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
                                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

replace with extra1/2 magic

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 11fa1a5564d4..3a835495fb53 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 = &min_hl;
-	lctl.extra2 = &max_hl;
-
-	return proc_dointvec_minmax(&lctl, 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 int one = 1;
+static 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		= &ipv6_devconf.hop_limit,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= addrconf_sysctl_hop_limit,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &one,
+		.extra2		= &two_five_five,
 	},
 	{
 		.procname	= "mtu",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
                                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to -1 meaning an unlimited number of retransmits.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3a835495fb53..6c63bf06fbcf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
 	if (idev->if_flags & IF_RA_RCVD)
 		goto out;
 
-	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
+	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
 		write_unlock(&idev->lock);
 		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
 			ndisc_send_rs(dev, &lladdr,
@@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
-		  ifp->idev->cnf.rtr_solicits > 0 &&
+		  ifp->idev->cnf.rtr_solicits != 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
 	read_unlock_bh(&ifp->idev->lock);
 
@@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits <= 0)
+	if (idev->cnf.rtr_solicits == 0)
 		return -EINVAL;
 
 	write_lock_bh(&idev->lock);
@@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static int minus_one = -1;
 static int one = 1;
 static int two_five_five = 255;
 
@@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
                                   ` (2 preceding siblings ...)
  2016-09-25 10:52                 ` [PATCH v3 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
                                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Accessible via:
  /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval

For now we default it to the same value as the normal interval.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/linux/ipv6.h      |  1 +
 include/net/addrconf.h    |  1 +
 include/uapi/linux/ipv6.h |  1 +
 net/ipv6/addrconf.c       | 11 +++++++++++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c6dbcd84a2c7..7e9a789be5e0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -18,6 +18,7 @@ struct ipv6_devconf {
 	__s32		dad_transmits;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
+	__s32		rtr_solicit_max_interval;
 	__s32		rtr_solicit_delay;
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9826d3a9464c..275e5af4c2f4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,6 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 395876060f50..8c2772340c3f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -177,6 +177,7 @@ enum {
 	DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
 	DEVCONF_DROP_UNSOLICITED_NA,
 	DEVCONF_KEEP_ADDR_ON_DOWN,
+	DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c63bf06fbcf..255be34cdbce 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
 	array[DEVCONF_RTR_SOLICIT_INTERVAL] =
 		jiffies_to_msecs(cnf->rtr_solicit_interval);
+	array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
+		jiffies_to_msecs(cnf->rtr_solicit_max_interval);
 	array[DEVCONF_RTR_SOLICIT_DELAY] =
 		jiffies_to_msecs(cnf->rtr_solicit_delay);
 	array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
@@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "router_solicitation_max_interval",
+		.data		= &ipv6_devconf.rtr_solicit_max_interval,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.procname	= "router_solicitation_delay",
 		.data		= &ipv6_devconf.rtr_solicit_delay,
 		.maxlen		= sizeof(int),
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
                                   ` (3 preceding siblings ...)
  2016-09-25 10:52                 ` [PATCH v3 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
                                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 255be34cdbce..6384a1cde056 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,24 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 initial)
+{
+	u32 r = (9 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = initial * (u64)r >> 20;       /* ~ multiply by 0.9 .. 1.1 */
+	return v;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
+{
+	u32 r = (19 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = cur * (u64)r >> 20;           /* ~ multiply by 1.9 .. 2.1 */
+	if (v > ceiling) {
+		r -= 1 << 20;
+		v = ceiling * (u64)r >> 20;   /* ~ multiply by 0.9 .. 1.1 */
+	}
+	return v;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3716,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3993,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5153,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
                                   ` (4 preceding siblings ...)
  2016-09-25 10:52                 ` [PATCH v3 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 10:52                 ` [PATCH v3 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  2016-09-25 12:51                 ` (unknown) David Miller
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitation_max_interval
  /proc/sys/net/ipv6/conf/default/router_solicitation_max_interval
from 4 seconds to 1 hour.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 275e5af4c2f4..8f3677269f9a 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,7 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
-#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v3 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited)
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
                                   ` (5 preceding siblings ...)
  2016-09-25 10:52                 ` [PATCH v3 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
@ 2016-09-25 10:52                 ` Maciej Żenczykowski
  2016-09-25 12:51                 ` (unknown) David Miller
  7 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 10:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitations
  /proc/sys/net/ipv6/conf/default/router_solicitations
from 3 to unlimited.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 8f3677269f9a..f2d072787947 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -1,7 +1,7 @@
 #ifndef _ADDRCONF_H
 #define _ADDRCONF_H
 
-#define MAX_RTR_SOLICITATIONS		3
+#define MAX_RTR_SOLICITATIONS		-1		/* unlimited */
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
 #define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff
  2016-09-25 10:04             ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl David Miller
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
@ 2016-09-25 11:03               ` Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                                   ` (6 more replies)
  1 sibling, 7 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

Hi,

This patch series implements RFC7559 style backoff of IPv6 router
solicitation requests.

Patches 1 and 2 are minor cleanup and stand on their own.

Patch 3 allows a (potentially) infinite number of RS'es to be sent
when the rtr_solicits sysctl is set to -1 (this depends on patch 1).

Patch 4 is just boilerplate to add a new sysctl for the maximum
backoff period.

Patch 5 implements the backoff algorithm (and depends on the previous
patches).

Patches 6 and 7 switch the defaults over to enable this by default.

[PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in
[PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
[PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
[PATCH v4 4/7] ipv6 addrconf: add new sysctl
[PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation
[PATCH v4 6/7] ipv6 addrconf: change default
[PATCH v4 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS


Changes v3->v4:
  added subject line to cover letter

Changes v2->v3:
  added cover letter

Changes v1->v2:
  avoid 64-bit divisions to fix 32-bit build errors

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-26  9:13                   ` Erik Kline
  2016-09-25 11:03                 ` [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
                                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..11fa1a5564d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6044,8 +6044,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 *)&ipv6_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

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-26  9:14                   ` Erik Kline
  2016-09-25 11:03                 ` [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
                                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

replace with extra1/2 magic

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 11fa1a5564d4..3a835495fb53 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 = &min_hl;
-	lctl.extra2 = &max_hl;
-
-	return proc_dointvec_minmax(&lctl, 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 int one = 1;
+static 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		= &ipv6_devconf.hop_limit,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= addrconf_sysctl_hop_limit,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &one,
+		.extra2		= &two_five_five,
 	},
 	{
 		.procname	= "mtu",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-26  9:23                   ` Erik Kline
  2016-09-25 11:03                 ` [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
                                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to -1 meaning an unlimited number of retransmits.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3a835495fb53..6c63bf06fbcf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
 	if (idev->if_flags & IF_RA_RCVD)
 		goto out;
 
-	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
+	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
 		write_unlock(&idev->lock);
 		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
 			ndisc_send_rs(dev, &lladdr,
@@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
-		  ifp->idev->cnf.rtr_solicits > 0 &&
+		  ifp->idev->cnf.rtr_solicits != 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
 	read_unlock_bh(&ifp->idev->lock);
 
@@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits <= 0)
+	if (idev->cnf.rtr_solicits == 0)
 		return -EINVAL;
 
 	write_lock_bh(&idev->lock);
@@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static int minus_one = -1;
 static int one = 1;
 static int two_five_five = 255;
 
@@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
                                   ` (2 preceding siblings ...)
  2016-09-25 11:03                 ` [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-26  9:29                   ` Erik Kline
  2016-09-26 13:59                   ` Hannes Frederic Sowa
  2016-09-25 11:03                 ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
                                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

Accessible via:
  /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval

For now we default it to the same value as the normal interval.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/linux/ipv6.h      |  1 +
 include/net/addrconf.h    |  1 +
 include/uapi/linux/ipv6.h |  1 +
 net/ipv6/addrconf.c       | 11 +++++++++++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c6dbcd84a2c7..7e9a789be5e0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -18,6 +18,7 @@ struct ipv6_devconf {
 	__s32		dad_transmits;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
+	__s32		rtr_solicit_max_interval;
 	__s32		rtr_solicit_delay;
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9826d3a9464c..275e5af4c2f4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,6 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 395876060f50..8c2772340c3f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -177,6 +177,7 @@ enum {
 	DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
 	DEVCONF_DROP_UNSOLICITED_NA,
 	DEVCONF_KEEP_ADDR_ON_DOWN,
+	DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c63bf06fbcf..255be34cdbce 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
 	array[DEVCONF_RTR_SOLICIT_INTERVAL] =
 		jiffies_to_msecs(cnf->rtr_solicit_interval);
+	array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
+		jiffies_to_msecs(cnf->rtr_solicit_max_interval);
 	array[DEVCONF_RTR_SOLICIT_DELAY] =
 		jiffies_to_msecs(cnf->rtr_solicit_delay);
 	array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
@@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "router_solicitation_max_interval",
+		.data		= &ipv6_devconf.rtr_solicit_max_interval,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.procname	= "router_solicitation_delay",
 		.data		= &ipv6_devconf.rtr_solicit_delay,
 		.maxlen		= sizeof(int),
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
                                   ` (3 preceding siblings ...)
  2016-09-25 11:03                 ` [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-26 13:53                   ` Hannes Frederic Sowa
  2016-09-25 11:03                 ` [PATCH v4 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 31 +++++++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 255be34cdbce..6384a1cde056 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,24 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 initial)
+{
+	u32 r = (9 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = initial * (u64)r >> 20;       /* ~ multiply by 0.9 .. 1.1 */
+	return v;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
+{
+	u32 r = (19 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));
+	s32 v = cur * (u64)r >> 20;           /* ~ multiply by 1.9 .. 2.1 */
+	if (v > ceiling) {
+		r -= 1 << 20;
+		v = ceiling * (u64)r >> 20;   /* ~ multiply by 0.9 .. 1.1 */
+	}
+	return v;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3716,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3993,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5153,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
                                   ` (4 preceding siblings ...)
  2016-09-25 11:03                 ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  2016-09-25 11:03                 ` [PATCH v4 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitation_max_interval
  /proc/sys/net/ipv6/conf/default/router_solicitation_max_interval
from 4 seconds to 1 hour.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 275e5af4c2f4..8f3677269f9a 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,7 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
-#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v4 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited)
  2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
                                   ` (5 preceding siblings ...)
  2016-09-25 11:03                 ` [PATCH v4 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
@ 2016-09-25 11:03                 ` Maciej Żenczykowski
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-25 11:03 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitations
  /proc/sys/net/ipv6/conf/default/router_solicitations
from 3 to unlimited.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 8f3677269f9a..f2d072787947 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -1,7 +1,7 @@
 #ifndef _ADDRCONF_H
 #define _ADDRCONF_H
 
-#define MAX_RTR_SOLICITATIONS		3
+#define MAX_RTR_SOLICITATIONS		-1		/* unlimited */
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
 #define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* (unknown)
  2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
                                   ` (6 preceding siblings ...)
  2016-09-25 10:52                 ` [PATCH v3 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
@ 2016-09-25 12:51                 ` David Miller
  7 siblings, 0 replies; 64+ messages in thread
From: David Miller @ 2016-09-25 12:51 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, netdev, ek, lorenzo


This posting needs an actual Subject line, saying something like:

	[PATCH net-next v3 0/7] Add RFC7559 style ipv6 soliciation backoff support

This text will go into the merge commit I create should I apply
this patch series.

In any event, using a blank Subject line is never appropriate.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-25 11:03                 ` [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-26  9:13                   ` Erik Kline
  0 siblings, 0 replies; 64+ messages in thread
From: Erik Kline @ 2016-09-26  9:13 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, Lorenzo Colitti

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 2f1f5d439788..11fa1a5564d4 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6044,8 +6044,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 *)&ipv6_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
>

Acked-by: Erik Kline <ek@google.com>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-25 11:03                 ` [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-26  9:14                   ` Erik Kline
  2016-09-26  9:37                     ` Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Erik Kline @ 2016-09-26  9:14 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, Lorenzo Colitti

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> replace with extra1/2 magic
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 11fa1a5564d4..3a835495fb53 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 = &min_hl;
> -       lctl.extra2 = &max_hl;
> -
> -       return proc_dointvec_minmax(&lctl, 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 int one = 1;
> +static int two_five_five = 255;

Should these be const as well?

> +
>  static const struct ctl_table addrconf_sysctl[] = {
>         {
>                 .procname       = "forwarding",
> @@ -5726,7 +5715,9 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .data           = &ipv6_devconf.hop_limit,
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
> -               .proc_handler   = addrconf_sysctl_hop_limit,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = &one,
> +               .extra2         = &two_five_five,
>         },
>         {
>                 .procname       = "mtu",
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-25 11:03                 ` [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-26  9:23                   ` Erik Kline
  0 siblings, 0 replies; 64+ messages in thread
From: Erik Kline @ 2016-09-26  9:23 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, Lorenzo Colitti

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to -1 meaning an unlimited number of retransmits.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3a835495fb53..6c63bf06fbcf 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
>         if (idev->if_flags & IF_RA_RCVD)
>                 goto out;
>
> -       if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
> +       if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
>                 write_unlock(&idev->lock);
>                 if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
>                         ndisc_send_rs(dev, &lladdr,
> @@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
>         send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
>         send_rs = send_mld &&
>                   ipv6_accept_ra(ifp->idev) &&
> -                 ifp->idev->cnf.rtr_solicits > 0 &&
> +                 ifp->idev->cnf.rtr_solicits != 0 &&
>                   (dev->flags&IFF_LOOPBACK) == 0;
>         read_unlock_bh(&ifp->idev->lock);
>
> @@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
>                 return -EINVAL;
>         if (!ipv6_accept_ra(idev))
>                 return -EINVAL;
> -       if (idev->cnf.rtr_solicits <= 0)
> +       if (idev->cnf.rtr_solicits == 0)
>                 return -EINVAL;
>
>         write_lock_bh(&idev->lock);
> @@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
>         return ret;
>  }
>
> +static int minus_one = -1;

Same question from part 2: const as well?

>  static int one = 1;
>  static int two_five_five = 255;
>
> @@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .data           = &ipv6_devconf.rtr_solicits,
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
> -               .proc_handler   = proc_dointvec,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = &minus_one,
>         },
>         {
>                 .procname       = "router_solicitation_interval",
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-25 11:03                 ` [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-26  9:29                   ` Erik Kline
  2016-09-26 13:59                   ` Hannes Frederic Sowa
  1 sibling, 0 replies; 64+ messages in thread
From: Erik Kline @ 2016-09-26  9:29 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, Lorenzo Colitti

On 25 September 2016 at 20:03, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> From: Maciej Żenczykowski <maze@google.com>
>
> Accessible via:
>   /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval
>
> For now we default it to the same value as the normal interval.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  include/linux/ipv6.h      |  1 +
>  include/net/addrconf.h    |  1 +
>  include/uapi/linux/ipv6.h |  1 +
>  net/ipv6/addrconf.c       | 11 +++++++++++
>  4 files changed, 14 insertions(+)
>
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index c6dbcd84a2c7..7e9a789be5e0 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -18,6 +18,7 @@ struct ipv6_devconf {
>         __s32           dad_transmits;
>         __s32           rtr_solicits;
>         __s32           rtr_solicit_interval;
> +       __s32           rtr_solicit_max_interval;
>         __s32           rtr_solicit_delay;
>         __s32           force_mld_version;
>         __s32           mldv1_unsolicited_report_interval;
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index 9826d3a9464c..275e5af4c2f4 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -3,6 +3,7 @@
>
>  #define MAX_RTR_SOLICITATIONS          3
>  #define RTR_SOLICITATION_INTERVAL      (4*HZ)
> +#define RTR_SOLICITATION_MAX_INTERVAL  (4*HZ)
>
>  #define MIN_VALID_LIFETIME             (2*3600)        /* 2 hours */
>
> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> index 395876060f50..8c2772340c3f 100644
> --- a/include/uapi/linux/ipv6.h
> +++ b/include/uapi/linux/ipv6.h
> @@ -177,6 +177,7 @@ enum {
>         DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
>         DEVCONF_DROP_UNSOLICITED_NA,
>         DEVCONF_KEEP_ADDR_ON_DOWN,
> +       DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
>         DEVCONF_MAX
>  };
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 6c63bf06fbcf..255be34cdbce 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
>         .dad_transmits          = 1,
>         .rtr_solicits           = MAX_RTR_SOLICITATIONS,
>         .rtr_solicit_interval   = RTR_SOLICITATION_INTERVAL,
> +       .rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
>         .rtr_solicit_delay      = MAX_RTR_SOLICITATION_DELAY,
>         .use_tempaddr           = 0,
>         .temp_valid_lft         = TEMP_VALID_LIFETIME,
> @@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>         .dad_transmits          = 1,
>         .rtr_solicits           = MAX_RTR_SOLICITATIONS,
>         .rtr_solicit_interval   = RTR_SOLICITATION_INTERVAL,
> +       .rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
>         .rtr_solicit_delay      = MAX_RTR_SOLICITATION_DELAY,
>         .use_tempaddr           = 0,
>         .temp_valid_lft         = TEMP_VALID_LIFETIME,
> @@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
>         array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
>         array[DEVCONF_RTR_SOLICIT_INTERVAL] =
>                 jiffies_to_msecs(cnf->rtr_solicit_interval);
> +       array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
> +               jiffies_to_msecs(cnf->rtr_solicit_max_interval);
>         array[DEVCONF_RTR_SOLICIT_DELAY] =
>                 jiffies_to_msecs(cnf->rtr_solicit_delay);
>         array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
> @@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .proc_handler   = proc_dointvec_jiffies,
>         },
>         {
> +               .procname       = "router_solicitation_max_interval",
> +               .data           = &ipv6_devconf.rtr_solicit_max_interval,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec_jiffies,
> +       },
> +       {
>                 .procname       = "router_solicitation_delay",
>                 .data           = &ipv6_devconf.rtr_solicit_delay,
>                 .maxlen         = sizeof(int),
> --
> 2.8.0.rc3.226.g39d4020
>

Acked-by: Erik Kline <ek@google.com>

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-26  9:14                   ` Erik Kline
@ 2016-09-26  9:37                     ` Maciej Żenczykowski
  0 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-26  9:37 UTC (permalink / raw)
  To: Erik Kline; +Cc: David S . Miller, netdev, Lorenzo Colitti

>> +static int one = 1;
>> +static int two_five_five = 255;
>
> Should these be const as well?

It would be nice, but you actually get compile time warnings if you do that.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-25 11:03                 ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-26 13:53                   ` Hannes Frederic Sowa
  2016-09-27  9:42                     ` Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Hannes Frederic Sowa @ 2016-09-26 13:53 UTC (permalink / raw)
  To: Maciej Żenczykowski, Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

On 25.09.2016 13:03, Maciej Żenczykowski wrote:
> +static inline s32 rfc3315_s14_backoff_init(s32 initial)
> +{
> +	u32 r = (9 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));

^

> +	s32 v = initial * (u64)r >> 20;       /* ~ multiply by 0.9 .. 1.1 */
> +	return v;
> +}
> +
> +static inline s32 rfc3315_s14_backoff_update(s32 cur, s32 ceiling)
> +{
> +	u32 r = (19 << 20) / 10 + (prandom_u32() % ((2 << 20) / 10 + 1));

Please just use do_div here and go back to the first version of the
patch. Variable names could be more aligned with the RFC maybe?

> +	s32 v = cur * (u64)r >> 20;           /* ~ multiply by 1.9 .. 2.1 */
> +	if (v > ceiling) {
> +		r -= 1 << 20;
> +		v = ceiling * (u64)r >> 20;   /* ~ multiply by 0.9 .. 1.1 */
> +	}
> +	return v;
> +}

Thanks,
Hannes

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-25 11:03                 ` [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
  2016-09-26  9:29                   ` Erik Kline
@ 2016-09-26 13:59                   ` Hannes Frederic Sowa
  2016-09-27  2:30                     ` Maciej Żenczykowski
  1 sibling, 1 reply; 64+ messages in thread
From: Hannes Frederic Sowa @ 2016-09-26 13:59 UTC (permalink / raw)
  To: Maciej Żenczykowski, Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti

On 25.09.2016 13:03, Maciej Żenczykowski wrote:
> +		.procname	= "router_solicitation_max_interval",
> +		.data		= &ipv6_devconf.rtr_solicit_max_interval,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_jiffies,
> +	},
> +	{

Is seconds granular enough?

Thanks,
Hannes

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-25  9:59             ` [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-26 15:34               ` Lorenzo Colitti
  2016-09-27  2:23                 ` Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Lorenzo Colitti @ 2016-09-26 15:34 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, Erik Kline

On Sun, Sep 25, 2016 at 6:59 PM, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> +                 ifp->idev->cnf.rtr_solicits != 0 &&

Given that some of this patch checks for == -1, and some of it checks
for != 0... is it possible that setting the value to something
unexpected like -3 will cause any issues to the stack? (Other than
just rendering IPv6 unusable on this interface, which seems like a
given.)

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-26 15:34               ` Lorenzo Colitti
@ 2016-09-27  2:23                 ` Maciej Żenczykowski
  2016-09-27  2:25                   ` Erik Kline
  0 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  2:23 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: David S . Miller, netdev, Erik Kline

> Given that some of this patch checks for == -1, and some of it checks
> for != 0... is it possible that setting the value to something
> unexpected like -3 will cause any issues to the stack? (Other than
> just rendering IPv6 unusable on this interface, which seems like a
> given.)

You shouldn't be able to set it to -3, that's what the extra1 is for...

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-27  2:23                 ` Maciej Żenczykowski
@ 2016-09-27  2:25                   ` Erik Kline
  0 siblings, 0 replies; 64+ messages in thread
From: Erik Kline @ 2016-09-27  2:25 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Lorenzo Colitti, David S . Miller, netdev

On 27 September 2016 at 11:23, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>> Given that some of this patch checks for == -1, and some of it checks
>> for != 0... is it possible that setting the value to something
>> unexpected like -3 will cause any issues to the stack? (Other than
>> just rendering IPv6 unusable on this interface, which seems like a
>> given.)
>
> You shouldn't be able to set it to -3, that's what the extra1 is for...

the proc_dointvec_minmax reference with &minus_one means you shouldn't
be able to set it below -1.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-26 13:59                   ` Hannes Frederic Sowa
@ 2016-09-27  2:30                     ` Maciej Żenczykowski
  2016-09-27  9:49                       ` Hannes Frederic Sowa
  0 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  2:30 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti

> Is seconds granular enough?

The only reason why one would ever want to go into fractions of
seconds would be some sort of unittesting with very low delays.

In any normal environment the max is going to be tens if not hundreds
or thousands of seconds.

Also note that the delay and interval (ie. not max interval) are also
currently exported in seconds, so having more granularity for
max_seconds is kind of pointless.

I have been considering whether I could make proc_dointvec_jiffies
accept floating point input (and output) though... although that seems
a little harder and probably out of scope of this change.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-26 13:53                   ` Hannes Frederic Sowa
@ 2016-09-27  9:42                     ` Maciej Żenczykowski
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
                                         ` (2 more replies)
  0 siblings, 3 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:42 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti

> Please just use do_div here and go back to the first version of the
> patch. Variable names could be more aligned with the RFC maybe?

So I tried:

static inline s32 rfc3315_s14_backoff_init(s32 irt)
 {
       /* multiply 'initial retransmission time' by 0.9 .. 1.1 */
       u64 tmp = (900000 + prandom_u32() % 200001) * (u64)irt;
       do_div(tmp, 1000000);
       return (s32)tmp;
 }

static inline s32 rfc3315_s14_backoff_update(s32 rt, s32 mrt)
 {
       /* multiply 'retransmission timeout' by 1.9 .. 2.1 */
       u64 tmp = (1900000 + prandom_u32() % 200001) * (u64)rt;
       do_div(tmp, 1000000);
       if ((s32)tmp > mrt) {
               /* multiply 'maximum retransmission time' by 0.9 .. 1.1 */
               tmp = (900000 + prandom_u32() % 200001) * (u64)mrt;
               do_div(tmp, 1000000);
        }
       return (s32)tmp;
}

but then building for i386 I get:

ERROR: "__udivdi3" [net/netfilter/xt_hashlimit.ko] undefined!

which happens even at net-next/master itself.

Anyway, I'll resubmit assuming the above is what you're looking for...

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-27  2:30                     ` Maciej Żenczykowski
@ 2016-09-27  9:49                       ` Hannes Frederic Sowa
  2016-09-27 10:08                         ` Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: Hannes Frederic Sowa @ 2016-09-27  9:49 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti

On 27.09.2016 04:30, Maciej Żenczykowski wrote:
>> Is seconds granular enough?
> 
> The only reason why one would ever want to go into fractions of
> seconds would be some sort of unittesting with very low delays.
> 
> In any normal environment the max is going to be tens if not hundreds
> or thousands of seconds.
> 
> Also note that the delay and interval (ie. not max interval) are also
> currently exported in seconds, so having more granularity for
> max_seconds is kind of pointless.
> 
> I have been considering whether I could make proc_dointvec_jiffies
> accept floating point input (and output) though... although that seems
> a little harder and probably out of scope of this change.

Good point. Using ms should actually be easy, instead of
proc_dointvec_jiffies you can use proc_dointvec_ms_jiffies.

It seems good practice to add a _ms then to the sysctl, too.

I am fine with both ways.

Bye,
Hannes

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH v5 0/7] implement rfc7559 ipv6 router solicitation backoff
  2016-09-27  9:42                     ` Maciej Żenczykowski
@ 2016-09-27  9:52                       ` Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
                                           ` (6 more replies)
  2016-09-27  9:58                       ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Hannes Frederic Sowa
  2016-09-27 10:20                       ` Hannes Frederic Sowa
  2 siblings, 7 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

Hi,

This patch series implements RFC7559 style backoff of IPv6 router
solicitation requests.

Patches 1 and 2 are minor cleanup and stand on their own.

Patch 3 allows a (potentially) infinite number of RS'es to be sent
when the rtr_solicits sysctl is set to -1 (this depends on patch 1).

Patch 4 is just boilerplate to add a new sysctl for the maximum
backoff period.

Patch 5 implements the backoff algorithm (and depends on the previous
patches).

Patches 6 and 7 switch the defaults over to enable this by default
(defaults come from the RFC).

[PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in
[PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
[PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
[PATCH v5 4/7] ipv6 addrconf: add new sysctl
[PATCH v5 5/7] ipv6 addrconf: implement RFC7559 router solicitation
[PATCH v5 6/7] ipv6 addrconf: change default
[PATCH v5 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS

Changes v4->v5:
  added 'const' qualifier to extra1/2 constants - requires (void*) casting
  switched away from shifts by 20 to do_div(..., 1000000)
  switched to variable names from the rfc, elaborated a bit in the comments

Changes v3->v4:
  added subject line to cover letter

Changes v2->v3:
  added cover letter

Changes v1->v2:
  avoid 64-bit divisions to fix 32-bit build errors

^ permalink raw reply	[flat|nested] 64+ messages in thread

* [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27 10:16                           ` YOSHIFUJI Hideaki
  2016-09-27  9:52                         ` [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
                                           ` (5 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..11fa1a5564d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6044,8 +6044,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 *)&ipv6_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

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27 10:18                           ` YOSHIFUJI Hideaki
  2016-09-27  9:52                         ` [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
                                           ` (4 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

replace with extra1/2 magic

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 11fa1a5564d4..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 = &min_hl;
-	lctl.extra2 = &max_hl;
-
-	return proc_dointvec_minmax(&lctl, 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		= &ipv6_devconf.hop_limit,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= addrconf_sysctl_hop_limit,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= (void *)&one,
+		.extra2		= (void *)&two_five_five,
 	},
 	{
 		.procname	= "mtu",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27 10:20                           ` YOSHIFUJI Hideaki
  2016-09-27  9:52                         ` [PATCH v5 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
                                           ` (3 subsequent siblings)
  6 siblings, 1 reply; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to -1 meaning an unlimited number of retransmits.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8bd2d06eefe7..1e59c0034916 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
 	if (idev->if_flags & IF_RA_RCVD)
 		goto out;
 
-	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
+	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
 		write_unlock(&idev->lock);
 		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
 			ndisc_send_rs(dev, &lladdr,
@@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
-		  ifp->idev->cnf.rtr_solicits > 0 &&
+		  ifp->idev->cnf.rtr_solicits != 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
 	read_unlock_bh(&ifp->idev->lock);
 
@@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
 		return -EINVAL;
-	if (idev->cnf.rtr_solicits <= 0)
+	if (idev->cnf.rtr_solicits == 0)
 		return -EINVAL;
 
 	write_lock_bh(&idev->lock);
@@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static const int minus_one = -1;
 static const int one = 1;
 static const int two_five_five = 255;
 
@@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= (void *)&minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
                                           ` (2 preceding siblings ...)
  2016-09-27  9:52                         ` [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
                                           ` (2 subsequent siblings)
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

Accessible via:
  /proc/sys/net/ipv6/conf/*/router_solicitation_max_interval

For now we default it to the same value as the normal interval.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/linux/ipv6.h      |  1 +
 include/net/addrconf.h    |  1 +
 include/uapi/linux/ipv6.h |  1 +
 net/ipv6/addrconf.c       | 11 +++++++++++
 4 files changed, 14 insertions(+)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index c6dbcd84a2c7..7e9a789be5e0 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -18,6 +18,7 @@ struct ipv6_devconf {
 	__s32		dad_transmits;
 	__s32		rtr_solicits;
 	__s32		rtr_solicit_interval;
+	__s32		rtr_solicit_max_interval;
 	__s32		rtr_solicit_delay;
 	__s32		force_mld_version;
 	__s32		mldv1_unsolicited_report_interval;
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9826d3a9464c..275e5af4c2f4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,6 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 395876060f50..8c2772340c3f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -177,6 +177,7 @@ enum {
 	DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
 	DEVCONF_DROP_UNSOLICITED_NA,
 	DEVCONF_KEEP_ADDR_ON_DOWN,
+	DEVCONF_RTR_SOLICIT_MAX_INTERVAL,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1e59c0034916..84c46950876a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -187,6 +187,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -232,6 +233,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.dad_transmits		= 1,
 	.rtr_solicits		= MAX_RTR_SOLICITATIONS,
 	.rtr_solicit_interval	= RTR_SOLICITATION_INTERVAL,
+	.rtr_solicit_max_interval = RTR_SOLICITATION_MAX_INTERVAL,
 	.rtr_solicit_delay	= MAX_RTR_SOLICITATION_DELAY,
 	.use_tempaddr		= 0,
 	.temp_valid_lft		= TEMP_VALID_LIFETIME,
@@ -4891,6 +4893,8 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_RTR_SOLICITS] = cnf->rtr_solicits;
 	array[DEVCONF_RTR_SOLICIT_INTERVAL] =
 		jiffies_to_msecs(cnf->rtr_solicit_interval);
+	array[DEVCONF_RTR_SOLICIT_MAX_INTERVAL] =
+		jiffies_to_msecs(cnf->rtr_solicit_max_interval);
 	array[DEVCONF_RTR_SOLICIT_DELAY] =
 		jiffies_to_msecs(cnf->rtr_solicit_delay);
 	array[DEVCONF_FORCE_MLD_VERSION] = cnf->force_mld_version;
@@ -5771,6 +5775,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "router_solicitation_max_interval",
+		.data		= &ipv6_devconf.rtr_solicit_max_interval,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.procname	= "router_solicitation_delay",
 		.data		= &ipv6_devconf.rtr_solicit_delay,
 		.maxlen		= sizeof(int),
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
                                           ` (3 preceding siblings ...)
  2016-09-27  9:52                         ` [PATCH v5 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

This implements:
  https://tools.ietf.org/html/rfc7559

Backoff is performed according to RFC3315 section 14:
  https://tools.ietf.org/html/rfc3315#section-14

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/if_inet6.h |  1 +
 net/ipv6/addrconf.c    | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 1c8b6820b694..515352c6280a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -201,6 +201,7 @@ struct inet6_dev {
 	struct ipv6_devstat	stats;
 
 	struct timer_list	rs_timer;
+	__s32			rs_interval;	/* in jiffies */
 	__u8			rs_probes;
 
 	__u8			addr_gen_mode;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 84c46950876a..dc287f57c39b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -112,6 +112,27 @@ static inline u32 cstamp_delta(unsigned long cstamp)
 	return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
 }
 
+static inline s32 rfc3315_s14_backoff_init(s32 irt)
+{
+	/* multiply 'initial retransmission time' by 0.9 .. 1.1 */
+	u64 tmp = (900000 + prandom_u32() % 200001) * (u64)irt;
+	do_div(tmp, 1000000);
+	return (s32)tmp;
+}
+
+static inline s32 rfc3315_s14_backoff_update(s32 rt, s32 mrt)
+{
+	/* multiply 'retransmission timeout' by 1.9 .. 2.1 */
+	u64 tmp = (1900000 + prandom_u32() % 200001) * (u64)rt;
+	do_div(tmp, 1000000);
+	if ((s32)tmp > mrt) {
+		/* multiply 'maximum retransmission time' by 0.9 .. 1.1 */
+		tmp = (900000 + prandom_u32() % 200001) * (u64)mrt;
+		do_div(tmp, 1000000);
+	}
+	return (s32)tmp;
+}
+
 #ifdef CONFIG_SYSCTL
 static int addrconf_sysctl_register(struct inet6_dev *idev);
 static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3698,11 +3719,13 @@ static void addrconf_rs_timer(unsigned long data)
 			goto put;
 
 		write_lock(&idev->lock);
+		idev->rs_interval = rfc3315_s14_backoff_update(
+			idev->rs_interval, idev->cnf.rtr_solicit_max_interval);
 		/* The wait after the last probe can be shorter */
 		addrconf_mod_rs_timer(idev, (idev->rs_probes ==
 					     idev->cnf.rtr_solicits) ?
 				      idev->cnf.rtr_solicit_delay :
-				      idev->cnf.rtr_solicit_interval);
+				      idev->rs_interval);
 	} else {
 		/*
 		 * Note: we do not support deprecated "all on-link"
@@ -3973,10 +3996,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 		write_lock_bh(&ifp->idev->lock);
 		spin_lock(&ifp->lock);
+		ifp->idev->rs_interval = rfc3315_s14_backoff_init(
+			ifp->idev->cnf.rtr_solicit_interval);
 		ifp->idev->rs_probes = 1;
 		ifp->idev->if_flags |= IF_RS_SENT;
-		addrconf_mod_rs_timer(ifp->idev,
-				      ifp->idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(ifp->idev, ifp->idev->rs_interval);
 		spin_unlock(&ifp->lock);
 		write_unlock_bh(&ifp->idev->lock);
 	}
@@ -5132,8 +5156,10 @@ update_lft:
 
 	if (update_rs) {
 		idev->if_flags |= IF_RS_SENT;
+		idev->rs_interval = rfc3315_s14_backoff_init(
+			idev->cnf.rtr_solicit_interval);
 		idev->rs_probes = 1;
-		addrconf_mod_rs_timer(idev, idev->cnf.rtr_solicit_interval);
+		addrconf_mod_rs_timer(idev, idev->rs_interval);
 	}
 
 	/* Well, that's kinda nasty ... */
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
                                           ` (4 preceding siblings ...)
  2016-09-27  9:52                         ` [PATCH v5 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  2016-09-27  9:52                         ` [PATCH v5 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitation_max_interval
  /proc/sys/net/ipv6/conf/default/router_solicitation_max_interval
from 4 seconds to 1 hour.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 275e5af4c2f4..8f3677269f9a 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -3,7 +3,7 @@
 
 #define MAX_RTR_SOLICITATIONS		3
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
-#define RTR_SOLICITATION_MAX_INTERVAL	(4*HZ)
+#define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
 #define MIN_VALID_LIFETIME		(2*3600)	/* 2 hours */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* [PATCH v5 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited)
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
                                           ` (5 preceding siblings ...)
  2016-09-27  9:52                         ` [PATCH v5 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
@ 2016-09-27  9:52                         ` Maciej Żenczykowski
  6 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27  9:52 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, Erik Kline, Lorenzo Colitti, Hannes Frederic Sowa

From: Maciej Żenczykowski <maze@google.com>

This changes:
  /proc/sys/net/ipv6/conf/all/router_solicitations
  /proc/sys/net/ipv6/conf/default/router_solicitations
from 3 to unlimited.

This is the https://tools.ietf.org/html/rfc7559 recommended default.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 include/net/addrconf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 8f3677269f9a..f2d072787947 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -1,7 +1,7 @@
 #ifndef _ADDRCONF_H
 #define _ADDRCONF_H
 
-#define MAX_RTR_SOLICITATIONS		3
+#define MAX_RTR_SOLICITATIONS		-1		/* unlimited */
 #define RTR_SOLICITATION_INTERVAL	(4*HZ)
 #define RTR_SOLICITATION_MAX_INTERVAL	(3600*HZ)	/* 1 hour */
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-27  9:42                     ` Maciej Żenczykowski
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
@ 2016-09-27  9:58                       ` Hannes Frederic Sowa
  2016-09-27 10:20                       ` Hannes Frederic Sowa
  2 siblings, 0 replies; 64+ messages in thread
From: Hannes Frederic Sowa @ 2016-09-27  9:58 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti

On 27.09.2016 11:42, Maciej Żenczykowski wrote:
>> Please just use do_div here and go back to the first version of the
>> patch. Variable names could be more aligned with the RFC maybe?
> 
> So I tried:
> 
> static inline s32 rfc3315_s14_backoff_init(s32 irt)
>  {
>        /* multiply 'initial retransmission time' by 0.9 .. 1.1 */
>        u64 tmp = (900000 + prandom_u32() % 200001) * (u64)irt;
>        do_div(tmp, 1000000);
>        return (s32)tmp;
>  }
> 
> static inline s32 rfc3315_s14_backoff_update(s32 rt, s32 mrt)
>  {
>        /* multiply 'retransmission timeout' by 1.9 .. 2.1 */
>        u64 tmp = (1900000 + prandom_u32() % 200001) * (u64)rt;
>        do_div(tmp, 1000000);
>        if ((s32)tmp > mrt) {
>                /* multiply 'maximum retransmission time' by 0.9 .. 1.1 */
>                tmp = (900000 + prandom_u32() % 200001) * (u64)mrt;
>                do_div(tmp, 1000000);
>         }
>        return (s32)tmp;
> }
> 
> but then building for i386 I get:
> 
> ERROR: "__udivdi3" [net/netfilter/xt_hashlimit.ko] undefined!
> 
> which happens even at net-next/master itself.
> 
> Anyway, I'll resubmit assuming the above is what you're looking for...

I think the __udivdi3 comes from the fact you are doing the modulo
operation and the reciprocal divide optimization doesn't work in this
case thus you end up with the call to libgcc.

Can you use the remainder from the do_div operation also?

u32 r = prandom_u32();
u64 tmp = (900000 + do_div(r,200001)) * (u64)irt;

Depending on if you keep the values in ms or jiffies, maybe it would
make sense to simply use msecs_to_jiffies and vice versa?

Thanks,
Hannes

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
  2016-09-27  9:49                       ` Hannes Frederic Sowa
@ 2016-09-27 10:08                         ` Maciej Żenczykowski
  0 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27 10:08 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti

> Good point. Using ms should actually be easy, instead of
> proc_dointvec_jiffies you can use proc_dointvec_ms_jiffies.

Yes, I'm aware of this, but 'proc_dointvec_ms_jiffies' seems to be a
bit of a hack, and especially for large settings like this exporting
them to userspace in units of ms instead of seconds seems to impose
unnecessary cognitive burden on the user.

(furthermore the other two related settings are exported as seconds)

> It seems good practice to add a _ms then to the sysctl, too.

If we wanted to do that, we'd have to add _ms versions of
router_solicitation_{delay,interval}.
This seems like pointless duplication - since we can't actually delete
the older seconds one.

There isn't really any real world scenario I can think of (besides
unit-testing) where ms granularity would be useful.

If we were to actually fix proc_dointvec_jiffies to accept fractional
seconds, we'd fix all of these interfaces in one fell swoop...

I took a look at what this would take, and there doesn't really appear
to be a nice way to do it :-(
The lower level conversion functions get a character buffer and
convert it to/from an int (or maybe long).

We'd either need to switch these over to take a void* or add an
entirely separate implementation of proc_dointvec_jiffies...

(at which point it could probably also parse units (ns, us, ms, s, m,
h, d) on input if we wanted to...)

> I am fine with both ways.

I'm going to leave this as is (especially since I send out v5 before I
saw your comment)

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl
  2016-09-27  9:52                         ` [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
@ 2016-09-27 10:16                           ` YOSHIFUJI Hideaki
  0 siblings, 0 replies; 64+ messages in thread
From: YOSHIFUJI Hideaki @ 2016-09-27 10:16 UTC (permalink / raw)
  To: Maciej Żenczykowski, Maciej Żenczykowski, David S . Miller
  Cc: hideaki.yoshifuji, netdev, Erik Kline, Lorenzo Colitti,
	Hannes Frederic Sowa

Hi,

Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 2f1f5d439788..11fa1a5564d4 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6044,8 +6044,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 *)&ipv6_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);
> 

This seems nothing to do with the RFC7559 changes.
Why don't you submit this as a separate patch?

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
  2016-09-27  9:52                         ` [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
@ 2016-09-27 10:18                           ` YOSHIFUJI Hideaki
  0 siblings, 0 replies; 64+ messages in thread
From: YOSHIFUJI Hideaki @ 2016-09-27 10:18 UTC (permalink / raw)
  To: Maciej Żenczykowski, Maciej Żenczykowski, David S . Miller
  Cc: hideaki.yoshifuji, netdev, Erik Kline, Lorenzo Colitti,
	Hannes Frederic Sowa

Hi,

Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> replace with extra1/2 magic
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 11fa1a5564d4..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 = &min_hl;
> -	lctl.extra2 = &max_hl;
> -
> -	return proc_dointvec_minmax(&lctl, 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		= &ipv6_devconf.hop_limit,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= addrconf_sysctl_hop_limit,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= (void *)&one,
> +		.extra2		= (void *)&two_five_five,
>  	},
>  	{
>  		.procname	= "mtu",
> 

Please submit this in a different series of patches
(like 1/7).

--yoshfuji

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff
  2016-09-27  9:42                     ` Maciej Żenczykowski
  2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
  2016-09-27  9:58                       ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Hannes Frederic Sowa
@ 2016-09-27 10:20                       ` Hannes Frederic Sowa
  2 siblings, 0 replies; 64+ messages in thread
From: Hannes Frederic Sowa @ 2016-09-27 10:20 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti, vpai

[cc Vishwanath Pai]

On 27.09.2016 11:42, Maciej Żenczykowski wrote:
>> Please just use do_div here and go back to the first version of the
>> patch. Variable names could be more aligned with the RFC maybe?
> 
> So I tried:
> 
> static inline s32 rfc3315_s14_backoff_init(s32 irt)
>  {
>        /* multiply 'initial retransmission time' by 0.9 .. 1.1 */
>        u64 tmp = (900000 + prandom_u32() % 200001) * (u64)irt;
>        do_div(tmp, 1000000);
>        return (s32)tmp;
>  }
> 
> static inline s32 rfc3315_s14_backoff_update(s32 rt, s32 mrt)
>  {
>        /* multiply 'retransmission timeout' by 1.9 .. 2.1 */
>        u64 tmp = (1900000 + prandom_u32() % 200001) * (u64)rt;
>        do_div(tmp, 1000000);
>        if ((s32)tmp > mrt) {
>                /* multiply 'maximum retransmission time' by 0.9 .. 1.1 */
>                tmp = (900000 + prandom_u32() % 200001) * (u64)mrt;
>                do_div(tmp, 1000000);
>         }
>        return (s32)tmp;
> }
> 
> but then building for i386 I get:
> 
> ERROR: "__udivdi3" [net/netfilter/xt_hashlimit.ko] undefined!

Hmm, evidently we have some u64 divisions in xt_hashlimit.c which should
be replaced by do_div?

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-27  9:52                         ` [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
@ 2016-09-27 10:20                           ` YOSHIFUJI Hideaki
  2016-09-27 10:39                             ` Maciej Żenczykowski
  0 siblings, 1 reply; 64+ messages in thread
From: YOSHIFUJI Hideaki @ 2016-09-27 10:20 UTC (permalink / raw)
  To: Maciej Żenczykowski, Maciej Żenczykowski, David S . Miller
  Cc: hideaki.yoshifuji, netdev, Erik Kline, Lorenzo Colitti,
	Hannes Frederic Sowa



Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> This allows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to -1 meaning an unlimited number of retransmits.
> 

We could say "< 0 means infinite" and we can reduce changes here.

--yoshfuji

> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/addrconf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8bd2d06eefe7..1e59c0034916 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3687,7 +3687,7 @@ static void addrconf_rs_timer(unsigned long data)
>  	if (idev->if_flags & IF_RA_RCVD)
>  		goto out;
>  
> -	if (idev->rs_probes++ < idev->cnf.rtr_solicits) {
> +	if (idev->rs_probes++ < idev->cnf.rtr_solicits || idev->cnf.rtr_solicits == -1) {
>  		write_unlock(&idev->lock);
>  		if (!ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
>  			ndisc_send_rs(dev, &lladdr,
> @@ -3949,7 +3949,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
>  	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
>  	send_rs = send_mld &&
>  		  ipv6_accept_ra(ifp->idev) &&
> -		  ifp->idev->cnf.rtr_solicits > 0 &&
> +		  ifp->idev->cnf.rtr_solicits != 0 &&
>  		  (dev->flags&IFF_LOOPBACK) == 0;
>  	read_unlock_bh(&ifp->idev->lock);
>  
> @@ -5099,7 +5099,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
>  		return -EINVAL;
>  	if (!ipv6_accept_ra(idev))
>  		return -EINVAL;
> -	if (idev->cnf.rtr_solicits <= 0)
> +	if (idev->cnf.rtr_solicits == 0)
>  		return -EINVAL;
>  
>  	write_lock_bh(&idev->lock);
> @@ -5699,6 +5699,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
>  	return ret;
>  }
>  
> +static const int minus_one = -1;
>  static const int one = 1;
>  static const int two_five_five = 255;
>  
> @@ -5759,7 +5760,8 @@ static const struct ctl_table addrconf_sysctl[] = {
>  		.data		= &ipv6_devconf.rtr_solicits,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= (void *)&minus_one,
>  	},
>  	{
>  		.procname	= "router_solicitation_interval",
> 

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
  2016-09-27 10:20                           ` YOSHIFUJI Hideaki
@ 2016-09-27 10:39                             ` Maciej Żenczykowski
  0 siblings, 0 replies; 64+ messages in thread
From: Maciej Żenczykowski @ 2016-09-27 10:39 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti,
	Hannes Frederic Sowa

That wouldn't really simplify much.

This change currently has 5 lines.
3 of those would be needed anyway if we were to define anything < 0 to
mean infinite.

Yes, you could get rid of the two lines with minus_one in them, but
this way we can also use -2 to mean something else in the future if we
ever want to.

^ permalink raw reply	[flat|nested] 64+ messages in thread

end of thread, other threads:[~2016-09-27 10:39 UTC | newest]

Thread overview: 64+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-24 16:01 Implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
2016-09-24 16:05 ` [PATCH 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
2016-09-25  2:14     ` Maciej Żenczykowski
2016-09-25  2:14       ` [PATCH] " Maciej Żenczykowski
2016-09-25  9:33         ` David Miller
2016-09-25  9:59           ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
2016-09-25  9:59             ` [PATCH v2 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
2016-09-25  9:59             ` [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
2016-09-26 15:34               ` Lorenzo Colitti
2016-09-27  2:23                 ` Maciej Żenczykowski
2016-09-27  2:25                   ` Erik Kline
2016-09-25  9:59             ` [PATCH v2 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
2016-09-25  9:59             ` [PATCH v2 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
2016-09-25  9:59             ` [PATCH v2 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
2016-09-25  9:59             ` [PATCH v2 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
2016-09-25 10:04             ` [PATCH v2 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl David Miller
2016-09-25 10:52               ` (unknown), Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
2016-09-25 10:52                 ` [PATCH v3 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
2016-09-25 12:51                 ` (unknown) David Miller
2016-09-25 11:03               ` [PATCH v4 0/7] implement rfc7559 ipv6 router solicitation backoff Maciej Żenczykowski
2016-09-25 11:03                 ` [PATCH v4 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
2016-09-26  9:13                   ` Erik Kline
2016-09-25 11:03                 ` [PATCH v4 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
2016-09-26  9:14                   ` Erik Kline
2016-09-26  9:37                     ` Maciej Żenczykowski
2016-09-25 11:03                 ` [PATCH v4 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
2016-09-26  9:23                   ` Erik Kline
2016-09-25 11:03                 ` [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
2016-09-26  9:29                   ` Erik Kline
2016-09-26 13:59                   ` Hannes Frederic Sowa
2016-09-27  2:30                     ` Maciej Żenczykowski
2016-09-27  9:49                       ` Hannes Frederic Sowa
2016-09-27 10:08                         ` Maciej Żenczykowski
2016-09-25 11:03                 ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
2016-09-26 13:53                   ` Hannes Frederic Sowa
2016-09-27  9:42                     ` Maciej Żenczykowski
2016-09-27  9:52                       ` [PATCH v5 0/7] implement rfc7559 ipv6 " Maciej Żenczykowski
2016-09-27  9:52                         ` [PATCH v5 1/7] ipv6 addrconf: enable use of proc_dointvec_minmax in addrconf_sysctl Maciej Żenczykowski
2016-09-27 10:16                           ` YOSHIFUJI Hideaki
2016-09-27  9:52                         ` [PATCH v5 2/7] ipv6 addrconf: remove addrconf_sysctl_hop_limit() Maciej Żenczykowski
2016-09-27 10:18                           ` YOSHIFUJI Hideaki
2016-09-27  9:52                         ` [PATCH v5 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited Maciej Żenczykowski
2016-09-27 10:20                           ` YOSHIFUJI Hideaki
2016-09-27 10:39                             ` Maciej Żenczykowski
2016-09-27  9:52                         ` [PATCH v5 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval' Maciej Żenczykowski
2016-09-27  9:52                         ` [PATCH v5 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Maciej Żenczykowski
2016-09-27  9:52                         ` [PATCH v5 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
2016-09-27  9:52                         ` [PATCH v5 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
2016-09-27  9:58                       ` [PATCH v4 5/7] ipv6 addrconf: implement RFC7559 router solicitation backoff Hannes Frederic Sowa
2016-09-27 10:20                       ` Hannes Frederic Sowa
2016-09-25 11:03                 ` [PATCH v4 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
2016-09-25 11:03                 ` [PATCH v4 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 6/7] ipv6 addrconf: change default RTR_SOLICITATION_MAX_INTERVAL from 4s to 1h Maciej Żenczykowski
2016-09-24 16:05   ` [PATCH 7/7] ipv6 addrconf: change default MAX_RTR_SOLICITATIONS from 3 to -1 (unlimited) Maciej Żenczykowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).