tweak in6_selectsrc()

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

tweak in6_selectsrc()

Martin Pieuchot
Diff below removes the 'struct route_in6' argument from in6_selectsrc().

It is only used by in6_pcbselsrc() so move the code there.  This reduces
differences with IPv4 and help me to get rid of 'struct route*'.

ok?

Index: net/if_vxlan.c
===================================================================
RCS file: /cvs/src/sys/net/if_vxlan.c,v
retrieving revision 1.52
diff -u -p -r1.52 if_vxlan.c
--- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
+++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
@@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
  ip6->ip6_hlim = ip6_defhlim;
 
  if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
- error = in6_selectsrc(&in6a, satosin6(dst), NULL, NULL,
+ error = in6_selectsrc(&in6a, satosin6(dst), NULL,
     sc->sc_rdomain);
  if (error != 0) {
  m_freem(m);
Index: netinet6/in6_src.c
===================================================================
RCS file: /cvs/src/sys/netinet6/in6_src.c,v
retrieving revision 1.80
diff -u -p -r1.80 in6_src.c
--- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
+++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
@@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
  struct route_in6 *ro = &inp->inp_route6;
  struct in6_addr *laddr = &inp->inp_laddr6;
  u_int rtableid = inp->inp_rtableid;
-
  struct ifnet *ifp = NULL;
  struct in6_addr *dst;
  struct in6_ifaddr *ia6 = NULL;
@@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
  return (0);
  }
 
- return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
+ error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
+ if (error != EADDRNOTAVAIL)
+ return (error);
+
+ /*
+ * If route is known or can be allocated now,
+ * our src addr is taken from the i/f, else punt.
+ */
+ if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
+    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
+ rtfree(ro->ro_rt);
+ ro->ro_rt = NULL;
+ }
+ if (ro->ro_rt == NULL) {
+ struct sockaddr_in6 *sa6;
+
+ /* No route yet, so try to acquire one */
+ bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
+ ro->ro_tableid = rtableid;
+ sa6 = &ro->ro_dst;
+ sa6->sin6_family = AF_INET6;
+ sa6->sin6_len = sizeof(struct sockaddr_in6);
+ sa6->sin6_addr = *dst;
+ sa6->sin6_scope_id = dstsock->sin6_scope_id;
+ ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
+    RT_RESOLVE, ro->ro_tableid);
+ }
+
+ /*
+ * in_pcbconnect() checks out IFF_LOOPBACK to skip using
+ * the address. But we don't know why it does so.
+ * It is necessary to ensure the scope even for lo0
+ * so doesn't check out IFF_LOOPBACK.
+ */
+
+ if (ro->ro_rt) {
+ ifp = if_get(ro->ro_rt->rt_ifidx);
+ if (ifp != NULL) {
+ ia6 = in6_ifawithscope(ifp, dst, rtableid);
+ if_put(ifp);
+ }
+ if (ia6 == NULL) /* xxx scope error ?*/
+ ia6 = ifatoia6(ro->ro_rt->rt_ifa);
+ }
+ if (ia6 == NULL)
+ return (EHOSTUNREACH); /* no route */
+
+ *in6src = &ia6->ia_addr.sin6_addr;
+ return (0);
 }
 
 /*
@@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
  */
 int
 in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
-    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
+    struct ip6_moptions *mopts, unsigned int rtableid)
 {
  struct ifnet *ifp = NULL;
  struct in6_addr *dst;
@@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
  *in6src = &ia6->ia_addr.sin6_addr;
  return (0);
  }
- }
-
- /*
- * If route is known or can be allocated now,
- * our src addr is taken from the i/f, else punt.
- */
- if (ro) {
- if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
-    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
- rtfree(ro->ro_rt);
- ro->ro_rt = NULL;
- }
- if (ro->ro_rt == NULL) {
- struct sockaddr_in6 *sa6;
-
- /* No route yet, so try to acquire one */
- bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
- ro->ro_tableid = rtableid;
- sa6 = &ro->ro_dst;
- sa6->sin6_family = AF_INET6;
- sa6->sin6_len = sizeof(struct sockaddr_in6);
- sa6->sin6_addr = *dst;
- sa6->sin6_scope_id = dstsock->sin6_scope_id;
- ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
-    RT_RESOLVE, ro->ro_tableid);
- }
-
- /*
- * in_pcbconnect() checks out IFF_LOOPBACK to skip using
- * the address. But we don't know why it does so.
- * It is necessary to ensure the scope even for lo0
- * so doesn't check out IFF_LOOPBACK.
- */
-
- if (ro->ro_rt) {
- ifp = if_get(ro->ro_rt->rt_ifidx);
- if (ifp != NULL) {
- ia6 = in6_ifawithscope(ifp, dst, rtableid);
- if_put(ifp);
- }
- if (ia6 == NULL) /* xxx scope error ?*/
- ia6 = ifatoia6(ro->ro_rt->rt_ifa);
- }
- if (ia6 == NULL)
- return (EHOSTUNREACH); /* no route */
-
- *in6src = &ia6->ia_addr.sin6_addr;
- return (0);
  }
 
  return (EADDRNOTAVAIL);
Index: netinet6/ip6_var.h
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
retrieving revision 1.64
diff -u -p -r1.64 ip6_var.h
--- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
+++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
@@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
 int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
     struct inpcb *, struct ip6_pktopts *);
 int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
-    struct ip6_moptions *, struct route_in6 *, u_int);
+    struct ip6_moptions *, unsigned int);
 struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct ip6_pktopts *,
     struct route_in6 *, unsigned int rtableid);
 

