[PATCH iproute2-next 0/9] ipaddress: Make print_linkinfo_brief() static

[PATCH iproute2-next 0/9] ipaddress: Make print_linkinfo_brief() static

[Serhey Popovych]

With this series I propose to make print_linkinfo_brief() static in
favor of print_linkinfo() as single point for linkinfo printing.

Changes presented with this series tested using following script:

\#!/bin/bash

iproute2_dir="$1"
iface='eth0.2'

pushd "$iproute2_dir" &>/dev/null

for i in new old; do
	DIR="/tmp/$i"
	mkdir -p "$DIR"

	ln -snf ip.$i ip/ip

	# normal
	ip/ip link show                  >"$DIR/ip-link-show"
	ip/ip -4 addr show               >"$DIR/ip-4-addr-show"
	ip/ip -6 addr show               >"$DIR/ip-6-addr-show"
	ip/ip addr show dev "$iface"     >"$DIR/ip-addr-show-$iface"

	# brief
	ip/ip -br link show              >"$DIR/ip-br-link-show"
	ip/ip -br -4 addr show           >"$DIR/ip-br-4-addr-show"
	ip/ip -br -6 addr show           >"$DIR/ip-br-6-addr-show"
	ip/ip -br addr show dev "$iface" >"$DIR/ip-br-addr-show-$iface"
done
rm -f ip/ip

diff -urN /tmp/{old,new} |sed -n -Ee'/^(-{3}|\+{3})[[:space:]]+/!p'
rc=$?

popd &>/dev/null
exit $rc

Expected results : <no output>
Actual results   : <no output>

Although test coverage is far from ideal in my opinion it covers most
important aspects of the changes presented by the series.

All this work is done in prepare of iplink_get() enhancements to support
attribute parse that finally will be used to simplify ip/tunnel
RTM_GETLINK code.

As always reviews, comments, suggestions and criticism is welcome.

Thanks,
Serhii

Serhey Popovych (9):
  ipaddress: Abstract IFA_LABEL matching code
  ipaddress: ll_map: Replace ll_idx_n2a() with ll_index_to_name()
  utils: Reimplement ll_idx_n2a() and introduce ll_idx_a2n()
  ipaddress: Improve print_linkinfo()
  ipaddress: Simplify print_linkinfo_brief() and it's usage
  lib: Correct object file dependencies
  utils: Introduce and use get_ifname_rta()
  utils: Introduce and use print_name_and_link() to print name@link
  ipaddress: Make print_linkinfo_brief() static

 bridge/link.c    |   21 ++---
 include/ll_map.h |    4 +-
 include/utils.h  |    5 ++
 ip/ip_common.h   |    2 -
 ip/ipaddress.c   |  224 ++++++++++++++----------------------------------------
 ip/iplink.c      |    5 +-
 lib/Makefile     |    4 +-
 lib/ll_map.c     |   31 +++++---
 lib/utils.c      |   68 +++++++++++++++++
 9 files changed, 162 insertions(+), 202 deletions(-)

-- 
1.7.10.4

[PATCH iproute2-next 1/9] ipaddress: Abstract IFA_LABEL matching code

[Serhey Popovych]

There at least two places in ip/ipaddress.c where we match IFA_LABEL
against filter.label if that is given.

Get rid of "common" if () statement for inet_addr_match_rta() and
ifa_label_match_rta(): it is not common because first will check for
filter.pfx.family != AF_UNSPEC inside and second for filter.label being
non NULL.

This allows us to further simplify down code and prepare for
ll_idx_n2a() replacement with ll_index_to_name() without 80 columns
checkpatch notice.

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 4707c2b..38ef5d7 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1470,6 +1470,22 @@ static int get_filter(const char *arg)
 	return 0;
 }
 
