[PATCH iproute2 v2 0/3] iptnl/tunnel: Open JSON section, kill code duplication and print iptnl mode

[PATCH iproute2 v2 0/3] iptnl/tunnel: Open JSON section, kill code duplication and print iptnl mode

[Serhey Popovych]

In this series I present following improvements and fixes:

  1) Add missing open_json_context() to link_iptnl.c
     for "encap" options.

  2) Get rid of code duplication when parsing tunnel mode
     parameters in link_iptnl.c.

  3) Add code to link_iptnl.c to print tunnel mode to be
     inline with ip6tnl.

See individual patch description message for details.

This is v2. Fixed proto value for ipip tunnel mode:
"proto ipip" vs "proto ip4ip4" previously.

Thanks,
Serhii

Serhey Popovych (3):
  link_iptnl: Kill code duplication
  link_iptnl: Print tunnel mode
  link_iptnl: Open "encap" JSON object

 ip/link_iptnl.c |   45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

-- 
1.7.10.4

[PATCH iproute2 v2 1/3] link_iptnl: Kill code duplication

[Serhey Popovych]

Both sit and ipip "mode" parameter handling nearly the same.
Except for sit we have "ip6ip" mode: check it only when
configuring sit.

Note that there is no need strcmp(lu->id, "ipip"): if it is
not sit it is "ipip" because we have only these two link util
defined in module.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_iptnl.c |   27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 8a8f5dd..d4d935b 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -230,11 +230,11 @@ get_failed:
 		} else if (strcmp(lu->id, "sit") == 0 &&
 			   strcmp(*argv, "isatap") == 0) {
 			iflags |= SIT_ISATAP;
-		} else if (strcmp(lu->id, "sit") == 0 &&
-			   strcmp(*argv, "mode") == 0) {
+		} else if (strcmp(*argv, "mode") == 0) {
 			NEXT_ARG();
-			if (strcmp(*argv, "ipv6/ipv4") == 0 ||
-			    strcmp(*argv, "ip6ip") == 0)
+			if (strcmp(lu->id, "sit") == 0 &&
+			    (strcmp(*argv, "ipv6/ipv4") == 0 ||
+			     strcmp(*argv, "ip6ip") == 0))
 				proto = IPPROTO_IPV6;
 			else if (strcmp(*argv, "ipv4/ipv4") == 0 ||
 				 strcmp(*argv, "ipip") == 0 ||
@@ -248,21 +248,6 @@ get_failed:
 				proto = 0;
 			else
 				invarg("Cannot guess tunnel mode.", *argv);
-		} else if (strcmp(lu->id, "ipip") == 0 &&
-			   strcmp(*argv, "mode") == 0) {
-			NEXT_ARG();
-			if (strcmp(*argv, "ipv4/ipv4") == 0 ||
-				 strcmp(*argv, "ipip") == 0 ||
-				 strcmp(*argv, "ip4ip4") == 0)
-				proto = IPPROTO_IPIP;
-			else if (strcmp(*argv, "mpls/ipv4") == 0 ||
-				   strcmp(*argv, "mplsip") == 0)
-				proto = IPPROTO_MPLS;
-			else if (strcmp(*argv, "any/ipv4") == 0 ||
-				 strcmp(*argv, "any") == 0)
-				proto = 0;
-			else
-				invarg("Cannot guess tunnel mode.", *argv);
 		} else if (strcmp(*argv, "noencap") == 0) {
 			encaptype = TUNNEL_ENCAP_NONE;
 		} else if (strcmp(*argv, "encap") == 0) {
@@ -337,6 +322,7 @@ get_failed:
 		exit(-1);
 	}
 
+	addattr8(n, 1024, IFLA_IPTUN_PROTO, proto);
 	if (metadata) {
 		addattr_l(n, 1024, IFLA_IPTUN_COLLECT_METADATA, NULL, 0);
 		return 0;
@@ -355,9 +341,6 @@ get_failed:
 	addattr16(n, 1024, IFLA_IPTUN_ENCAP_SPORT, htons(encapsport));
 	addattr16(n, 1024, IFLA_IPTUN_ENCAP_DPORT, htons(encapdport));
 
-	if (strcmp(lu->id, "ipip") == 0 || strcmp(lu->id, "sit") == 0)
-		addattr8(n, 1024, IFLA_IPTUN_PROTO, proto);
-
 	if (strcmp(lu->id, "sit") == 0) {
 		addattr16(n, 1024, IFLA_IPTUN_FLAGS, iflags);
 		if (ip6rdprefixlen) {
-- 
1.7.10.4

[PATCH iproute2 v2 2/3] link_iptnl: Print tunnel mode

[Serhey Popovych]

Tunnel mode does not appear in parameters print for iptnl
supported tunnels like ipip and sit, while printed for
ip6tnl.

Print tunnel mode as "proto" field name for JSON and
without any name when printing to cli to follow ip6tnl
behaviour.

For non JSON output we have:

   $ ip -d link show dev sit1

Before:
-------
17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
    link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
    sit remote any local X.X.X.X ...
        ~~~

After:
------
17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
    link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
    sit any remote any local X.X.X.X ...
        ^^^

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_iptnl.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index d4d935b..b6ef95d 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -372,6 +372,23 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	if (tb[IFLA_IPTUN_COLLECT_METADATA])
 		print_bool(PRINT_ANY, "external", "external ", true);
 
+	if (tb[IFLA_IPTUN_PROTO]) {
+		switch (rta_getattr_u8(tb[IFLA_IPTUN_PROTO])) {
+		case IPPROTO_IPIP:
+			print_string(PRINT_ANY, "proto", "%s ", "ipip");
+			break;
+		case IPPROTO_IPV6:
+			print_string(PRINT_ANY, "proto", "%s ", "ip6ip");
+			break;
+		case IPPROTO_MPLS:
+			print_string(PRINT_ANY, "proto", "%s ", "mplsip");
+			break;
+		case 0:
+			print_string(PRINT_ANY, "proto", "%s ", "any");
+			break;
+		}
+	}
+
 	if (tb[IFLA_IPTUN_REMOTE]) {
 		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
 
-- 
1.7.10.4

Re: [PATCH iproute2 v2 2/3] link_iptnl: Print tunnel mode

[Stephen Hemminger]

On Tue,  2 Jan 2018 23:27:58 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> Tunnel mode does not appear in parameters print for iptnl
> supported tunnels like ipip and sit, while printed for
> ip6tnl.
> 
> Print tunnel mode as "proto" field name for JSON and
> without any name when printing to cli to follow ip6tnl
> behaviour.
> 
> For non JSON output we have:
> 
>    $ ip -d link show dev sit1
> 
> Before:
> -------
> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>     sit remote any local X.X.X.X ...
>         ~~~
> 
> After:
> ------
> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>     sit any remote any local X.X.X.X ...
>         ^^^
> 
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>

Applied all three to master.
Thanks.

[PATCH iproute2 v2 3/3] link_iptnl: Open "encap" JSON object

[Serhey Popovych]

It seems missing pair of open_json_object()/close_json_object()
in iptnl implementation.

Note that we open "encap" JSON object in ip6tnl.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_iptnl.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index b6ef95d..24a0f0c 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -498,6 +498,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		__u16 sport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_SPORT]);
 		__u16 dport = rta_getattr_u16(tb[IFLA_IPTUN_ENCAP_DPORT]);
 
+		open_json_object("encap");
 		print_string(PRINT_FP, NULL, "encap ", NULL);
 		switch (type) {
 		case TUNNEL_ENCAP_FOU:
-- 
1.7.10.4

Re: [PATCH iproute2 2/3] link_iptnl: Print tunnel mode

[Stephen Hemminger]

On Tue,  2 Jan 2018 17:29:18 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> Tunnel mode does not appear in parameters print for iptnl
> supported tunnels like ipip and sit, while printed for
> ip6tnl.
> 
> Print tunnel mode with "proto" field for JSON and without
> any name when printing to cli to follow ip6tnl behaviour.
> 
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
>  ip/link_iptnl.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
> index d4d935b..afd1696 100644
> --- a/ip/link_iptnl.c
> +++ b/ip/link_iptnl.c
> @@ -372,6 +372,23 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
>  	if (tb[IFLA_IPTUN_COLLECT_METADATA])
>  		print_bool(PRINT_ANY, "external", "external ", true);
>  
> +	if (tb[IFLA_IPTUN_PROTO]) {
> +		switch (rta_getattr_u8(tb[IFLA_IPTUN_PROTO])) {
> +		case IPPROTO_IPIP:
> +			print_string(PRINT_ANY, "proto", "%s ", "ip4ip4");
> +			break;
> +		case IPPROTO_IPV6:
> +			print_string(PRINT_ANY, "proto", "%s ", "ip6ip");
> +			break;
> +		case IPPROTO_MPLS:
> +			print_string(PRINT_ANY, "proto", "%s ", "mplsip");
> +			break;
> +		case 0:
> +			print_string(PRINT_ANY, "proto", "%s ", "any");
> +			break;
> +		}
> +	}
> +
>  	if (tb[IFLA_IPTUN_REMOTE]) {
>  		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
>  

In iproute2 utilities the output format must match the input format.
I don't see "proto ip4ip4" as a valid option.

[PATCH iproute2 v2 2/3] link_iptnl: Print tunnel mode

[Serhey Popovych]

Tunnel mode does not appear in parameters print for iptnl
supported tunnels like ipip and sit, while printed for
ip6tnl.

Print tunnel mode as "proto" field name for JSON and
without any name when printing to cli to follow ip6tnl
behaviour.

For non JSON output we have:

   $ ip -d link show dev sit1

Before:
-------
17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
    link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
    sit remote any local X.X.X.X ...
        ~~~

After:
------
17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
    link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
    sit any remote any local X.X.X.X ...
        ^^^

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
v2: Addressed comments: "proto ipip" vs "proto ip4ip4" for
    IPPROTO_IPIP tunnel type. Add example to message.

 ip/link_iptnl.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index d4d935b..b6ef95d 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -372,6 +372,23 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	if (tb[IFLA_IPTUN_COLLECT_METADATA])
 		print_bool(PRINT_ANY, "external", "external ", true);
 
+	if (tb[IFLA_IPTUN_PROTO]) {
+		switch (rta_getattr_u8(tb[IFLA_IPTUN_PROTO])) {
+		case IPPROTO_IPIP:
+			print_string(PRINT_ANY, "proto", "%s ", "ipip");
+			break;
+		case IPPROTO_IPV6:
+			print_string(PRINT_ANY, "proto", "%s ", "ip6ip");
+			break;
+		case IPPROTO_MPLS:
+			print_string(PRINT_ANY, "proto", "%s ", "mplsip");
+			break;
+		case 0:
+			print_string(PRINT_ANY, "proto", "%s ", "any");
+			break;
+		}
+	}
+
 	if (tb[IFLA_IPTUN_REMOTE]) {
 		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
 
-- 
1.7.10.4

Re: [PATCH iproute2 v2 2/3] link_iptnl: Print tunnel mode

[Stephen Hemminger]

On Tue,  2 Jan 2018 23:02:45 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> Tunnel mode does not appear in parameters print for iptnl
> supported tunnels like ipip and sit, while printed for
> ip6tnl.
> 
> Print tunnel mode as "proto" field name for JSON and
> without any name when printing to cli to follow ip6tnl
> behaviour.
> 
> For non JSON output we have:
> 
>    $ ip -d link show dev sit1
> 
> Before:
> -------
> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>     sit remote any local X.X.X.X ...
>         ~~~
> 
> After:
> ------
> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>     sit any remote any local X.X.X.X ...
>         ^^^
> 
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
> v2: Addressed comments: "proto ipip" vs "proto ip4ip4" for
>     IPPROTO_IPIP tunnel type. Add example to message.
> 
>  ip/link_iptnl.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
> index d4d935b..b6ef95d 100644
> --- a/ip/link_iptnl.c
> +++ b/ip/link_iptnl.c
> @@ -372,6 +372,23 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
>  	if (tb[IFLA_IPTUN_COLLECT_METADATA])
>  		print_bool(PRINT_ANY, "external", "external ", true);
>  
> +	if (tb[IFLA_IPTUN_PROTO]) {
> +		switch (rta_getattr_u8(tb[IFLA_IPTUN_PROTO])) {
> +		case IPPROTO_IPIP:
> +			print_string(PRINT_ANY, "proto", "%s ", "ipip");
> +			break;
> +		case IPPROTO_IPV6:
> +			print_string(PRINT_ANY, "proto", "%s ", "ip6ip");
> +			break;
> +		case IPPROTO_MPLS:
> +			print_string(PRINT_ANY, "proto", "%s ", "mplsip");
> +			break;
> +		case 0:
> +			print_string(PRINT_ANY, "proto", "%s ", "any");
> +			break;
> +		}
> +	}
> +
>  	if (tb[IFLA_IPTUN_REMOTE]) {
>  		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);

Thanks for fixing the ip4 value.

When you fix one thing, you need to resend whole patch series.

Re: [PATCH iproute2 v2 2/3] link_iptnl: Print tunnel mode

[Serhey Popovych]

[-- Attachment #1.1: Type: text/plain, Size: 2268 bytes --]

Stephen Hemminger wrote:
> On Tue,  2 Jan 2018 23:02:45 +0200
> Serhey Popovych <serhe.popovych@gmail.com> wrote:
> 
>> Tunnel mode does not appear in parameters print for iptnl
>> supported tunnels like ipip and sit, while printed for
>> ip6tnl.
>>
>> Print tunnel mode as "proto" field name for JSON and
>> without any name when printing to cli to follow ip6tnl
>> behaviour.
>>
>> For non JSON output we have:
>>
>>    $ ip -d link show dev sit1
>>
>> Before:
>> -------
>> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>>     sit remote any local X.X.X.X ...
>>         ~~~
>>
>> After:
>> ------
>> 17: sit1@NONE: <NOARP> mtu 1480 qdisc noop state DOWN ...
>>     link/sit X.X.X.X brd 0.0.0.0 promiscuity 0
>>     sit any remote any local X.X.X.X ...
>>         ^^^
>>
>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>> ---
>> v2: Addressed comments: "proto ipip" vs "proto ip4ip4" for
>>     IPPROTO_IPIP tunnel type. Add example to message.
>>
>>  ip/link_iptnl.c |   17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
>> index d4d935b..b6ef95d 100644
>> --- a/ip/link_iptnl.c
>> +++ b/ip/link_iptnl.c
>> @@ -372,6 +372,23 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
>>  	if (tb[IFLA_IPTUN_COLLECT_METADATA])
>>  		print_bool(PRINT_ANY, "external", "external ", true);
>>  
>> +	if (tb[IFLA_IPTUN_PROTO]) {
>> +		switch (rta_getattr_u8(tb[IFLA_IPTUN_PROTO])) {
>> +		case IPPROTO_IPIP:
>> +			print_string(PRINT_ANY, "proto", "%s ", "ipip");
>> +			break;
>> +		case IPPROTO_IPV6:
>> +			print_string(PRINT_ANY, "proto", "%s ", "ip6ip");
>> +			break;
>> +		case IPPROTO_MPLS:
>> +			print_string(PRINT_ANY, "proto", "%s ", "mplsip");
>> +			break;
>> +		case 0:
>> +			print_string(PRINT_ANY, "proto", "%s ", "any");
>> +			break;
>> +		}
>> +	}
>> +
>>  	if (tb[IFLA_IPTUN_REMOTE]) {
>>  		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
> 
> Thanks for fixing the ip4 value.
> 
> When you fix one thing, you need to resend whole patch series.

Submitted v2 series. Thank you for the tip!

> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]