[LEDE-DEV] [PATCH netifd] system-linux: add VXLAN support

Matthias Schiffer mschiffer at universe-factory.net
Sat Feb 25 03:30:25 PST 2017


On 02/25/2017 11:51 AM, Felix Fietkau wrote:
> On 2017-02-24 20:52, Hans Dedecker wrote:
>> On Fri, Feb 24, 2017 at 6:12 PM, Matthias Schiffer
>> <mschiffer at universe-factory.net> wrote:
>>> On 02/24/2017 05:53 PM, Hans Dedecker wrote:
>>>> On Thursday, 23 February 2017 22:23:32 CET Matthias Schiffer wrote:
>>>>> VXLAN shares many attributes with the tunnel devices, so it is implemented
>>>>> as a new tunnel type. The 'remote' attribute can be used for an unicast
>>>>> peer or a multicast group.
>>>>>
>>>>> The IANA-assigned port 4789 is used by default, instead of the non-standard
>>>>> port Linux defaults to.
>>>>>
>>>>> Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>
>>>>> ---
>>>>>
>>>>> I've also added a matching proto package in my staging tree. Seems to work
>>>>> fine and I could probably just push this myself, but I prefer to get a
>>>>> least a little review as I'm not really familar with the netifd code.
>>>>>
>>>>>
>>>>>  system-linux.c | 153
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- system.c       |
>>>>>  3 ++
>>>>>  system.h       |   3 ++
>>>>>  3 files changed, 158 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/system-linux.c b/system-linux.c
>>>>> index f4d6c25..fa698cf 100644
>>>>> --- a/system-linux.c
>>>>> +++ b/system-linux.c
>>>>> @@ -4,6 +4,7 @@
>>>>>   * Copyright (C) 2013 Jo-Philipp Wich <jow at openwrt.org>
>>>>>   * Copyright (C) 2013 Steven Barth <steven at midlink.org>
>>>>>   * Copyright (C) 2014 Gioacchino Mazzurco <gio at eigenlab.org>
>>>>> + * Copyright (C) 2017 Matthias Schiffer <mschiffer at universe-factory.net>
>>>>>   *
>>>>>   * This program is free software; you can redistribute it and/or modify
>>>>>   * it under the terms of the GNU General Public License version 2
>>>>> @@ -25,6 +26,7 @@
>>>>>  #include <net/if_arp.h>
>>>>>
>>>>>  #include <arpa/inet.h>
>>>>> +#include <netinet/ether.h>
>>>>>  #include <netinet/in.h>
>>>>>
>>>>>  #include <linux/rtnetlink.h>
>>>>> @@ -2540,6 +2542,148 @@ failure:
>>>>>  }
>>>>>  #endif
>>>>>
>>>>> +#ifdef IFLA_VXLAN_MAX
>>>>> +static int system_add_vxlan(const char *name, const unsigned int link,
>>>>> struct blob_attr **tb, bool v6) +{
>>>>> +    struct nl_msg *msg;
>>>>> +    struct nlattr *linkinfo, *data;
>>>>> +    struct ifinfomsg iim = { .ifi_family = AF_UNSPEC, };
>>>>> +    struct blob_attr *cur;
>>>>> +    int ret = 0;
>>>>> +
>>>>> +    msg = nlmsg_alloc_simple(RTM_NEWLINK, NLM_F_REQUEST | NLM_F_CREATE |
>>>>> NLM_F_EXCL); +
>>>>> +    if (!msg)
>>>>> +            return -1;
>>>>> +
>>>>> +    nlmsg_append(msg, &iim, sizeof(iim), 0);
>>>>> +
>>>>> +    nla_put_string(msg, IFLA_IFNAME, name);
>>>>> +
>>>>> +    if ((cur = tb[TUNNEL_ATTR_MACADDR])) {
>>>>> +            struct ether_addr *ea = ether_aton(blobmsg_get_string(cur));
>>>>> +            if (!ea) {
>>>>> +                    ret = -EINVAL;
>>>>> +                    goto failure;
>>>>> +            }
>>>>> +
>>>>> +            nla_put(msg, IFLA_ADDRESS, ETH_ALEN, ea);
>>>>> +    }
>>>>> +
>>>>> +    if ((cur = tb[TUNNEL_ATTR_MTU])) {
>>>>> +            uint32_t mtu = blobmsg_get_u32(cur);
>>>>> +            nla_put_u32(msg, IFLA_MTU, mtu);
>>>>> +    }
>>>>> +
>>>>> +    if (!(linkinfo = nla_nest_start(msg, IFLA_LINKINFO))) {
>>>>> +            ret = -ENOMEM;
>>>>> +            goto failure;
>>>>> +    }
>>>>> +
>>>>> +    nla_put_string(msg, IFLA_INFO_KIND, "vxlan");
>>>>> +
>>>>> +    if (!(data = nla_nest_start(msg, IFLA_INFO_DATA))) {
>>>>> +            ret = -ENOMEM;
>>>>> +            goto failure;
>>>>> +    }
>>>>> +
>>>>> +    if (link)
>>>>> +            nla_put_u32(msg, IFLA_VXLAN_LINK, link);
>>>>> +
>>>>> +    if ((cur = tb[TUNNEL_ATTR_ID])) {
>>>>> +            uint32_t id = blobmsg_get_u32(cur);
>>>>> +            if (id >= (1u << 24) - 1) {
>>>>> +                    ret = -EINVAL;
>>>>> +                    goto failure;
>>>>> +            }
>>>>> +
>>>>> +            nla_put_u32(msg, IFLA_VXLAN_ID, id);
>>>>> +    }
>>>>> +
>>>>> +    if (v6) {
>>>>> +            struct in6_addr in6buf;
>>>>> +            if ((cur = tb[TUNNEL_ATTR_LOCAL])) {
>>>>> +                    if (inet_pton(AF_INET6, blobmsg_data(cur), &in6buf) < 1) {
>>>>> +                            ret = -EINVAL;
>>>>> +                            goto failure;
>>>>> +                    }
>>>>> +                    nla_put(msg, IFLA_VXLAN_LOCAL6, sizeof(in6buf), &in6buf);
>>>>> +            }
>>>>> +
>>>>> +            if ((cur = tb[TUNNEL_ATTR_REMOTE])) {
>>>>> +                    if (inet_pton(AF_INET6, blobmsg_data(cur), &in6buf) < 1) {
>>>>> +                            ret = -EINVAL;
>>>>> +                            goto failure;
>>>>> +                    }
>>>>> +                    nla_put(msg, IFLA_VXLAN_GROUP6, sizeof(in6buf), &in6buf);
>>>>> +            }
>>>>> +    } else {
>>>>> +            struct in_addr inbuf;
>>>>> +
>>>>> +            if ((cur = tb[TUNNEL_ATTR_LOCAL])) {
>>>>> +                    if (inet_pton(AF_INET, blobmsg_data(cur), &inbuf) < 1) {
>>>>> +                            ret = -EINVAL;
>>>>> +                            goto failure;
>>>>> +                    }
>>>>> +                    nla_put(msg, IFLA_VXLAN_LOCAL, sizeof(inbuf), &inbuf);
>>>>> +            }
>>>>> +
>>>>> +            if ((cur = tb[TUNNEL_ATTR_REMOTE])) {
>>>>> +                    if (inet_pton(AF_INET, blobmsg_data(cur), &inbuf) < 1) {
>>>>> +                            ret = -EINVAL;
>>>>> +                            goto failure;
>>>>> +                    }
>>>>> +                    nla_put(msg, IFLA_VXLAN_GROUP, sizeof(inbuf), &inbuf);
>>>>> +            }
>>>>> +    }
>>>>> +
>>>>> +    uint32_t port = 4789;
>>>>> +    if ((cur = tb[TUNNEL_ATTR_PORT])) {
>>>>> +            port = blobmsg_get_u32(cur);
>>>>> +            if (port < 1 || port > 65535) {
>>>>> +                    ret = -EINVAL;
>>>>> +                    goto failure;
>>>>> +            }
>>>>> +    }
>>>>> +    nla_put_u16(msg, IFLA_VXLAN_PORT, htons(port));
>>>>> +
>>>>> +    if ((cur = tb[TUNNEL_ATTR_TOS])) {
>>>>> +            char *str = blobmsg_get_string(cur);
>>>>> +            unsigned tos = 1;
>>>>> +
>>>>> +            if (strcmp(str, "inherit")) {
>>>>> +                    if (!system_tos_aton(str, &tos))
>>>>> +                            return -EINVAL;
>>>>> +            }
>>>>> +
>>>>> +            nla_put_u8(msg, IFLA_VXLAN_TOS, tos);
>>>>> +    }
>>>>> +
>>>>> +    if ((cur = tb[TUNNEL_ATTR_TTL])) {
>>>>> +            uint32_t ttl = blobmsg_get_u32(cur);
>>>>> +            if (ttl < 1 || ttl > 255) {
>>>>> +                    ret = -EINVAL;
>>>>> +                    goto failure;
>>>>> +            }
>>>>> +
>>>>> +            nla_put_u8(msg, IFLA_VXLAN_TTL, ttl);
>>>>> +    }
>>>>> +
>>>>> +    nla_nest_end(msg, data);
>>>>> +    nla_nest_end(msg, linkinfo);
>>>>> +
>>>>> +    ret = system_rtnl_call(msg);
>>>>> +    if (ret)
>>>>> +            D(SYSTEM, "Error adding vxlan '%s': %d\n", name, ret);
>>>>> +
>>>>> +    return ret;
>>>>> +
>>>>> +failure:
>>>>> +    nlmsg_free(msg);
>>>>> +    return ret;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>>  static int system_add_proto_tunnel(const char *name, const uint8_t proto,
>>>>> const unsigned int link, struct blob_attr **tb) {
>>>>>      struct blob_attr *cur;
>>>>> @@ -2609,7 +2753,8 @@ static int __system_del_ip_tunnel(const char *name,
>>>>> struct blob_attr **tb)
>>>>>
>>>>>      if (!strcmp(str, "greip") || !strcmp(str, "gretapip") ||
>>>>>          !strcmp(str, "greip6") || !strcmp(str, "gretapip6") ||
>>>>> -        !strcmp(str, "vtiip") || !strcmp(str, "vtiip6"))
>>>>> +        !strcmp(str, "vtiip") || !strcmp(str, "vtiip6") ||
>>>>> +        !strcmp(str, "vxlan") || !strcmp(str, "vxlan6"))
>>>>>              return system_link_del(name);
>>>>>      else
>>>>>              return tunnel_ioctl(name, SIOCDELTUNNEL, NULL);
>>>>> @@ -2833,6 +2978,12 @@ failure:
>>>>>      } else if (!strcmp(str, "vtiip6")) {
>>>>>              return system_add_vti_tunnel(name, "vti6", link, tb, true);
>>>>>  #endif
>>>>> +#ifdef IFLA_VXLAN_MAX
>>>>> +    } else if(!strcmp(str, "vxlan")) {
>>>>> +            return system_add_vxlan(name, link, tb, false);
>>>>> +    } else if(!strcmp(str, "vxlan6")) {
>>>>> +            return system_add_vxlan(name, link, tb, true);
>>>>> +#endif
>>>>>  #endif
>>>>>      } else if (!strcmp(str, "ipip")) {
>>>>>              return system_add_proto_tunnel(name, IPPROTO_IPIP, link, tb);
>>>>> diff --git a/system.c b/system.c
>>>>> index e57084f..981d16c 100644
>>>>> --- a/system.c
>>>>> +++ b/system.c
>>>>> @@ -28,6 +28,9 @@ static const struct blobmsg_policy
>>>>> tunnel_attrs[__TUNNEL_ATTR_MAX] = { [TUNNEL_ATTR_LINK] = { .name = "link",
>>>>> .type = BLOBMSG_TYPE_STRING }, [TUNNEL_ATTR_FMRS] = { .name = "fmrs", .type
>>>>> = BLOBMSG_TYPE_ARRAY }, [TUNNEL_ATTR_INFO] = { .name = "info", .type =
>>>>> BLOBMSG_TYPE_STRING }, +     [TUNNEL_ATTR_ID] = { .name = "id", .type =
>>>>> BLOBMSG_TYPE_INT32 }, +      [TUNNEL_ATTR_PORT] = { .name = "port", .type =
>>>>> BLOBMSG_TYPE_INT32 }, +      [TUNNEL_ATTR_MACADDR] = { .name = "macaddr", .type
>>>>> = BLOBMSG_TYPE_STRING }, };
>>>>>
>>>>>  const struct uci_blob_param_list tunnel_attr_list = {
>>>>> diff --git a/system.h b/system.h
>>>>> index 6077b95..2c4df38 100644
>>>>> --- a/system.h
>>>>> +++ b/system.h
>>>>> @@ -35,6 +35,9 @@ enum tunnel_param {
>>>>>      TUNNEL_ATTR_LINK,
>>>>>      TUNNEL_ATTR_FMRS,
>>>>>      TUNNEL_ATTR_INFO,
>>>>> +    TUNNEL_ATTR_ID,
>>>>> +    TUNNEL_ATTR_PORT,
>>>>> +    TUNNEL_ATTR_MACADDR,
>>>> GRE specific parameters like  ikey, okey, icsum, ocum, iseqno, oseqno were put
>>>> into the TUNNEL_ATTR_INFO parameter to avoid to define too much tunnel protocol
>>>> specific attributes definitions. Maybe it's possible to use the same approach
>>>> for the vxlan specific parameters like id, port, macaddr ?
>>>> Otherwise the rest looks of for me.
>>>
>>> Kinda seems to me like defeating the purpose of having a blobmsg for
>>> this... Munching everything together into a single string feels more
>>> acceptable to me for simple on/off flags than for more complex data like
>>> numbers and MAC addresses, but I think I wouldn't have chosen such a
>>> solution in any case.
>> Choice was based on a tradeoff between a continuously growing tunnel
>> attribute list with protocol specific unrelated parameters or trying
>> to get a bit of structure in the different tunnel protocol parameters.
>> Having said that I'm open for other suggestions
> I think I prefer the growing tunnel attribute list over weird and
> seemingly arbitrary stuffing of attributes into a single string.
> 
> - Felix
> 

How about adding a nested table with tunnel-specific attributes, similar to
the IFLA_INFO_DATA netlink attribute?

Matthias


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/lede-dev/attachments/20170225/45b9a0ba/attachment-0001.sig>


More information about the Lede-dev mailing list