+static int ifa_label_match_rta(int ifindex, const struct rtattr *rta)
+{
+	const char *label;
+	SPRINT_BUF(b1);
+
+	if (!filter.label)
+		return 0;
+
+	if (rta)
+		label = RTA_DATA(rta);
+	else
+		label = ll_idx_n2a(ifindex, b1);
+
+	return fnmatch(filter.label, label, 0);
+}
+
 int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		   void *arg)
 {
@@ -1508,21 +1524,13 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		return 0;
 	if ((filter.flags ^ ifa_flags) & filter.flagmask)
 		return 0;
-	if (filter.label) {
-		SPRINT_BUF(b1);
-		const char *label;
-
-		if (rta_tb[IFA_LABEL])
-			label = RTA_DATA(rta_tb[IFA_LABEL]);
-		else
-			label = ll_idx_n2a(ifa->ifa_index, b1);
-		if (fnmatch(filter.label, label, 0) != 0)
-			return 0;
-	}
 
 	if (filter.family && filter.family != ifa->ifa_family)
 		return 0;
 
+	if (ifa_label_match_rta(ifa->ifa_index, rta_tb[IFA_LABEL]))
+		return 0;
+
 	if (inet_addr_match_rta(&filter.pfx, rta_tb[IFA_LOCAL]))
 		return 0;
 
@@ -1878,25 +1886,14 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
 
 			if ((filter.flags ^ ifa_flags) & filter.flagmask)
 				continue;
-			if (filter.pfx.family || filter.label) {
-				struct rtattr *rta =
-					tb[IFA_LOCAL] ? : tb[IFA_ADDRESS];
-
-				if (inet_addr_match_rta(&filter.pfx, rta))
-					continue;
-
-				if (filter.label) {
-					SPRINT_BUF(b1);
-					const char *label;
-
-					if (tb[IFA_LABEL])
-						label = RTA_DATA(tb[IFA_LABEL]);
-					else
-						label = ll_idx_n2a(ifa->ifa_index, b1);
-					if (fnmatch(filter.label, label, 0) != 0)
-						continue;
-				}
-			}
+
+			if (ifa_label_match_rta(ifa->ifa_index, tb[IFA_LABEL]))
+				continue;
+
+			if (!tb[IFA_LOCAL])
+				tb[IFA_LOCAL] = tb[IFA_ADDRESS];
+			if (inet_addr_match_rta(&filter.pfx, tb[IFA_LOCAL]))
+				continue;
 
 			ok = 1;
 			break;
-- 
1.7.10.4

[PATCH iproute2-next 2/9] ipaddress: ll_map: Replace ll_idx_n2a() with ll_index_to_name()

[Serhey Popovych]

There is no reentrancy as well as deferred result usage for all cases
where ll_idx_n2a() being used: it is safe to use ll_index_to_name() that
internally calls ll_idx_n2a() with static buffer to hold result.

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 38ef5d7..366a304 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -978,14 +978,13 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
 	if (tb[IFLA_LINK]) {
-		SPRINT_BUF(b1);
 		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
 
 		if (iflink == 0) {
 			snprintf(buf, sizeof(buf), "%s@NONE", name);
 			print_null(PRINT_JSON, "link", NULL, NULL);
 		} else {
-			const char *link = ll_idx_n2a(iflink, b1);
+			const char *link = ll_index_to_name(iflink);
 
 			print_string(PRINT_JSON, "link", NULL, link);
 			snprintf(buf, sizeof(buf), "%s@%s", name, link);
@@ -1122,12 +1121,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
 				print_int(PRINT_ANY,
 					  "link_index", "@if%d: ", iflink);
 			else {
-				SPRINT_BUF(b1);
-
 				print_string(PRINT_ANY,
 					     "link",
 					     "@%s: ",
-					     ll_idx_n2a(iflink, b1));
+					     ll_index_to_name(iflink));
 				m_flag = ll_index_to_flags(iflink);
 				m_flag = !(m_flag & IFF_UP);
 			}
@@ -1149,12 +1146,12 @@ int print_linkinfo(const struct sockaddr_nl *who,
 			     "qdisc %s ",
 			     rta_getattr_str(tb[IFLA_QDISC]));
 	if (tb[IFLA_MASTER]) {
-		SPRINT_BUF(b1);
+		int master = rta_getattr_u32(tb[IFLA_MASTER]);
 
 		print_string(PRINT_ANY,
 			     "master",
 			     "master %s ",
-			     ll_idx_n2a(rta_getattr_u32(tb[IFLA_MASTER]), b1));
+			     ll_index_to_name(master));
 	}
 
 	if (tb[IFLA_OPERSTATE])
@@ -1473,7 +1470,6 @@ static int get_filter(const char *arg)
 static int ifa_label_match_rta(int ifindex, const struct rtattr *rta)
 {
 	const char *label;
-	SPRINT_BUF(b1);
 
 	if (!filter.label)
 		return 0;
@@ -1481,7 +1477,7 @@ static int ifa_label_match_rta(int ifindex, const struct rtattr *rta)
 	if (rta)
 		label = RTA_DATA(rta);
 	else
-		label = ll_idx_n2a(ifindex, b1);
+		label = ll_index_to_name(ifindex);
 
 	return fnmatch(filter.label, label, 0);
 }
-- 
1.7.10.4

[PATCH iproute2-next 3/9] utils: Reimplement ll_idx_n2a() and introduce ll_idx_a2n()

[Serhey Popovych]

Now all users of ll_idx_n2a() replaced with ll_index_to_name() we can
move it's functionality to ll_index_to_name() and implement index to
name conversion using snprintf() and "if%u".

Use %u specifier in "if%..." template consistently: network device
indexes are always greather than zero.

Also introduce ll_idx_n2a() conterpart: ll_idx_a2n() that is used
to translate name of the "if%u" form to index using sscanf().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/ll_map.h |    4 +++-
 lib/ll_map.c     |   31 +++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/include/ll_map.h b/include/ll_map.h
index c8474e6..8546ff9 100644
--- a/include/ll_map.h
+++ b/include/ll_map.h
@@ -8,9 +8,11 @@ int ll_remember_index(const struct sockaddr_nl *who,
 void ll_init_map(struct rtnl_handle *rth);
 unsigned ll_name_to_index(const char *name);
 const char *ll_index_to_name(unsigned idx);
-const char *ll_idx_n2a(unsigned idx, char *buf);
 int ll_index_to_type(unsigned idx);
 int ll_index_to_flags(unsigned idx);
 unsigned namehash(const char *str);
 
+const char *ll_idx_n2a(unsigned int idx);
+unsigned int ll_idx_a2n(const char *name);
+
 #endif /* __LL_MAP_H__ */
diff --git a/lib/ll_map.c b/lib/ll_map.c
index f65614f..0afe689 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -136,8 +136,26 @@ int ll_remember_index(const struct sockaddr_nl *who,
 	return 0;
 }
 
-const char *ll_idx_n2a(unsigned idx, char *buf)
+const char *ll_idx_n2a(unsigned int idx)
 {
+	static char buf[IFNAMSIZ];
+
+	snprintf(buf, sizeof(buf), "if%u", idx);
+	return buf;
+}
+
+unsigned int ll_idx_a2n(const char *name)
+{
+	unsigned int idx;
+
+	if (sscanf(name, "if%u", &idx) != 1)
+		return 0;
+	return idx;
+}
+
+const char *ll_index_to_name(unsigned int idx)
+{
+	static char buf[IFNAMSIZ];
 	const struct ll_cache *im;
 
 	if (idx == 0)
@@ -148,18 +166,11 @@ const char *ll_idx_n2a(unsigned idx, char *buf)
 		return im->name;
 
 	if (if_indextoname(idx, buf) == NULL)
-		snprintf(buf, IFNAMSIZ, "if%d", idx);
+		snprintf(buf, IFNAMSIZ, "if%u", idx);
 
 	return buf;
 }
 
-const char *ll_index_to_name(unsigned idx)
-{
-	static char nbuf[IFNAMSIZ];
-
-	return ll_idx_n2a(idx, nbuf);
-}
-
 int ll_index_to_type(unsigned idx)
 {
 	const struct ll_cache *im;
@@ -196,7 +207,7 @@ unsigned ll_name_to_index(const char *name)
 
 	idx = if_nametoindex(name);
 	if (idx == 0)
-		sscanf(name, "if%u", &idx);
+		idx = ll_idx_a2n(name);
 	return idx;
 }
 
-- 
1.7.10.4

[PATCH iproute2-next 4/9] ipaddress: Improve print_linkinfo()

[Serhey Popovych]

There are few places to improve:

  1) return -1 when entry is filtered instead of zero, which means
     accept entry: ipaddress_list_flush_or_save() the only user of this

  2) use ll_idx_n2a() as last resort to translate name to index for
     "should never happen" cases when cache shouldn't be considered

  3) replace open coded access to IFLA_IFNAME attribute data by
     RTA_DATA() with rta_getattr_str()

  4) simplify ifname printing since name is never NULL, thanks to (2).

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 366a304..02197e2 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -943,14 +943,14 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
 	if (tb[IFLA_IFNAME] == NULL) {
 		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
-		name = "<nil>";
+		name = ll_idx_n2a(ifi->ifi_index);
 	} else {
 		name = rta_getattr_str(tb[IFLA_IFNAME]);
 	}
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
-	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
+	    fnmatch(filter.label, name, 0))
 		return -1;
 
 	if (tb[IFLA_GROUP]) {
@@ -1052,6 +1052,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
 	int len = n->nlmsg_len;
+	const char *name;
 	unsigned int m_flag = 0;
 
 	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
@@ -1062,18 +1063,22 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		return -1;
 
 	if (filter.ifindex && ifi->ifi_index != filter.ifindex)
-		return 0;
+		return -1;
 	if (filter.up && !(ifi->ifi_flags&IFF_UP))
-		return 0;
+		return -1;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL)
+	if (tb[IFLA_IFNAME] == NULL) {
 		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
+		name = ll_idx_n2a(ifi->ifi_index);
+	} else {
+		name = rta_getattr_str(tb[IFLA_IFNAME]);
+	}
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
-	    fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
-		return 0;
+	    fnmatch(filter.label, name, 0))
+		return -1;
 
 	if (tb[IFLA_GROUP]) {
 		int group = rta_getattr_u32(tb[IFLA_GROUP]);
@@ -1100,16 +1105,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
 	print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
-	if (tb[IFLA_IFNAME]) {
-		print_color_string(PRINT_ANY,
-				   COLOR_IFNAME,
-				   "ifname", "%s",
-				   rta_getattr_str(tb[IFLA_IFNAME]));
-	} else {
-		print_null(PRINT_JSON, "ifname", NULL, NULL);
-		print_color_null(PRINT_FP, COLOR_IFNAME,
-				 "ifname", "%s", "<nil>");
-	}
+	print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", name);
 
 	if (tb[IFLA_LINK]) {
 		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-- 
1.7.10.4

[PATCH iproute2-next 5/9] ipaddress: Simplify print_linkinfo_brief() and it's usage

[Serhey Popovych]

Simplify calling code in ipaddr_list_flush_or_save() by introducing
intermediate variable of @struct nlmsghdr, drop duplicated code:
print_linkinfo_brief() never returns values other than <= 0 so we can
move print_selected_addrinfo() outside of each block.

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 02197e2..89e3cc4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -918,7 +918,7 @@ static void print_link_stats(FILE *fp, struct nlmsghdr *n)
 }
 
 int print_linkinfo_brief(const struct sockaddr_nl *who,
-				struct nlmsghdr *n, void *arg)
+			 struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = (FILE *)arg;
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
@@ -2177,24 +2177,21 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
 		ipaddr_filter(&linfo, ainfo);
 
 	for (l = linfo.head; l; l = l->next) {
-		int res = 0;
-		struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
+		struct nlmsghdr *n = &l->h;
+		struct ifinfomsg *ifi = NLMSG_DATA(n);
+		int res;
 
 		open_json_object(NULL);
-		if (brief) {
-			if (print_linkinfo_brief(NULL, &l->h, stdout) == 0)
-				if (filter.family != AF_PACKET)
-					print_selected_addrinfo(ifi,
-								ainfo->head,
-								stdout);
-		} else if (no_link ||
-			   (res = print_linkinfo(NULL, &l->h, stdout)) >= 0) {
-			if (filter.family != AF_PACKET)
-				print_selected_addrinfo(ifi,
-							ainfo->head, stdout);
-			if (res > 0 && !do_link && show_stats)
-				print_link_stats(stdout, &l->h);
-		}
+		if (brief)
+			res = print_linkinfo_brief(NULL, n, stdout);
+		else if (no_link)
+			res = 0;
+		else
+			res = print_linkinfo(NULL, n, stdout);
+		if (res >= 0 && filter.family != AF_PACKET)
+			print_selected_addrinfo(ifi, ainfo->head, stdout);
+		if (res > 0 && !do_link && show_stats)
+			print_link_stats(stdout, n);
 		close_json_object();
 	}
 	fflush(stdout);
-- 
1.7.10.4

[PATCH iproute2-next 6/9] lib: Correct object file dependencies

[Serhey Popovych]

Neither internal libnetlink nor libgenl depends on ll_map.o: prepare for
upcoming changes that brings much more cleaner dependency between
utils.o and ll_map.o.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 lib/Makefile |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index 7b34ed5..bab8cbf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -3,11 +3,11 @@ include ../config.mk
 
 CFLAGS += -fPIC
 
-UTILOBJ = utils.o rt_names.o ll_types.o ll_proto.o ll_addr.o \
+UTILOBJ = utils.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
 	inet_proto.o namespace.o json_writer.o json_print.o \
 	names.o color.o bpf.o exec.o fs.o
 
-NLOBJ=libgenl.o ll_map.o libnetlink.o
+NLOBJ=libgenl.o libnetlink.o
 
 all: libnetlink.a libutil.a
 
-- 
1.7.10.4

[PATCH iproute2-next 7/9] utils: Introduce and use get_ifname_rta()

[Serhey Popovych]

Be consistent in handling of IFLA_IFNAME attribute in all places: if
there is no attribute report bug to stderr and use ll_idx_n2a() as
last measure to get name in "if%u" format instead of "<nil>".

Use check_ifname() to validate network device name: this catches both
unexpected return from kernel and ll_idx_n2a().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/link.c   |    8 ++++----
 include/utils.h |    1 +
 ip/ipaddress.c  |   20 ++++++++------------
 lib/utils.c     |   19 +++++++++++++++++++
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index 870ebe0..a11cbb1 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -99,9 +99,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = arg;
-	int len = n->nlmsg_len;
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
+	int len = n->nlmsg_len;
+	const char *name;
 
 	len -= NLMSG_LENGTH(sizeof(*ifi));
 	if (len < 0) {
@@ -117,10 +118,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 
 	parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED);
 
-	if (tb[IFLA_IFNAME] == NULL) {
-		fprintf(stderr, "BUG: nil ifname\n");
+	name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
+	if (!name)
 		return -1;
-	}
 
 	if (n->nlmsg_type == RTM_DELLINK)
 		fprintf(fp, "Deleted ");
diff --git a/include/utils.h b/include/utils.h
index f81928a..ced5b7b 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -173,6 +173,7 @@ void duparg(const char *, const char *) __attribute__((noreturn));
 void duparg2(const char *, const char *) __attribute__((noreturn));
 int check_ifname(const char *);
 int get_ifname(char *, const char *);
+const char *get_ifname_rta(int ifindex, const struct rtattr *rta);
 int matches(const char *arg, const char *pattern);
 int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits);
 int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rta);
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 89e3cc4..458d47c 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -941,12 +941,10 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 		return -1;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL) {
-		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
-		name = ll_idx_n2a(ifi->ifi_index);
-	} else {
-		name = rta_getattr_str(tb[IFLA_IFNAME]);
-	}
+
+	name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
+	if (!name)
+		return -1;
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
@@ -1068,12 +1066,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		return -1;
 
 	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-	if (tb[IFLA_IFNAME] == NULL) {
-		fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
-		name = ll_idx_n2a(ifi->ifi_index);
-	} else {
-		name = rta_getattr_str(tb[IFLA_IFNAME]);
-	}
+
+	name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
+	if (!name)
+		return -1;
 
 	if (filter.label &&
 	    (!filter.family || filter.family == AF_PACKET) &&
diff --git a/lib/utils.c b/lib/utils.c
index 8e15625..22fe637 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -871,6 +871,25 @@ int get_ifname(char *buf, const char *name)
 	return ret;
 }
 
