Re: merge nd6_rs_input() and nd6_ra_input()

2017-11-03 Thread Florian Obser 
On Fri, Nov 03, 2017 at 01:37:40PM +, Martin Pieuchot wrote:
> 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) (:

unfortunately they are still used for redirects :(

-- 
I'm not entirely sure you are real.

Re: merge nd6_rs_input() and nd6_ra_input()

2017-11-03 Thread Martin Pieuchot 
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_src, src, sizeof(src)),
> -

merge nd6_rs_input() and nd6_ra_input()

2017-11-02 Thread Florian Obser 
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?

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_src, src, sizeof(src)),
-   inet_ntop(AF_INET6, >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())
-   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, );
-   if (nd6_options() < 0) {
-   nd6log(