diff mbox series

[iproute2,1/9] iplink: Use ll_index_to_name() instead of if_indextoname()

Message ID 1515778774-24173-2-git-send-email-serhe.popovych@gmail.com
State Changes Requested, archived
Delegated to: stephen hemminger
Headers show
Series ip/tunnel: Improve tunnel parameters printing | expand

Commit Message

Serhey Popovych Jan. 12, 2018, 5:39 p.m. UTC 
There are two reasons for switching to cached variant:

  1) ll_index_to_name() may return result from cache,
     eliminating expensive ioctl() to the kernel.

     Note that most of the code already switched from plain
     if_indextoname() to ll_index_to_name() to cached variant
     in print path because in most cases cache populated.

  2) It always return name in the form "if%d", even if
     entry is not in cache and ioctl() fails. This drops
     "link_index" from JSON output.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 bridge/fdb.c          |    4 ++--
 bridge/link.c         |   18 ++++++------------
 ip/iplink_bond.c      |   25 ++++---------------------
 ip/iplink_vxlan.c     |    8 ++------
 ip/iproute_lwtunnel.c |    7 ++-----
 ip/link_gre.c         |   12 +++++-------
 ip/link_gre6.c        |   12 +++++-------
 ip/link_ip6tnl.c      |   12 +++++-------
 ip/link_iptnl.c       |   12 +++++-------
 ip/link_vti.c         |    7 ++-----
 ip/link_vti6.c        |   10 ++++------
 11 files changed, 42 insertions(+), 85 deletions(-)

Comments

Stephen Hemminger Jan. 17, 2018, 6:53 p.m. UTC | #1 
On Fri, 12 Jan 2018 19:39:26 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

This looks fine, but minor nuisances from checkpatch

> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index 376713b..2cc0268 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -219,10 +219,10 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  		unsigned int ifindex = rta_getattr_u32(tb[NDA_IFINDEX]);
>  
>  		if (ifindex) {
> -			char ifname[IF_NAMESIZE];
> +			const char *ifname;
>  
>  			if (!tb[NDA_LINK_NETNSID] &&
> -			    if_indextoname(ifindex, ifname)) {
> +			    (ifname = ll_index_to_name(ifindex))) {
>  				if (jw_global)

Please rearrange to avoid assignment in conditional.
			ifname = ll_index_to_name(ifindex);
			if (ifname && !tb[NDA_LINK_NETNSID]) {


> @@ -135,14 +132,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>  
>  	if (tb[IFLA_LINK]) {
> -		SPRINT_BUF(b1);
>  		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
>  
> -		if (iflink == 0)
> -			fprintf(fp, "@NONE: ");
> -		else
> -			fprintf(fp, "@%s: ",
> -				if_indextoname(iflink, b1));
> +		fprintf(fp, "@%s: ", iflink ? ll_index_to_name(iflink) : "NONE");

Break long line here.


ERROR: do not use assignment in if condition
#265: FILE: ip/link_gre6.c:418:
+	if (tb[IFLA_GRE_LINK] &&

ERROR: do not use assignment in if condition
#296: FILE: ip/link_ip6tnl.c:381:
+	if (tb[IFLA_IPTUN_LINK] &&

ERROR: do not use assignment in if condition
#327: FILE: ip/link_iptnl.c:411:
+	if (tb[IFLA_IPTUN_LINK] &&

ERROR: do not use assignment in if condition
#368: FILE: ip/link_vti6.c:193:
+	if (tb[IFLA_VTI_LINK] &&
diff mbox series

Patch

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 376713b..2cc0268 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -219,10 +219,10 @@  int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		unsigned int ifindex = rta_getattr_u32(tb[NDA_IFINDEX]);
 
 		if (ifindex) {
-			char ifname[IF_NAMESIZE];
+			const char *ifname;
 
 			if (!tb[NDA_LINK_NETNSID] &&
-			    if_indextoname(ifindex, ifname)) {
+			    (ifname = ll_index_to_name(ifindex))) {
 				if (jw_global)
 					jsonw_string_field(jw_global, "viaIf",
 							   ifname);
diff --git a/bridge/link.c b/bridge/link.c
index e2371d0..9c846c9 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -26,8 +26,6 @@  static const char *port_states[] = {
 	[BR_STATE_BLOCKING] = "blocking",
 };
 
-extern char *if_indextoname(unsigned int __ifindex, char *__ifname);
-
 static void print_link_flags(FILE *fp, unsigned int flags)
 {
 	fprintf(fp, "<");
@@ -104,7 +102,6 @@  int print_linkinfo(const struct sockaddr_nl *who,
 	int len = n->nlmsg_len;
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct rtattr *tb[IFLA_MAX+1];
-	char b1[IFNAMSIZ];
 
 	len -= NLMSG_LENGTH(sizeof(*ifi));
 	if (len < 0) {
@@ -135,14 +132,9 @@  int print_linkinfo(const struct sockaddr_nl *who,
 		print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
 
 	if (tb[IFLA_LINK]) {
-		SPRINT_BUF(b1);
 		int iflink = rta_getattr_u32(tb[IFLA_LINK]);
 
-		if (iflink == 0)
-			fprintf(fp, "@NONE: ");
-		else
-			fprintf(fp, "@%s: ",
-				if_indextoname(iflink, b1));
+		fprintf(fp, "@%s: ", iflink ? ll_index_to_name(iflink) : "NONE");
 	} else
 		fprintf(fp, ": ");
 
@@ -151,9 +143,11 @@  int print_linkinfo(const struct sockaddr_nl *who,
 	if (tb[IFLA_MTU])
 		fprintf(fp, "mtu %u ", rta_getattr_u32(tb[IFLA_MTU]));
 
-	if (tb[IFLA_MASTER])
-		fprintf(fp, "master %s ",
-			if_indextoname(rta_getattr_u32(tb[IFLA_MASTER]), b1));
+	if (tb[IFLA_MASTER]) {
+		int master = rta_getattr_u32(tb[IFLA_MASTER]);
+
+		fprintf(fp, "master %s ", ll_index_to_name(master));
+	}
 
 	if (tb[IFLA_PROTINFO]) {
 		if (tb[IFLA_PROTINFO]->rta_type & NLA_F_NESTED) {
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 2b5cf4f..45900c8 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -382,19 +382,9 @@  static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_BOND_ACTIVE_SLAVE] &&
 	    (ifindex = rta_getattr_u32(tb[IFLA_BOND_ACTIVE_SLAVE]))) {
-		char buf[IFNAMSIZ];
-		const char *n = if_indextoname(ifindex, buf);
+		const char *n = ll_index_to_name(ifindex);
 
-		if (n)
-			print_string(PRINT_ANY,
-				     "active_slave",
-				     "active_slave %s ",
-				     n);
-		else
-			print_uint(PRINT_ANY,
-				   "active_slave_index",
-				   "active_slave %u ",
-				   ifindex);
+		print_string(PRINT_ANY, "active_slave", "active_slave %s ", n);
 	}
 
 	if (tb[IFLA_BOND_MIIMON])
@@ -481,16 +471,9 @@  static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_BOND_PRIMARY] &&
 	    (ifindex = rta_getattr_u32(tb[IFLA_BOND_PRIMARY]))) {
-		char buf[IFNAMSIZ];
-		const char *n = if_indextoname(ifindex, buf);
+		const char *n = ll_index_to_name(ifindex);
 
-		if (n)
-			print_string(PRINT_ANY, "primary", "primary %s ", n);
-		else
-			print_uint(PRINT_ANY,
-				   "primary_index",
-				   "primary %u ",
-				   ifindex);
+		print_string(PRINT_ANY, "primary", "primary %s ", n);
 	}
 
 	if (tb[IFLA_BOND_PRIMARY_RESELECT]) {
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 661eaa7..a6c964a 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -397,7 +397,6 @@  static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int link;
 	__u8 tos;
 	__u32 maxaddr;
-	char s2[64];
 
 	if (!tb)
 		return;
@@ -469,12 +468,9 @@  static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_VXLAN_LINK] &&
 	    (link = rta_getattr_u32(tb[IFLA_VXLAN_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_VXLAN_PORT_RANGE]) {
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 740da7c..a8d7171 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -196,7 +196,6 @@  static int read_action_type(const char *name)
 static void print_encap_seg6local(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[SEG6_LOCAL_MAX + 1];
-	char ifbuf[IFNAMSIZ];
 	int action;
 
 	parse_rtattr_nested(tb, SEG6_LOCAL_MAX, encap);
@@ -229,15 +228,13 @@  static void print_encap_seg6local(FILE *fp, struct rtattr *encap)
 	if (tb[SEG6_LOCAL_IIF]) {
 		int iif = rta_getattr_u32(tb[SEG6_LOCAL_IIF]);
 
-		fprintf(fp, "iif %s ",
-			if_indextoname(iif, ifbuf) ?: "<unknown>");
+		fprintf(fp, "iif %s ", ll_index_to_name(iif));
 	}
 
 	if (tb[SEG6_LOCAL_OIF]) {
 		int oif = rta_getattr_u32(tb[SEG6_LOCAL_OIF]);
 
-		fprintf(fp, "oif %s ",
-			if_indextoname(oif, ifbuf) ?: "<unknown>");
+		fprintf(fp, "oif %s ", ll_index_to_name(oif));
 	}
 }
 
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 3c0b6d6..d876bad 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -359,6 +359,7 @@  static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
+	unsigned int link;
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 
@@ -380,14 +381,11 @@  static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_GRE_LINK] && rta_getattr_u32(tb[IFLA_GRE_LINK])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_GRE_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_GRE_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 55bd1fb..93f3a96 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -377,6 +377,7 @@  static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
+	unsigned int link;
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
@@ -414,14 +415,11 @@  static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_GRE_LINK] && rta_getattr_u32(tb[IFLA_GRE_LINK])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_GRE_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_GRE_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_GRE_TTL]) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index bbc7878..0cfb2c6 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -334,6 +334,7 @@  get_failed:
 static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
+	unsigned int link;
 	int flags = 0;
 	__u32 flowinfo = 0;
 
@@ -377,14 +378,11 @@  static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			     rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_LOCAL]));
 	}
 
-	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_IPTUN_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 24a0f0c..3204e5e 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -364,6 +364,7 @@  static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
+	unsigned int link;
 	__u16 prefixlen, type;
 
 	if (!tb)
@@ -407,14 +408,11 @@  static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
-		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_IPTUN_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_int(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_IPTUN_TTL]) {
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 2b0fab2..7ae2e3c 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -194,12 +194,9 @@  static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_VTI_LINK] &&
 	    (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_VTI_IKEY] &&
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 74c246d..f7c40d3 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -190,13 +190,11 @@  static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	print_string(PRINT_ANY, "local", "local %s ", local);
 
-	if (tb[IFLA_VTI_LINK] && (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
-		const char *n = if_indextoname(link, s2);
+	if (tb[IFLA_VTI_LINK] &&
+	    (link = rta_getattr_u32(tb[IFLA_VTI_LINK]))) {
+		const char *n = ll_index_to_name(link);
 
-		if (n)
-			print_string(PRINT_ANY, "link", "dev %s ", n);
-		else
-			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
+		print_string(PRINT_ANY, "link", "dev %s ", n);
 	}
 
 	if (tb[IFLA_VTI_IKEY]) {