+const char *get_ifname_rta(int ifindex, const struct rtattr *rta)
+{
+	const char *name;
+
+	if (rta) {
+		name = rta_getattr_str(rta);
+	} else {
+		fprintf(stderr,
+			"BUG: device with ifindex %d has nil ifname\n",
+			ifindex);
+		name = ll_idx_n2a(ifindex);
+	}
+
+	if (check_ifname(name))
+		return NULL;
+
+	return name;
+}
+
 int matches(const char *cmd, const char *pattern)
 {
 	int len = strlen(cmd);
-- 
1.7.10.4

[PATCH iproute2-next 8/9] utils: Introduce and use print_name_and_link() to print name@link

[Serhey Popovych]

There is at least three places implementing same things: two in
ipaddress.c print_linkinfo() & print_linkinfo_brief() and one in
bridge/link.c.

They are diverge from each other very little: bridge/link.c does not
support JSON output at the moment and print_linkinfo_brief() does not
handle IFLA_LINK_NETNS case.

Introduce and use print_name_and_link() routine to handle name@link
output in all possible variations; respect IFLA_LINK_NETNS attribute to
handle case when link is in different namespace; use ll_idx_n2a() for
interface name instead of "<nil>" to share logic with other code (e.g.
ll_name_to_index() and ll_index_to_name()) supporting such template.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/link.c   |   13 +++----------
 include/utils.h |    4 ++++
 ip/ipaddress.c  |   44 ++------------------------------------------
 lib/utils.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index a11cbb1..90c9734 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -125,20 +125,13 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (n->nlmsg_type == RTM_DELLINK)
 		fprintf(fp, "Deleted ");
 
-	fprintf(fp, "%d: %s ", ifi->ifi_index,
-		tb[IFLA_IFNAME] ? rta_getattr_str(tb[IFLA_IFNAME]) : "<nil>");
+	fprintf(fp, "%d: ", ifi->ifi_index);
+
+	print_name_and_link("%s: ", COLOR_NONE, name, tb);
 
 	if (tb[IFLA_OPERSTATE])
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
 
-	if (tb[IFLA_LINK]) {
-		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-
-		fprintf(fp, "@%s: ",
-			iflink ? ll_index_to_name(iflink) : "NONE");
-	} else
-		fprintf(fp, ": ");
-
 	print_link_flags(fp, ifi->ifi_flags);
 
 	if (tb[IFLA_MTU])
diff --git a/include/utils.h b/include/utils.h
index ced5b7b..fe8f6d4 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -12,6 +12,7 @@
 #include "libnetlink.h"
 #include "ll_map.h"
 #include "rtm_map.h"
+#include "json_print.h"
 
 extern int preferred_family;
 extern int human_readable;
@@ -240,6 +241,9 @@ void print_escape_buf(const __u8 *buf, size_t len, const char *escape);
 int print_timestamp(FILE *fp);
 void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n);
 
