On Fri, Jan 26, 2007 at 09:13:31AM +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote: > In article <[EMAIL PROTECTED]> (at Thu, 25 Jan 2007 14:45:00 -0500), Neil > Horman <[EMAIL PROTECTED]> says:
New patch attached with most of your suggestions incorporated. I've a few comments mixed in for some of the suggestions that I think need further discussion > If optimistic_dad is disabled, flags should be IFA_F_TEMPORARY, > not IFA_F_TEMPORARY|IFA_F_OPTIMISTIC. > > Another idea is to use IFA_F_OPTIMISTIC not > IFA_F_OPTIMISTIC|IFA_F_TENTATIVE until the DAD has been finished. > I'm currently setting the OPTIMISTIC flag in every location that its possibly needed, and then clearing it in addrconf_dad_start if that interface is not participating in optimistic dad. I do this because the RFC in section 3.1 indicates that manually configured addresses should not set the optimistic flag. If I removed the OPTIMISTIC flag from the locations it gets set in the patch and then only set it for participating interfaces in addrconf_dad_start, I would need to have some way to tell if the address in question was manually configured (to avoid setting it in that case). At present I see no clear way to do that, but if you have a suggestion, I'll happily change this around. <snip> > > ((ifa_result->flags & > (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC)) == 0) <snip> > ditto. > Done > > @@ -2123,7 +2133,8 @@ static void addrconf_add_linklocal(struct inet6_dev > > *idev, struct in6_addr *addr > > { > > struct inet6_ifaddr * ifp; > > > > - ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, IFA_F_PERMANENT); > > + ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, > > + IFA_F_PERMANENT|IFA_F_OPTIMISTIC); > > if (!IS_ERR(ifp)) { > > addrconf_dad_start(ifp, 0); > > in6_ifa_put(ifp); > > Please do not always put IFA_F_OPTIMISTIC. > Again, same reasoning as above for why I set optimistic in this way. > > > > + /* > > + * Optimistic nodes need to joing the anycast address > > + * right away > > + */ > > + if (ifp->flags & IFA_F_OPTIMISTIC) > > + addrconf_join_anycast(ifp); > > + > > if (ifp->prefix_len != 128 && (ifp->flags&IFA_F_PERMANENT)) > > addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev, 0, > > flags); > > Should we join anycast even if the node is a host (not a router)?! > I've moved this to the spot below where we check if we are forwarding, so the new patch attached does not join anycast if we are a router. > When you add a call to "addrconf_join_anycast()", > you must consider when to leave this. > I think addrconf_dad_completed is appropriate for this. New patch leaves anycast group when we are no longer optimistic > > > @@ -2573,6 +2594,18 @@ static void addrconf_dad_start(struct inet6_ifaddr > > *ifp, u32 flags) > > addrconf_dad_stop(ifp); > > return; > > } > > + > > + /* > > + * Forwarding devices (routers) should not use > > + * optimistic addresses > > + * Nor should interfaces that don't know the > > + * Source address for their default gateway > > + * RFC 4429 Sec 3.3 > > + */ > > + if ((ipv6_devconf.forwarding) || > > + (ifp->rt == NULL)) > > + ifp->flags &= ~IFA_F_OPTIMISTIC; > > + > > addrconf_dad_kick(ifp); > > spin_unlock_bh(&ifp->lock); > > out: > > Please test this condition when you are adding the > address. > Are you sure? There are several locations where we add an address that we start dad on. Shall I test this in all of those places? It would seem more appropriate to do it in addrconf_dad_start, as I do. Or am I missing something? > BTW, you have not implemented the later condition, > right? Sefault gatewa is not tested. > I thought I was testing it. I was under the impression from my reading of the code the ifp->rt held the rt6_info pointer for the default gateway, so if its not set, we don't know the default gateway. I may well be missing something here though. Please let me know. > > index 6a9f616..fcd22e3 100644 > > --- a/net/ipv6/ndisc.c > > +++ b/net/ipv6/ndisc.c > > @@ -498,7 +498,21 @@ static void ndisc_send_na(struct net_device *dev, > > struct neighbour *neigh, > > msg->icmph.icmp6_unused = 0; > > msg->icmph.icmp6_router = router; > > msg->icmph.icmp6_solicited = solicited; > > - msg->icmph.icmp6_override = override; > > + if (!ifp || !(ifp->flags & IFA_F_OPTIMISTIC)) > > + msg->icmph.icmp6_override = override; > > + else { > > + /* > > + * We must clear the override flag on all > > + * neighbor advertisements from source > > + * addresses that are OPTIMISTIC - RFC 4429 > > + * section 2.2 > > + */ > > + if (override) > > + printk(KERN_WARNING > > + "Disallowing override flag for OPTIMISTIC > > addr\n"); > > + msg->icmph.icmp6_override = 0; > > + } > > + > > Ifp is already put. Please clear "override" in the code > where we try getting temporary source address for NS. > Done. I've moved the check to before the in6_ifa_put, and added a flag to skip adding the sllao option if we find we are optimistic. > > @@ -622,9 +637,20 @@ void ndisc_send_rs(struct net_device *dev, struct > > in6_addr *saddr, > > + /* > > + * Check the source address. If its OPTIMISTIC > > + * and addr_len is non-zero (implying the sllao option) > > + * then don't send the RS (RFC 4429, section 2.2) > > + */ > > + ifp = ipv6_get_ifaddr(saddr, dev, 1); > > + > > + if ((!ifp) || ((ifp->flags & IFA_F_OPTIMISTIC) && dev->addr_len)) > > + return; > > + > > ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr, > > dev->ifindex); > > > > I disagree. Please send RS in other way. > Choose another address, or send it without SLLAO. > Done More thoughts/comments appreciated. Thanks & Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> include/linux/if_addr.h | 1 include/linux/ipv6.h | 2 + include/linux/sysctl.h | 1 include/net/addrconf.h | 4 +- net/ipv6/addrconf.c | 78 +++++++++++++++++++++++++++++++++++++++--------- net/ipv6/mcast.c | 4 +- net/ipv6/ndisc.c | 74 +++++++++++++++++++++++++++++++-------------- 7 files changed, 124 insertions(+), 40 deletions(-) diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h index d557e4c..43f3bed 100644 --- a/include/linux/if_addr.h +++ b/include/linux/if_addr.h @@ -39,6 +39,7 @@ enum #define IFA_F_TEMPORARY IFA_F_SECONDARY #define IFA_F_NODAD 0x02 +#define IFA_F_OPTIMISTIC 0x04 #define IFA_F_HOMEADDRESS 0x10 #define IFA_F_DEPRECATED 0x20 #define IFA_F_TENTATIVE 0x40 diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h index f824113..5d37abf 100644 --- a/include/linux/ipv6.h +++ b/include/linux/ipv6.h @@ -177,6 +177,7 @@ struct ipv6_devconf { #endif #endif __s32 proxy_ndp; + __s32 optimistic_dad; void *sysctl; }; @@ -205,6 +206,7 @@ enum { DEVCONF_RTR_PROBE_INTERVAL, DEVCONF_ACCEPT_RA_RT_INFO_MAX_PLEN, DEVCONF_PROXY_NDP, + DEVCONF_OPTIMISTIC_DAD, DEVCONF_MAX }; diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 81480e6..972a33a 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -570,6 +570,7 @@ enum { NET_IPV6_RTR_PROBE_INTERVAL=21, NET_IPV6_ACCEPT_RA_RT_INFO_MAX_PLEN=22, NET_IPV6_PROXY_NDP=23, + NET_IPV6_OPTIMISTIC_DAD=24, __NET_IPV6_MAX }; diff --git a/include/net/addrconf.h b/include/net/addrconf.h index 88df8fc..d248a19 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -73,7 +73,9 @@ extern int ipv6_get_saddr(struct dst_entry *dst, extern int ipv6_dev_get_saddr(struct net_device *dev, struct in6_addr *daddr, struct in6_addr *saddr); -extern int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *); +extern int ipv6_get_lladdr(struct net_device *dev, + struct in6_addr *, + unsigned char banned_flags); extern int ipv6_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2); extern void addrconf_join_solict(struct net_device *dev, diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 2a7e461..fc29815 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -830,7 +830,8 @@ retry: ift = !max_addresses || ipv6_count_addresses(idev) < max_addresses ? ipv6_add_addr(idev, &addr, tmp_plen, - ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK, IFA_F_TEMPORARY) : NULL; + ipv6_addr_type(&addr)&IPV6_ADDR_SCOPE_MASK, + IFA_F_TEMPORARY|IFA_F_OPTIMISTIC) : NULL; if (!ift || IS_ERR(ift)) { in6_ifa_put(ifp); in6_dev_put(idev); @@ -962,13 +963,14 @@ int ipv6_dev_get_saddr(struct net_device *daddr_dev, * - Tentative Address (RFC2462 section 5.4) * - A tentative address is not considered * "assigned to an interface" in the traditional - * sense. + * sense, unless it is also flagged as optimistic. * - Candidate Source Address (section 4) * - In any case, anycast addresses, multicast * addresses, and the unspecified address MUST * NOT be included in a candidate set. */ - if (ifa->flags & IFA_F_TENTATIVE) + if ((ifa->flags & IFA_F_TENTATIVE) && + (!(ifa->flags & IFA_F_OPTIMISTIC))) continue; if (unlikely(score.addr_type == IPV6_ADDR_ANY || score.addr_type & IPV6_ADDR_MULTICAST)) { @@ -1027,15 +1029,17 @@ int ipv6_dev_get_saddr(struct net_device *daddr_dev, } } - /* Rule 3: Avoid deprecated address */ + /* Rule 3: Avoid deprecated and optimistic address */ if (hiscore.rule < 3) { if (ipv6_saddr_preferred(hiscore.addr_type) || - !(ifa_result->flags & IFA_F_DEPRECATED)) + ((ifa_result->flags & + (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC) == 0))) hiscore.attrs |= IPV6_SADDR_SCORE_PREFERRED; hiscore.rule++; } if (ipv6_saddr_preferred(score.addr_type) || - !(ifa->flags & IFA_F_DEPRECATED)) { + ((ifa_result->flags & + (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC) == 0))) { score.attrs |= IPV6_SADDR_SCORE_PREFERRED; if (!(hiscore.attrs & IPV6_SADDR_SCORE_PREFERRED)) { score.rule = 3; @@ -1174,7 +1178,8 @@ int ipv6_get_saddr(struct dst_entry *dst, } -int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr) +int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr, + unsigned char banned_flags) { struct inet6_dev *idev; int err = -EADDRNOTAVAIL; @@ -1185,7 +1190,7 @@ int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr) read_lock_bh(&idev->lock); for (ifp=idev->addr_list; ifp; ifp=ifp->if_next) { - if (ifp->scope == IFA_LINK && !(ifp->flags&IFA_F_TENTATIVE)) { + if (ifp->scope == IFA_LINK && !(ifp->flags & banned_flags)) { ipv6_addr_copy(addr, &ifp->addr); err = 0; break; @@ -1751,6 +1756,7 @@ ok: update_lft = create = 1; ifp->cstamp = jiffies; + ifp->flags |= IFA_F_OPTIMISTIC; addrconf_dad_start(ifp, RTF_ADDRCONF|RTF_PREFIX_RT); } @@ -1945,7 +1951,11 @@ static int inet6_addr_add(int ifindex, struct in6_addr *pfx, int plen, ifp->prefered_lft = prefered_lft; ifp->tstamp = jiffies; spin_unlock_bh(&ifp->lock); - + /* + * Note that section 3.1 of RFC 4429 indicates + * That the Optimistic flag should not be set for + * manually configured addresses + */ addrconf_dad_start(ifp, 0); in6_ifa_put(ifp); addrconf_verify(0); @@ -2123,7 +2133,8 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, struct in6_addr *addr { struct inet6_ifaddr * ifp; - ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, IFA_F_PERMANENT); + ifp = ipv6_add_addr(idev, addr, 64, IFA_LINK, + IFA_F_PERMANENT|IFA_F_OPTIMISTIC); if (!IS_ERR(ifp)) { addrconf_dad_start(ifp, 0); in6_ifa_put(ifp); @@ -2190,7 +2201,7 @@ ipv6_inherit_linklocal(struct inet6_dev *idev, struct net_device *link_dev) { struct in6_addr lladdr; - if (!ipv6_get_lladdr(link_dev, &lladdr)) { + if (!ipv6_get_lladdr(link_dev, &lladdr, IFA_F_TENTATIVE)) { addrconf_add_linklocal(idev, &lladdr); return 0; } @@ -2527,7 +2538,11 @@ static void addrconf_dad_kick(struct inet6_ifaddr *ifp) unsigned long rand_num; struct inet6_dev *idev = ifp->idev; - rand_num = net_random() % (idev->cnf.rtr_solicit_delay ? : 1); + if (ifp->flags & IFA_F_OPTIMISTIC) + rand_num = 0; + else + rand_num = net_random() % (idev->cnf.rtr_solicit_delay ? : 1); + ifp->probes = idev->cnf.dad_transmits; addrconf_mod_timer(ifp, AC_DAD, rand_num); } @@ -2537,6 +2552,9 @@ static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags) struct inet6_dev *idev = ifp->idev; struct net_device *dev = idev->dev; + if (!idev->cnf.optimistic_dad) + ifp->flags &= ~IFA_F_OPTIMISTIC; + addrconf_join_solict(dev, &ifp->addr); if (ifp->prefix_len != 128 && (ifp->flags&IFA_F_PERMANENT)) @@ -2553,7 +2571,7 @@ static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags) if (dev->flags&(IFF_NOARP|IFF_LOOPBACK) || !(ifp->flags&IFA_F_TENTATIVE) || ifp->flags & IFA_F_NODAD) { - ifp->flags &= ~IFA_F_TENTATIVE; + ifp->flags &= ~(IFA_F_TENTATIVE|IFA_F_OPTIMISTIC); spin_unlock_bh(&ifp->lock); read_unlock_bh(&idev->lock); @@ -2573,6 +2591,25 @@ static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags) addrconf_dad_stop(ifp); return; } + + /* + * Forwarding devices (routers) should not use + * optimistic addresses + * Nor should interfaces that don't know the + * Source address for their default gateway + * RFC 4429 Sec 3.3 + */ + if ((ipv6_devconf.forwarding) || + (ifp->rt == NULL)) + ifp->flags &= ~IFA_F_OPTIMISTIC; + + /* + * Optimistic nodes need to join the anycast address + * right away + */ + if (ifp->flags & IFA_F_OPTIMISTIC) + addrconf_join_anycast(ifp); + addrconf_dad_kick(ifp); spin_unlock_bh(&ifp->lock); out: @@ -2597,7 +2634,7 @@ static void addrconf_dad_timer(unsigned long data) * DAD was successful */ - ifp->flags &= ~IFA_F_TENTATIVE; + ifp->flags &= ~(IFA_F_TENTATIVE|IFA_F_OPTIMISTIC); spin_unlock_bh(&ifp->lock); read_unlock_bh(&idev->lock); @@ -2627,6 +2664,9 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp) * Configure the address for reception. Now it is valid. */ + if (ifp->flags & IFA_F_OPTIMISTIC) + addrconf_leave_anycast(ifp); + ipv6_ifa_notify(RTM_NEWADDR, ifp); /* If added prefix is link local and forwarding is off, @@ -3398,6 +3438,7 @@ static void inline ipv6_store_devconf(struct ipv6_devconf *cnf, #endif #endif array[DEVCONF_PROXY_NDP] = cnf->proxy_ndp; + array[DEVCONF_OPTIMISTIC_DAD] = cnf->optimistic_dad; } static inline size_t inet6_if_nlmsg_size(void) @@ -3918,6 +3959,15 @@ static struct addrconf_sysctl_table .proc_handler = &proc_dointvec, }, { + .ctl_name = NET_IPV6_OPTIMISTIC_DAD, + .procname = "optimistic_dad", + .data = &ipv6_devconf.optimistic_dad, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec, + + }, + { .ctl_name = 0, /* sentinel */ } }, diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index 882cde4..9c5273c 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -1411,7 +1411,7 @@ static struct sk_buff *mld_newpack(struct net_device *dev, int size) skb_reserve(skb, LL_RESERVED_SPACE(dev)); - if (ipv6_get_lladdr(dev, &addr_buf)) { + if (ipv6_get_lladdr(dev, &addr_buf, IFA_F_TENTATIVE)) { /* <draft-ietf-magma-mld-source-05.txt>: * use unspecified address as the source address * when a valid link-local address is not available. @@ -1789,7 +1789,7 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type) skb_reserve(skb, LL_RESERVED_SPACE(dev)); - if (ipv6_get_lladdr(dev, &addr_buf)) { + if (ipv6_get_lladdr(dev, &addr_buf, IFA_F_TENTATIVE)) { /* <draft-ietf-magma-mld-source-05.txt>: * use unspecified address as the source address * when a valid link-local address is not available. diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 6a9f616..6ed0357 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -447,6 +447,8 @@ static void ndisc_send_na(struct net_device *dev, struct neighbour *neigh, ifp = ipv6_get_ifaddr(solicited_addr, dev, 1); if (ifp) { src_addr = solicited_addr; + if (ifp->flags & IFA_F_OPTIMISTIC) + override = 0; in6_ifa_put(ifp); } else { if (ipv6_dev_get_saddr(dev, daddr, &tmpaddr)) @@ -498,7 +500,8 @@ static void ndisc_send_na(struct net_device *dev, struct neighbour *neigh, msg->icmph.icmp6_unused = 0; msg->icmph.icmp6_router = router; msg->icmph.icmp6_solicited = solicited; - msg->icmph.icmp6_override = override; + msg->icmph.icmp6_override = override; + /* Set the target address. */ ipv6_addr_copy(&msg->target, solicited_addr); @@ -542,7 +545,8 @@ void ndisc_send_ns(struct net_device *dev, struct neighbour *neigh, int send_llinfo; if (saddr == NULL) { - if (ipv6_get_lladdr(dev, &addr_buf)) + if (ipv6_get_lladdr(dev, &addr_buf, + (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC))) return; saddr = &addr_buf; } @@ -622,9 +626,21 @@ void ndisc_send_rs(struct net_device *dev, struct in6_addr *saddr, struct sk_buff *skb; struct icmp6hdr *hdr; __u8 * opt; + struct inet6_ifaddr *ifp; + int include_sllao_option = 1; int len; int err; + /* + * Check the source address. If its OPTIMISTIC + * and addr_len is non-zero (implying the sllao option) + * then don't send the RS (RFC 4429, section 2.2) + */ + ifp = ipv6_get_ifaddr(saddr, dev, 1); + + if ((!ifp) || ((ifp->flags & IFA_F_OPTIMISTIC) && dev->addr_len)) + include_sllao_option=0; + ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr, dev->ifindex); @@ -664,7 +680,7 @@ void ndisc_send_rs(struct net_device *dev, struct in6_addr *saddr, opt = (u8*) (hdr + 1); - if (dev->addr_len) + if (dev->addr_len && include_sllao_option) ndisc_fill_addr_option(opt, ND_OPT_SOURCE_LL_ADDR, dev->dev_addr, dev->addr_len, dev->type); @@ -746,6 +762,7 @@ static void ndisc_recv_ns(struct sk_buff *skb) int dad = ipv6_addr_any(saddr); int inc; int is_router; + int type; if (ipv6_addr_is_multicast(&msg->target)) { ND_PRINTK2(KERN_WARNING @@ -796,28 +813,39 @@ static void ndisc_recv_ns(struct sk_buff *skb) inc = ipv6_addr_is_multicast(daddr); if ((ifp = ipv6_get_ifaddr(&msg->target, dev, 1)) != NULL) { - if (ifp->flags & IFA_F_TENTATIVE) { - /* Address is tentative. If the source - is unspecified address, it is someone - does DAD, otherwise we ignore solicitations - until DAD timer expires. - */ - if (!dad) + + if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) { + if (dad) { + if (dev->type == ARPHRD_IEEE802_TR) { + unsigned char *sadr = skb->mac.raw; + if (((sadr[8] ^ dev->dev_addr[0]) & 0x7f) == 0 && + sadr[9] == dev->dev_addr[1] && + sadr[10] == dev->dev_addr[2] && + sadr[11] == dev->dev_addr[3] && + sadr[12] == dev->dev_addr[4] && + sadr[13] == dev->dev_addr[5]) { + /* looped-back to us */ + goto out; + } + } + + /* + * We are colliding with another node + * who is doing DAD + * so fail our DAD process + */ + addrconf_dad_failure(ifp); goto out; - if (dev->type == ARPHRD_IEEE802_TR) { - unsigned char *sadr = skb->mac.raw; - if (((sadr[8] ^ dev->dev_addr[0]) & 0x7f) == 0 && - sadr[9] == dev->dev_addr[1] && - sadr[10] == dev->dev_addr[2] && - sadr[11] == dev->dev_addr[3] && - sadr[12] == dev->dev_addr[4] && - sadr[13] == dev->dev_addr[5]) { - /* looped-back to us */ + } else { + /* + * This is not a dad solicitation. + * If we are an optimistic node, + * we should respond. + * Otherwise, we should ignore it. + */ + if (!(ifp->flags & IFA_F_OPTIMISTIC)) goto out; - } } - addrconf_dad_failure(ifp); - return; } idev = ifp->idev; @@ -1406,7 +1434,7 @@ void ndisc_send_redirect(struct sk_buff *skb, struct neighbour *neigh, dev = skb->dev; - if (ipv6_get_lladdr(dev, &saddr_buf)) { + if (ipv6_get_lladdr(dev, &saddr_buf, IFA_F_TENTATIVE)) { ND_PRINTK2(KERN_WARNING "ICMPv6 Redirect: no link-local address on %s\n", dev->name); - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html