Reply | Threaded
Open this post in threaded view
|

vxlan bug wrt IN6_ANY as source Was: Re: tweak in6_selectsrc()

Vincent Gross-5
On Tue, 29 Nov 2016 17:03:44 +0100
Martin Pieuchot <[hidden email]> wrote:

> Diff below removes the 'struct route_in6' argument from
> in6_selectsrc().
>
> It is only used by in6_pcbselsrc() so move the code there.  This
> reduces differences with IPv4 and help me to get rid of 'struct
> route*'.
>
> ok?

Reads ok, not tested yet.

Your diff is interesting in that is helped me to find a bug
in vxlan(4).

Build a tunnel like this:
$ doas ifconfig pair11 rdomain 11
$ doas ifconfig pair12 rdomain 12 patch pair11
$ doas ifconfig pair11 inet6 fd03::11/64 up
$ doas ifconfig pair12 inet6 fd03::12/64 up
$ doas ifconfig vxlan11 rdomain 11 tunneldomain 11 vnetid 10
$ doas ifconfig vxlan12 rdomain 12 tunneldomain 12 vnetid 10
$ doas ifconfig vxlan11 inet6 fd06::11/64 tunnel :: fd03::12 up
$ doas ifconfig vxlan12 inet6 fd06::12/64 tunnel :: fd03::11 up

Watch ping6 fail:
$ ping6 -V 11 fd06::12

Tweak the vxlans and see pings flow
$ doas ifconfig vxlan11 tunnel fd03::11 fd03::12
$ doas ifconfig vxlan12 tunnel fd03::12 fd03::11
$ ping6 -V 11 fd06::11


I think we should not allow at all empty source addresses, as it can
make things confusing when troubleshooting. goda@ yasuoka@ reyk@ :
what is your take on this ?


>
> Index: net/if_vxlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 if_vxlan.c
> --- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
> +++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
> @@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
>   ip6->ip6_hlim = ip6_defhlim;
>  
>   if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
> - error = in6_selectsrc(&in6a, satosin6(dst), NULL,
> NULL,
> + error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>      sc->sc_rdomain);
>   if (error != 0) {
>   m_freem(m);
> Index: netinet6/in6_src.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 in6_src.c
> --- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
> +++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
> @@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   struct route_in6 *ro = &inp->inp_route6;
>   struct in6_addr *laddr = &inp->inp_laddr6;
>   u_int rtableid = inp->inp_rtableid;
> -
>   struct ifnet *ifp = NULL;
>   struct in6_addr *dst;
>   struct in6_ifaddr *ia6 = NULL;
> @@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   return (0);
>   }
>  
> - return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
> + error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
> + if (error != EADDRNOTAVAIL)
> + return (error);
> +
> + /*
> + * If route is known or can be allocated now,
> + * our src addr is taken from the i/f, else punt.
> + */
> + if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
> +    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
> + rtfree(ro->ro_rt);
> + ro->ro_rt = NULL;
> + }
> + if (ro->ro_rt == NULL) {
> + struct sockaddr_in6 *sa6;
> +
> + /* No route yet, so try to acquire one */
> + bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
> + ro->ro_tableid = rtableid;
> + sa6 = &ro->ro_dst;
> + sa6->sin6_family = AF_INET6;
> + sa6->sin6_len = sizeof(struct sockaddr_in6);
> + sa6->sin6_addr = *dst;
> + sa6->sin6_scope_id = dstsock->sin6_scope_id;
> + ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
> +    RT_RESOLVE, ro->ro_tableid);
> + }
> +
> + /*
> + * in_pcbconnect() checks out IFF_LOOPBACK to skip using
> + * the address. But we don't know why it does so.
> + * It is necessary to ensure the scope even for lo0
> + * so doesn't check out IFF_LOOPBACK.
> + */
> +
> + if (ro->ro_rt) {
> + ifp = if_get(ro->ro_rt->rt_ifidx);
> + if (ifp != NULL) {
> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
> + if_put(ifp);
> + }
> + if (ia6 == NULL) /* xxx scope error ?*/
> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> + }
> + if (ia6 == NULL)
> + return (EHOSTUNREACH); /* no route */
> +
> + *in6src = &ia6->ia_addr.sin6_addr;
> + return (0);
>  }
>  
>  /*
> @@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   */
>  int
>  in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
> -    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
> +    struct ip6_moptions *mopts, unsigned int rtableid)
>  {
>   struct ifnet *ifp = NULL;
>   struct in6_addr *dst;
> @@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
>   *in6src = &ia6->ia_addr.sin6_addr;
>   return (0);
>   }
> - }
> -
> - /*
> - * If route is known or can be allocated now,
> - * our src addr is taken from the i/f, else punt.
> - */
> - if (ro) {
> - if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid !=
> rtableid) ||
> -    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))
> {
> - rtfree(ro->ro_rt);
> - ro->ro_rt = NULL;
> - }
> - if (ro->ro_rt == NULL) {
> - struct sockaddr_in6 *sa6;
> -
> - /* No route yet, so try to acquire one */
> - bzero(&ro->ro_dst, sizeof(struct
> sockaddr_in6));
> - ro->ro_tableid = rtableid;
> - sa6 = &ro->ro_dst;
> - sa6->sin6_family = AF_INET6;
> - sa6->sin6_len = sizeof(struct sockaddr_in6);
> - sa6->sin6_addr = *dst;
> - sa6->sin6_scope_id = dstsock->sin6_scope_id;
> - ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
> -    RT_RESOLVE, ro->ro_tableid);
> - }
> -
> - /*
> - * in_pcbconnect() checks out IFF_LOOPBACK to skip
> using
> - * the address. But we don't know why it does so.
> - * It is necessary to ensure the scope even for lo0
> - * so doesn't check out IFF_LOOPBACK.
> - */
> -
> - if (ro->ro_rt) {
> - ifp = if_get(ro->ro_rt->rt_ifidx);
> - if (ifp != NULL) {
> - ia6 = in6_ifawithscope(ifp, dst,
> rtableid);
> - if_put(ifp);
> - }
> - if (ia6 == NULL) /* xxx scope error ?*/
> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> - }
> - if (ia6 == NULL)
> - return (EHOSTUNREACH); /* no route */
> -
> - *in6src = &ia6->ia_addr.sin6_addr;
> - return (0);
>   }
>  
>   return (EADDRNOTAVAIL);
> Index: netinet6/ip6_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
> retrieving revision 1.64
> diff -u -p -r1.64 ip6_var.h
> --- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
> +++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
> @@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
>  int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
>      struct inpcb *, struct ip6_pktopts *);
>  int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
> -    struct ip6_moptions *, struct route_in6 *, u_int);
> +    struct ip6_moptions *, unsigned int);
>  struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct
> ip6_pktopts *, struct route_in6 *, unsigned int rtableid);
>  

