| Message ID | 1422446619-9502-1-git-send-email-ek@google.com |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jan 28, 2015 at 9:03 PM, Erik Kline <ek@google.com> wrote: > if (ipv6_addr_equal(&ifp->addr, addr) && > - !(ifp->flags&IFA_F_TENTATIVE) && > + (!(ifp->flags&banned_flags) || > + ifp->flags&IFA_F_OPTIMISTIC&~banned_flags) && Is this if statement correct? The intent here is "if ifp has IFA_F_OPTIMISTIC set, then IFA_F_TENTATIVE is allowed, even if the caller explicitly banned IFA_F_TENTATIVE", right? Not "if ifp has IFA_F_OPTIMISTIC set, then any flags are allowed, even ones explicitly baned by the caller". For example, suppose that: banned_flags = IFA_F_SECONDARY ifp->flags = IFA_F_SECONDARY | IFA_F_OPTIMISTIC In that case, won't the if statement match ifp, even though it contains a flag that is explicitly banned? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Erik, you have to address and respond to the feedback you received from Lorenzo Colitti. If you do not respond and handle that soon, I'm simply taking your patch out of my queue and you'll have to resubmit even if no changes are necessary. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 29, 2015 at 2:25 PM, Lorenzo Colitti <lorenzo@google.com> wrote: > On Wed, Jan 28, 2015 at 9:03 PM, Erik Kline <ek@google.com> wrote: >> if (ipv6_addr_equal(&ifp->addr, addr) && >> - !(ifp->flags&IFA_F_TENTATIVE) && >> + (!(ifp->flags&banned_flags) || >> + ifp->flags&IFA_F_OPTIMISTIC&~banned_flags) && > > Is this if statement correct? > > The intent here is "if ifp has IFA_F_OPTIMISTIC set, then > IFA_F_TENTATIVE is allowed, even if the caller explicitly banned > IFA_F_TENTATIVE", right? Not "if ifp has IFA_F_OPTIMISTIC set, then > any flags are allowed, even ones explicitly baned by the caller". For > example, suppose that: > > banned_flags = IFA_F_SECONDARY > ifp->flags = IFA_F_SECONDARY | IFA_F_OPTIMISTIC > > In that case, won't the if statement match ifp, even though it > contains a flag that is explicitly banned? Yep, true. It works for all existing inputs, but is not generally future-proof. :( I have a rewrite that explicitly defines the circumstances under which optimistic would be considered ok in this check. If there are changes in that definition in the future that one clause will need to be updated accordingly. Patch coming in minutes. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 31, 2015 at 10:31 AM, David Miller <davem@davemloft.net> wrote: > > Erik, you have to address and respond to the feedback you received > from Lorenzo Colitti. > > If you do not respond and handle that soon, I'm simply taking > your patch out of my queue and you'll have to resubmit even > if no changes are necessary. > > Thanks. Ack. I didn't get to it on Friday, nor was I able to get to the office on the weekend, but I have coming in minutes. Apologies, -Erik -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/addrconf.h b/include/net/addrconf.h index d13573b..80456f7 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -62,6 +62,9 @@ int addrconf_set_dstaddr(struct net *net, void __user *arg); int ipv6_chk_addr(struct net *net, const struct in6_addr *addr, const struct net_device *dev, int strict); +int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr, + const struct net_device *dev, int strict, + u32 banned_flags); #if defined(CONFIG_IPV6_MIP6) || defined(CONFIG_IPV6_MIP6_MODULE) int ipv6_chk_home_addr(struct net *net, const struct in6_addr *addr); diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index f7c8bbe..943da83 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1519,6 +1519,14 @@ static int ipv6_count_addresses(struct inet6_dev *idev) int ipv6_chk_addr(struct net *net, const struct in6_addr *addr, const struct net_device *dev, int strict) { + return ipv6_chk_addr_and_flags(net, addr, dev, strict, IFA_F_TENTATIVE); +} +EXPORT_SYMBOL(ipv6_chk_addr); + +int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr, + const struct net_device *dev, int strict, + u32 banned_flags) +{ struct inet6_ifaddr *ifp; unsigned int hash = inet6_addr_hash(addr); @@ -1527,7 +1535,8 @@ int ipv6_chk_addr(struct net *net, const struct in6_addr *addr, if (!net_eq(dev_net(ifp->idev->dev), net)) continue; if (ipv6_addr_equal(&ifp->addr, addr) && - !(ifp->flags&IFA_F_TENTATIVE) && + (!(ifp->flags&banned_flags) || + ifp->flags&IFA_F_OPTIMISTIC&~banned_flags) && (dev == NULL || ifp->idev->dev == dev || !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) { rcu_read_unlock_bh(); @@ -1538,7 +1547,7 @@ int ipv6_chk_addr(struct net *net, const struct in6_addr *addr, rcu_read_unlock_bh(); return 0; } -EXPORT_SYMBOL(ipv6_chk_addr); +EXPORT_SYMBOL(ipv6_chk_addr_and_flags); static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr, struct net_device *dev) diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 6828667..113fc6c 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -655,7 +655,9 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb) struct in6_addr *target = (struct in6_addr *)&neigh->primary_key; int probes = atomic_read(&neigh->probes); - if (skb && ipv6_chk_addr(dev_net(dev), &ipv6_hdr(skb)->saddr, dev, 1)) + if (skb && ipv6_chk_addr_and_flags(dev_net(dev), &ipv6_hdr(skb)->saddr, + dev, 1, + IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) saddr = &ipv6_hdr(skb)->saddr; probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES); if (probes < 0) {
RFC 4429 ("Optimistic DAD") states that optimistic addresses should be treated as deprecated addresses. From section 2.1: Unless noted otherwise, components of the IPv6 protocol stack should treat addresses in the Optimistic state equivalently to those in the Deprecated state, indicating that the address is available for use but should not be used if another suitable address is available. Optimistic addresses are indeed avoided when other addresses are available (i.e. at source address selection time), but they have not heretofore been available for things like explicit bind() and sendmsg() with struct in6_pktinfo, etc. This change makes optimistic addresses treated more like deprecated addresses than tentative ones. Signed-off-by: Erik Kline <ek@google.com> --- include/net/addrconf.h | 3 +++ net/ipv6/addrconf.c | 13 +++++++++++-- net/ipv6/ndisc.c | 4 +++- 3 files changed, 17 insertions(+), 3 deletions(-)