+unsigned int print_name_and_link(const char *fmt, enum color_attr color,
+				 const char *name, struct rtattr *tb[]);
+
 #define BIT(nr)                 (1UL << (nr))
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 458d47c..65c2559 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -925,7 +925,6 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	struct rtattr *tb[IFLA_MAX+1];
 	int len = n->nlmsg_len;
 	const char *name;
-	char buf[32] = { 0, };
 	unsigned int m_flag = 0;
 
 	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
@@ -975,25 +974,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	if (n->nlmsg_type == RTM_DELLINK)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
-	if (tb[IFLA_LINK]) {
-		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-
-		if (iflink == 0) {
-			snprintf(buf, sizeof(buf), "%s@NONE", name);
-			print_null(PRINT_JSON, "link", NULL, NULL);
-		} else {
-			const char *link = ll_index_to_name(iflink);
-
-			print_string(PRINT_JSON, "link", NULL, link);
-			snprintf(buf, sizeof(buf), "%s@%s", name, link);
-			m_flag = ll_index_to_flags(iflink);
-			m_flag = !(m_flag & IFF_UP);
-		}
-	} else
-		snprintf(buf, sizeof(buf), "%s", name);
-
-	print_string(PRINT_FP, NULL, "%-16s ", buf);
-	print_string(PRINT_JSON, "ifname", NULL, name);
+	m_flag = print_name_and_link("%-16s ", COLOR_NONE, name, tb);
 
 	if (tb[IFLA_OPERSTATE])
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
@@ -1101,29 +1082,8 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
 	print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