Reply | Threaded
Open this post in threaded view
|

Re: vxlan bug wrt IN6_ANY as source Was: Re: tweak in6_selectsrc()

YASUOKA Masahiko-4
Hi,

> I think we should not allow at all empty source addresses, as it can
> make things confusing when troubleshooting. goda@ yasuoka@ reyk@ :
> what is your take on this ?

I have the same opinion of you.  Since tunneling protocol like vxlan
(or etherip) is used with statically assigned IP address I don't see
any reason to omit specifying the source address.

--yasuoka

On Thu, 1 Dec 2016 08:35:34 +0100
Vincent Gross <[hidden email]> wrote:

> On Tue, 29 Nov 2016 17:03:44 +0100
> Martin Pieuchot <[hidden email]> wrote:
>
>> Diff below removes the 'struct route_in6' argument from
>> in6_selectsrc().
>>
>> It is only used by in6_pcbselsrc() so move the code there.  This
>> reduces differences with IPv4 and help me to get rid of 'struct
>> route*'.
>>
>> ok?
>
> Reads ok, not tested yet.
>
> Your diff is interesting in that is helped me to find a bug
> in vxlan(4).
>
> Build a tunnel like this:
> $ doas ifconfig pair11 rdomain 11
> $ doas ifconfig pair12 rdomain 12 patch pair11
> $ doas ifconfig pair11 inet6 fd03::11/64 up
> $ doas ifconfig pair12 inet6 fd03::12/64 up
> $ doas ifconfig vxlan11 rdomain 11 tunneldomain 11 vnetid 10
> $ doas ifconfig vxlan12 rdomain 12 tunneldomain 12 vnetid 10
> $ doas ifconfig vxlan11 inet6 fd06::11/64 tunnel :: fd03::12 up
> $ doas ifconfig vxlan12 inet6 fd06::12/64 tunnel :: fd03::11 up
>
> Watch ping6 fail:
> $ ping6 -V 11 fd06::12
>
> Tweak the vxlans and see pings flow
> $ doas ifconfig vxlan11 tunnel fd03::11 fd03::12
> $ doas ifconfig vxlan12 tunnel fd03::12 fd03::11
> $ ping6 -V 11 fd06::11
>
>
> I think we should not allow at all empty source addresses, as it can
> make things confusing when troubleshooting. goda@ yasuoka@ reyk@ :
> what is your take on this ?
>
>
>>
>> Index: net/if_vxlan.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_vxlan.c,v
>> retrieving revision 1.52
>> diff -u -p -r1.52 if_vxlan.c
>> --- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
>> +++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
>> @@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
>>   ip6->ip6_hlim = ip6_defhlim;
>>  
>>   if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
>> - error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>> NULL,
>> + error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>>      sc->sc_rdomain);
>>   if (error != 0) {
>>   m_freem(m);
>> Index: netinet6/in6_src.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
>> retrieving revision 1.80
>> diff -u -p -r1.80 in6_src.c
>> --- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
>> +++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
>> @@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>   struct route_in6 *ro = &inp->inp_route6;
>>   struct in6_addr *laddr = &inp->inp_laddr6;
>>   u_int rtableid = inp->inp_rtableid;
>> -
>>   struct ifnet *ifp = NULL;
>>   struct in6_addr *dst;
>>   struct in6_ifaddr *ia6 = NULL;
>> @@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>   return (0);
>>   }
>>  
>> - return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
>> + error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
>> + if (error != EADDRNOTAVAIL)
>> + return (error);
>> +
>> + /*
>> + * If route is known or can be allocated now,
>> + * our src addr is taken from the i/f, else punt.
>> + */
>> + if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
>> +    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
>> + rtfree(ro->ro_rt);
>> + ro->ro_rt = NULL;
>> + }
>> + if (ro->ro_rt == NULL) {
>> + struct sockaddr_in6 *sa6;
>> +
>> + /* No route yet, so try to acquire one */
>> + bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
>> + ro->ro_tableid = rtableid;
>> + sa6 = &ro->ro_dst;
>> + sa6->sin6_family = AF_INET6;
>> + sa6->sin6_len = sizeof(struct sockaddr_in6);
>> + sa6->sin6_addr = *dst;
>> + sa6->sin6_scope_id = dstsock->sin6_scope_id;
>> + ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>> +    RT_RESOLVE, ro->ro_tableid);
>> + }
>> +
>> + /*
>> + * in_pcbconnect() checks out IFF_LOOPBACK to skip using
>> + * the address. But we don't know why it does so.
>> + * It is necessary to ensure the scope even for lo0
>> + * so doesn't check out IFF_LOOPBACK.
>> + */
>> +
>> + if (ro->ro_rt) {
>> + ifp = if_get(ro->ro_rt->rt_ifidx);
>> + if (ifp != NULL) {
>> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
>> + if_put(ifp);
>> + }
>> + if (ia6 == NULL) /* xxx scope error ?*/
>> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>> + }
>> + if (ia6 == NULL)
>> + return (EHOSTUNREACH); /* no route */
>> +
>> + *in6src = &ia6->ia_addr.sin6_addr;
>> + return (0);
>>  }
>>  
>>  /*
>> @@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>   */
>>  int
>>  in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>> -    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
>> +    struct ip6_moptions *mopts, unsigned int rtableid)
>>  {
>>   struct ifnet *ifp = NULL;
>>   struct in6_addr *dst;
>> @@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
>>   *in6src = &ia6->ia_addr.sin6_addr;
>>   return (0);
>>   }
>> - }
>> -
>> - /*
>> - * If route is known or can be allocated now,
>> - * our src addr is taken from the i/f, else punt.
>> - */
>> - if (ro) {
>> - if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid !=
>> rtableid) ||
>> -    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))
>> {
>> - rtfree(ro->ro_rt);
>> - ro->ro_rt = NULL;
>> - }
>> - if (ro->ro_rt == NULL) {
>> - struct sockaddr_in6 *sa6;
>> -
>> - /* No route yet, so try to acquire one */
>> - bzero(&ro->ro_dst, sizeof(struct
>> sockaddr_in6));
>> - ro->ro_tableid = rtableid;
>> - sa6 = &ro->ro_dst;
>> - sa6->sin6_family = AF_INET6;
>> - sa6->sin6_len = sizeof(struct sockaddr_in6);
>> - sa6->sin6_addr = *dst;
>> - sa6->sin6_scope_id = dstsock->sin6_scope_id;
>> - ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>> -    RT_RESOLVE, ro->ro_tableid);
>> - }
>> -
>> - /*
>> - * in_pcbconnect() checks out IFF_LOOPBACK to skip
>> using
>> - * the address. But we don't know why it does so.
>> - * It is necessary to ensure the scope even for lo0
>> - * so doesn't check out IFF_LOOPBACK.
>> - */
>> -
>> - if (ro->ro_rt) {
>> - ifp = if_get(ro->ro_rt->rt_ifidx);
>> - if (ifp != NULL) {
>> - ia6 = in6_ifawithscope(ifp, dst,
>> rtableid);
>> - if_put(ifp);
>> - }
>> - if (ia6 == NULL) /* xxx scope error ?*/
>> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>> - }
>> - if (ia6 == NULL)
>> - return (EHOSTUNREACH); /* no route */
>> -
>> - *in6src = &ia6->ia_addr.sin6_addr;
>> - return (0);
>>   }
>>  
>>   return (EADDRNOTAVAIL);
>> Index: netinet6/ip6_var.h
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
>> retrieving revision 1.64
>> diff -u -p -r1.64 ip6_var.h
>> --- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
>> +++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
>> @@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
>>  int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
>>      struct inpcb *, struct ip6_pktopts *);
>>  int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
>> -    struct ip6_moptions *, struct route_in6 *, u_int);
>> +    struct ip6_moptions *, unsigned int);
>>  struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct
>> ip6_pktopts *, struct route_in6 *, unsigned int rtableid);
>>  
>

