[iproute PATCH 0/2] ip-address: fix type list inconsistencies

[iproute PATCH 0/2] ip-address: fix type list inconsistencies

[Phil Sutter]

The basic problem was differences in type list reported by help output
and stated in man page.

I decided to tackle the problem from both sides:
a) Make sure 'ip addr show' supports matching on all types reported.
b) Add missing types in either list.

This is still rather best-effort, actually things are quite messed up:
- Lists are not sorted, it's easy to miss something.
- The type list is duplicated four times as ip-link help and man page
  contain it, too.
- The kernel supports more types than listed here.
- We can't add but match on all types the kernel supports.

Phil Sutter (2):
  ip-address: Support filtering by slave type, too
  ip-address: Align type list in help and man page

 ip/ipaddress.c           | 58 +++++++++++++++++++++++++++---------------------
 man/man8/ip-address.8.in |  3 +++
 2 files changed, 36 insertions(+), 25 deletions(-)

-- 
2.8.2

[iproute PATCH 1/2] ip-address: Support filtering by slave type, too

[Phil Sutter]

This patch allows to query all interfaces enslaved to a bridge or bond
using the following syntax:

| ip addr show type bridge_slave

Filtering has to be done in userspace since the kernel does not support
filtering on IFLA_INFO_SLAVE_KIND.

Functionality introduced in this patch is not fully complete since it
does not allow to match on type and slave type at the same time, but it
doesn't prevent implementing a dedicated slave_type match, either.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipaddress.c | 52 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 8766530f7fa7c..56f68eb21c0fc 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -61,6 +61,7 @@ static struct
 	int group;
 	int master;
 	char *kind;
+	char *slave_kind;
 } filter;
 
 static int do_link;
@@ -206,18 +207,27 @@ static void print_linkmode(FILE *f, struct rtattr *tb)
 		fprintf(f, "mode %s ", link_modes[mode]);
 }
 
-static char *parse_link_kind(struct rtattr *tb)
+static char *parse_link_kind(struct rtattr *tb, bool slave)
 {
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
+	int attr = slave ? IFLA_INFO_SLAVE_KIND : IFLA_INFO_KIND;
 
 	parse_rtattr_nested(linkinfo, IFLA_INFO_MAX, tb);
 
-	if (linkinfo[IFLA_INFO_KIND])
-		return RTA_DATA(linkinfo[IFLA_INFO_KIND]);
+	if (linkinfo[attr])
+		return RTA_DATA(linkinfo[attr]);
 
 	return "";
 }
 
+static int match_link_kind(struct rtattr **tb, char *kind, bool slave)
+{
+	if (!tb[IFLA_LINKINFO])
+		return -1;
+
+	return strcmp(parse_link_kind(tb[IFLA_LINKINFO], slave), kind);
+}
+
 static void print_linktype(FILE *fp, struct rtattr *tb)
 {
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
@@ -680,16 +690,11 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	} else if (filter.master > 0)
 		return -1;
 
-	if (filter.kind) {
-		if (tb[IFLA_LINKINFO]) {
-			char *kind = parse_link_kind(tb[IFLA_LINKINFO]);
+	if (filter.kind && match_link_kind(tb, filter.kind, 0))
+		return -1;
 
-			if (strcmp(kind, filter.kind))
-				return -1;
-		} else {
-			return -1;
-		}
-	}
+	if (filter.slave_kind && match_link_kind(tb, filter.slave_kind, 1))
+		return -1;
 
 	if (n->nlmsg_type == RTM_DELLINK)
 		fprintf(fp, "Deleted ");
@@ -781,16 +786,11 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	} else if (filter.master > 0)
 		return -1;
 
-	if (filter.kind) {
-		if (tb[IFLA_LINKINFO]) {
-			char *kind = parse_link_kind(tb[IFLA_LINKINFO]);
+	if (filter.kind && match_link_kind(tb, filter.kind, 0))
+		return -1;
 
-			if (strcmp(kind, filter.kind))
-				return -1;
-		} else {
-			return -1;
-		}
-	}
+	if (filter.slave_kind && match_link_kind(tb, filter.slave_kind, 1))
+		return -1;
 
 	if (n->nlmsg_type == RTM_DELLINK)
 		fprintf(fp, "Deleted ");
