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 <vgr...@openbsd.org> wrote:
> On Tue, 29 Nov 2016 17:03:44 +0100
> Martin Pieuchot <m...@openbsd.org> 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);
>>
>
