diff mbox

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

Message ID 17f983c8843997caed814f647aadd9322cc401c4.1487884755.git.mschiffer@universe-factory.net
State Superseded
Headers show

Commit Message

Matthias Schiffer Feb. 23, 2017, 9:23 p.m. UTC 
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@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(-)

Comments

Hans Dedecker Feb. 24, 2017, 4:53 p.m. UTC | #1 
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@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@openwrt.org>
>   * Copyright (C) 2013 Steven Barth <steven@midlink.org>
>   * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
> + * Copyright (C) 2017 Matthias Schiffer <mschiffer@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.

Hans
>  	__TUNNEL_ATTR_MAX
>  };
Matthias Schiffer Feb. 24, 2017, 5:12 p.m. UTC | #2 
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@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@openwrt.org>
>>   * Copyright (C) 2013 Steven Barth <steven@midlink.org>
>>   * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
>> + * Copyright (C) 2017 Matthias Schiffer <mschiffer@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.

Matthias

> 
> Hans
>>  	__TUNNEL_ATTR_MAX
>>  };
> 
>
Hans Dedecker Feb. 24, 2017, 7:52 p.m. UTC | #3 
On Fri, Feb 24, 2017 at 6:12 PM, Matthias Schiffer
<mschiffer@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@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@openwrt.org>
>>>   * Copyright (C) 2013 Steven Barth <steven@midlink.org>
>>>   * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
>>> + * Copyright (C) 2017 Matthias Schiffer <mschiffer@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

Hans

>
> Matthias
>
>>
>> Hans
>>>      __TUNNEL_ATTR_MAX
>>>  };
>>
>>
>
>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
>
Felix Fietkau Feb. 25, 2017, 10:51 a.m. UTC | #4 
On 2017-02-24 20:52, Hans Dedecker wrote:
> On Fri, Feb 24, 2017 at 6:12 PM, Matthias Schiffer
> <mschiffer@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@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@openwrt.org>
>>>>   * Copyright (C) 2013 Steven Barth <steven@midlink.org>
>>>>   * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
>>>> + * Copyright (C) 2017 Matthias Schiffer <mschiffer@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
Matthias Schiffer Feb. 25, 2017, 11:30 a.m. UTC | #5 
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@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@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@openwrt.org>
>>>>>   * Copyright (C) 2013 Steven Barth <steven@midlink.org>
>>>>>   * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
>>>>> + * Copyright (C) 2017 Matthias Schiffer <mschiffer@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
diff mbox

Patch

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@openwrt.org>
  * Copyright (C) 2013 Steven Barth <steven@midlink.org>
  * Copyright (C) 2014 Gioacchino Mazzurco <gio@eigenlab.org>
+ * Copyright (C) 2017 Matthias Schiffer <mschiffer@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,
 	__TUNNEL_ATTR_MAX
 };