* [PATCH iproute2 1/3] ip/tunnel: Unify setup and accept zero address for local/remote endpoints
2017-12-13 19:35 [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Serhey Popovych
@ 2017-12-13 19:36 ` Serhey Popovych
2017-12-13 19:36 ` [PATCH iproute2 2/3] ip/tunnel: Use get_addr() instead of get_prefix() " Serhey Popovych
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Serhey Popovych @ 2017-12-13 19:36 UTC (permalink / raw)
To: netdev
It is fully legal to submit zero (INADDR_ANY/IN6ADDR_ANY_INIT)
value for local and/or remote endpoints for all tunnel drivers:
no need additionally check this in userspace.
Note that all tunnel specific code already can pass zero address
to the kernel.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/iptunnel.c | 10 ++--------
ip/link_gre.c | 6 ++----
ip/link_iptnl.c | 10 ++--------
ip/link_vti.c | 14 ++------------
ip/link_vti6.c | 26 ++++++++------------------
5 files changed, 16 insertions(+), 50 deletions(-)
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 208a1f0..ce610f8 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -127,16 +127,10 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
p->iph.frag_off = htons(IP_DF);
} else if (strcmp(*argv, "remote") == 0) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- p->iph.daddr = get_addr32(*argv);
- else
- p->iph.daddr = htonl(INADDR_ANY);
+ p->iph.daddr = get_addr32(*argv);
} else if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- p->iph.saddr = get_addr32(*argv);
- else
- p->iph.saddr = htonl(INADDR_ANY);
+ p->iph.saddr = get_addr32(*argv);
} else if (strcmp(*argv, "dev") == 0) {
NEXT_ARG();
medium = *argv;
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 43cb1af..6f82fb4 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -251,12 +251,10 @@ get_failed:
pmtudisc = 1;
} else if (!matches(*argv, "remote")) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- daddr = get_addr32(*argv);
+ daddr = get_addr32(*argv);
} else if (!matches(*argv, "local")) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- saddr = get_addr32(*argv);
+ saddr = get_addr32(*argv);
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
link = if_nametoindex(*argv);
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 4940b8b..521d6ba 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -195,16 +195,10 @@ get_failed:
while (argc > 0) {
if (strcmp(*argv, "remote") == 0) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- raddr = get_addr32(*argv);
- else
- raddr = 0;
+ raddr = get_addr32(*argv);
} else if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
- if (strcmp(*argv, "any"))
- laddr = get_addr32(*argv);
- else
- laddr = 0;
+ laddr = get_addr32(*argv);
} else if (matches(*argv, "dev") == 0) {
NEXT_ARG();
link = if_nametoindex(*argv);
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 07ac94e..05aefa3 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -167,20 +167,10 @@ get_failed:
okey = uval;
} else if (!matches(*argv, "remote")) {
NEXT_ARG();
- if (!strcmp(*argv, "any")) {
- fprintf(stderr, "invalid value for \"remote\": \"%s\"\n", *argv);
- exit(-1);
- } else {
- daddr = get_addr32(*argv);
- }
+ daddr = get_addr32(*argv);
} else if (!matches(*argv, "local")) {
NEXT_ARG();
- if (!strcmp(*argv, "any")) {
- fprintf(stderr, "invalid value for \"local\": \"%s\"\n", *argv);
- exit(-1);
- } else {
- saddr = get_addr32(*argv);
- }
+ saddr = get_addr32(*argv);
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
link = if_nametoindex(*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 6d08bfe..f665520 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -161,27 +161,17 @@ get_failed:
}
okey = uval;
} else if (!matches(*argv, "remote")) {
- NEXT_ARG();
- if (!strcmp(*argv, "any")) {
- fprintf(stderr, "invalid value for \"remote\": \"%s\"\n", *argv);
- exit(-1);
- } else {
- inet_prefix addr;
+ inet_prefix addr;
- get_prefix(&addr, *argv, AF_INET6);
- memcpy(&daddr, addr.data, addr.bytelen);
- }
- } else if (!matches(*argv, "local")) {
NEXT_ARG();
- if (!strcmp(*argv, "any")) {
- fprintf(stderr, "invalid value for \"local\": \"%s\"\n", *argv);
- exit(-1);
- } else {
- inet_prefix addr;
+ get_prefix(&addr, *argv, AF_INET6);
+ memcpy(&daddr, addr.data, addr.bytelen);
+ } else if (!matches(*argv, "local")) {
+ inet_prefix addr;
- get_prefix(&addr, *argv, AF_INET6);
- memcpy(&saddr, addr.data, addr.bytelen);
- }
+ NEXT_ARG();
+ get_prefix(&addr, *argv, AF_INET6);
+ memcpy(&saddr, addr.data, addr.bytelen);
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
link = if_nametoindex(*argv);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH iproute2 2/3] ip/tunnel: Use get_addr() instead of get_prefix() for local/remote endpoints
2017-12-13 19:35 [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Serhey Popovych
2017-12-13 19:36 ` [PATCH iproute2 1/3] ip/tunnel: Unify setup and accept zero address for local/remote endpoints Serhey Popovych
@ 2017-12-13 19:36 ` Serhey Popovych
2017-12-13 19:36 ` [PATCH iproute2 3/3] ip: gre: fix IFLA_GRE_LINK attribute sizing Serhey Popovych
2017-12-16 18:10 ` [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Serhey Popovych @ 2017-12-13 19:36 UTC (permalink / raw)
To: netdev
Manual page ip-link(8) states that both local and remote accept
IPADDR not PREFIX. Use get_addr() instead of get_prefix() to
parse local/remote endpoint address correctly.
Force corresponding address family instead of using preferred_family
to catch weired cases as shown below.
Before this patch it is possible to create tunnel with commands:
ip li add dev ip6gre2 type ip6gre local fe80::1/64 remote fe80::2/64
ip -4 li add dev ip6gre2 type ip6gre local 10.0.0.1/24 remote 10.0.0.2/24
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip6tunnel.c | 8 ++------
ip/link_gre6.c | 8 ++------
ip/link_ip6tnl.c | 12 ++++--------
ip/link_vti6.c | 8 ++++----
4 files changed, 12 insertions(+), 24 deletions(-)
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 4563e1e..b8db49c 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -170,17 +170,13 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
inet_prefix raddr;
NEXT_ARG();
- get_prefix(&raddr, *argv, preferred_family);
- if (raddr.family == AF_UNSPEC)
- invarg("\"remote\" address family is AF_UNSPEC", *argv);
+ get_addr(&raddr, *argv, AF_INET6);
memcpy(&p->raddr, &raddr.data, sizeof(p->raddr));
} else if (strcmp(*argv, "local") == 0) {
inet_prefix laddr;
NEXT_ARG();
- get_prefix(&laddr, *argv, preferred_family);
- if (laddr.family == AF_UNSPEC)
- invarg("\"local\" address family is AF_UNSPEC", *argv);
+ get_addr(&laddr, *argv, AF_INET6);
memcpy(&p->laddr, &laddr.data, sizeof(p->laddr));
} else if (strcmp(*argv, "dev") == 0) {
NEXT_ARG();
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 0a82eae..c22fded 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -257,17 +257,13 @@ get_failed:
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, preferred_family);
- if (addr.family == AF_UNSPEC)
- invarg("\"remote\" address family is AF_UNSPEC", *argv);
+ get_addr(&addr, *argv, AF_INET6);
memcpy(&raddr, &addr.data, sizeof(raddr));
} else if (!matches(*argv, "local")) {
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, preferred_family);
- if (addr.family == AF_UNSPEC)
- invarg("\"local\" address family is AF_UNSPEC", *argv);
+ get_addr(&addr, *argv, AF_INET6);
memcpy(&laddr, &addr.data, sizeof(laddr));
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 43287ab..4ab337f 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -184,18 +184,14 @@ get_failed:
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, preferred_family);
- if (addr.family == AF_UNSPEC)
- invarg("\"remote\" address family is AF_UNSPEC", *argv);
- memcpy(&raddr, addr.data, addr.bytelen);
+ get_addr(&addr, *argv, AF_INET6);
+ memcpy(&raddr, addr.data, sizeof(raddr));
} else if (strcmp(*argv, "local") == 0) {
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, preferred_family);
- if (addr.family == AF_UNSPEC)
- invarg("\"local\" address family is AF_UNSPEC", *argv);
- memcpy(&laddr, addr.data, addr.bytelen);
+ get_addr(&addr, *argv, AF_INET6);
+ memcpy(&laddr, addr.data, sizeof(laddr));
} else if (matches(*argv, "dev") == 0) {
NEXT_ARG();
link = if_nametoindex(*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index f665520..84824a5 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -164,14 +164,14 @@ get_failed:
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, AF_INET6);
- memcpy(&daddr, addr.data, addr.bytelen);
+ get_addr(&addr, *argv, AF_INET6);
+ memcpy(&daddr, addr.data, sizeof(daddr));
} else if (!matches(*argv, "local")) {
inet_prefix addr;
NEXT_ARG();
- get_prefix(&addr, *argv, AF_INET6);
- memcpy(&saddr, addr.data, addr.bytelen);
+ get_addr(&addr, *argv, AF_INET6);
+ memcpy(&saddr, addr.data, sizeof(saddr));
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
link = if_nametoindex(*argv);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH iproute2 3/3] ip: gre: fix IFLA_GRE_LINK attribute sizing
2017-12-13 19:35 [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Serhey Popovych
2017-12-13 19:36 ` [PATCH iproute2 1/3] ip/tunnel: Unify setup and accept zero address for local/remote endpoints Serhey Popovych
2017-12-13 19:36 ` [PATCH iproute2 2/3] ip/tunnel: Use get_addr() instead of get_prefix() " Serhey Popovych
@ 2017-12-13 19:36 ` Serhey Popovych
2017-12-16 18:10 ` [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Serhey Popovych @ 2017-12-13 19:36 UTC (permalink / raw)
To: netdev
Attribute IFLA_GRE_LINK is 32 bit long, not 8 bit.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 6f82fb4..09f1e44 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -155,7 +155,7 @@ get_failed:
tos = rta_getattr_u8(greinfo[IFLA_GRE_TOS]);
if (greinfo[IFLA_GRE_LINK])
- link = rta_getattr_u8(greinfo[IFLA_GRE_LINK]);
+ link = rta_getattr_u32(greinfo[IFLA_GRE_LINK]);
if (greinfo[IFLA_GRE_ENCAP_TYPE])
encaptype = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_TYPE]);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling
2017-12-13 19:35 [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling Serhey Popovych
` (2 preceding siblings ...)
2017-12-13 19:36 ` [PATCH iproute2 3/3] ip: gre: fix IFLA_GRE_LINK attribute sizing Serhey Popovych
@ 2017-12-16 18:10 ` Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2017-12-16 18:10 UTC (permalink / raw)
To: Serhey Popovych; +Cc: netdev
On Wed, 13 Dec 2017 21:35:59 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:
> In this series following problems addressed:
>
> 1) Size of IFLA_GRE_LINK attribute is 32 bits long in , not 8 bit.
>
> 2) Use get_addr() instead of get_prefix() to parse local/remote
> tunnel endpoints as IPADDR, not PREFIX as per ip-link(8).
>
> 3) No need to check if local/remote endpoints are zero (e.g. INADDR_ANY):
> it is fully legal value, accepted by the kernel.
>
> See individual patch description message for details.
>
> Thanks,
> Serhii
>
> Serhey Popovych (3):
> ip/tunnel: Unify setup and accept zero address for local/remote
> endpoints
> ip/tunnel: Use get_addr() instead of get_prefix() for local/remote
> endpoints
> ip: gre: fix IFLA_GRE_LINK attribute sizing
>
> ip/ip6tunnel.c | 8 ++------
> ip/iptunnel.c | 10 ++--------
> ip/link_gre.c | 8 +++-----
> ip/link_gre6.c | 8 ++------
> ip/link_ip6tnl.c | 12 ++++--------
> ip/link_iptnl.c | 10 ++--------
> ip/link_vti.c | 14 ++------------
> ip/link_vti6.c | 26 ++++++++------------------
> 8 files changed, 25 insertions(+), 71 deletions(-)
>
Looks good, applied
^ permalink raw reply [flat|nested] 5+ messages in thread