-	print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", name);
-
-	if (tb[IFLA_LINK]) {
-		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
 
-		if (iflink == 0)
-			print_null(PRINT_ANY, "link", "@%s: ", "NONE");
-		else {
-			if (tb[IFLA_LINK_NETNSID])
-				print_int(PRINT_ANY,
-					  "link_index", "@if%d: ", iflink);
-			else {
-				print_string(PRINT_ANY,
-					     "link",
-					     "@%s: ",
-					     ll_index_to_name(iflink));
-				m_flag = ll_index_to_flags(iflink);
-				m_flag = !(m_flag & IFF_UP);
-			}
-		}
-	} else {
-		print_string(PRINT_FP, NULL, ": ", NULL);
-	}
+	m_flag = print_name_and_link("%s: ", COLOR_IFNAME, name, tb);
 	print_link_flags(fp, ifi->ifi_flags, m_flag);
 
 	if (tb[IFLA_MTU])
diff --git a/lib/utils.c b/lib/utils.c
index 22fe637..106f8e2 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -33,6 +33,7 @@
 
 #include "rt_names.h"
 #include "utils.h"
+#include "ll_map.h"
 #include "namespace.h"
 
 int resolve_hosts;
@@ -1260,6 +1261,54 @@ int print_timestamp(FILE *fp)
 	return 0;
 }
 
+unsigned int print_name_and_link(const char *fmt, enum color_attr color,
+				 const char *name, struct rtattr *tb[])
+{
+	const char *link = NULL;
+	unsigned int m_flag = 0;
+	SPRINT_BUF(b1);
+
+	if (tb[IFLA_LINK]) {
+		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
+
+		if (iflink) {
+			if (tb[IFLA_LINK_NETNSID]) {
+				if (is_json_context()) {
+					print_int(PRINT_JSON,
+						  "link_index", NULL, iflink);
+				} else {
+					link = ll_idx_n2a(iflink);
+				}
+			} else {
+				link = ll_index_to_name(iflink);
+
+				if (is_json_context()) {
+					print_string(PRINT_JSON,
+						     "link", NULL, link);
+					link = NULL;
+				}
+
+				m_flag = ll_index_to_flags(iflink);
+				m_flag = !(m_flag & IFF_UP);
+			}
+		} else {
+			if (is_json_context())
+				print_null(PRINT_JSON, "link", NULL, NULL);
+			else
+				link = "NONE";
+		}
+
+		if (link) {
+			snprintf(b1, sizeof(b1), "%s@%s", name, link);
+			name = b1;
+		}
+	}
+
+	print_color_string(PRINT_ANY, color, "ifname", fmt, name);
+
+	return m_flag;
+}
+
 int cmdlineno;
 
 /* Like glibc getline but handle continuation lines and comments */
-- 
1.7.10.4

