netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-08-29 12:08 [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted Tore Anderson
@ 2011-08-29  9:47 ` Tore Anderson
  2011-09-06 17:57   ` David Miller
  2011-09-16 21:15 ` David Miller
  1 sibling, 1 reply; 8+ messages in thread
From: Tore Anderson @ 2011-08-29  9:47 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy
  Cc: Thomas Graf, netdev

This patch improves the logic determining when to send ICMPv6 Router
Solicitations, so that they are 1) always sent when the kernel is
accepting Router Advertisements, and 2) never sent when the kernel is
not accepting RAs. In other words, the operational setting of the
"accept_ra" sysctl is used.

The change also makes the special "Hybrid Router" forwarding mode
("forwarding" sysctl set to 2) operate exactly the same as the standard
Router mode (forwarding=1). The only difference between the two was
that RSes was being sent in the Hybrid Router mode only. The sysctl
documentation describing the special Hybrid Router mode has therefore
been removed.

Rationale for the change:

Currently, the value of forwarding sysctl is the only thing determining
whether or not to send RSes. If it has the value 0 or 2, they are sent,
otherwise they are not. This leads to inconsistent behaviour in the
following cases:

* accept_ra=0, forwarding=0
* accept_ra=0, forwarding=2
* accept_ra=1, forwarding=2
* accept_ra=2, forwarding=1

In the first three cases, the kernel will send RSes, even though it will
not accept any RAs received in reply. In the last case, it will not send
any RSes, even though it will accept and process any RAs received. (Most
routers will send unsolicited RAs periodically, so suppressing RSes in
the last case will merely delay auto-configuration, not prevent it.)

Also, it is my opinion that having the forwarding sysctl control RS
sending behaviour (completely independent of whether RAs are being
accepted or not) is simply not what most users would intuitively expect
to be the case.

Signed-off-by: Tore Anderson <tore@fud.no>
---
Note: This is a resend of a patch I inappropriately sent to the netdev
list only. This time the net/ipv6/ maintainers are on the recipient list,
in addition to Thomas Graf, the author of the Hybrid Router forwarding
mode (see commit c3bccac2fa76f1619dfe4fb7b9bee69de7f066d8).

 Documentation/networking/ip-sysctl.txt |   17 ++++++++---------
 net/ipv6/addrconf.c                    |    8 ++++----
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 8154699..98c8d42 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1045,6 +1045,11 @@ conf/interface/*:
 accept_ra - BOOLEAN
 	Accept Router Advertisements; autoconfigure using them.
 
+	It also determines whether or not to transmit Router
+	Solicitations. If and only if the functional setting is to
+	accept Router Advertisements, Router Solicitations will be
+	transmitted.
+
 	Possible values are:
 		0 Do not accept Router Advertisements.
 		1 Accept Router Advertisements if forwarding is disabled.
@@ -1115,14 +1120,14 @@ forwarding - BOOLEAN
 	Possible values are:
 		0 Forwarding disabled
 		1 Forwarding enabled
-		2 Forwarding enabled (Hybrid Mode)
 
 	FALSE (0):
 
 	By default, Host behaviour is assumed.  This means:
 
 	1. IsRouter flag is not set in Neighbour Advertisements.
-	2. Router Solicitations are being sent when necessary.
+	2. If accept_ra is TRUE (default), transmit Router
+	   Solicitations.
 	3. If accept_ra is TRUE (default), accept Router
 	   Advertisements (and do autoconfiguration).
 	4. If accept_redirects is TRUE (default), accept Redirects.
@@ -1133,16 +1138,10 @@ forwarding - BOOLEAN
 	This means exactly the reverse from the above:
 
 	1. IsRouter flag is set in Neighbour Advertisements.
-	2. Router Solicitations are not sent.
+	2. Router Solicitations are not sent unless accept_ra is 2.
 	3. Router Advertisements are ignored unless accept_ra is 2.
 	4. Redirects are ignored.
 
-	TRUE (2):
-
-	Hybrid mode. Same behaviour as TRUE, except for:
-
-	2. Router Solicitations are being sent when necessary.
-
 	Default: 0 (disabled) if global forwarding is disabled (default),
 		 otherwise 1 (enabled).
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f012ebd..d3e51cb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2969,12 +2969,12 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 	ipv6_ifa_notify(RTM_NEWADDR, ifp);
 
-	/* If added prefix is link local and forwarding is off,
-	   start sending router solicitations.
+	/* If added prefix is link local and we are prepared to process
+	   router advertisements, start sending router solicitations.
 	 */
 
-	if ((ifp->idev->cnf.forwarding == 0 ||
-	     ifp->idev->cnf.forwarding == 2) &&
+	if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
+	     ifp->idev->cnf.accept_ra == 2) &&
 	    ifp->idev->cnf.rtr_solicits > 0 &&
 	    (dev->flags&IFF_LOOPBACK) == 0 &&
 	    (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
-- 
1.7.6

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

* [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
@ 2011-08-29 12:08 Tore Anderson
  2011-08-29  9:47 ` Tore Anderson
  2011-09-16 21:15 ` David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Tore Anderson @ 2011-08-29 12:08 UTC (permalink / raw)
  To: netdev

This patch improves the logic determining when to send ICMPv6 Router
Solicitations, so that they are 1) always sent when the kernel is
accepting Router Advertisements, and 2) never sent when the kernel is
not accepting RAs. In other words, the operational setting of the
"accept_ra" sysctl is used.

The change also makes "Hybrid Router" forwarding mode ("forwarding"
sysctl set to 2) redundant, as the only thing that distinguished it from
the standard Router mode (forwarding=1) was that RSes was being sent. It
has therefore been removed.

Rationale for the change:

Currently, the value of forwarding sysctl is the only thing determining
whether or not to send RSes. If it has the value 0 or 2, they are sent,
otherwise they are not. This leads to inconsistent behaviour in the
following cases:

* accept_ra=0, forwarding=0
* accept_ra=0, forwarding=2
* accept_ra=1, forwarding=2
* accept_ra=2, forwarding=1

In the first three cases, the kernel will send RSes, even though it will
not accept any RAs received in reply. In the last case, it will not send
any RSes, even though it will accept and process any RAs received. (Most
routers will send unsolicited RAs periodically, so suppressing RSes in
the last case will merely delay auto-configuration, not prevent it.)

Also, it is my opinion that having the forwarding sysctl control RS
sending behaviour (completely independent of whether RAs are being
accepted or not) is simply not what most users would intuitively expect
to be the case.

Signed-off-by: Tore Anderson <tore@fud.no>
---
 Documentation/networking/ip-sysctl.txt |   17 ++++++++---------
 net/ipv6/addrconf.c                    |    8 ++++----
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 8154699..98c8d42 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1045,6 +1045,11 @@ conf/interface/*:
 accept_ra - BOOLEAN
 	Accept Router Advertisements; autoconfigure using them.
 
+	It also determines whether or not to transmit Router
+	Solicitations. If and only if the functional setting is to
+	accept Router Advertisements, Router Solicitations will be
+	transmitted.
+
 	Possible values are:
 		0 Do not accept Router Advertisements.
 		1 Accept Router Advertisements if forwarding is disabled.
@@ -1115,14 +1120,14 @@ forwarding - BOOLEAN
 	Possible values are:
 		0 Forwarding disabled
 		1 Forwarding enabled
-		2 Forwarding enabled (Hybrid Mode)
 
 	FALSE (0):
 
 	By default, Host behaviour is assumed.  This means:
 
 	1. IsRouter flag is not set in Neighbour Advertisements.
-	2. Router Solicitations are being sent when necessary.
+	2. If accept_ra is TRUE (default), transmit Router
+	   Solicitations.
 	3. If accept_ra is TRUE (default), accept Router
 	   Advertisements (and do autoconfiguration).
 	4. If accept_redirects is TRUE (default), accept Redirects.
@@ -1133,16 +1138,10 @@ forwarding - BOOLEAN
 	This means exactly the reverse from the above:
 
 	1. IsRouter flag is set in Neighbour Advertisements.
-	2. Router Solicitations are not sent.
+	2. Router Solicitations are not sent unless accept_ra is 2.
 	3. Router Advertisements are ignored unless accept_ra is 2.
 	4. Redirects are ignored.
 
-	TRUE (2):
-
-	Hybrid mode. Same behaviour as TRUE, except for:
-
-	2. Router Solicitations are being sent when necessary.
-
 	Default: 0 (disabled) if global forwarding is disabled (default),
 		 otherwise 1 (enabled).
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f012ebd..d3e51cb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2969,12 +2969,12 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 	ipv6_ifa_notify(RTM_NEWADDR, ifp);
 
-	/* If added prefix is link local and forwarding is off,
-	   start sending router solicitations.
+	/* If added prefix is link local and we are prepared to process
+	   router advertisements, start sending router solicitations.
 	 */
 
-	if ((ifp->idev->cnf.forwarding == 0 ||
-	     ifp->idev->cnf.forwarding == 2) &&
+	if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
+	     ifp->idev->cnf.accept_ra == 2) &&
 	    ifp->idev->cnf.rtr_solicits > 0 &&
 	    (dev->flags&IFF_LOOPBACK) == 0 &&
 	    (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
-- 
1.7.6

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-08-29  9:47 ` Tore Anderson
@ 2011-09-06 17:57   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2011-09-06 17:57 UTC (permalink / raw)
  To: tore; +Cc: kuznet, jmorris, yoshfuji, kaber, tgraf, netdev


We saw your first submission, you do not need to send your patch
again.

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-08-29 12:08 [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted Tore Anderson
  2011-08-29  9:47 ` Tore Anderson
@ 2011-09-16 21:15 ` David Miller
  2011-09-16 21:43   ` Tore Anderson
  1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2011-09-16 21:15 UTC (permalink / raw)
  To: tore; +Cc: netdev

From: Tore Anderson <tore@fud.no>
Date: Mon, 29 Aug 2011 14:08:33 +0200

> The change also makes "Hybrid Router" forwarding mode ("forwarding"
> sysctl set to 2) redundant, as the only thing that distinguished it from
> the standard Router mode (forwarding=1) was that RSes was being sent. It
> has therefore been removed.

You're not removing "accept_ra==2", it's still there in the test:

> -	if ((ifp->idev->cnf.forwarding == 0 ||
> -	     ifp->idev->cnf.forwarding == 2) &&
> +	if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
> +	     ifp->idev->cnf.accept_ra == 2) &&

And it does provide it's own unique behavior compared to
"accept_ra==1".

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-09-16 21:15 ` David Miller
@ 2011-09-16 21:43   ` Tore Anderson
  2011-09-16 21:49     ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Tore Anderson @ 2011-09-16 21:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

* David Miller

> From: Tore Anderson <tore@fud.no>
> Date: Mon, 29 Aug 2011 14:08:33 +0200
> 
>> The change also makes "Hybrid Router" forwarding mode ("forwarding"
>> sysctl set to 2) redundant, as the only thing that distinguished it from
>> the standard Router mode (forwarding=1) was that RSes was being sent. It
>> has therefore been removed.
> 
> You're not removing "accept_ra==2", it's still there in the test:
> 
>> -	if ((ifp->idev->cnf.forwarding == 0 ||
>> -	     ifp->idev->cnf.forwarding == 2) &&
>> +	if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
>> +	     ifp->idev->cnf.accept_ra == 2) &&
> 
> And it does provide it's own unique behavior compared to
> "accept_ra==1".

Hi David,

I'm not removing accept_ra==2, no, only forwarding==2. Or, more
precisely, I'm only removing the *documentation* for forwarding==2;
forwarding==2 will still work, but there's no difference from
forwarding==1 any longer.

-- 
Tore Anderson

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-09-16 21:43   ` Tore Anderson
@ 2011-09-16 21:49     ` David Miller
  2011-09-16 22:07       ` Tore Anderson
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2011-09-16 21:49 UTC (permalink / raw)
  To: tore; +Cc: netdev

From: Tore Anderson <tore@fud.no>
Date: Fri, 16 Sep 2011 23:43:01 +0200

> * David Miller
> 
>> From: Tore Anderson <tore@fud.no>
>> Date: Mon, 29 Aug 2011 14:08:33 +0200
>> 
>>> The change also makes "Hybrid Router" forwarding mode ("forwarding"
>>> sysctl set to 2) redundant, as the only thing that distinguished it from
>>> the standard Router mode (forwarding=1) was that RSes was being sent. It
>>> has therefore been removed.
>> 
>> You're not removing "accept_ra==2", it's still there in the test:
>> 
>>> -	if ((ifp->idev->cnf.forwarding == 0 ||
>>> -	     ifp->idev->cnf.forwarding == 2) &&
>>> +	if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
>>> +	     ifp->idev->cnf.accept_ra == 2) &&
>> 
>> And it does provide it's own unique behavior compared to
>> "accept_ra==1".
> 
> Hi David,
> 
> I'm not removing accept_ra==2, no, only forwarding==2. Or, more
> precisely, I'm only removing the *documentation* for forwarding==2;
> forwarding==2 will still work, but there's no difference from
> forwarding==1 any longer.

Ok, please make this more clear in your commit message.

Thank you.

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-09-16 21:49     ` David Miller
@ 2011-09-16 22:07       ` Tore Anderson
  2011-09-16 23:15         ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Tore Anderson @ 2011-09-16 22:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

* David Miller

>> I'm not removing accept_ra==2, no, only forwarding==2. Or, more
>> precisely, I'm only removing the *documentation* for forwarding==2;
>> forwarding==2 will still work, but there's no difference from
>> forwarding==1 any longer.
> 
> Ok, please make this more clear in your commit message.

I actually did improve the commit message in this regard when I re-sent
the patch, see <http://patchwork.ozlabs.org/patch/113625/>. Is that one
good enough?

-- 
Tore Anderson

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

* Re: [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted
  2011-09-16 22:07       ` Tore Anderson
@ 2011-09-16 23:15         ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2011-09-16 23:15 UTC (permalink / raw)
  To: tore; +Cc: netdev

From: Tore Anderson <tore@fud.no>
Date: Sat, 17 Sep 2011 00:07:45 +0200

> * David Miller
> 
>>> I'm not removing accept_ra==2, no, only forwarding==2. Or, more
>>> precisely, I'm only removing the *documentation* for forwarding==2;
>>> forwarding==2 will still work, but there's no difference from
>>> forwarding==1 any longer.
>> 
>> Ok, please make this more clear in your commit message.
> 
> I actually did improve the commit message in this regard when I re-sent
> the patch, see <http://patchwork.ozlabs.org/patch/113625/>. Is that one
> good enough?

Looks good, applied, thanks!

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

end of thread, other threads:[~2011-09-16 23:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-29 12:08 [PATCH] ipv6: Send ICMPv6 RSes only when RAs are accepted Tore Anderson
2011-08-29  9:47 ` Tore Anderson
2011-09-06 17:57   ` David Miller
2011-09-16 21:15 ` David Miller
2011-09-16 21:43   ` Tore Anderson
2011-09-16 21:49     ` David Miller
2011-09-16 22:07       ` Tore Anderson
2011-09-16 23:15         ` David Miller

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).