From 17df3d607d43a0a37468a0e3863fcb27ff55300d Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:16 +0200 Subject: [PATCH 1/9] ipaddress: Abstract IFA_LABEL matching code 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 Signed-off-by: David Ahern --- ip/ipaddress.c | 53 ++++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 6990b815..ad69d09b 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -1305,6 +1305,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) { @@ -1343,21 +1359,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; @@ -1713,25 +1721,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 (ifa_label_match_rta(ifa->ifa_index, tb[IFA_LABEL])) + 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 (!tb[IFA_LOCAL]) + tb[IFA_LOCAL] = tb[IFA_ADDRESS]; + if (inet_addr_match_rta(&filter.pfx, tb[IFA_LOCAL])) + continue; ok = 1; break; From d92cc2d087b0efde6152acf5e52ec4bf469d9805 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:17 +0200 Subject: [PATCH 2/9] ipaddress: ll_map: Replace ll_idx_n2a() with ll_index_to_name() 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. While there print master network device name using correct color. Signed-off-by: Serhey Popovych Signed-off-by: David Ahern --- ip/ipaddress.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index ad69d09b..0daba8c0 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -813,14 +813,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); @@ -957,12 +956,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); } @@ -984,12 +981,13 @@ 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)); + print_color_string(PRINT_ANY, + COLOR_IFNAME, + "master", + "master %s ", + ll_index_to_name(master)); } if (tb[IFLA_OPERSTATE]) @@ -1308,7 +1306,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; @@ -1316,7 +1313,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); } From fe269b6e7c5de9870ffa5c3889a51dbb26e4e6ff Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:18 +0200 Subject: [PATCH 3/9] utils: Reimplement ll_idx_n2a() and introduce ll_idx_a2n() 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 Signed-off-by: David Ahern --- 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 c8474e6e..8546ff92 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 f65614fa..0afe6897 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; } From 9516823051cee49d1eddd87aa3ecb7770f0f1c7b Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:19 +0200 Subject: [PATCH 4/9] ipaddress: Improve print_linkinfo() 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 Signed-off-by: David Ahern --- ip/ipaddress.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 0daba8c0..6eac3707 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -778,14 +778,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 = ""; + 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]) { @@ -887,6 +887,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) @@ -897,18 +898,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]); @@ -935,16 +940,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", ""); - } + print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", name); if (tb[IFLA_LINK]) { int iflink = rta_getattr_u32(tb[IFLA_LINK]); From 1bccd1e43b949cc3696f5060fe0928439e6f7bef Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:20 +0200 Subject: [PATCH 5/9] ipaddress: Simplify print_linkinfo_brief() and it's usage 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 Signed-off-by: David Ahern --- ip/ipaddress.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 6eac3707..749178de 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -753,7 +753,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); @@ -2013,24 +2013,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); From 0cec58dac4c249a6773a39e00b0f13530534cda7 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:21 +0200 Subject: [PATCH 6/9] lib: Correct object file dependencies 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. However ll_map.o depends on libnetlink.o functions so we need to provide libnetlink.a after libutil.a in LIBNETLINK at global Makefile. Tested using make clean && make -j4. No problems so far. Signed-off-by: Serhey Popovych Signed-off-by: David Ahern --- Makefile | 2 +- lib/Makefile | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 32587db3..b526d3b5 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ YACCFLAGS = -d -t -v SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma man -LIBNETLINK=../lib/libnetlink.a ../lib/libutil.a +LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a LDLIBS += $(LIBNETLINK) all: config.mk diff --git a/lib/Makefile b/lib/Makefile index 7b34ed5f..bab8cbf5 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 From fcac9665265510735252760b7a0040f9adaa34a2 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:22 +0200 Subject: [PATCH 7/9] utils: Introduce and use get_ifname_rta() 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 "". Use check_ifname() to validate network device name: this catches both unexpected return from kernel and ll_idx_n2a(). Signed-off-by: Serhey Popovych Signed-off-by: David Ahern --- 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 870ebe05..a11cbb11 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 867e5404..84ca873e 100644 --- a/include/utils.h +++ b/include/utils.h @@ -183,6 +183,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 749178de..08d2576b 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -776,12 +776,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) && @@ -903,12 +901,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 cbe5d8de..6a62c1ed 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -872,6 +872,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); From f5b50a18ae8cf47531b3a1801f963693b59c79e3 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:23 +0200 Subject: [PATCH 8/9] utils: Introduce and use print_name_and_link() to print name@link 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 "" 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 Signed-off-by: David Ahern --- 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 a11cbb11..90c9734d 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]) : ""); + 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 84ca873e..75ddb4ae 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; @@ -250,6 +251,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 08d2576b..90cb5e57 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -760,7 +760,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) @@ -810,25 +809,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])); @@ -936,29 +917,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 6a62c1ed..61af123e 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; @@ -1261,6 +1262,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 */ From c956e9a934280e1a1e0c913515f69a76dd804cd3 Mon Sep 17 00:00:00 2001 From: Serhey Popovych Date: Thu, 15 Feb 2018 23:23:24 +0200 Subject: [PATCH 9/9] ipaddress: Make print_linkinfo_brief() static 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 Signed-off-by: David Ahern --- 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 1397d99b..e4e628b5 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 90cb5e57..13804539 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -752,63 +752,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]) @@ -868,6 +817,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; @@ -916,6 +866,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); @@ -949,7 +902,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, @@ -965,8 +917,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", @@ -1066,7 +1016,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 ", @@ -1077,7 +1026,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 ", @@ -1116,7 +1064,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; } @@ -1971,14 +1919,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 3d7f7fa2..74c377c8 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);