Reply | Threaded
Open this post in threaded view
|

Re: vxlan bug wrt IN6_ANY as source Was: Re: tweak in6_selectsrc()

Reyk Floeter-2
In reply to this post by Vincent Gross-5

> On 01.12.2016, at 08:35, Vincent Gross <[hidden email]> wrote:
>
> On Tue, 29 Nov 2016 17:03:44 +0100
> Martin Pieuchot <[hidden email]> wrote:
>
>> Diff below removes the 'struct route_in6' argument from
>> in6_selectsrc().
>>
>> It is only used by in6_pcbselsrc() so move the code there.  This
>> reduces differences with IPv4 and help me to get rid of 'struct
>> route*'.
>>
>> ok?
>
> Reads ok, not tested yet.
>
> Your diff is interesting in that is helped me to find a bug
> in vxlan(4).
>
> Build a tunnel like this:
> $ doas ifconfig pair11 rdomain 11
> $ doas ifconfig pair12 rdomain 12 patch pair11
> $ doas ifconfig pair11 inet6 fd03::11/64 up
> $ doas ifconfig pair12 inet6 fd03::12/64 up
> $ doas ifconfig vxlan11 rdomain 11 tunneldomain 11 vnetid 10
> $ doas ifconfig vxlan12 rdomain 12 tunneldomain 12 vnetid 10
> $ doas ifconfig vxlan11 inet6 fd06::11/64 tunnel :: fd03::12 up
> $ doas ifconfig vxlan12 inet6 fd06::12/64 tunnel :: fd03::11 up
>
> Watch ping6 fail:
> $ ping6 -V 11 fd06::12
>
> Tweak the vxlans and see pings flow
> $ doas ifconfig vxlan11 tunnel fd03::11 fd03::12
> $ doas ifconfig vxlan12 tunnel fd03::12 fd03::11
> $ ping6 -V 11 fd06::11
>
>
> I think we should not allow at all empty source addresses, as it can
> make things confusing when troubleshooting. goda@ yasuoka@ reyk@ :
> what is your take on this ?
>