Re: [PATCH iproute2-next 8/9] utils: Introduce and use print_name_and_link() to print name@link

[David Ahern]

On 2/5/18 12:49 PM, Serhey Popovych wrote:
> There is at least three places implementing same things: two in
> ipaddress.c print_linkinfo() & print_linkinfo_brief() and one in
> bridge/link.c.
> 
> They are diverge from each other very little: bridge/link.c does not
> support JSON output at the moment and print_linkinfo_brief() does not
> handle IFLA_LINK_NETNS case.
> 
> Introduce and use print_name_and_link() routine to handle name@link
> output in all possible variations; respect IFLA_LINK_NETNS attribute to
> handle case when link is in different namespace; use ll_idx_n2a() for
> interface name instead of "<nil>" to share logic with other code (e.g.
> ll_name_to_index() and ll_index_to_name()) supporting such template.
> 
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
>  bridge/link.c   |   13 +++----------
>  include/utils.h |    4 ++++
>  ip/ipaddress.c  |   44 ++------------------------------------------
>  lib/utils.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 52 deletions(-)
> 

This patch is causing a diff on my system:

# ip  -br add sh > /tmp/1
# ip/ip  -br add sh > /tmp/2
# diff /tmp/1 /tmp/2
8c8
< veth-out@br3     UP             fe80::18a8:89ff:fee7:55c5/64
---
> veth-out@if7     UP             fe80::18a8:89ff:fee7:55c5/64

So the current ip resolves ifindex 7 to br3:

# ip li sh dev br3
7: br3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master
vrf3 state UP mode DEFAULT group default qlen 1000

where your patch causes if%d to be printed.

Re: [PATCH iproute2-next 8/9] utils: Introduce and use print_name_and_link() to print name@link

[Serhey Popovych]

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

David Ahern wrote:
> On 2/5/18 12:49 PM, Serhey Popovych wrote:
>> There is at least three places implementing same things: two in
>> ipaddress.c print_linkinfo() & print_linkinfo_brief() and one in
>> bridge/link.c.
>>
>> They are diverge from each other very little: bridge/link.c does not
>> support JSON output at the moment and print_linkinfo_brief() does not
>> handle IFLA_LINK_NETNS case.
>>
>> Introduce and use print_name_and_link() routine to handle name@link
>> output in all possible variations; respect IFLA_LINK_NETNS attribute to
>> handle case when link is in different namespace; use ll_idx_n2a() for
>> interface name instead of "<nil>" to share logic with other code (e.g.
>> ll_name_to_index() and ll_index_to_name()) supporting such template.
>>
>> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
>> ---
>>  bridge/link.c   |   13 +++----------
>>  include/utils.h |    4 ++++
>>  ip/ipaddress.c  |   44 ++------------------------------------------
>>  lib/utils.c     |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 58 insertions(+), 52 deletions(-)
>>
> 
> This patch is causing a diff on my system:
> 
> # ip  -br add sh > /tmp/1
> # ip/ip  -br add sh > /tmp/2
> # diff /tmp/1 /tmp/2
> 8c8
> < veth-out@br3     UP             fe80::18a8:89ff:fee7:55c5/64
> ---
>> veth-out@if7     UP             fe80::18a8:89ff:fee7:55c5/64
> 
> So the current ip resolves ifindex 7 to br3:
> 
> # ip li sh dev br3
> 7: br3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master
> vrf3 state UP mode DEFAULT group default qlen 1000
> 
> where your patch causes if%d to be printed.
> 

That's interesting. I guess output comes from ll_idx_n2a() in this
change when both IFLA_LINK and IFLA_LINK_NETNS is seen.

My guess about this case is following:

  1) veth-out is of "veth" rtnl kind. (ip -d li sh dev veth-out).

  2) according to drivers/net/veth.c veth_get_iflink() and
     veth_get_link_net() IFLA_LINK and IFLA_LINK_NETNS are taken
     from peer device.

  3) seeing @br3 in current ip output looks confusing according to (2)
     as veth do not link to something other than it's peer that is in
     different network namespace.

From (3) I guess @br3 is incorrect value and caused by missing
IFLA_LINK_NETNS handling in old print_linkinfo_brief(): it always
calls ll_index_to_name().

Could you provide some more details about your setup if above guess is
wrong.

Especially following ones:

  1) ip -d li sh dev veth-out  (get the rtnl kind)

  2) ip -d li sh dev br3 (get the rtnl kind)

  3) uname -r or cat /proc/version


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

Re: [PATCH iproute2-next 8/9] utils: Introduce and use print_name_and_link() to print name@link

[David Ahern]

On 2/7/18 12:36 AM, Serhey Popovych wrote:
>>
>> This patch is causing a diff on my system:
>>
>> # ip  -br add sh > /tmp/1
>> # ip/ip  -br add sh > /tmp/2
>> # diff /tmp/1 /tmp/2
>> 8c8
>> < veth-out@br3     UP             fe80::18a8:89ff:fee7:55c5/64
>> ---
>>> veth-out@if7     UP             fe80::18a8:89ff:fee7:55c5/64
>>
>> So the current ip resolves ifindex 7 to br3:
>>
>> # ip li sh dev br3
>> 7: br3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master
>> vrf3 state UP mode DEFAULT group default qlen 1000
>>
>> where your patch causes if%d to be printed.
>>
> 
> That's interesting. I guess output comes from ll_idx_n2a() in this
> change when both IFLA_LINK and IFLA_LINK_NETNS is seen.
> 
> My guess about this case is following:
> 
>   1) veth-out is of "veth" rtnl kind. (ip -d li sh dev veth-out).
> 
>   2) according to drivers/net/veth.c veth_get_iflink() and
>      veth_get_link_net() IFLA_LINK and IFLA_LINK_NETNS are taken
>      from peer device.
> 
>   3) seeing @br3 in current ip output looks confusing according to (2)
>      as veth do not link to something other than it's peer that is in
>      different network namespace.
> 
> From (3) I guess @br3 is incorrect value and caused by missing
> IFLA_LINK_NETNS handling in old print_linkinfo_brief(): it always
> calls ll_index_to_name().
> 

