On 02/11/17(Thu) 23:25, Florian Obser wrote:
> We are processing Router Solicitation / Advertisement messages only
> for the Source Link-layer Address Options.
> Merge nd6_rs_input() and nd6_ra_input() into one generic function.
Ok, but I'd suggest a different name: nd6_rtr_cache().
I'd also suggest you look at killing the others functions in this file,
I doubt they are still in use since you introduced slaacd(8) (:
>
> OK?
>
> diff --git netinet6/icmp6.c netinet6/icmp6.c
> index 421280690c9..b5e12169584 100644
> --- netinet6/icmp6.c
> +++ netinet6/icmp6.c
> @@ -637,32 +637,23 @@ icmp6_input(struct mbuf **mp, int *offp, int proto, int
> af)
> break;
>
> case ND_ROUTER_SOLICIT:
> - if (code != 0)
> - goto badcode;
> - if (icmp6len < sizeof(struct nd_router_solicit))
> - goto badlen;
> - if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
> - /* give up local */
> - nd6_rs_input(m, off, icmp6len);
> - m = NULL;
> - goto freeit;
> - }
> - nd6_rs_input(n, off, icmp6len);
> - /* m stays. */
> - break;
> -
> case ND_ROUTER_ADVERT:
> if (code != 0)
> goto badcode;
> - if (icmp6len < sizeof(struct nd_router_advert))
> + if ((icmp6->icmp6_type == ND_ROUTER_SOLICIT && icmp6len <
> + sizeof(struct nd_router_solicit)) ||
> + (icmp6->icmp6_type == ND_ROUTER_ADVERT && icmp6len <
> + sizeof(struct nd_router_advert)))
> goto badlen;
> +
> if ((n = m_copym(m, 0, M_COPYALL, M_DONTWAIT)) == NULL) {
> /* give up local */
> - nd6_ra_input(m, off, icmp6len);
> + nd6_cache_from_rtr_input(m, off, icmp6len,
> + icmp6->icmp6_type);
> m = NULL;
> goto freeit;
> }
> - nd6_ra_input(n, off, icmp6len);
> + nd6_cache_from_rtr_input(n, off, icmp6len, icmp6->icmp6_type);
> /* m stays. */
> break;
>
> diff --git netinet6/nd6.h netinet6/nd6.h
> index fa63806d040..ce273321a25 100644
> --- netinet6/nd6.h
> +++ netinet6/nd6.h
> @@ -192,9 +192,8 @@ void nd6_ns_output(struct ifnet *, struct in6_addr *,
> caddr_t nd6_ifptomac(struct ifnet *);
> void nd6_dad_start(struct ifaddr *);
> void nd6_dad_stop(struct ifaddr *);
> -void nd6_ra_input(struct mbuf *, int, int);
>
> -void nd6_rs_input(struct mbuf *, int, int);
> +void nd6_cache_from_rtr_input(struct mbuf *, int, int, int);
>
> int in6_ifdel(struct ifnet *, struct in6_addr *);
> void rt6_flush(struct in6_addr *, struct ifnet *);
> diff --git netinet6/nd6_rtr.c netinet6/nd6_rtr.c
> index 51772d5814e..18397b8267b 100644
> --- netinet6/nd6_rtr.c
> +++ netinet6/nd6_rtr.c
> @@ -60,111 +60,15 @@
> int rt6_deleteroute(struct rtentry *, void *, unsigned int);
>
> /*
> - * Receive Router Solicitation Message - just for routers.
> - * Router solicitation/advertisement is mostly managed by userland program
> - * (rtadvd) so here we have no function like nd6_ra_output().
> - *
> - * Based on RFC 2461
> + * Process Source Link-layer Address Options from
> + * Router Solicitation / Advertisement Messages.
> */
> void
> -nd6_rs_input(struct mbuf *m, int off, int icmp6len)
> +nd6_cache_from_rtr_input(struct mbuf *m, int off, int icmp6len, int
> icmp6_type)
> {
> struct ifnet *ifp;
> struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
> struct nd_router_solicit *nd_rs;
> - struct in6_addr saddr6 = ip6->ip6_src;
> -#if 0
> - struct in6_addr daddr6 = ip6->ip6_dst;
> -#endif
> - char *lladdr = NULL;
> - int lladdrlen = 0;
> -#if 0
> - struct sockaddr_dl *sdl = NULL;
> - struct llinfo_nd6 *ln = NULL;
> - struct rtentry *rt = NULL;
> - int is_newentry;
> -#endif
> - union nd_opts ndopts;
> - char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN];
> -
> - /* If I'm not a router, ignore it. XXX - too restrictive? */
> - if (!ip6_forwarding)
> - goto freeit;
> -
> - /* Sanity checks */
> - if (ip6->ip6_hlim != 255) {
> - nd6log((LOG_ERR,
> - "nd6_rs_input: invalid hlim (%d) from %s to %s on %u\n",
> - ip6->ip6_hlim,
> - inet_ntop(AF_INET6, &ip6->ip6_src, src, sizeof(src)),
> - inet_ntop(AF_INET6, &ip6->ip6_dst, dst, sizeof(dst)),
> - m->m_pkthdr.ph_ifidx));
> - goto bad;
> - }
> -
> - /*
> - * Don't update the neighbor cache, if src = ::.
> - * This indicates that the src has no IP address assigned yet.
> - */
> - if (IN6_IS_ADDR_UNSPECIFIED(&saddr6))
> - goto freeit;
> -
> - IP6_EXTHDR_GET(nd_rs, struct nd_router_solicit *, m, off, icmp6len);
> - if (nd_rs == NULL) {
> - icmp6stat_inc(icp6s_tooshort);
> - return;
> - }
> -
> - icmp6len -= sizeof(*nd_rs);
> - nd6_option_init(nd_rs + 1, icmp6len, &ndopts);
> - if (nd6_options(&ndopts) < 0) {
> - nd6log((LOG_INFO,
> - "nd6_rs_input: invalid ND option, ignored\n"));
> - /* nd6_options have incremented stats */
> - goto freeit;
> - }
> -
> - if (ndopts.nd_opts_src_lladdr) {
> - lladdr = (char *)(ndopts.nd_opts_src_lladdr + 1);
> - lladdrlen = ndopts.nd_opts_src_lladdr->nd_opt_len << 3;
> - }
> -
> - ifp = if_get(m->m_pkthdr.ph_ifidx);
> - if (ifp == NULL)
> - goto freeit;
> -
> - if (lladdr && ((ifp->if_addrlen + 2 + 7) & ~7) != lladdrlen) {
> - nd6log((LOG_INFO,
> - "nd6_rs_input: lladdrlen mismatch for %s "
> - "(if %d, RS packet %d)\n",
> - inet_ntop(AF_INET6, &saddr6, src, sizeof(src)),
> - ifp->if_addrlen, lladdrlen - 2));
> - if_put(ifp);
> - goto bad;
> - }
> -
> - nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, ND_ROUTER_SOLICIT, 0);
> - if_put(ifp);
> -
> - freeit:
> - m_freem(m);
> - return;
> -
> - bad:
> - icmp6stat_inc(icp6s_badrs);
> - m_freem(m);
> -}
> -
> -/*
> - * Receive Router Advertisement Message.
> - *
> - * Based on RFC 2461
> - */
> -void
> -nd6_ra_input(struct mbuf *m, int off, int icmp6len)
> -{
> - struct ifnet *ifp;
> - struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
> struct nd_router_advert *nd_ra;
> struct in6_addr saddr6 = ip6->ip6_src;
> char *lladdr = NULL;
> @@ -172,35 +76,62 @@ nd6_ra_input(struct mbuf *m, int off, int icmp6len)
> union nd_opts ndopts;
> char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN];
>
> + KASSERT(icmp6_type == ND_ROUTER_SOLICIT || icmp6_type ==
> + ND_ROUTER_ADVERT);
> +
> /* Sanity checks */
> if (ip6->ip6_hlim != 255) {
> nd6log((LOG_ERR,
> - "nd6_ra_input: invalid hlim (%d) from %s to %s on %u\n",
> - ip6->ip6_hlim,
> + "%s: invalid hlim (%d) from %s to %s on %u\n",
> + __func__, ip6->ip6_hlim,
> inet_ntop(AF_INET6, &ip6->ip6_src, src, sizeof(src)),
> inet_ntop(AF_INET6, &ip6->ip6_dst, dst, sizeof(dst)),
> m->m_pkthdr.ph_ifidx));
> goto bad;
> }
>
> - if (!IN6_IS_ADDR_LINKLOCAL(&saddr6)) {
> - nd6log((LOG_ERR,
> - "nd6_ra_input: src %s is not link-local\n",
> - inet_ntop(AF_INET6, &saddr6, src, sizeof(src))));
> - goto bad;
> - }
> -
> - IP6_EXTHDR_GET(nd_ra, struct nd_router_advert *, m, off, icmp6len);
> - if (nd_ra == NULL) {
> - icmp6stat_inc(icp6s_tooshort);
> - return;
> + switch (icmp6_type) {
> + case ND_ROUTER_SOLICIT:
> + /*
> + * Don't update the neighbor cache, if src = ::.
> + * This indicates that the src has no IP address assigned yet.
> + */
> + if (IN6_IS_ADDR_UNSPECIFIED(&saddr6))
> + goto freeit;
> +
> + IP6_EXTHDR_GET(nd_rs, struct nd_router_solicit *, m, off,
> + icmp6len);
> + if (nd_rs == NULL) {
> + icmp6stat_inc(icp6s_tooshort);
> + return;
> + }
> +
> + icmp6len -= sizeof(*nd_rs);
> + nd6_option_init(nd_rs + 1, icmp6len, &ndopts);
> + break;
> + case ND_ROUTER_ADVERT:
> + if (!IN6_IS_ADDR_LINKLOCAL(&saddr6)) {
> + nd6log((LOG_ERR,
> + "%s: src %s is not link-local\n", __func__,
> + inet_ntop(AF_INET6, &saddr6, src, sizeof(src))));
> + goto bad;
> + }
> +
> + IP6_EXTHDR_GET(nd_ra, struct nd_router_advert *, m, off,
> + icmp6len);
> + if (nd_ra == NULL) {
> + icmp6stat_inc(icp6s_tooshort);
> + return;
> + }
> +
> + icmp6len -= sizeof(*nd_ra);
> + nd6_option_init(nd_ra + 1, icmp6len, &ndopts);
> + break;
> }
>
> - icmp6len -= sizeof(*nd_ra);
> - nd6_option_init(nd_ra + 1, icmp6len, &ndopts);
> if (nd6_options(&ndopts) < 0) {
> nd6log((LOG_INFO,
> - "nd6_ra_input: invalid ND option, ignored\n"));
> + "%s: invalid ND option, ignored\n", __func__));
> /* nd6_options have incremented stats */
> goto freeit;
> }
> @@ -216,15 +147,14 @@ nd6_ra_input(struct mbuf *m, int off, int icmp6len)
>
> if (lladdr && ((ifp->if_addrlen + 2 + 7) & ~7) != lladdrlen) {
> nd6log((LOG_INFO,
> - "nd6_ra_input: lladdrlen mismatch for %s "
> - "(if %d, RA packet %d)\n",
> - inet_ntop(AF_INET6, &saddr6, src, sizeof(src)),
> + "%s: lladdrlen mismatch for %s (if %d, RA/RS packet %d)\n",
> + __func__, inet_ntop(AF_INET6, &saddr6, src, sizeof(src)),
> ifp->if_addrlen, lladdrlen - 2));
> if_put(ifp);
> goto bad;
> }
>
> - nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, ND_ROUTER_ADVERT, 0);
> + nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, icmp6_type, 0);
> if_put(ifp);
>
> freeit:
> @@ -232,7 +162,8 @@ nd6_ra_input(struct mbuf *m, int off, int icmp6len)
> return;
>
> bad:
> - icmp6stat_inc(icp6s_badra);
> + icmp6stat_inc(icmp6_type == ND_ROUTER_SOLICIT ? icp6s_badrs :
> + icp6s_badra);
> m_freem(m);
> }
>
>
> --
> I'm not entirely sure you are real.
>
