* [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
@ 2017-09-12 15:46 Matteo Croce
2017-09-15 21:12 ` David Miller
2017-09-19 23:44 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Matteo Croce @ 2017-09-12 15:46 UTC (permalink / raw)
To: netdev, linux-doc; +Cc: Erik Kline, David S . Miller
Currently, writing into
net.ipv6.conf.all.{accept_dad,use_optimistic,optimistic_dad} has no effect.
Fix handling of these flags by:
- using the maximum of global and per-interface values for the
accept_dad flag. That is, if at least one of the two values is
non-zero, enable DAD on the interface. If at least one value is
set to 2, enable DAD and disable IPv6 operation on the interface if
MAC-based link-local address was found
- using the logical OR of global and per-interface values for the
optimistic_dad flag. If at least one of them is set to one, optimistic
duplicate address detection (RFC 4429) is enabled on the interface
- using the logical OR of global and per-interface values for the
use_optimistic flag. If at least one of them is set to one,
optimistic addresses won't be marked as deprecated during source address
selection on the interface.
While at it, as we're modifying the prototype for ipv6_use_optimistic_addr(),
drop inline, and let the compiler decide.
Fixes: 7fd2561e4ebd ("net: ipv6: Add a sysctl to make optimistic addresses useful candidates")
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
Documentation/networking/ip-sysctl.txt | 18 ++++++++++++++----
net/ipv6/addrconf.c | 27 ++++++++++++++++++++-------
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index b3345d0fe0a6..77f4de59dc9c 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1680,6 +1680,9 @@ accept_dad - INTEGER
2: Enable DAD, and disable IPv6 operation if MAC-based duplicate
link-local address has been found.
+ DAD operation and mode on a given interface will be selected according
+ to the maximum value of conf/{all,interface}/accept_dad.
+
force_tllao - BOOLEAN
Enable sending the target link-layer address option even when
responding to a unicast neighbor solicitation.
@@ -1727,16 +1730,23 @@ suppress_frag_ndisc - INTEGER
optimistic_dad - BOOLEAN
Whether to perform Optimistic Duplicate Address Detection (RFC 4429).
- 0: disabled (default)
- 1: enabled
+ 0: disabled (default)
+ 1: enabled
+
+ Optimistic Duplicate Address Detection for the interface will be enabled
+ if at least one of conf/{all,interface}/optimistic_dad is set to 1,
+ it will be disabled otherwise.
use_optimistic - BOOLEAN
If enabled, do not classify optimistic addresses as deprecated during
source address selection. Preferred addresses will still be chosen
before optimistic addresses, subject to other ranking in the source
address selection algorithm.
- 0: disabled (default)
- 1: enabled
+ 0: disabled (default)
+ 1: enabled
+
+ This will be enabled if at least one of
+ conf/{all,interface}/use_optimistic is set to 1, disabled otherwise.
stable_secret - IPv6 address
This IPv6 address will be used as a secret to generate IPv6
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c2e2a78787ec..774d8794248a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1399,10 +1399,18 @@ static inline int ipv6_saddr_preferred(int type)
return 0;
}
-static inline bool ipv6_use_optimistic_addr(struct inet6_dev *idev)
+static bool ipv6_use_optimistic_addr(struct net *net,
+ struct inet6_dev *idev)
{
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
- return idev && idev->cnf.optimistic_dad && idev->cnf.use_optimistic;
+ if (!idev)
+ return false;
+ if (!net->ipv6.devconf_all->optimistic_dad && !idev->cnf.optimistic_dad)
+ return false;
+ if (!net->ipv6.devconf_all->use_optimistic && !idev->cnf.use_optimistic)
+ return false;
+
+ return true;
#else
return false;
#endif
@@ -1472,7 +1480,7 @@ static int ipv6_get_saddr_eval(struct net *net,
/* Rule 3: Avoid deprecated and optimistic addresses */
u8 avoid = IFA_F_DEPRECATED;
- if (!ipv6_use_optimistic_addr(score->ifa->idev))
+ if (!ipv6_use_optimistic_addr(net, score->ifa->idev))
avoid |= IFA_F_OPTIMISTIC;
ret = ipv6_saddr_preferred(score->addr_type) ||
!(score->ifa->flags & avoid);
@@ -2460,7 +2468,8 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
int max_addresses = in6_dev->cnf.max_addresses;
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
- if (in6_dev->cnf.optimistic_dad &&
+ if ((net->ipv6.devconf_all->optimistic_dad ||
+ in6_dev->cnf.optimistic_dad) &&
!net->ipv6.devconf_all->forwarding && sllao)
addr_flags |= IFA_F_OPTIMISTIC;
#endif
@@ -3051,7 +3060,8 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
u32 addr_flags = flags | IFA_F_PERMANENT;
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
- if (idev->cnf.optimistic_dad &&
+ if ((dev_net(idev->dev)->ipv6.devconf_all->optimistic_dad ||
+ idev->cnf.optimistic_dad) &&
!dev_net(idev->dev)->ipv6.devconf_all->forwarding)
addr_flags |= IFA_F_OPTIMISTIC;
#endif
@@ -3810,6 +3820,7 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
goto out;
if (dev->flags&(IFF_NOARP|IFF_LOOPBACK) ||
+ dev_net(dev)->ipv6.devconf_all->accept_dad < 1 ||
idev->cnf.accept_dad < 1 ||
!(ifp->flags&IFA_F_TENTATIVE) ||
ifp->flags & IFA_F_NODAD) {
@@ -3841,7 +3852,7 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
*/
if (ifp->flags & IFA_F_OPTIMISTIC) {
ip6_ins_rt(ifp->rt);
- if (ipv6_use_optimistic_addr(idev)) {
+ if (ipv6_use_optimistic_addr(dev_net(dev), idev)) {
/* Because optimistic nodes can use this address,
* notify listeners. If DAD fails, RTM_DELADDR is sent.
*/
@@ -3897,7 +3908,9 @@ static void addrconf_dad_work(struct work_struct *w)
action = DAD_ABORT;
ifp->state = INET6_IFADDR_STATE_POSTDAD;
- if (idev->cnf.accept_dad > 1 && !idev->cnf.disable_ipv6 &&
+ if ((dev_net(idev->dev)->ipv6.devconf_all->accept_dad > 1 ||
+ idev->cnf.accept_dad > 1) &&
+ !idev->cnf.disable_ipv6 &&
!(ifp->flags & IFA_F_STABLE_PRIVACY)) {
struct in6_addr addr;
--
2.13.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
2017-09-12 15:46 [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers Matteo Croce
@ 2017-09-15 21:12 ` David Miller
2017-09-28 4:47 ` Erik Kline
2017-09-19 23:44 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2017-09-15 21:12 UTC (permalink / raw)
To: mcroce; +Cc: netdev, linux-doc, ek
From: Matteo Croce <mcroce@redhat.com>
Date: Tue, 12 Sep 2017 17:46:37 +0200
> Currently, writing into
> net.ipv6.conf.all.{accept_dad,use_optimistic,optimistic_dad} has no effect.
> Fix handling of these flags by:
>
> - using the maximum of global and per-interface values for the
> accept_dad flag. That is, if at least one of the two values is
> non-zero, enable DAD on the interface. If at least one value is
> set to 2, enable DAD and disable IPv6 operation on the interface if
> MAC-based link-local address was found
>
> - using the logical OR of global and per-interface values for the
> optimistic_dad flag. If at least one of them is set to one, optimistic
> duplicate address detection (RFC 4429) is enabled on the interface
>
> - using the logical OR of global and per-interface values for the
> use_optimistic flag. If at least one of them is set to one,
> optimistic addresses won't be marked as deprecated during source address
> selection on the interface.
>
> While at it, as we're modifying the prototype for ipv6_use_optimistic_addr(),
> drop inline, and let the compiler decide.
>
> Fixes: 7fd2561e4ebd ("net: ipv6: Add a sysctl to make optimistic addresses useful candidates")
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Erik, please review.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
2017-09-12 15:46 [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers Matteo Croce
2017-09-15 21:12 ` David Miller
@ 2017-09-19 23:44 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2017-09-19 23:44 UTC (permalink / raw)
To: mcroce; +Cc: netdev, linux-doc, ek
From: Matteo Croce <mcroce@redhat.com>
Date: Tue, 12 Sep 2017 17:46:37 +0200
> Currently, writing into
> net.ipv6.conf.all.{accept_dad,use_optimistic,optimistic_dad} has no effect.
> Fix handling of these flags by:
>
> - using the maximum of global and per-interface values for the
> accept_dad flag. That is, if at least one of the two values is
> non-zero, enable DAD on the interface. If at least one value is
> set to 2, enable DAD and disable IPv6 operation on the interface if
> MAC-based link-local address was found
>
> - using the logical OR of global and per-interface values for the
> optimistic_dad flag. If at least one of them is set to one, optimistic
> duplicate address detection (RFC 4429) is enabled on the interface
>
> - using the logical OR of global and per-interface values for the
> use_optimistic flag. If at least one of them is set to one,
> optimistic addresses won't be marked as deprecated during source address
> selection on the interface.
>
> While at it, as we're modifying the prototype for ipv6_use_optimistic_addr(),
> drop inline, and let the compiler decide.
>
> Fixes: 7fd2561e4ebd ("net: ipv6: Add a sysctl to make optimistic addresses useful candidates")
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
2017-09-15 21:12 ` David Miller
@ 2017-09-28 4:47 ` Erik Kline
2017-09-28 11:22 ` Erik Kline
0 siblings, 1 reply; 6+ messages in thread
From: Erik Kline @ 2017-09-28 4:47 UTC (permalink / raw)
To: David Miller; +Cc: mcroce, netdev, linux-doc
[-- Attachment #1: Type: text/plain, Size: 604 bytes --]
> Erik, please review.
I apologize for the delay. I see that you've already applied this, and
it's mostly LGTM except I have one thing I'm not seeing clearly.
The documentation accept_dad now claims:
DAD operation and mode on a given interface will be selected according
to the maximum value of conf/{all,interface}/accept_dad.
but I'm try to square this with my reading of the changes to
addrconf_dad_begin(). I think setting all.accept_dad to 0 but
ifname.accept_dad to non-0 still results in the short-circuit call to
addrconf_dad_completed().
Am I just not seeing (thinking) straight?
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4835 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
2017-09-28 4:47 ` Erik Kline
@ 2017-09-28 11:22 ` Erik Kline
2017-09-29 10:47 ` Matteo Croce
0 siblings, 1 reply; 6+ messages in thread
From: Erik Kline @ 2017-09-28 11:22 UTC (permalink / raw)
To: David Miller; +Cc: mcroce, netdev, linux-doc
[-- Attachment #1: Type: text/plain, Size: 952 bytes --]
On 28 September 2017 at 13:47, Erik Kline <ek@google.com> wrote:
>> Erik, please review.
>
> I apologize for the delay. I see that you've already applied this, and
> it's mostly LGTM except I have one thing I'm not seeing clearly.
>
> The documentation accept_dad now claims:
>
> DAD operation and mode on a given interface will be selected according
> to the maximum value of conf/{all,interface}/accept_dad.
>
> but I'm try to square this with my reading of the changes to
> addrconf_dad_begin(). I think setting all.accept_dad to 0 but
> ifname.accept_dad to non-0 still results in the short-circuit call to
> addrconf_dad_completed().
>
> Am I just not seeing (thinking) straight?
Upon further reflection, doesn't the whole premise of this change
means that it's no longer possible to selectively disable these
features if they are set on "all"? Or are we saying that this mode is
only support with "default" enable + "ifname" disable?
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4835 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
2017-09-28 11:22 ` Erik Kline
@ 2017-09-29 10:47 ` Matteo Croce
0 siblings, 0 replies; 6+ messages in thread
From: Matteo Croce @ 2017-09-29 10:47 UTC (permalink / raw)
To: Erik Kline; +Cc: David Miller, netdev, linux-doc
On Thu, Sep 28, 2017 at 1:22 PM, Erik Kline <ek@google.com> wrote:
> Upon further reflection, doesn't the whole premise of this change
> means that it's no longer possible to selectively disable these
> features if they are set on "all"? Or are we saying that this mode is
> only support with "default" enable + "ifname" disable?
Hi Erik, thanks for the review.
Yes the behaviour seems wrong when writing all.accept_dad respect what
the documentation says.
BTW the previous behaviour was not defined, I put them in OR because
that's what other handlers do, eg. send_redirects.
If you think that it's better to put them in AND we can change the
documentation accordingly.
What do you think?
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-29 10:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-12 15:46 [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers Matteo Croce
2017-09-15 21:12 ` David Miller
2017-09-28 4:47 ` Erik Kline
2017-09-28 11:22 ` Erik Kline
2017-09-29 10:47 ` Matteo Croce
2017-09-19 23:44 ` 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).