What do you mean with "confusing when troubleshooting"? Why is it confusing? VXLAN is an protocol over UDP/IP and like any other connection in the system it does not require you to select the source address. It just happens to run in the kernel. Do you always run "ssh -b 10.1.1.2"?

If it really helps to make the code or the network stack easier, I would be fine with the change/limitation, but I do not agree with the argument.

Reyk

>
>>
>> Index: net/if_vxlan.c
>> ===================================================================
>> RCS file: /cvs/src/sys/net/if_vxlan.c,v
>> retrieving revision 1.52
>> diff -u -p -r1.52 if_vxlan.c
>> --- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
>> +++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
>> @@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
>> ip6->ip6_hlim = ip6_defhlim;
>>
>> if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
>> - error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>> NULL,
>> + error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>>    sc->sc_rdomain);
>> if (error != 0) {
>> m_freem(m);
>> Index: netinet6/in6_src.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
>> retrieving revision 1.80
>> diff -u -p -r1.80 in6_src.c
>> --- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
>> +++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
>> @@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
>> struct route_in6 *ro = &inp->inp_route6;
>> struct in6_addr *laddr = &inp->inp_laddr6;
>> u_int rtableid = inp->inp_rtableid;
>> -
>> struct ifnet *ifp = NULL;
>> struct in6_addr *dst;
>> struct in6_ifaddr *ia6 = NULL;
>> @@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
>> return (0);
>> }
>>
>> - return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
>> + error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
>> + if (error != EADDRNOTAVAIL)
>> + return (error);
>> +
>> + /*
>> + * If route is known or can be allocated now,
>> + * our src addr is taken from the i/f, else punt.
>> + */
>> + if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
>> +    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
>> + rtfree(ro->ro_rt);
>> + ro->ro_rt = NULL;
>> + }
>> + if (ro->ro_rt == NULL) {
>> + struct sockaddr_in6 *sa6;
>> +
>> + /* No route yet, so try to acquire one */
>> + bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
>> + ro->ro_tableid = rtableid;
>> + sa6 = &ro->ro_dst;
>> + sa6->sin6_family = AF_INET6;
>> + sa6->sin6_len = sizeof(struct sockaddr_in6);
>> + sa6->sin6_addr = *dst;
>> + sa6->sin6_scope_id = dstsock->sin6_scope_id;
>> + ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>> +    RT_RESOLVE, ro->ro_tableid);
>> + }
>> +
>> + /*
>> + * in_pcbconnect() checks out IFF_LOOPBACK to skip using
>> + * the address. But we don't know why it does so.
>> + * It is necessary to ensure the scope even for lo0
>> + * so doesn't check out IFF_LOOPBACK.
>> + */
>> +
>> + if (ro->ro_rt) {
>> + ifp = if_get(ro->ro_rt->rt_ifidx);
>> + if (ifp != NULL) {
>> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
>> + if_put(ifp);
>> + }
>> + if (ia6 == NULL) /* xxx scope error ?*/
>> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>> + }
>> + if (ia6 == NULL)
>> + return (EHOSTUNREACH); /* no route */
>> +
>> + *in6src = &ia6->ia_addr.sin6_addr;
>> + return (0);
>> }
>>
>> /*
>> @@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>  */
>> int
>> in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>> -    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
>> +    struct ip6_moptions *mopts, unsigned int rtableid)
>> {
>> struct ifnet *ifp = NULL;
>> struct in6_addr *dst;
>> @@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
>> *in6src = &ia6->ia_addr.sin6_addr;
>> return (0);
>> }
>> - }
>> -
>> - /*
>> - * If route is known or can be allocated now,
>> - * our src addr is taken from the i/f, else punt.
>> - */
>> - if (ro) {
>> - if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid !=
>> rtableid) ||
>> -    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))
>> {
>> - rtfree(ro->ro_rt);
>> - ro->ro_rt = NULL;
>> - }
>> - if (ro->ro_rt == NULL) {
>> - struct sockaddr_in6 *sa6;
>> -
>> - /* No route yet, so try to acquire one */
>> - bzero(&ro->ro_dst, sizeof(struct
>> sockaddr_in6));
>> - ro->ro_tableid = rtableid;
>> - sa6 = &ro->ro_dst;
>> - sa6->sin6_family = AF_INET6;
>> - sa6->sin6_len = sizeof(struct sockaddr_in6);
>> - sa6->sin6_addr = *dst;
>> - sa6->sin6_scope_id = dstsock->sin6_scope_id;
>> - ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>> -    RT_RESOLVE, ro->ro_tableid);
>> - }
>> -
>> - /*
>> - * in_pcbconnect() checks out IFF_LOOPBACK to skip
>> using
>> - * the address. But we don't know why it does so.
>> - * It is necessary to ensure the scope even for lo0
>> - * so doesn't check out IFF_LOOPBACK.
>> - */
>> -
>> - if (ro->ro_rt) {
>> - ifp = if_get(ro->ro_rt->rt_ifidx);
>> - if (ifp != NULL) {
>> - ia6 = in6_ifawithscope(ifp, dst,
>> rtableid);
>> - if_put(ifp);
>> - }
>> - if (ia6 == NULL) /* xxx scope error ?*/
>> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>> - }
>> - if (ia6 == NULL)
>> - return (EHOSTUNREACH); /* no route */
>> -
>> - *in6src = &ia6->ia_addr.sin6_addr;
>> - return (0);
>> }
>>
>> return (EADDRNOTAVAIL);
>> Index: netinet6/ip6_var.h
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
>> retrieving revision 1.64
>> diff -u -p -r1.64 ip6_var.h
>> --- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
>> +++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
>> @@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
>> int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
>>    struct inpcb *, struct ip6_pktopts *);
>> int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
>> -    struct ip6_moptions *, struct route_in6 *, u_int);
>> +    struct ip6_moptions *, unsigned int);
>> struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct
>> ip6_pktopts *, struct route_in6 *, unsigned int rtableid);
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: vxlan bug wrt IN6_ANY as source Was: Re: tweak in6_selectsrc()

