lkml.org 
[lkml]   [2017]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH net-next 3/3] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses
    From
    Date
    On 03/14/2017 04:28 PM, Jiri Benc wrote:
    > On Fri, 10 Mar 2017 23:39:44 +0100, Matthias Schiffer wrote:
    >> @@ -233,17 +234,30 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs, __be32 vni)
    >> vni = 0;
    >>
    >> hlist_for_each_entry_rcu(vxlan, vni_head(vs, vni), hlist) {
    >> - if (vxlan->default_dst.remote_vni == vni)
    >> - return vxlan;
    >> + if (vxlan->default_dst.remote_vni != vni)
    >> + continue;
    >> +
    >> + if (IS_ENABLED(CONFIG_IPV6)) {
    >> + const struct vxlan_config *cfg = &vxlan->cfg;
    >> +
    >> + if (cfg->remote_ifindex != 0 &&
    >> + cfg->remote_ifindex != ifindex &&
    >> + cfg->saddr.sa.sa_family == AF_INET6 &&
    >> + (ipv6_addr_type(&cfg->saddr.sin6.sin6_addr) &
    >> + IPV6_ADDR_LINKLOCAL))
    >
    > Calculating this (especially ipv6_addr_type) on every received packet
    > looks unnecessarily expensive. Just store the fact the the local
    > address is link-local in a flag during config. And compare the flag
    > first before considering remote_ifindex.
    >
    > This is especially important for lwtunnels which can have anything in
    > the saddr and remote_ifindex, yet those fields are ignored and the
    > vni 0 entry has to be returned. It also means that the link-local flag
    > must not be set for lwtunnels.
    >
    >> +#if IS_ENABLED(CONFIG_IPV6)
    >> + if (conf->remote_ifindex != tmp->cfg.remote_ifindex &&
    >> + conf->saddr.sa.sa_family == AF_INET6 &&
    >> + tmp->cfg.saddr.sa.sa_family == AF_INET6 &&
    >> + (ipv6_addr_type(&conf->saddr.sin6.sin6_addr) &
    >> + IPV6_ADDR_LINKLOCAL) &&
    >> + (ipv6_addr_type(&tmp->cfg.saddr.sin6.sin6_addr) &
    >> + IPV6_ADDR_LINKLOCAL))
    >> + continue;
    >> +#endif
    >
    > In patch 1, you're checking for either source and destination address
    > being link-local, while here you're consider only those that have both
    > addresses link-local.
    >
    > Wouldn't it be better to plainly reject configuration that has one
    > address link-local but not the other one?

    While ensuring that the destination address is link-local iff the source
    address is would also be an option, it didn't seem too useful as the
    destination address will be a multicast address anyways in "normal" VXLAN
    configurations. If we really want to check this, I guess the valid
    combinations are:

    source link-local - destination link-local UC
    source link-local - destination link-local MC
    source global/... - destination global/... UC
    source global/... - destination any MC

    Does this make sense?


    >
    > Jiri

    Thanks for your comments, I'll send a v2 soon.

    Matthias




    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2017-03-15 15:31    [W:2.414 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site