From cc9c1dfaee04030f1c2a80fb28d99c62ce0fca6a Mon Sep 17 00:00:00 2001 From: Konstantin Shemyak Date: Thu, 26 Nov 2015 18:22:05 +0200 Subject: [PATCH 1/7] ip_tunnel: determine tunnel address family from the tunnel type On 24.11.2015 02:26, Stephen Hemminger wrote: > On Thu, 12 Nov 2015 21:10:08 +0000 > Konstantin Shemyak wrote: > >> When creating an IP tunnel over IPv6, the address family must be passed in >> the option, e.g. >> >> ip -6 tunnel add mode ip6gre local 1::1 remote 2::2 >> >> This makes it impossible to create both IPv4 and IPv6 tunnels in one batch. >> >> In fact the address family option is redundant here, as each tunnel mode is >> relevant for only one address family. >> The patch determines whether the applicable address family is AF_INET6 >> instead of the default AF_INET and makes the "-6" option unnecessary for >> "ip tunnel add". >> >> Signed-off-by: Konstantin Shemyak >> --- >> ip/iptunnel.c | 26 ++++++++++++++++++++++++++ >> testsuite/tests/ip/tunnel/add_tunnel.t | 14 ++++++++++++++ >> 2 files changed, 40 insertions(+) >> create mode 100755 testsuite/tests/ip/tunnel/add_tunnel.t >> >> diff --git a/ip/iptunnel.c b/ip/iptunnel.c >> index 78fa988..7826a37 100644 >> --- a/ip/iptunnel.c >> +++ b/ip/iptunnel.c >> @@ -629,8 +629,34 @@ static int do_6rd(int argc, char **argv) >> return tnl_6rd_ioctl(cmd, medium, &ip6rd); >> } >> >> +static int tunnel_mode_is_ipv6(char *tunnel_mode) { >> + char *ipv6_modes[] = { >> + "ipv6/ipv6", "ip6ip6", >> + "vti6", >> + "ip/ipv6", "ipv4/ipv6", "ipip6", "ip4ip6", >> + "ip6gre", "gre/ipv6", >> + "any/ipv6", "any" >> + }; >> + int i; >> + >> + for (i = 0; i < sizeof(ipv6_modes) / sizeof(char *); i++) { >> + if (strcmp(ipv6_modes[i], tunnel_mode) == 0) >> + return 1; >> + } >> + return 0; >> +} >> + > > The ipv6_modes table should be static const. Thank you for the note! attached the corrected patch. > Also is it possible to use strstr for ipv6 and ip6 or even strchr(tunnel_mode, '6') > to simplify this? There is IPv6 tunnel mode 'any', and IPv4 tunnel mode 'ipv6/ip' (aka 'sit'). It looks to me that attempts to find some substring match would not make the code much shorter, but definitely less readable. Konstantin Shemyak. >From 42d27db0055c3a114fe6eb86d680bef9ec098ad4 Mon Sep 17 00:00:00 2001 From: Konstantin Shemyak Date: Thu, 12 Nov 2015 20:52:02 +0200 Subject: [PATCH] Tunnel address family is determined from the tunnel mode When the tunnel mode already tells the IP address family, "ip tunnel" command determines it and does not require option "-4"/"-6" to be passed. This makes possible creating both IPv4 and IPv6 tunnels in one batch. Signed-off-by: Konstantin Shemyak --- ip/iptunnel.c | 26 ++++++++++++++++++++++++++ testsuite/tests/ip/tunnel/add_tunnel.t | 14 ++++++++++++++ 2 files changed, 40 insertions(+) create mode 100755 testsuite/tests/ip/tunnel/add_tunnel.t diff --git a/ip/iptunnel.c b/ip/iptunnel.c index b9552edc..096bbe4e 100644 --- a/ip/iptunnel.c +++ b/ip/iptunnel.c @@ -570,8 +570,34 @@ static int do_6rd(int argc, char **argv) return tnl_6rd_ioctl(cmd, medium, &ip6rd); } +static int tunnel_mode_is_ipv6(char *tunnel_mode) { + static const char *ipv6_modes[] = { + "ipv6/ipv6", "ip6ip6", + "vti6", + "ip/ipv6", "ipv4/ipv6", "ipip6", "ip4ip6", + "ip6gre", "gre/ipv6", + "any/ipv6", "any" + }; + int i; + + for (i = 0; i < sizeof(ipv6_modes) / sizeof(ipv6_modes[0]); i++) { + if (strcmp(ipv6_modes[i], tunnel_mode) == 0) + return 1; + } + return 0; +} + int do_iptunnel(int argc, char **argv) { + int i; + + for (i = 0; i < argc - 1; i++) { + if (strcmp(argv[i], "mode") == 0) { + if (tunnel_mode_is_ipv6(argv[i + 1])) + preferred_family = AF_INET6; + break; + } + } switch (preferred_family) { case AF_UNSPEC: preferred_family = AF_INET; diff --git a/testsuite/tests/ip/tunnel/add_tunnel.t b/testsuite/tests/ip/tunnel/add_tunnel.t new file mode 100755 index 00000000..18f6e370 --- /dev/null +++ b/testsuite/tests/ip/tunnel/add_tunnel.t @@ -0,0 +1,14 @@ +#!/bin/sh + +source lib/generic.sh + +TUNNEL_NAME="tunnel_test_ip" + +ts_log "[Testing add/del tunnels]" + +ts_ip "$0" "Add GRE tunnel over IPv4" tunnel add name $TUNNEL_NAME mode gre local 1.1.1.1 remote 2.2.2.2 +ts_ip "$0" "Del GRE tunnel over IPv4" tunnel del $TUNNEL_NAME + +ts_ip "$0" "Add GRE tunnel over IPv6" tunnel add name $TUNNEL_NAME mode ip6gre local dead:beef::1 remote dead:beef::2 +ts_ip "$0" "Del GRE tunnel over IPv6" tunnel del $TUNNEL_NAME + From a96a5d94c6bd58cb455c66a38cff6077841e7aab Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Sun, 29 Nov 2015 12:05:39 -0800 Subject: [PATCH 2/7] iptunnel: cleanup code Make iptunnel pass checkpatch (mostly). --- ip/iptunnel.c | 57 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/ip/iptunnel.c b/ip/iptunnel.c index 096bbe4e..ce9ee320 100644 --- a/ip/iptunnel.c +++ b/ip/iptunnel.c @@ -50,7 +50,8 @@ static void usage(void) static void set_tunnel_proto(struct ip_tunnel_parm *p, int proto) { if (p->iph.protocol && p->iph.protocol != proto) { - fprintf(stderr,"You managed to ask for more than one tunnel mode.\n"); + fprintf(stderr, + "You managed to ask for more than one tunnel mode.\n"); exit(-1); } p->iph.protocol = proto; @@ -91,7 +92,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p) set_tunnel_proto(p, IPPROTO_IPIP); p->i_flags |= VTI_ISVTI; } else { - fprintf(stderr,"Unknown tunnel mode \"%s\"\n", *argv); + fprintf(stderr, + "Unknown tunnel mode \"%s\"\n", *argv); exit(-1); } } else if (strcmp(*argv, "key") == 0) { @@ -144,6 +146,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p) strcmp(*argv, "hoplimit") == 0 || strcmp(*argv, "hlim") == 0) { __u8 uval; + NEXT_ARG(); if (strcmp(*argv, "inherit") != 0) { if (get_u8(&uval, *argv, 0)) @@ -155,6 +158,7 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p) matches(*argv, "dsfield") == 0) { char *dsfield; __u32 uval; + NEXT_ARG(); dsfield = *argv; strsep(&dsfield, "/"); @@ -169,15 +173,17 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p) p->iph.tos |= uval; } } else { - if (strcmp(*argv, "name") == 0) { + if (strcmp(*argv, "name") == 0) NEXT_ARG(); - } else if (matches(*argv, "help") == 0) + else if (matches(*argv, "help") == 0) usage(); + if (p->name[0]) duparg2("name", *argv); strncpy(p->name, *argv, IFNAMSIZ - 1); if (cmd == SIOCCHGTUNNEL && count == 0) { struct ip_tunnel_parm old_p; + memset(&old_p, 0, sizeof(old_p)); if (tnl_get_ioctl(*argv, &old_p)) return -1; @@ -268,8 +274,10 @@ static int do_add(int cmd, int argc, char **argv) return -1; } - if (!(basedev = tnl_defname(&p))) { - fprintf(stderr, "cannot determine tunnel mode (ipip, gre, vti or sit)\n"); + basedev = tnl_defname(&p); + if (!basedev) { + fprintf(stderr, + "cannot determine tunnel mode (ipip, gre, vti or sit)\n"); return -1; } @@ -312,18 +320,18 @@ static void print_tunnel(struct ip_tunnel_parm *p) prl[0].addr = htonl(INADDR_ANY); if (!tnl_prl_ioctl(SIOCGETPRL, p->name, prl)) - for (i = 1; i < sizeof(prl) / sizeof(prl[0]); i++) - { - if (prl[i].addr != htonl(INADDR_ANY)) { - printf(" %s %s ", - (prl[i].flags & PRL_DEFAULT) ? "pdr" : "pr", - format_host(AF_INET, 4, &prl[i].addr, s1, sizeof(s1))); + for (i = 1; i < ARRAY_SIZE(prl); i++) { + if (prl[i].addr != htonl(INADDR_ANY)) { + printf(" %s %s ", + (prl[i].flags & PRL_DEFAULT) ? "pdr" : "pr", + format_host(AF_INET, 4, &prl[i].addr, s1, sizeof(s1))); + } } - } } if (p->link) { const char *n = ll_index_to_name(p->link); + if (n) printf(" dev %s", n); } @@ -381,6 +389,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p) char buf[512]; int err = -1; FILE *fp = fopen("/proc/net/dev", "r"); + if (fp == NULL) { perror("fopen"); return -1; @@ -404,7 +413,8 @@ static int do_tunnels_list(struct ip_tunnel_parm *p) char *ptr; buf[sizeof(buf) - 1] = 0; - if ((ptr = strchr(buf, ':')) == NULL || + ptr = strchr(buf, ':'); + if (ptr == NULL || (*ptr++ = 0, sscanf(buf, "%s", name) != 1)) { fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n"); goto end; @@ -463,7 +473,8 @@ static int do_show(int argc, char **argv) if (parse_args(argc, argv, SIOCGETTUNNEL, &p) < 0) return -1; - if (!(basedev = tnl_defname(&p))) + basedev = tnl_defname(&p); + if (!basedev) return do_tunnels_list(&p); if (tnl_get_ioctl(p.name[0] ? p.name : basedev, &p)) @@ -507,11 +518,13 @@ static int do_prl(int argc, char **argv) strncpy(medium, *argv, IFNAMSIZ-1); devname++; } else { - fprintf(stderr,"Invalid PRL parameter \"%s\"\n", *argv); + fprintf(stderr, + "Invalid PRL parameter \"%s\"\n", *argv); exit(-1); } if (count > 1) { - fprintf(stderr,"One PRL entry at a time\n"); + fprintf(stderr, + "One PRL entry at a time\n"); exit(-1); } argc--; argv++; @@ -557,7 +570,8 @@ static int do_6rd(int argc, char **argv) strncpy(medium, *argv, IFNAMSIZ-1); devname++; } else { - fprintf(stderr,"Invalid 6RD parameter \"%s\"\n", *argv); + fprintf(stderr, + "Invalid 6RD parameter \"%s\"\n", *argv); exit(-1); } argc--; argv++; @@ -570,8 +584,9 @@ static int do_6rd(int argc, char **argv) return tnl_6rd_ioctl(cmd, medium, &ip6rd); } -static int tunnel_mode_is_ipv6(char *tunnel_mode) { - static const char *ipv6_modes[] = { +static int tunnel_mode_is_ipv6(char *tunnel_mode) +{ + static const char * const ipv6_modes[] = { "ipv6/ipv6", "ip6ip6", "vti6", "ip/ipv6", "ipv4/ipv6", "ipip6", "ip4ip6", @@ -580,7 +595,7 @@ static int tunnel_mode_is_ipv6(char *tunnel_mode) { }; int i; - for (i = 0; i < sizeof(ipv6_modes) / sizeof(ipv6_modes[0]); i++) { + for (i = 0; i < ARRAY_SIZE(ipv6_modes); i++) { if (strcmp(ipv6_modes[i], tunnel_mode) == 0) return 1; } From 0f7543322c5fd64d70672578979cf74226f54b64 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 2 Dec 2015 13:50:22 +0100 Subject: [PATCH 3/7] route: ignore RTAX_HOPLIMIT of value -1 Older kernels use -1 internally as indicator to use the sysctl default, but they still export the setting. Newer kernels use 0 to indicate that (which is why the conversion from -1 to 0 was done here), but they also stopped exporting the value. Since the meaning of -1 is clear, treat it equally like default on newer kernels (which is to not print anything). Signed-off-by: Phil Sutter --- ip/iproute.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/ip/iproute.c b/ip/iproute.c index aed1038e..c42ea0b9 100644 --- a/ip/iproute.c +++ b/ip/iproute.c @@ -577,24 +577,23 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) if (mxrta[i] == NULL) continue; - if (i < sizeof(mx_names)/sizeof(char*) && mx_names[i]) - fprintf(fp, " %s", mx_names[i]); - else - fprintf(fp, " metric %d", i); - if (mxlock & (1< Date: Thu, 3 Dec 2015 17:13:48 +0100 Subject: [PATCH 4/7] libnetlink: don't confuse variables in rtnl_talk() There is two variables named 'len' in rtnl_talk. In fact, commit c079e121a73a didn't work. For example, it was possible to trigger a seg fault with this command: $ ip link set gre2 type ip6gre hoplimit 32 Let's rename the argument len to maxlen. Fixes: c079e121a73a ("libnetlink: add size argument to rtnl_talk") Reported-by: Thomas Faivre Signed-off-by: Nicolas Dichtel --- lib/libnetlink.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/libnetlink.c b/lib/libnetlink.c index 922ec2d9..16582144 100644 --- a/lib/libnetlink.c +++ b/lib/libnetlink.c @@ -332,7 +332,7 @@ int rtnl_dump_filter_nc(struct rtnl_handle *rth, } int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, - struct nlmsghdr *answer, size_t len) + struct nlmsghdr *answer, size_t maxlen) { int status; unsigned seq; @@ -415,7 +415,7 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, } else if (!err->error) { if (answer) memcpy(answer, h, - MIN(len, h->nlmsg_len)); + MIN(maxlen, h->nlmsg_len)); return 0; } @@ -427,7 +427,7 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, if (answer) { memcpy(answer, h, - MIN(len, h->nlmsg_len)); + MIN(maxlen, h->nlmsg_len)); return 0; } From 8a23f820457b309fc30d093109ef1e6aa57842c1 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Tue, 8 Dec 2015 12:24:44 -0800 Subject: [PATCH 5/7] vrf: Add support for table names Currently, the table id for VRF devices requires an integer. Convert it to use rtnl_rttable_a2n which handles table names from the iproute2 directory. This also fixes a bug in the original commit where table name are not properly handled. Fixes: 15faa0a30bed ("add support for VRF device") Signed-off-by: David Ahern --- ip/iplink_vrf.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/ip/iplink_vrf.c b/ip/iplink_vrf.c index 913a2892..9b4b7728 100644 --- a/ip/iplink_vrf.c +++ b/ip/iplink_vrf.c @@ -28,12 +28,6 @@ static void explain(void) vrf_explain(stderr); } -static int table_arg(void) -{ - fprintf(stderr,"Error: argument of \"table\" must be 0-32767 and currently unused\n"); - return -1; -} - static int vrf_parse_opt(struct link_util *lu, int argc, char **argv, struct nlmsghdr *n) { @@ -43,9 +37,8 @@ static int vrf_parse_opt(struct link_util *lu, int argc, char **argv, NEXT_ARG(); - table = atoi(*argv); - if (table > 32767) - return table_arg(); + if (rtnl_rttable_a2n(&table, *argv)) + invarg("invalid table ID\n", *argv); addattr32(n, 1024, IFLA_VRF_TABLE, table); } else if (matches(*argv, "help") == 0) { explain(); From b08b5ff128874f94a1bc9dd8e178fa0e57c11c61 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 10 Dec 2015 13:24:51 +0100 Subject: [PATCH 6/7] tc.8: Fix reference to tc-tcindex.8 Just a typo there, it's spelled correctly in SEE ALSO section.. Signed-off-by: Phil Sutter --- man/man8/tc.8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/man8/tc.8 b/man/man8/tc.8 index 6275c4b3..4e99dcad 100644 --- a/man/man8/tc.8 +++ b/man/man8/tc.8 @@ -181,7 +181,7 @@ Match Resource Reservation Protocol (RSVP) packets. .TP tcindex Filter packets based on traffic control index. See -.BR tc-index (8). +.BR tc-tcindex (8). .TP u32 Generic filtering on arbitrary packet data, assisted by syntax to abstract common operations. See From 654ae881de57467642c8c2ed16ffc3a8d57fafa2 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 10 Dec 2015 08:52:10 -0800 Subject: [PATCH 7/7] ip: fix format string when reading statistics The tunnel code was doing sscanf(buf, "%ld", &x) where x was unsigned long. --- ip/ip6tunnel.c | 2 +- ip/iptunnel.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c index 320d2539..1737d884 100644 --- a/ip/ip6tunnel.c +++ b/ip/ip6tunnel.c @@ -354,7 +354,7 @@ static int do_tunnels_list(struct ip6_tnl_parm2 *p) fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n"); goto end; } - if (sscanf(ptr, "%ld%ld%ld%ld%ld%ld%ld%*d%ld%ld%ld%ld%ld%ld%ld", + if (sscanf(ptr, "%lu%lu%lu%lu%lu%lu%lu%*d%lu%lu%lu%lu%lu%lu%lu", &rx_bytes, &rx_packets, &rx_errs, &rx_drops, &rx_fifo, &rx_frame, &rx_multi, &tx_bytes, &tx_packets, &tx_errs, &tx_drops, diff --git a/ip/iptunnel.c b/ip/iptunnel.c index ce9ee320..a3ff99bd 100644 --- a/ip/iptunnel.c +++ b/ip/iptunnel.c @@ -419,7 +419,7 @@ static int do_tunnels_list(struct ip_tunnel_parm *p) fprintf(stderr, "Wrong format for /proc/net/dev. Giving up.\n"); goto end; } - if (sscanf(ptr, "%ld%ld%ld%ld%ld%ld%ld%*d%ld%ld%ld%ld%ld%ld%ld", + if (sscanf(ptr, "%lu%lu%lu%lu%lu%lu%lu%*d%lu%lu%lu%lu%lu%lu%lu", &rx_bytes, &rx_packets, &rx_errs, &rx_drops, &rx_fifo, &rx_frame, &rx_multi, &tx_bytes, &tx_packets, &tx_errs, &tx_drops,