you are right; if7 is the right output so the current 'br3' is a bug and
it only happens with the brief output.

[PATCH iproute2-next 9/9] ipaddress: Make print_linkinfo_brief() static

[Serhey Popovych]

It shares lot of code with print_linkinfo(): drop duplicated part,
change parameters list, make it static and call from print_linkinfo()
after common path.

While there move SPRINT_BUF() to the function scope from blocks to
avoid duplication and use "%s" to print "\n" to help compiler optimize
exit for both print_linkinfo_brief() and normal paths.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ip_common.h |    2 --
 ip/ipaddress.c |   76 ++++++++------------------------------------------------
 ip/iplink.c    |    5 +---
 3 files changed, 11 insertions(+), 72 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 1397d99..e4e628b 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -29,8 +29,6 @@ struct link_filter {
 int get_operstate(const char *name);
 int print_linkinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg);
-int print_linkinfo_brief(const struct sockaddr_nl *who,
-			 struct nlmsghdr *n, void *arg);
 int print_addrinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg);
 int print_addrlabel(const struct sockaddr_nl *who,
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 65c2559..417b899 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -917,63 +917,12 @@ static void print_link_stats(FILE *fp, struct nlmsghdr *n)
 	fprintf(fp, "%s", _SL_);
 }
 
-int print_linkinfo_brief(const struct sockaddr_nl *who,
-			 struct nlmsghdr *n, void *arg)
+static int print_linkinfo_brief(FILE *fp, const char *name,
+				const struct ifinfomsg *ifi,
+				struct rtattr *tb[])
 {
-	FILE *fp = (FILE *)arg;
-	struct ifinfomsg *ifi = NLMSG_DATA(n);
-	struct rtattr *tb[IFLA_MAX+1];
-	int len = n->nlmsg_len;
-	const char *name;
 	unsigned int m_flag = 0;
 
-	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
-		return -1;
-
-	len -= NLMSG_LENGTH(sizeof(*ifi));
-	if (len < 0)
-		return -1;
-
-	if (filter.ifindex && ifi->ifi_index != filter.ifindex)
-		return -1;
-	if (filter.up && !(ifi->ifi_flags&IFF_UP))
-		return -1;
-
-	parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-
-	name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
-	if (!name)
-		return -1;
-
-	if (filter.label &&
-	    (!filter.family || filter.family == AF_PACKET) &&
-	    fnmatch(filter.label, name, 0))
-		return -1;
-
-	if (tb[IFLA_GROUP]) {
-		int group = rta_getattr_u32(tb[IFLA_GROUP]);
-
-		if (filter.group != -1 && group != filter.group)
-			return -1;
-	}
-
-	if (tb[IFLA_MASTER]) {
-		int master = rta_getattr_u32(tb[IFLA_MASTER]);
-
-		if (filter.master > 0 && master != filter.master)
-			return -1;
-	} else if (filter.master > 0)
-		return -1;
-
-	if (filter.kind && match_link_kind(tb, filter.kind, 0))
-		return -1;
-
-	if (filter.slave_kind && match_link_kind(tb, filter.slave_kind, 1))
-		return -1;
-
-	if (n->nlmsg_type == RTM_DELLINK)
-		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
-
 	m_flag = print_name_and_link("%-16s ", COLOR_NONE, name, tb);
 
 	if (tb[IFLA_OPERSTATE])
@@ -1033,6 +982,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	const char *name;
 	unsigned int m_flag = 0;
+	SPRINT_BUF(b1);
 
 	if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
 		return 0;
@@ -1081,6 +1031,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (n->nlmsg_type == RTM_DELLINK)
 		print_bool(PRINT_ANY, "deleted", "Deleted ", true);
 
+	if (brief)
+		return print_linkinfo_brief(fp, name, ifi, tb);
+
 	print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
 
 	m_flag = print_name_and_link("%s: ", COLOR_IFNAME, name, tb);
@@ -1113,7 +1066,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		print_linkmode(fp, tb[IFLA_LINKMODE]);
 
 	if (tb[IFLA_GROUP]) {
-		SPRINT_BUF(b1);
 		int group = rta_getattr_u32(tb[IFLA_GROUP]);
 
 		print_string(PRINT_ANY,
@@ -1129,8 +1081,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		print_link_event(fp, rta_getattr_u32(tb[IFLA_EVENT]));
 
 	if (!filter.family || filter.family == AF_PACKET || show_details) {
-		SPRINT_BUF(b1);
-
 		print_string(PRINT_FP, NULL, "%s", _SL_);
 		print_string(PRINT_ANY,
 			     "link_type",
@@ -1230,7 +1180,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
 				     rta_getattr_str(tb[IFLA_PHYS_PORT_NAME]));
 
 		if (tb[IFLA_PHYS_PORT_ID]) {
-			SPRINT_BUF(b1);
 			print_string(PRINT_ANY,
 				     "phys_port_id",
 				     "portid %s ",
@@ -1241,7 +1190,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		}
 
 		if (tb[IFLA_PHYS_SWITCH_ID]) {
-			SPRINT_BUF(b1);
 			print_string(PRINT_ANY,
 				     "phys_switch_id",
 				     "switchid %s ",
@@ -1280,7 +1228,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		close_json_array(PRINT_JSON, NULL);
 	}
 
-	print_string(PRINT_FP, NULL, "\n", NULL);
+	print_string(PRINT_FP, NULL, "%s", "\n");
 	fflush(fp);
 	return 1;
 }
@@ -2135,14 +2083,10 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
 	for (l = linfo.head; l; l = l->next) {
 		struct nlmsghdr *n = &l->h;
 		struct ifinfomsg *ifi = NLMSG_DATA(n);
-		int res;
+		int res = 0;
 
 		open_json_object(NULL);
-		if (brief)
-			res = print_linkinfo_brief(NULL, n, stdout);
-		else if (no_link)
-			res = 0;
-		else
+		if (brief || !no_link)
 			res = print_linkinfo(NULL, n, stdout);
 		if (res >= 0 && filter.family != AF_PACKET)
 			print_selected_addrinfo(ifi, ainfo->head, stdout);
diff --git a/ip/iplink.c b/ip/iplink.c
index 3d7f7fa..74c377c 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1075,10 +1075,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
 		return -2;
 
 	open_json_object(NULL);
-	if (brief)
-		print_linkinfo_brief(NULL, answer, stdout);
-	else
-		print_linkinfo(NULL, answer, stdout);
+	print_linkinfo(NULL, answer, stdout);
 	close_json_object();
 
 	free(answer);
-- 
1.7.10.4

Re: [PATCH iproute2-next 0/9] ipaddress: Make print_linkinfo_brief() static

[Serhey Popovych]

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

Serhey Popovych wrote:
> With this series I propose to make print_linkinfo_brief() static in
> favor of print_linkinfo() as single point for linkinfo printing.
> 
> Changes presented with this series tested using following script:
> 
> \#!/bin/bash
> 
> iproute2_dir="$1"
> iface='eth0.2'
> 
> pushd "$iproute2_dir" &>/dev/null
> 
> for i in new old; do
> 	DIR="/tmp/$i"
> 	mkdir -p "$DIR"
> 
> 	ln -snf ip.$i ip/ip
> 
> 	# normal
> 	ip/ip link show                  >"$DIR/ip-link-show"
> 	ip/ip -4 addr show               >"$DIR/ip-4-addr-show"
> 	ip/ip -6 addr show               >"$DIR/ip-6-addr-show"
> 	ip/ip addr show dev "$iface"     >"$DIR/ip-addr-show-$iface"
> 
> 	# brief
> 	ip/ip -br link show              >"$DIR/ip-br-link-show"
> 	ip/ip -br -4 addr show           >"$DIR/ip-br-4-addr-show"
> 	ip/ip -br -6 addr show           >"$DIR/ip-br-6-addr-show"
> 	ip/ip -br addr show dev "$iface" >"$DIR/ip-br-addr-show-$iface"
> done
> rm -f ip/ip
> 
> diff -urN /tmp/{old,new} |sed -n -Ee'/^(-{3}|\+{3})[[:space:]]+/!p'
> rc=$?
> 
> popd &>/dev/null
> exit $rc
> 
> Expected results : <no output>
> Actual results   : <no output>
> 
> Although test coverage is far from ideal in my opinion it covers most
> important aspects of the changes presented by the series.
> 
> All this work is done in prepare of iplink_get() enhancements to support
> attribute parse that finally will be used to simplify ip/tunnel
> RTM_GETLINK code.
> 
> As always reviews, comments, suggestions and criticism is welcome.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (9):
>   ipaddress: Abstract IFA_LABEL matching code
>   ipaddress: ll_map: Replace ll_idx_n2a() with ll_index_to_name()
>   utils: Reimplement ll_idx_n2a() and introduce ll_idx_a2n()
>   ipaddress: Improve print_linkinfo()
>   ipaddress: Simplify print_linkinfo_brief() and it's usage
>   lib: Correct object file dependencies
>   utils: Introduce and use get_ifname_rta()
>   utils: Introduce and use print_name_and_link() to print name@link
>   ipaddress: Make print_linkinfo_brief() static
> 
>  bridge/link.c    |   21 ++---
>  include/ll_map.h |    4 +-
>  include/utils.h  |    5 ++
>  ip/ip_common.h   |    2 -
>  ip/ipaddress.c   |  224 ++++++++++++++----------------------------------------
>  ip/iplink.c      |    5 +-
>  lib/Makefile     |    4 +-
>  lib/ll_map.c     |   31 +++++---
>  lib/utils.c      |   68 +++++++++++++++++
>  9 files changed, 162 insertions(+), 202 deletions(-)
> 

Any comments on this series? Should I perform some additional testing?



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

Re: [PATCH iproute2-next 0/9] ipaddress: Make print_linkinfo_brief() static

[David Ahern]

On 2/14/18 1:09 PM, Serhey Popovych wrote:

> 
> Any comments on this series? Should I perform some additional testing?
> 
> 

I thought I had applied them. Can you re-send and cc me?

Re: [PATCH iproute2-next 0/9] ipaddress: Make print_linkinfo_brief() static

[Serhey Popovych]

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

David Ahern wrote:
> On 2/14/18 1:09 PM, Serhey Popovych wrote:
> 
>>
>> Any comments on this series? Should I perform some additional testing?
>>
>>
> 
> I thought I had applied them. Can you re-send and cc me?
> 
Done. v2 is broken due to missing subject line. v3 is ready.



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