Reyk Floeter-2

> On 02.12.2016, at 11:31, Reyk Floeter <[hidden email]> wrote:
>
>>
>> On 01.12.2016, at 08:35, Vincent Gross <[hidden email]> wrote:
>>
>> On Tue, 29 Nov 2016 17:03:44 +0100
>> Martin Pieuchot <[hidden email]> wrote:
>>
>>> Diff below removes the 'struct route_in6' argument from
>>> in6_selectsrc().
>>>
>>> It is only used by in6_pcbselsrc() so move the code there.  This
>>> reduces differences with IPv4 and help me to get rid of 'struct
>>> route*'.
>>>
>>> ok?
>>
>> Reads ok, not tested yet.
>>
>> Your diff is interesting in that is helped me to find a bug
>> in vxlan(4).
>>
>> Build a tunnel like this:
>> $ doas ifconfig pair11 rdomain 11
>> $ doas ifconfig pair12 rdomain 12 patch pair11
>> $ doas ifconfig pair11 inet6 fd03::11/64 up
>> $ doas ifconfig pair12 inet6 fd03::12/64 up
>> $ doas ifconfig vxlan11 rdomain 11 tunneldomain 11 vnetid 10
>> $ doas ifconfig vxlan12 rdomain 12 tunneldomain 12 vnetid 10
>> $ doas ifconfig vxlan11 inet6 fd06::11/64 tunnel :: fd03::12 up
>> $ doas ifconfig vxlan12 inet6 fd06::12/64 tunnel :: fd03::11 up
>>
>> Watch ping6 fail:
>> $ ping6 -V 11 fd06::12
>>
>> Tweak the vxlans and see pings flow
>> $ doas ifconfig vxlan11 tunnel fd03::11 fd03::12
>> $ doas ifconfig vxlan12 tunnel fd03::12 fd03::11
>> $ ping6 -V 11 fd06::11
>>
>>
>> I think we should not allow at all empty source addresses, as it can
>> make things confusing when troubleshooting. goda@ yasuoka@ reyk@ :
>> what is your take on this ?
>>
>
> What do you mean with "confusing when troubleshooting"? Why is it confusing? VXLAN is an protocol over UDP/IP and like any other connection in the system it does not require you to select the source address. It just happens to run in the kernel. Do you always run "ssh -b 10.1.1.2"?
>
> If it really helps to make the code or the network stack easier, I would be fine with the change/limitation, but I do not agree with the argument.
>

And I have to note that VXLAN is not like etherip because it runs over UDP with proper ports etc.

While most configurations would use a static source address indeed, I don't see the "special need" to enforce it.

Reyk