@@ -1621,8 +1621,16 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
 				invarg("Device does not exist\n", *argv);
 			filter.master = ifindex;
 		} else if (strcmp(*argv, "type") == 0) {
+			int soff;
+
 			NEXT_ARG();
-			filter.kind = *argv;
+			soff = strlen(*argv) - strlen("_slave");
+			if (!strcmp(*argv + soff, "_slave")) {
+				(*argv)[soff] = '\0';
+				filter.slave_kind = *argv;
+			} else {
+				filter.kind = *argv;
+			}
 		} else {
 			if (strcmp(*argv, "dev") == 0) {
 				NEXT_ARG();
-- 
2.8.2

Re: [iproute PATCH 1/2] ip-address: Support filtering by slave type, too

[David Ahern]

On 6/28/16 7:07 AM, Phil Sutter wrote:
> This patch allows to query all interfaces enslaved to a bridge or bond
> using the following syntax:

works for vrf's too.

Re: [iproute PATCH 1/2] ip-address: Support filtering by slave type, too

[Stephen Hemminger]

On Tue, 28 Jun 2016 15:07:16 +0200
Phil Sutter <phil@nwl.cc> wrote:

> +static int match_link_kind(struct rtattr **tb, char *kind, bool slave)

const char *kind ??

> +{
> +	if (!tb[IFLA_LINKINFO])
> +		return -1;
> +
> +	return strcmp(parse_link_kind(tb[IFLA_LINKINFO], slave), kind);
> +}
> +

[iproute PATCH] ip-address: constify match_link_kind arg

[Phil Sutter]

Since the function won't ever change the data 'kind' is pointing at, it
can sanely be made const.

Fixes: e0513807f6dbb ("ip-address: Support filtering by slave type, too")
Suggested-by: Stephen Hemminger <shemming@brocade.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipaddress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index d4d649505e15a..02bc9029d6b20 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -220,7 +220,7 @@ static char *parse_link_kind(struct rtattr *tb, bool slave)
 	return "";
 }
 
-static int match_link_kind(struct rtattr **tb, char *kind, bool slave)
+static int match_link_kind(struct rtattr **tb, const char *kind, bool slave)
 {
 	if (!tb[IFLA_LINKINFO])
 		return -1;
-- 
2.8.2

[iproute PATCH 2/2] ip-address: Align type list in help and man page

[Phil Sutter]

This adds missing entries on both sides until they are identical.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipaddress.c           | 6 +++---
 man/man8/ip-address.8.in | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 56f68eb21c0fc..d4d649505e15a 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -95,9 +95,9 @@ static void usage(void)
 	fprintf(stderr, "LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n");
 	fprintf(stderr, "LFT := forever | SECONDS\n");
 	fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
-	fprintf(stderr, "          bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n");
-	fprintf(stderr, "          gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n");
-	fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf }\n");
+	fprintf(stderr, "          bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan | lowpan |\n");
+	fprintf(stderr, "          gre | gretap | ip6gre | ip6gretap | vti | nlmon | can |\n");
+	fprintf(stderr, "          bond_slave | ipvlan | geneve | bridge_slave | vrf | hsr}\n");
 
 	exit(-1);
 }
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index 8d34adb336af4..3cbe4181f7e36 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -98,7 +98,9 @@ ip-address \- protocol address management
 .ti -8
 .IR TYPE " := [ "
 .BR bridge " | "
+.BR bridge_slave " |"
 .BR bond " | "
+.BR bond_slave " |"
 .BR can " | "
 .BR dummy " | "
 .BR hsr " | "
@@ -118,6 +120,7 @@ ip-address \- protocol address management
 .BR ip6gre " |"
 .BR ip6gretap " |"
 .BR vti " |"
+.BR vrf " |"
 .BR nlmon " |"
 .BR ipvlan " |"
 .BR lowpan " |"
-- 
2.8.2