On Mon, Aug 22, 2016 at 01:50:53PM +0200, Martin Pieuchot wrote:
> Diff below replace in6_selectsrc() by rtalloc(9) for source address
> selection when send NS and NA.
OK bluhm@
> While this can be a controversial change, it's the only way I found to
> move forward to change the existing route caching mechanism. My goal
> is to stop passing a 'struct route{_in6}' to ip{,6}_output() and only
> give it a *valid* 'struct rtentry'. But since too many code paths
> currently use a 'struct route{,_in6}', I'm doing small steps.
>
> That means that we're going to do 2 route lookups per NS/NA packets
> until ip6_output() is changed to take a 'struct rtentry'. That's fine
> since such packets are not sent too often.
>
> Index: netinet6/nd6_nbr.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 nd6_nbr.c
> --- netinet6/nd6_nbr.c 22 Aug 2016 10:33:22 -0000 1.109
> +++ netinet6/nd6_nbr.c 22 Aug 2016 11:35:40 -0000
> @@ -368,10 +368,6 @@ nd6_ns_output(struct ifnet *ifp, struct
> int icmp6len;
> int maxlen;
> caddr_t mac;
> - struct route_in6 ro;
> -
> - bzero(&ro, sizeof(ro));
> - ro.ro_tableid = ifp->if_rdomain;
>
> if (IN6_IS_ADDR_MULTICAST(taddr6))
> return;
> @@ -467,24 +463,23 @@ nd6_ns_output(struct ifnet *ifp, struct
> if (saddr6 && in6ifa_ifpwithaddr(ifp, saddr6))
> src_sa.sin6_addr = *saddr6;
> else {
> - struct in6_addr *src0;
> - int error;
> + struct rtentry *rt;
>
> - bcopy(&dst_sa, &ro.ro_dst, sizeof(dst_sa));
> - error = in6_selectsrc(&src0, &dst_sa, NULL, &ro,
> + rt = rtalloc(sin6tosa(&dst_sa), RT_RESOLVE,
> m->m_pkthdr.ph_rtableid);
> - if (error) {
> + if (!rtisvalid(rt)) {
> char addr[INET6_ADDRSTRLEN];
>
> nd6log((LOG_DEBUG,
> - "nd6_ns_output: source can't be "
> - "determined: dst=%s, error=%d\n",
> - inet_ntop(AF_INET6, &dst_sa.sin6_addr,
> - addr, sizeof(addr)),
> - error));
> + "%s: source can't be determined: dst=%s\n",
> + __func__, inet_ntop(AF_INET6,
> + &dst_sa.sin6_addr, addr, sizeof(addr))));
> + rtfree(rt);
> goto bad;
> }
> - src_sa.sin6_addr = *src0;
> + src_sa.sin6_addr =
> + ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> + rtfree(rt);
> }
> } else {
> /*
> @@ -537,20 +532,12 @@ nd6_ns_output(struct ifnet *ifp, struct
> nd_ns->nd_ns_cksum = 0;
> m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
>
> - ip6_output(m, NULL, &ro, dad ? IPV6_UNSPECSRC : 0, &im6o, NULL);
> + ip6_output(m, NULL, NULL, dad ? IPV6_UNSPECSRC : 0, &im6o, NULL);
> icmp6stat.icp6s_outhist[ND_NEIGHBOR_SOLICIT]++;
> -
> - if (ro.ro_rt) { /* we don't cache this route. */
> - rtfree(ro.ro_rt);
> - }
> return;
>
> bad:
> - if (ro.ro_rt) {
> - rtfree(ro.ro_rt);
> - }
> m_freem(m);
> - return;
> }
>
> /*
> @@ -913,18 +900,13 @@ nd6_na_output(struct ifnet *ifp, struct
> struct sockaddr *sdl0)
> {
> struct mbuf *m;
> + struct rtentry *rt = NULL;
> struct ip6_hdr *ip6;
> struct nd_neighbor_advert *nd_na;
> struct ip6_moptions im6o;
> - struct sockaddr_in6 src_sa, dst_sa;
> - struct in6_addr *src0;
> - int icmp6len, maxlen, error;
> - caddr_t mac;
> - struct route_in6 ro;
> -
> - mac = NULL;
> - bzero(&ro, sizeof(ro));
> - ro.ro_tableid = ifp->if_rdomain;
> + struct sockaddr_in6 dst_sa;
> + int icmp6len, maxlen;
> + caddr_t mac = NULL;
>
> /* estimate the size of message */
> maxlen = sizeof(*ip6) + sizeof(*nd_na);
> @@ -969,10 +951,9 @@ nd6_na_output(struct ifnet *ifp, struct
> ip6->ip6_vfc |= IPV6_VERSION;
> ip6->ip6_nxt = IPPROTO_ICMPV6;
> ip6->ip6_hlim = 255;
> - bzero(&src_sa, sizeof(src_sa));
> bzero(&dst_sa, sizeof(dst_sa));
> - src_sa.sin6_len = dst_sa.sin6_len = sizeof(struct sockaddr_in6);
> - src_sa.sin6_family = dst_sa.sin6_family = AF_INET6;
> + dst_sa.sin6_len = sizeof(struct sockaddr_in6);
> + dst_sa.sin6_family = AF_INET6;
> dst_sa.sin6_addr = *daddr6;
> if (IN6_IS_ADDR_UNSPECIFIED(daddr6)) {
> /* reply to DAD */
> @@ -989,20 +970,18 @@ nd6_na_output(struct ifnet *ifp, struct
> /*
> * Select a source whose scope is the same as that of the dest.
> */
> - bcopy(&dst_sa, &ro.ro_dst, sizeof(dst_sa));
> - error = in6_selectsrc(&src0, &dst_sa, NULL, &ro,
> - m->m_pkthdr.ph_rtableid);
> - if (error) {
> + rt = rtalloc(sin6tosa(&dst_sa), RT_RESOLVE, ifp->if_rdomain);
> + if (!rtisvalid(rt)) {
> char addr[INET6_ADDRSTRLEN];
>
> - nd6log((LOG_DEBUG, "nd6_na_output: source can't be "
> - "determined: dst=%s, error=%d\n",
> - inet_ntop(AF_INET6, &dst_sa.sin6_addr, addr, sizeof(addr)),
> - error));
> + nd6log((LOG_DEBUG, "%s: source can't be determined: dst=%s\n",
> + __func__, inet_ntop(AF_INET6, &dst_sa.sin6_addr, addr,
> + sizeof(addr))));
> + rtfree(rt);
> goto bad;
> }
> - src_sa.sin6_addr = *src0;
> - ip6->ip6_src = src_sa.sin6_addr;
> + ip6->ip6_src = ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
> + rtfree(rt);
> nd_na = (struct nd_neighbor_advert *)(ip6 + 1);
> nd_na->nd_na_type = ND_NEIGHBOR_ADVERT;
> nd_na->nd_na_code = 0;
> @@ -1059,20 +1038,12 @@ nd6_na_output(struct ifnet *ifp, struct
> nd_na->nd_na_cksum = 0;
> m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
>
> - ip6_output(m, NULL, &ro, 0, &im6o, NULL);
> + ip6_output(m, NULL, NULL, 0, &im6o, NULL);
> icmp6stat.icp6s_outhist[ND_NEIGHBOR_ADVERT]++;
> -
> - if (ro.ro_rt) { /* we don't cache this route. */
> - rtfree(ro.ro_rt);
> - }
> return;
>
> bad:
> - if (ro.ro_rt) {
> - rtfree(ro.ro_rt);
> - }
> m_freem(m);
> - return;
> }
>
> caddr_t