>
>>
>>>
>>> Index: net/if_vxlan.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/net/if_vxlan.c,v
>>> retrieving revision 1.52
>>> diff -u -p -r1.52 if_vxlan.c
>>> --- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
>>> +++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
>>> @@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
>>> ip6->ip6_hlim = ip6_defhlim;
>>>
>>> if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
>>> - error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>>> NULL,
>>> + error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>>>    sc->sc_rdomain);
>>> if (error != 0) {
>>> m_freem(m);
>>> Index: netinet6/in6_src.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
>>> retrieving revision 1.80
>>> diff -u -p -r1.80 in6_src.c
>>> --- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
>>> +++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
>>> @@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>> struct route_in6 *ro = &inp->inp_route6;
>>> struct in6_addr *laddr = &inp->inp_laddr6;
>>> u_int rtableid = inp->inp_rtableid;
>>> -
>>> struct ifnet *ifp = NULL;
>>> struct in6_addr *dst;
>>> struct in6_ifaddr *ia6 = NULL;
>>> @@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>> return (0);
>>> }
>>>
>>> - return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
>>> + error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
>>> + if (error != EADDRNOTAVAIL)
>>> + return (error);
>>> +
>>> + /*
>>> + * If route is known or can be allocated now,
>>> + * our src addr is taken from the i/f, else punt.
>>> + */
>>> + if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
>>> +    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
>>> + rtfree(ro->ro_rt);
>>> + ro->ro_rt = NULL;
>>> + }
>>> + if (ro->ro_rt == NULL) {
>>> + struct sockaddr_in6 *sa6;
>>> +
>>> + /* No route yet, so try to acquire one */
>>> + bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
>>> + ro->ro_tableid = rtableid;
>>> + sa6 = &ro->ro_dst;
>>> + sa6->sin6_family = AF_INET6;
>>> + sa6->sin6_len = sizeof(struct sockaddr_in6);
>>> + sa6->sin6_addr = *dst;
>>> + sa6->sin6_scope_id = dstsock->sin6_scope_id;
>>> + ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>>> +    RT_RESOLVE, ro->ro_tableid);
>>> + }
>>> +
>>> + /*
>>> + * in_pcbconnect() checks out IFF_LOOPBACK to skip using
>>> + * the address. But we don't know why it does so.
>>> + * It is necessary to ensure the scope even for lo0
>>> + * so doesn't check out IFF_LOOPBACK.
>>> + */
>>> +
>>> + if (ro->ro_rt) {
>>> + ifp = if_get(ro->ro_rt->rt_ifidx);
>>> + if (ifp != NULL) {
>>> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
>>> + if_put(ifp);
>>> + }
>>> + if (ia6 == NULL) /* xxx scope error ?*/
>>> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>>> + }
>>> + if (ia6 == NULL)
>>> + return (EHOSTUNREACH); /* no route */
>>> +
>>> + *in6src = &ia6->ia_addr.sin6_addr;
>>> + return (0);
>>> }
>>>
>>> /*
>>> @@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
>>> */
>>> int
>>> in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>>> -    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
>>> +    struct ip6_moptions *mopts, unsigned int rtableid)
>>> {
>>> struct ifnet *ifp = NULL;
>>> struct in6_addr *dst;
>>> @@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
>>> *in6src = &ia6->ia_addr.sin6_addr;
>>> return (0);
>>> }
>>> - }
>>> -
>>> - /*
>>> - * If route is known or can be allocated now,
>>> - * our src addr is taken from the i/f, else punt.
>>> - */
>>> - if (ro) {
>>> - if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid !=
>>> rtableid) ||
>>> -    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst))
>>> {
>>> - rtfree(ro->ro_rt);
>>> - ro->ro_rt = NULL;
>>> - }
>>> - if (ro->ro_rt == NULL) {
>>> - struct sockaddr_in6 *sa6;
>>> -
>>> - /* No route yet, so try to acquire one */
>>> - bzero(&ro->ro_dst, sizeof(struct
>>> sockaddr_in6));
>>> - ro->ro_tableid = rtableid;
>>> - sa6 = &ro->ro_dst;
>>> - sa6->sin6_family = AF_INET6;
>>> - sa6->sin6_len = sizeof(struct sockaddr_in6);
>>> - sa6->sin6_addr = *dst;
>>> - sa6->sin6_scope_id = dstsock->sin6_scope_id;
>>> - ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>>> -    RT_RESOLVE, ro->ro_tableid);
>>> - }
>>> -
>>> - /*
>>> - * in_pcbconnect() checks out IFF_LOOPBACK to skip
>>> using
>>> - * the address. But we don't know why it does so.
>>> - * It is necessary to ensure the scope even for lo0
>>> - * so doesn't check out IFF_LOOPBACK.
>>> - */
>>> -
>>> - if (ro->ro_rt) {
>>> - ifp = if_get(ro->ro_rt->rt_ifidx);
>>> - if (ifp != NULL) {
>>> - ia6 = in6_ifawithscope(ifp, dst,
>>> rtableid);
>>> - if_put(ifp);
>>> - }
>>> - if (ia6 == NULL) /* xxx scope error ?*/
>>> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>>> - }
>>> - if (ia6 == NULL)
>>> - return (EHOSTUNREACH); /* no route */
>>> -
>>> - *in6src = &ia6->ia_addr.sin6_addr;
>>> - return (0);
>>> }
>>>
>>> return (EADDRNOTAVAIL);
>>> Index: netinet6/ip6_var.h
>>> ===================================================================
>>> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
>>> retrieving revision 1.64
>>> diff -u -p -r1.64 ip6_var.h
>>> --- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
>>> +++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
>>> @@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
>>> int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
>>>    struct inpcb *, struct ip6_pktopts *);
>>> int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
>>> -    struct ip6_moptions *, struct route_in6 *, u_int);
>>> +    struct ip6_moptions *, unsigned int);
>>> struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct
>>> ip6_pktopts *, struct route_in6 *, unsigned int rtableid);

Reply | Threaded
Open this post in threaded view
|

Re: tweak in6_selectsrc()

Alexander Bluhm
In reply to this post by Martin Pieuchot
On Tue, Nov 29, 2016 at 05:03:44PM +0100, Martin Pieuchot wrote:
> Diff below removes the 'struct route_in6' argument from in6_selectsrc().
>
> It is only used by in6_pcbselsrc() so move the code there.  This reduces
> differences with IPv4 and help me to get rid of 'struct route*'.

This changes behavior.  Before there was no route lookup for the
cases IN6_IS_ADDR_LINKLOCAL, IN6_IS_ADDR_MC_LINKLOCAL,
IN6_IS_ADDR_MC_INTFACELOCAL, IN6_IS_ADDR_MULTICAST.  They can also
return EADDRNOTAVAIL, now we use the route as fallback.  This is
also different for IPv4, there the route lookup is not done in the
IN_MULTICAST case.

I don't know wether that is better or worse.

bluhm

>
> ok?
>
> Index: net/if_vxlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vxlan.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 if_vxlan.c
> --- net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
> +++ net/if_vxlan.c 29 Nov 2016 15:52:41 -0000
> @@ -768,7 +768,7 @@ vxlan_encap6(struct ifnet *ifp, struct m
>   ip6->ip6_hlim = ip6_defhlim;
>  
>   if (IN6_IS_ADDR_UNSPECIFIED(&satosin6(src)->sin6_addr)) {
> - error = in6_selectsrc(&in6a, satosin6(dst), NULL, NULL,
> + error = in6_selectsrc(&in6a, satosin6(dst), NULL,
>      sc->sc_rdomain);
>   if (error != 0) {
>   m_freem(m);
> Index: netinet6/in6_src.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 in6_src.c
> --- netinet6/in6_src.c 2 Sep 2016 13:53:44 -0000 1.80
> +++ netinet6/in6_src.c 29 Nov 2016 15:56:56 -0000
> @@ -99,7 +99,6 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   struct route_in6 *ro = &inp->inp_route6;
>   struct in6_addr *laddr = &inp->inp_laddr6;
>   u_int rtableid = inp->inp_rtableid;
> -
>   struct ifnet *ifp = NULL;
>   struct in6_addr *dst;
>   struct in6_ifaddr *ia6 = NULL;
> @@ -172,7 +171,55 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   return (0);
>   }
>  
> - return in6_selectsrc(in6src, dstsock, mopts, ro, rtableid);
> + error = in6_selectsrc(in6src, dstsock, mopts, rtableid);
> + if (error != EADDRNOTAVAIL)
> + return (error);
> +
> + /*
> + * If route is known or can be allocated now,
> + * our src addr is taken from the i/f, else punt.
> + */
> + if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
> +    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
> + rtfree(ro->ro_rt);
> + ro->ro_rt = NULL;
> + }
> + if (ro->ro_rt == NULL) {
> + struct sockaddr_in6 *sa6;
> +
> + /* No route yet, so try to acquire one */
> + bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
> + ro->ro_tableid = rtableid;
> + sa6 = &ro->ro_dst;
> + sa6->sin6_family = AF_INET6;
> + sa6->sin6_len = sizeof(struct sockaddr_in6);
> + sa6->sin6_addr = *dst;
> + sa6->sin6_scope_id = dstsock->sin6_scope_id;
> + ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
> +    RT_RESOLVE, ro->ro_tableid);
> + }
> +
> + /*
> + * in_pcbconnect() checks out IFF_LOOPBACK to skip using
> + * the address. But we don't know why it does so.
> + * It is necessary to ensure the scope even for lo0
> + * so doesn't check out IFF_LOOPBACK.
> + */
> +
> + if (ro->ro_rt) {
> + ifp = if_get(ro->ro_rt->rt_ifidx);
> + if (ifp != NULL) {
> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
> + if_put(ifp);
> + }
> + if (ia6 == NULL) /* xxx scope error ?*/
> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> + }
> + if (ia6 == NULL)
> + return (EHOSTUNREACH); /* no route */
> +
> + *in6src = &ia6->ia_addr.sin6_addr;
> + return (0);
>  }
>  
>  /*
> @@ -183,7 +230,7 @@ in6_pcbselsrc(struct in6_addr **in6src,
>   */
>  int
>  in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
> -    struct ip6_moptions *mopts, struct route_in6 *ro, u_int rtableid)
> +    struct ip6_moptions *mopts, unsigned int rtableid)
>  {
>   struct ifnet *ifp = NULL;
>   struct in6_addr *dst;
> @@ -239,54 +286,6 @@ in6_selectsrc(struct in6_addr **in6src,
>   *in6src = &ia6->ia_addr.sin6_addr;
>   return (0);
>   }
> - }
> -
> - /*
> - * If route is known or can be allocated now,
> - * our src addr is taken from the i/f, else punt.
> - */
> - if (ro) {
> - if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
> -    !IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, dst)) {
> - rtfree(ro->ro_rt);
> - ro->ro_rt = NULL;
> - }
> - if (ro->ro_rt == NULL) {
> - struct sockaddr_in6 *sa6;
> -
> - /* No route yet, so try to acquire one */
> - bzero(&ro->ro_dst, sizeof(struct sockaddr_in6));
> - ro->ro_tableid = rtableid;
> - sa6 = &ro->ro_dst;
> - sa6->sin6_family = AF_INET6;
> - sa6->sin6_len = sizeof(struct sockaddr_in6);
> - sa6->sin6_addr = *dst;
> - sa6->sin6_scope_id = dstsock->sin6_scope_id;
> - ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
> -    RT_RESOLVE, ro->ro_tableid);
> - }
> -
> - /*
> - * in_pcbconnect() checks out IFF_LOOPBACK to skip using
> - * the address. But we don't know why it does so.
> - * It is necessary to ensure the scope even for lo0
> - * so doesn't check out IFF_LOOPBACK.
> - */
> -
> - if (ro->ro_rt) {
> - ifp = if_get(ro->ro_rt->rt_ifidx);
> - if (ifp != NULL) {
> - ia6 = in6_ifawithscope(ifp, dst, rtableid);
> - if_put(ifp);
> - }
> - if (ia6 == NULL) /* xxx scope error ?*/
> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> - }
> - if (ia6 == NULL)
> - return (EHOSTUNREACH); /* no route */
> -
> - *in6src = &ia6->ia_addr.sin6_addr;
> - return (0);
>   }
>  
>   return (EADDRNOTAVAIL);
> Index: netinet6/ip6_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_var.h,v
> retrieving revision 1.64
> diff -u -p -r1.64 ip6_var.h
> --- netinet6/ip6_var.h 24 Aug 2016 09:41:12 -0000 1.64
> +++ netinet6/ip6_var.h 29 Nov 2016 15:58:04 -0000
> @@ -297,7 +297,7 @@ int none_input(struct mbuf **, int *, in
>  int in6_pcbselsrc(struct in6_addr **, struct sockaddr_in6 *,
>      struct inpcb *, struct ip6_pktopts *);
>  int in6_selectsrc(struct in6_addr **, struct sockaddr_in6 *,
> -    struct ip6_moptions *, struct route_in6 *, u_int);
> +    struct ip6_moptions *, unsigned int);
>  struct rtentry *in6_selectroute(struct sockaddr_in6 *, struct ip6_pktopts *,
>      struct route_in6 *, unsigned int rtableid);
>