Netdev List
 help / color / mirror / Atom feed
* [iproute PATCH 4/7] utils: make rt_addr_n2a() non-reentrant by default
From: Phil Sutter @ 2016-03-22 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1458671719-14361-1-git-send-email-phil@nwl.cc>

There is only a single user who needs it to be reentrant (not really,
but it's safer like this), add rt_addr_n2a_r() for it to use.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/utils.h       |  3 ++-
 ip/ip6tunnel.c        |  2 +-
 ip/iplink_bond.c      |  5 +----
 ip/ipmroute.c         |  7 ++-----
 ip/ipprefix.c         |  5 +----
 ip/iproute.c          | 10 +++-------
 ip/iproute_lwtunnel.c |  7 ++-----
 ip/iprule.c           |  8 ++------
 ip/iptunnel.c         |  2 +-
 ip/ipxfrm.c           | 29 ++++++-----------------------
 ip/link_ip6tnl.c      |  7 ++-----
 ip/xfrm_monitor.c     | 16 +++-------------
 lib/utils.c           | 11 +++++++++--
 tc/f_flower.c         |  7 ++-----
 14 files changed, 37 insertions(+), 82 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index 84083b0dbba71..bc2cbce0cc303 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -125,8 +125,9 @@ int af_byte_len(int af);
 const char *format_host_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
 const char *format_host(int af, int lne, const void *addr);
-const char *rt_addr_n2a(int af, int len, const void *addr,
+const char *rt_addr_n2a_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
+const char *rt_addr_n2a(int af, int len, const void *addr);
 
 int read_family(const char *name);
 const char *family_name(int family);
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index d588645eb942a..c02fa0746ab69 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -78,7 +78,7 @@ static void print_tunnel(struct ip6_tnl_parm2 *p)
 	       p->name,
 	       tnl_strproto(p->proto),
 	       format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
-	       rt_addr_n2a(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
+	       rt_addr_n2a_r(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
 	if (p->link) {
 		const char *n = ll_index_to_name(p->link);
 
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 45473f66da065..7da58e4556c07 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -411,7 +411,6 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 	if (tb[IFLA_BOND_ARP_IP_TARGET]) {
 		struct rtattr *iptb[BOND_MAX_ARP_TARGETS + 1];
-		char buf[INET_ADDRSTRLEN];
 		int i;
 
 		parse_rtattr_nested(iptb, BOND_MAX_ARP_TARGETS,
@@ -425,9 +424,7 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 				fprintf(f, "%s",
 					rt_addr_n2a(AF_INET,
 						    RTA_PAYLOAD(iptb[i]),
-						    RTA_DATA(iptb[i]),
-						    buf,
-						    INET_ADDRSTRLEN));
+						    RTA_DATA(iptb[i])));
 			if (i < BOND_MAX_ARP_TARGETS-1 && iptb[i+1])
 				fprintf(f, ",");
 		}
diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index 34543c00ecb75..2b9f892a62630 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -58,7 +58,6 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	struct rtmsg *r = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[RTA_MAX+1];
-	char abuf[256];
 	char obuf[256];
 
 	SPRINT_BUF(b1);
@@ -126,16 +125,14 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		len = snprintf(obuf, sizeof(obuf),
 			       "(%s, ", rt_addr_n2a(family,
 						    RTA_PAYLOAD(tb[RTA_SRC]),
-						    RTA_DATA(tb[RTA_SRC]),
-						    abuf, sizeof(abuf)));
+						    RTA_DATA(tb[RTA_SRC])));
 	else
 		len = sprintf(obuf, "(unknown, ");
 	if (tb[RTA_DST])
 		snprintf(obuf + len, sizeof(obuf) - len,
 			 "%s)", rt_addr_n2a(family,
 					    RTA_PAYLOAD(tb[RTA_DST]),
-					    RTA_DATA(tb[RTA_DST]),
-					    abuf, sizeof(abuf)));
+					    RTA_DATA(tb[RTA_DST])));
 	else
 		snprintf(obuf + len, sizeof(obuf) - len, "unknown) ");
 
diff --git a/ip/ipprefix.c b/ip/ipprefix.c
index 2524f784965b5..4d986dbc1a5d1 100644
--- a/ip/ipprefix.c
+++ b/ip/ipprefix.c
@@ -75,15 +75,12 @@ int print_prefix(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[PREFIX_ADDRESS]) {
 		struct in6_addr *pfx;
-		char abuf[256];
 
 		pfx = (struct in6_addr *)RTA_DATA(tb[PREFIX_ADDRESS]);
 
-		memset(abuf, '\0', sizeof(abuf));
 		fprintf(fp, "%s", rt_addr_n2a(family,
 					      RTA_PAYLOAD(tb[PREFIX_ADDRESS]),
-					      pfx,
-					      abuf, sizeof(abuf)));
+					      pfx));
 	}
 	fprintf(fp, "/%u ", prefix->prefix_len);
 
diff --git a/ip/iproute.c b/ip/iproute.c
index 8315ad3e6c6ac..1ec9294290b2a 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -318,7 +318,6 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	struct rtmsg *r = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[RTA_MAX+1];
-	char abuf[256];
 	int host_len;
 	__u32 table;
 
@@ -373,8 +372,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_dst_len != host_len) {
 			fprintf(fp, "%s/%u ", rt_addr_n2a(r->rtm_family,
 						       RTA_PAYLOAD(tb[RTA_DST]),
-						       RTA_DATA(tb[RTA_DST]),
-						       abuf, sizeof(abuf)),
+						       RTA_DATA(tb[RTA_DST])),
 				r->rtm_dst_len
 				);
 		} else {
@@ -392,8 +390,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_src_len != host_len) {
 			fprintf(fp, "from %s/%u ", rt_addr_n2a(r->rtm_family,
 						       RTA_PAYLOAD(tb[RTA_SRC]),
-						       RTA_DATA(tb[RTA_SRC]),
-						       abuf, sizeof(abuf)),
+						       RTA_DATA(tb[RTA_SRC])),
 				r->rtm_src_len
 				);
 		} else {
@@ -452,8 +449,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, " src %s ",
 			rt_addr_n2a(r->rtm_family,
 				    RTA_PAYLOAD(tb[RTA_PREFSRC]),
-				    RTA_DATA(tb[RTA_PREFSRC]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[RTA_PREFSRC])));
 	}
 	if (tb[RTA_PRIORITY])
 		fprintf(fp, " metric %u ", rta_getattr_u32(tb[RTA_PRIORITY]));
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 42abe3753ac6a..53d3ad4e6ea8e 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -70,7 +70,6 @@ static void print_encap_mpls(FILE *fp, struct rtattr *encap)
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[LWTUNNEL_IP_MAX+1];
-	char abuf[256];
 
 	parse_rtattr_nested(tb, LWTUNNEL_IP_MAX, encap);
 
@@ -81,15 +80,13 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap)
 		fprintf(fp, "src %s ",
 			rt_addr_n2a(AF_INET,
 				    RTA_PAYLOAD(tb[LWTUNNEL_IP_SRC]),
-				    RTA_DATA(tb[LWTUNNEL_IP_SRC]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[LWTUNNEL_IP_SRC])));
 
 	if (tb[LWTUNNEL_IP_DST])
 		fprintf(fp, "dst %s ",
 			rt_addr_n2a(AF_INET,
 				    RTA_PAYLOAD(tb[LWTUNNEL_IP_DST]),
-				    RTA_DATA(tb[LWTUNNEL_IP_DST]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[LWTUNNEL_IP_DST])));
 
 	if (tb[LWTUNNEL_IP_TTL])
 		fprintf(fp, "ttl %d ", rta_getattr_u8(tb[LWTUNNEL_IP_TTL]));
diff --git a/ip/iprule.c b/ip/iprule.c
index 345b3b042307d..3fd510efe5e1b 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -57,8 +57,6 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	int host_len = -1;
 	__u32 table;
 	struct rtattr *tb[FRA_MAX+1];
-	char abuf[256];
-
 	SPRINT_BUF(b1);
 
 	if (n->nlmsg_type != RTM_NEWRULE && n->nlmsg_type != RTM_DELRULE)
@@ -87,8 +85,7 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_src_len != host_len) {
 			fprintf(fp, "from %s/%u ", rt_addr_n2a(r->rtm_family,
 						       RTA_PAYLOAD(tb[FRA_SRC]),
-						       RTA_DATA(tb[FRA_SRC]),
-						       abuf, sizeof(abuf)),
+						       RTA_DATA(tb[FRA_SRC])),
 				r->rtm_src_len
 				);
 		} else {
@@ -107,8 +104,7 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_dst_len != host_len) {
 			fprintf(fp, "to %s/%u ", rt_addr_n2a(r->rtm_family,
 						       RTA_PAYLOAD(tb[FRA_DST]),
-						       RTA_DATA(tb[FRA_DST]),
-						       abuf, sizeof(abuf)),
+						       RTA_DATA(tb[FRA_DST])),
 				r->rtm_dst_len
 				);
 		} else {
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index bfee99561615a..e3161d81beded 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -309,7 +309,7 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 	       p->name,
 	       tnl_strproto(p->iph.protocol),
 	       p->iph.daddr ? format_host_r(AF_INET, 4, &p->iph.daddr, s1, sizeof(s1)) : "any",
-	       p->iph.saddr ? rt_addr_n2a(AF_INET, 4, &p->iph.saddr, s2, sizeof(s2)) : "any");
+	       p->iph.saddr ? rt_addr_n2a_r(AF_INET, 4, &p->iph.saddr, s2, sizeof(s2)) : "any");
 
 	if (p->iph.protocol == IPPROTO_IPV6 && (p->i_flags & SIT_ISATAP)) {
 		struct ip_tunnel_prl prl[16];
diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 860414d377b47..8741ff3b302a9 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -285,17 +285,11 @@ void xfrm_id_info_print(xfrm_address_t *saddr, struct xfrm_id *id,
 			__u8 mode, __u32 reqid, __u16 family, int force_spi,
 			FILE *fp, const char *prefix, const char *title)
 {
-	char abuf[256];
-
 	if (title)
 		fputs(title, fp);
 
-	memset(abuf, '\0', sizeof(abuf));
-	fprintf(fp, "src %s ", rt_addr_n2a(family, sizeof(*saddr),
-					   saddr, abuf, sizeof(abuf)));
-	memset(abuf, '\0', sizeof(abuf));
-	fprintf(fp, "dst %s", rt_addr_n2a(family, sizeof(id->daddr),
-					  &id->daddr, abuf, sizeof(abuf)));
+	fprintf(fp, "src %s ", rt_addr_n2a(family, sizeof(*saddr), saddr));
+	fprintf(fp, "dst %s", rt_addr_n2a(family, sizeof(id->daddr), &id->daddr));
 	fprintf(fp, "%s", _SL_);
 
 	if (prefix)
@@ -447,7 +441,6 @@ void xfrm_lifetime_print(struct xfrm_lifetime_cfg *cfg,
 void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 			 FILE *fp, const char *prefix)
 {
-	char abuf[256];
 	__u16 f;
 
 	f = sel->family;
@@ -459,16 +452,12 @@ void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
 	if (prefix)
 		fputs(prefix, fp);
 
-	memset(abuf, '\0', sizeof(abuf));
 	fprintf(fp, "src %s/%u ",
-		rt_addr_n2a(f, sizeof(sel->saddr), &sel->saddr,
-			    abuf, sizeof(abuf)),
+		rt_addr_n2a(f, sizeof(sel->saddr), &sel->saddr),
 		sel->prefixlen_s);
 
-	memset(abuf, '\0', sizeof(abuf));
 	fprintf(fp, "dst %s/%u ",
-		rt_addr_n2a(f, sizeof(sel->daddr), &sel->daddr,
-			    abuf, sizeof(abuf)),
+		rt_addr_n2a(f, sizeof(sel->daddr), &sel->daddr),
 		sel->prefixlen_d);
 
 	if (sel->proto)
@@ -740,7 +729,6 @@ void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
 
 	if (tb[XFRMA_ENCAP]) {
 		struct xfrm_encap_tmpl *e;
-		char abuf[256];
 
 		if (prefix)
 			fputs(prefix, fp);
@@ -768,10 +756,8 @@ void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
 		fprintf(fp, "sport %u ", ntohs(e->encap_sport));
 		fprintf(fp, "dport %u ", ntohs(e->encap_dport));
 
-		memset(abuf, '\0', sizeof(abuf));
 		fprintf(fp, "addr %s",
-			rt_addr_n2a(family, sizeof(e->encap_oa), &e->encap_oa,
-				    abuf, sizeof(abuf)));
+			rt_addr_n2a(family, sizeof(e->encap_oa), &e->encap_oa));
 		fprintf(fp, "%s", _SL_);
 	}
 
@@ -783,7 +769,6 @@ void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
 	}
 
 	if (tb[XFRMA_COADDR]) {
-		char abuf[256];
 		xfrm_address_t *coa;
 
 		if (prefix)
@@ -798,10 +783,8 @@ void xfrm_xfrma_print(struct rtattr *tb[], __u16 family,
 			return;
 		}
 
-		memset(abuf, '\0', sizeof(abuf));
 		fprintf(fp, "%s",
-			rt_addr_n2a(family, sizeof(*coa), coa,
-				    abuf, sizeof(abuf)));
+			rt_addr_n2a(family, sizeof(*coa), coa));
 		fprintf(fp, "%s", _SL_);
 	}
 
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index ab5e50a205983..4b32fe5aa8088 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -260,7 +260,6 @@ get_failed:
 
 static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[256];
 	char s2[64];
 	int flags = 0;
 	__u32 flowinfo = 0;
@@ -292,16 +291,14 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		fprintf(f, "remote %s ",
 			rt_addr_n2a(AF_INET6,
 				    RTA_PAYLOAD(tb[IFLA_IPTUN_REMOTE]),
-				    RTA_DATA(tb[IFLA_IPTUN_REMOTE]),
-				    s1, sizeof(s1)));
+				    RTA_DATA(tb[IFLA_IPTUN_REMOTE])));
 	}
 
 	if (tb[IFLA_IPTUN_LOCAL]) {
 		fprintf(f, "local %s ",
 			rt_addr_n2a(AF_INET6,
 				    RTA_PAYLOAD(tb[IFLA_IPTUN_LOCAL]),
-				    RTA_DATA(tb[IFLA_IPTUN_LOCAL]),
-				    s1, sizeof(s1)));
+				    RTA_DATA(tb[IFLA_IPTUN_LOCAL])));
 	}
 
 	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
diff --git a/ip/xfrm_monitor.c b/ip/xfrm_monitor.c
index 96b52a44b8a85..9d01a2aab5018 100644
--- a/ip/xfrm_monitor.c
+++ b/ip/xfrm_monitor.c
@@ -227,12 +227,8 @@ static void xfrm_ae_flags_print(__u32 flags, void *arg)
 
 static void xfrm_usersa_print(const struct xfrm_usersa_id *sa_id, __u32 reqid, FILE *fp)
 {
-	char buf[256];
-
-	buf[0] = 0;
 	fprintf(fp, "dst %s ",
-		rt_addr_n2a(sa_id->family, sizeof(sa_id->daddr), &sa_id->daddr,
-			    buf, sizeof(buf)));
+		rt_addr_n2a(sa_id->family, sizeof(sa_id->daddr), &sa_id->daddr));
 
 	fprintf(fp, " reqid 0x%x", reqid);
 
@@ -245,15 +241,12 @@ static int xfrm_ae_print(const struct sockaddr_nl *who,
 {
 	FILE *fp = (FILE *)arg;
 	struct xfrm_aevent_id *id = NLMSG_DATA(n);
-	char abuf[256];
 
 	fprintf(fp, "Async event ");
 	xfrm_ae_flags_print(id->flags, arg);
 	fprintf(fp, "\n\t");
-	memset(abuf, '\0', sizeof(abuf));
 	fprintf(fp, "src %s ", rt_addr_n2a(id->sa_id.family,
-					   sizeof(id->saddr), &id->saddr,
-					   abuf, sizeof(abuf)));
+					   sizeof(id->saddr), &id->saddr));
 
 	xfrm_usersa_print(&id->sa_id, id->reqid, fp);
 
@@ -265,10 +258,7 @@ static int xfrm_ae_print(const struct sockaddr_nl *who,
 
 static void xfrm_print_addr(FILE *fp, int family, xfrm_address_t *a)
 {
-	char buf[256];
-
-	buf[0] = 0;
-	fprintf(fp, "%s", rt_addr_n2a(family, sizeof(*a), a, buf, sizeof(buf)));
+	fprintf(fp, "%s", rt_addr_n2a(family, sizeof(*a), a));
 }
 
 static int xfrm_mapping_print(const struct sockaddr_nl *who,
diff --git a/lib/utils.c b/lib/utils.c
index 22a5ef8fa8213..9b303a6b096bc 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -702,7 +702,7 @@ int __get_user_hz(void)
 	return sysconf(_SC_CLK_TCK);
 }
 
-const char *rt_addr_n2a(int af, int len, const void *addr, char *buf, int buflen)
+const char *rt_addr_n2a_r(int af, int len, const void *addr, char *buf, int buflen)
 {
 	switch (af) {
 	case AF_INET:
@@ -725,6 +725,13 @@ const char *rt_addr_n2a(int af, int len, const void *addr, char *buf, int buflen
 	}
 }
 
+const char *rt_addr_n2a(int af, int len, const void *addr)
+{
+	static char buf[256];
+
+	return rt_addr_n2a_r(af, len, addr, buf, 256);
+}
+
 int read_family(const char *name)
 {
 	int family = AF_UNSPEC;
@@ -832,7 +839,7 @@ const char *format_host_r(int af, int len, const void *addr,
 			return n;
 	}
 #endif
-	return rt_addr_n2a(af, len, addr, buf, buflen);
+	return rt_addr_n2a_r(af, len, addr, buf, buflen);
 }
 
 const char *format_host(int af, int len, const void *addr)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 6139fe08651a4..40f8c595b2eec 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -394,7 +394,6 @@ static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
 				 struct rtattr *addr6_attr,
 				 struct rtattr *mask6_attr)
 {
-	SPRINT_BUF(b1);
 	struct rtattr *addr_attr;
 	struct rtattr *mask_attr;
 	int family;
@@ -418,16 +417,14 @@ static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
 		return;
 	fprintf(f, "\n  %s %s", name, rt_addr_n2a(family,
 						  RTA_PAYLOAD(addr_attr),
-						  RTA_DATA(addr_attr),
-						  b1, sizeof(b1)));
+						  RTA_DATA(addr_attr)));
 	if (!mask_attr || RTA_PAYLOAD(mask_attr) != len)
 		return;
 	bits = __mask_bits(RTA_DATA(mask_attr), len);
 	if (bits < 0)
 		fprintf(f, "/%s", rt_addr_n2a(family,
 					      RTA_PAYLOAD(mask_attr),
-					      RTA_DATA(mask_attr),
-					      b1, sizeof(b1)));
+					      RTA_DATA(mask_attr)));
 	else if (bits < len * 8)
 		fprintf(f, "/%d", bits);
 }
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH 7/7] lib/ll_addr: improve ll_addr_n2a() a bit
From: Phil Sutter @ 2016-03-22 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1458671719-14361-1-git-send-email-phil@nwl.cc>

Apart from making the code a bit more compact and efficient, this also
prevents a potential buffer overflow if the passed buffer is really too
small: Although correctly decrementing the size parameter passed to
snprintf, it could become negative which would then wrap since snprintf
uses (unsigned) size_t for the parameter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 lib/ll_addr.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/lib/ll_addr.c b/lib/ll_addr.c
index 2ce9abfbb8c69..465ed6fa4d9a2 100644
--- a/lib/ll_addr.c
+++ b/lib/ll_addr.c
@@ -41,18 +41,9 @@ const char *ll_addr_n2a(const unsigned char *addr, int alen, int type, char *buf
 	if (alen == 16 && type == ARPHRD_TUNNEL6) {
 		return inet_ntop(AF_INET6, addr, buf, blen);
 	}
-	l = 0;
-	for (i=0; i<alen; i++) {
-		if (i==0) {
-			snprintf(buf+l, blen, "%02x", addr[i]);
-			blen -= 2;
-			l += 2;
-		} else {
-			snprintf(buf+l, blen, ":%02x", addr[i]);
-			blen -= 3;
-			l += 3;
-		}
-	}
+	snprintf(buf, blen, "%02x", addr[0]);
+	for (i = 1, l = 2; i < alen && l < blen; i++, l += 3)
+		snprintf(buf + l, blen - l, ":%02x", addr[i]);
 	return buf;
 }
 
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH 3/7] make format_host non-reentrant by default
From: Phil Sutter @ 2016-03-22 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1458671719-14361-1-git-send-email-phil@nwl.cc>

There are only three users which require it to be reentrant, the rest is
fine without. Instead, provide a reentrant format_host_r() for users
which need it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 bridge/fdb.c          |  4 +---
 include/utils.h       |  3 ++-
 ip/ip6tunnel.c        |  2 +-
 ip/ipaddress.c        | 13 ++++---------
 ip/ipaddrlabel.c      |  4 +---
 ip/iplink_geneve.c    |  5 ++---
 ip/iplink_vxlan.c     | 13 ++++++-------
 ip/ipmaddr.c          |  6 +-----
 ip/ipneigh.c          |  4 +---
 ip/iproute.c          | 21 +++++++--------------
 ip/iproute_lwtunnel.c |  4 +---
 ip/iprule.c           |  9 +++------
 ip/iptoken.c          |  4 +---
 ip/iptunnel.c         |  6 +++---
 ip/link_gre.c         |  5 ++---
 ip/link_gre6.c        |  5 ++---
 ip/link_iptnl.c       |  7 +++----
 ip/link_vti.c         |  5 ++---
 ip/link_vti6.c        |  5 ++---
 ip/tcp_metrics.c      |  6 ++----
 lib/utils.c           |  9 ++++++++-
 misc/ss.c             |  4 ++--
 tc/m_nat.c            |  4 ++--
 23 files changed, 59 insertions(+), 89 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 88f1b63c233e9..e8c314a3c0771 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -104,7 +104,6 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "dev %s ", ll_index_to_name(r->ndm_ifindex));
 
 	if (tb[NDA_DST]) {
-		SPRINT_BUF(abuf);
 		int family = AF_INET;
 
 		if (RTA_PAYLOAD(tb[NDA_DST]) == sizeof(struct in6_addr))
@@ -113,8 +112,7 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "dst %s ",
 			format_host(family,
 				    RTA_PAYLOAD(tb[NDA_DST]),
-				    RTA_DATA(tb[NDA_DST]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[NDA_DST])));
 	}
 
 	if (tb[NDA_VLAN]) {
diff --git a/include/utils.h b/include/utils.h
index c43427c35a6cc..84083b0dbba71 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -122,8 +122,9 @@ int addr64_n2a(__u64 addr, char *buff, size_t len);
 int af_bit_len(int af);
 int af_byte_len(int af);
 
-const char *format_host(int af, int len, const void *addr,
+const char *format_host_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
+const char *format_host(int af, int lne, const void *addr);
 const char *rt_addr_n2a(int af, int len, const void *addr,
 			       char *buf, int buflen);
 
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 2e9d3ed40d003..d588645eb942a 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -77,7 +77,7 @@ static void print_tunnel(struct ip6_tnl_parm2 *p)
 	printf("%s: %s/ipv6 remote %s local %s",
 	       p->name,
 	       tnl_strproto(p->proto),
-	       format_host(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
+	       format_host_r(AF_INET6, 16, &p->raddr, s1, sizeof(s1)),
 	       rt_addr_n2a(AF_INET6, 16, &p->laddr, s2, sizeof(s2)));
 	if (p->link) {
 		const char *n = ll_index_to_name(p->link);
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 90d7b1096c3aa..03c8c03cd4a17 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -964,7 +964,6 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	/* Use local copy of ifa_flags to not interfere with filtering code */
 	unsigned int ifa_flags;
 	struct rtattr *rta_tb[IFA_MAX+1];
-	char abuf[256];
 
 	SPRINT_BUF(b1);
 
@@ -1061,8 +1060,7 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s",
 		              format_host(ifa->ifa_family,
 		                          RTA_PAYLOAD(rta_tb[IFA_LOCAL]),
-		                          RTA_DATA(rta_tb[IFA_LOCAL]),
-		                          abuf, sizeof(abuf)));
+		                          RTA_DATA(rta_tb[IFA_LOCAL])));
 		if (rta_tb[IFA_ADDRESS] &&
 		    memcmp(RTA_DATA(rta_tb[IFA_ADDRESS]),
 		           RTA_DATA(rta_tb[IFA_LOCAL]),
@@ -1071,8 +1069,7 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 			color_fprintf(fp, ifa_family_color(ifa->ifa_family),
 			              "%s", format_host(ifa->ifa_family,
 			              RTA_PAYLOAD(rta_tb[IFA_ADDRESS]),
-			              RTA_DATA(rta_tb[IFA_ADDRESS]),
-			              abuf, sizeof(abuf)));
+			              RTA_DATA(rta_tb[IFA_ADDRESS])));
 		}
 		fprintf(fp, "/%d ", ifa->ifa_prefixlen);
 	}
@@ -1085,16 +1082,14 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s ",
 			format_host(ifa->ifa_family,
 				    RTA_PAYLOAD(rta_tb[IFA_BROADCAST]),
-				    RTA_DATA(rta_tb[IFA_BROADCAST]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(rta_tb[IFA_BROADCAST])));
 	}
 	if (rta_tb[IFA_ANYCAST]) {
 		fprintf(fp, "any ");
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s ",
 			format_host(ifa->ifa_family,
 				    RTA_PAYLOAD(rta_tb[IFA_ANYCAST]),
-				    RTA_DATA(rta_tb[IFA_ANYCAST]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(rta_tb[IFA_ANYCAST])));
 	}
 	fprintf(fp, "scope %s ", rtnl_rtscope_n2a(ifa->ifa_scope, b1, sizeof(b1)));
 	if (ifa_flags & IFA_F_SECONDARY) {
diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c
index 05c44978ff37a..6076bb952297f 100644
--- a/ip/ipaddrlabel.c
+++ b/ip/ipaddrlabel.c
@@ -60,7 +60,6 @@ int print_addrlabel(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg
 	struct ifaddrlblmsg *ifal = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[IFAL_MAX+1];
-	char abuf[256];
 
 	if (n->nlmsg_type != RTM_NEWADDRLABEL && n->nlmsg_type != RTM_DELADDRLABEL)
 		return 0;
@@ -78,8 +77,7 @@ int print_addrlabel(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg
 		fprintf(fp, "prefix %s/%u ",
 			format_host(ifal->ifal_family,
 				    RTA_PAYLOAD(tb[IFAL_ADDRESS]),
-				    RTA_DATA(tb[IFAL_ADDRESS]),
-				    abuf, sizeof(abuf)),
+				    RTA_DATA(tb[IFAL_ADDRESS])),
 			ifal->ifal_prefixlen);
 	}
 
diff --git a/ip/iplink_geneve.c b/ip/iplink_geneve.c
index 40c08ec270d15..c6e7bbe0ce20e 100644
--- a/ip/iplink_geneve.c
+++ b/ip/iplink_geneve.c
@@ -138,7 +138,6 @@ static int geneve_parse_opt(struct link_util *lu, int argc, char **argv,
 static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	__u32 vni;
-	char s1[1024];
 	__u8 tos;
 
 	if (!tb)
@@ -156,7 +155,7 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 		if (addr)
 			fprintf(f, "remote %s ",
-				format_host(AF_INET, 4, &addr, s1, sizeof(s1)));
+				format_host(AF_INET, 4, &addr));
 	} else if (tb[IFLA_GENEVE_REMOTE6]) {
 		struct in6_addr addr;
 
@@ -164,7 +163,7 @@ static void geneve_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (memcmp(&addr, &in6addr_any, sizeof(addr)) != 0) {
 			if (IN6_IS_ADDR_MULTICAST(&addr))
 				fprintf(f, "remote %s ",
-					format_host(AF_INET6, sizeof(struct in6_addr), &addr, s1, sizeof(s1)));
+					format_host(AF_INET6, sizeof(struct in6_addr), &addr));
 		}
 	}
 
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 5c23cf534ce81..543522e139ff4 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -310,7 +310,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int link;
 	__u8 tos;
 	__u32 maxaddr;
-	char s1[1024];
 	char s2[64];
 
 	if (!tb)
@@ -329,10 +328,10 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (addr) {
 			if (IN_MULTICAST(ntohl(addr)))
 				fprintf(f, "group %s ",
-					format_host(AF_INET, 4, &addr, s1, sizeof(s1)));
+					format_host(AF_INET, 4, &addr));
 			else
 				fprintf(f, "remote %s ",
-					format_host(AF_INET, 4, &addr, s1, sizeof(s1)));
+					format_host(AF_INET, 4, &addr));
 		}
 	} else if (tb[IFLA_VXLAN_GROUP6]) {
 		struct in6_addr addr;
@@ -341,10 +340,10 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (memcmp(&addr, &in6addr_any, sizeof(addr)) != 0) {
 			if (IN6_IS_ADDR_MULTICAST(&addr))
 				fprintf(f, "group %s ",
-					format_host(AF_INET6, sizeof(struct in6_addr), &addr, s1, sizeof(s1)));
+					format_host(AF_INET6, sizeof(struct in6_addr), &addr));
 			else
 				fprintf(f, "remote %s ",
-					format_host(AF_INET6, sizeof(struct in6_addr), &addr, s1, sizeof(s1)));
+					format_host(AF_INET6, sizeof(struct in6_addr), &addr));
 		}
 	}
 
@@ -353,14 +352,14 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 
 		if (addr)
 			fprintf(f, "local %s ",
-				format_host(AF_INET, 4, &addr, s1, sizeof(s1)));
+				format_host(AF_INET, 4, &addr));
 	} else if (tb[IFLA_VXLAN_LOCAL6]) {
 		struct in6_addr addr;
 
 		memcpy(&addr, RTA_DATA(tb[IFLA_VXLAN_LOCAL6]), sizeof(struct in6_addr));
 		if (memcmp(&addr, &in6addr_any, sizeof(addr)) != 0)
 			fprintf(f, "local %s ",
-				format_host(AF_INET6, sizeof(struct in6_addr), &addr, s1, sizeof(s1)));
+				format_host(AF_INET6, sizeof(struct in6_addr), &addr));
 	}
 
 	if (tb[IFLA_VXLAN_LINK] &&
diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index 347990b5c8f52..c3673979f9761 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -204,8 +204,6 @@ static void print_maddr(FILE *fp, struct ma_info *list)
 						    list->addr.bytelen, 0,
 						    b1, sizeof(b1)));
 	} else {
-		char abuf[256];
-
 		switch (list->addr.family) {
 		case AF_INET:
 			fprintf(fp, "inet  ");
@@ -219,9 +217,7 @@ static void print_maddr(FILE *fp, struct ma_info *list)
 		}
 		fprintf(fp, "%s",
 			format_host(list->addr.family,
-				    -1,
-				    list->addr.data,
-				    abuf, sizeof(abuf)));
+				    -1, list->addr.data));
 	}
 	if (list->users != 1)
 		fprintf(fp, " users %d", list->users);
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 6458c8eaa6b1e..583aad30dc820 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -198,7 +198,6 @@ int print_neigh(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	struct ndmsg *r = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[NDA_MAX+1];
-	char abuf[256];
 	static int logit = 1;
 
 	if (n->nlmsg_type != RTM_NEWNEIGH && n->nlmsg_type != RTM_DELNEIGH &&
@@ -281,8 +280,7 @@ int print_neigh(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "%s ",
 			format_host(r->ndm_family,
 				    RTA_PAYLOAD(tb[NDA_DST]),
-				    RTA_DATA(tb[NDA_DST]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[NDA_DST])));
 	}
 	if (!filter.index && r->ndm_ifindex)
 		fprintf(fp, "dev %s ", ll_index_to_name(r->ndm_ifindex));
diff --git a/ip/iproute.c b/ip/iproute.c
index 19b28701d4da1..8315ad3e6c6ac 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -380,8 +380,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		} else {
 			fprintf(fp, "%s ", format_host(r->rtm_family,
 						       RTA_PAYLOAD(tb[RTA_DST]),
-						       RTA_DATA(tb[RTA_DST]),
-						       abuf, sizeof(abuf))
+						       RTA_DATA(tb[RTA_DST]))
 				);
 		}
 	} else if (r->rtm_dst_len) {
@@ -400,8 +399,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		} else {
 			fprintf(fp, "from %s ", format_host(r->rtm_family,
 						       RTA_PAYLOAD(tb[RTA_SRC]),
-						       RTA_DATA(tb[RTA_SRC]),
-						       abuf, sizeof(abuf))
+						       RTA_DATA(tb[RTA_SRC]))
 				);
 		}
 	} else if (r->rtm_src_len) {
@@ -410,8 +408,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	if (tb[RTA_NEWDST]) {
 		fprintf(fp, "as to %s ", format_host(r->rtm_family,
 						  RTA_PAYLOAD(tb[RTA_NEWDST]),
-						  RTA_DATA(tb[RTA_NEWDST]),
-						  abuf, sizeof(abuf))
+						  RTA_DATA(tb[RTA_NEWDST]))
 			);
 	}
 
@@ -427,8 +424,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "via %s ",
 			format_host(r->rtm_family,
 				    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-				    RTA_DATA(tb[RTA_GATEWAY]),
-				    abuf, sizeof(abuf)));
+				    RTA_DATA(tb[RTA_GATEWAY])));
 	}
 	if (tb[RTA_VIA]) {
 		size_t len = RTA_PAYLOAD(tb[RTA_VIA]) - 2;
@@ -436,8 +432,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 		fprintf(fp, "via %s %s ",
 			family_name(via->rtvia_family),
-			format_host(via->rtvia_family, len, via->rtvia_addr,
-				    abuf, sizeof(abuf)));
+			format_host(via->rtvia_family, len, via->rtvia_addr));
 	}
 	if (tb[RTA_OIF] && filter.oifmask != -1)
 		fprintf(fp, "dev %s ", ll_index_to_name(*(int *)RTA_DATA(tb[RTA_OIF])));
@@ -671,8 +666,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 					fprintf(fp, " via %s ",
 						format_host(r->rtm_family,
 							    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-							    RTA_DATA(tb[RTA_GATEWAY]),
-							    abuf, sizeof(abuf)));
+							    RTA_DATA(tb[RTA_GATEWAY])));
 				}
 				if (tb[RTA_VIA]) {
 					size_t len = RTA_PAYLOAD(tb[RTA_VIA]) - 2;
@@ -680,8 +674,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 					fprintf(fp, "via %s %s ",
 						family_name(via->rtvia_family),
-						format_host(via->rtvia_family, len, via->rtvia_addr,
-							    abuf, sizeof(abuf)));
+						format_host(via->rtvia_family, len, via->rtvia_addr));
 				}
 				if (tb[RTA_FLOW]) {
 					__u32 to = rta_getattr_u32(tb[RTA_FLOW]);
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index a53e11d9478e9..42abe3753ac6a 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -58,15 +58,13 @@ static const char *format_encap_type(int type)
 static void print_encap_mpls(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[MPLS_IPTUNNEL_MAX+1];
-	char abuf[256];
 
 	parse_rtattr_nested(tb, MPLS_IPTUNNEL_MAX, encap);
 
 	if (tb[MPLS_IPTUNNEL_DST])
 		fprintf(fp, " %s ", format_host(AF_MPLS,
 			RTA_PAYLOAD(tb[MPLS_IPTUNNEL_DST]),
-			RTA_DATA(tb[MPLS_IPTUNNEL_DST]),
-			abuf, sizeof(abuf)));
+			RTA_DATA(tb[MPLS_IPTUNNEL_DST])));
 }
 
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
diff --git a/ip/iprule.c b/ip/iprule.c
index dc45c6b43e606..345b3b042307d 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -94,8 +94,7 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		} else {
 			fprintf(fp, "from %s ", format_host(r->rtm_family,
 						       RTA_PAYLOAD(tb[FRA_SRC]),
-						       RTA_DATA(tb[FRA_SRC]),
-						       abuf, sizeof(abuf))
+						       RTA_DATA(tb[FRA_SRC]))
 				);
 		}
 	} else if (r->rtm_src_len) {
@@ -115,8 +114,7 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		} else {
 			fprintf(fp, "to %s ", format_host(r->rtm_family,
 						       RTA_PAYLOAD(tb[FRA_DST]),
-						       RTA_DATA(tb[FRA_DST]),
-						       abuf, sizeof(abuf)));
+						       RTA_DATA(tb[FRA_DST])));
 		}
 	} else if (r->rtm_dst_len) {
 		fprintf(fp, "to 0/%d ", r->rtm_dst_len);
@@ -191,8 +189,7 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			fprintf(fp, "map-to %s ",
 				format_host(r->rtm_family,
 					    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-					    RTA_DATA(tb[RTA_GATEWAY]),
-					    abuf, sizeof(abuf)));
+					    RTA_DATA(tb[RTA_GATEWAY])));
 		} else
 			fprintf(fp, "masquerade");
 	} else if (r->rtm_type == FR_ACT_GOTO) {
diff --git a/ip/iptoken.c b/ip/iptoken.c
index 1d22c03c63ab7..02fe98e249526 100644
--- a/ip/iptoken.c
+++ b/ip/iptoken.c
@@ -51,7 +51,6 @@ static int print_token(const struct sockaddr_nl *who, struct nlmsghdr *n, void *
 	int len = n->nlmsg_len;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *ltb[IFLA_INET6_MAX + 1];
-	char abuf[256];
 
 	if (n->nlmsg_type != RTM_NEWLINK)
 		return -1;
@@ -82,8 +81,7 @@ static int print_token(const struct sockaddr_nl *who, struct nlmsghdr *n, void *
 	fprintf(fp, "token %s ",
 		format_host(ifi->ifi_family,
 			    RTA_PAYLOAD(ltb[IFLA_INET6_TOKEN]),
-			    RTA_DATA(ltb[IFLA_INET6_TOKEN]),
-			    abuf, sizeof(abuf)));
+			    RTA_DATA(ltb[IFLA_INET6_TOKEN])));
 	fprintf(fp, "dev %s ", ll_index_to_name(ifi->ifi_index));
 	fprintf(fp, "\n");
 	fflush(fp);
diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 65a4e6e9c1a5a..bfee99561615a 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -308,7 +308,7 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 	printf("%s: %s/ip remote %s local %s",
 	       p->name,
 	       tnl_strproto(p->iph.protocol),
-	       p->iph.daddr ? format_host(AF_INET, 4, &p->iph.daddr, s1, sizeof(s1)) : "any",
+	       p->iph.daddr ? format_host_r(AF_INET, 4, &p->iph.daddr, s1, sizeof(s1)) : "any",
 	       p->iph.saddr ? rt_addr_n2a(AF_INET, 4, &p->iph.saddr, s2, sizeof(s2)) : "any");
 
 	if (p->iph.protocol == IPPROTO_IPV6 && (p->i_flags & SIT_ISATAP)) {
@@ -324,7 +324,7 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 				if (prl[i].addr != htonl(INADDR_ANY)) {
 					printf(" %s %s ",
 					       (prl[i].flags & PRL_DEFAULT) ? "pdr" : "pr",
-					       format_host(AF_INET, 4, &prl[i].addr, s1, sizeof(s1)));
+					       format_host(AF_INET, 4, &prl[i].addr));
 				}
 			}
 	}
@@ -360,7 +360,7 @@ static void print_tunnel(struct ip_tunnel_parm *p)
 		       ip6rd.prefixlen);
 		if (ip6rd.relay_prefix) {
 			printf(" 6rd-relay_prefix %s/%u",
-			       format_host(AF_INET, 4, &ip6rd.relay_prefix, s1, sizeof(s1)),
+			       format_host(AF_INET, 4, &ip6rd.relay_prefix),
 			       ip6rd.relay_prefixlen);
 		}
 	}
diff --git a/ip/link_gre.c b/ip/link_gre.c
index c50963d79d3c5..bcf003aaa5d7a 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -339,7 +339,6 @@ get_failed:
 
 static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[1024];
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
@@ -353,7 +352,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_REMOTE]);
 
 		if (addr)
-			remote = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			remote = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "remote %s ", remote);
@@ -362,7 +361,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_LOCAL]);
 
 		if (addr)
-			local = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			local = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "local %s ", local);
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a48ea8b1ebae3..bddfc0ff97aa0 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -302,7 +302,6 @@ get_failed:
 
 static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[1024];
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
@@ -327,7 +326,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		memcpy(&addr, RTA_DATA(tb[IFLA_GRE_REMOTE]), sizeof(addr));
 
 		if (memcmp(&addr, &in6_addr_any, sizeof(addr)))
-			remote = format_host(AF_INET6, sizeof(addr), &addr, s1, sizeof(s1));
+			remote = format_host(AF_INET6, sizeof(addr), &addr);
 	}
 
 	fprintf(f, "remote %s ", remote);
@@ -338,7 +337,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		memcpy(&addr, RTA_DATA(tb[IFLA_GRE_LOCAL]), sizeof(addr));
 
 		if (memcmp(&addr, &in6_addr_any, sizeof(addr)))
-			local = format_host(AF_INET6, sizeof(addr), &addr, s1, sizeof(s1));
+			local = format_host(AF_INET6, sizeof(addr), &addr);
 	}
 
 	fprintf(f, "local %s ", local);
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 04568ffbd81bf..8411a6a00a1b4 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -339,7 +339,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
 
 		if (addr)
-			remote = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			remote = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "remote %s ", remote);
@@ -348,7 +348,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_LOCAL]);
 
 		if (addr)
-			local = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			local = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "local %s ", local);
@@ -404,8 +404,7 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		       prefixlen);
 		if (relayprefix) {
 			printf("6rd-relay_prefix %s/%u ",
-			       format_host(AF_INET, 4, &relayprefix, s1,
-					   sizeof(s1)),
+			       format_host(AF_INET, 4, &relayprefix),
 			       relayprefixlen);
 		}
 	}
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 587993035e8d5..8052e7514989a 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -198,7 +198,6 @@ get_failed:
 
 static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[1024];
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
@@ -210,7 +209,7 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		unsigned int addr = *(__u32 *)RTA_DATA(tb[IFLA_VTI_REMOTE]);
 
 		if (addr)
-			remote = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			remote = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "remote %s ", remote);
@@ -219,7 +218,7 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		unsigned int addr = *(__u32 *)RTA_DATA(tb[IFLA_VTI_LOCAL]);
 
 		if (addr)
-			local = format_host(AF_INET, 4, &addr, s1, sizeof(s1));
+			local = format_host(AF_INET, 4, &addr);
 	}
 
 	fprintf(f, "local %s ", local);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 9a62eb5aa370d..9bcf7fe9dea46 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -197,7 +197,6 @@ get_failed:
 
 static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	char s1[1024];
 	char s2[64];
 	const char *local = "any";
 	const char *remote = "any";
@@ -210,7 +209,7 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_VTI_REMOTE]) {
 		memcpy(&daddr, RTA_DATA(tb[IFLA_VTI_REMOTE]), sizeof(daddr));
 
-		remote = format_host(AF_INET6, 16, &daddr, s1, sizeof(s1));
+		remote = format_host(AF_INET6, 16, &daddr);
 	}
 
 	fprintf(f, "remote %s ", remote);
@@ -218,7 +217,7 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_VTI_LOCAL]) {
 		memcpy(&saddr, RTA_DATA(tb[IFLA_VTI_LOCAL]), sizeof(saddr));
 
-		local = format_host(AF_INET6, 16, &saddr, s1, sizeof(s1));
+		local = format_host(AF_INET6, 16, &saddr);
 	}
 
 	fprintf(f, "local %s ", local);
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index 3a649917191a8..f82604f458ada 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -95,7 +95,6 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	struct genlmsghdr *ghdr;
 	struct rtattr *attrs[TCP_METRICS_ATTR_MAX + 1], *a;
 	int len = n->nlmsg_len;
-	char abuf[256];
 	inet_prefix daddr, saddr;
 	int family, i, atype, stype, dlen = 0, slen = 0;
 
@@ -194,7 +193,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		fprintf(fp, "Deleted ");
 
 	fprintf(fp, "%s",
-		format_host(family, dlen, &daddr.data, abuf, sizeof(abuf)));
+		format_host(family, dlen, &daddr.data));
 
 	a = attrs[TCP_METRICS_ATTR_AGE];
 	if (a) {
@@ -298,8 +297,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	if (slen) {
 		fprintf(fp, " source %s",
-			format_host(family, slen, &saddr.data, abuf,
-				    sizeof(abuf)));
+			format_host(family, slen, &saddr.data));
 	}
 
 	fprintf(fp, "\n");
diff --git a/lib/utils.c b/lib/utils.c
index fa35f4d044064..22a5ef8fa8213 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -818,7 +818,7 @@ static const char *resolve_address(const void *addr, int len, int af)
 }
 #endif
 
-const char *format_host(int af, int len, const void *addr,
+const char *format_host_r(int af, int len, const void *addr,
 			char *buf, int buflen)
 {
 #ifdef RESOLVE_HOSTNAMES
@@ -835,6 +835,13 @@ const char *format_host(int af, int len, const void *addr,
 	return rt_addr_n2a(af, len, addr, buf, buflen);
 }
 
+const char *format_host(int af, int len, const void *addr)
+{
+	static char buf[256];
+
+	return format_host_r(af, len, addr, buf, 256);
+}
+
 
 char *hexstring_n2a(const __u8 *str, int len, char *buf, int blen)
 {
diff --git a/misc/ss.c b/misc/ss.c
index 192389cc82371..38cf3312a746e 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1016,10 +1016,10 @@ static void inet_addr_print(const inet_prefix *a, int port, unsigned int ifindex
 			buf[0] = '*';
 			buf[1] = 0;
 		} else {
-			ap = format_host(AF_INET, 4, a->data, buf, sizeof(buf));
+			ap = format_host(AF_INET, 4, a->data);
 		}
 	} else {
-		ap = format_host(a->family, 16, a->data, buf, sizeof(buf));
+		ap = format_host(a->family, 16, a->data);
 		est_len = strlen(ap);
 		if (est_len <= addr_width)
 			est_len = addr_width;
diff --git a/tc/m_nat.c b/tc/m_nat.c
index deb071da90daf..4b90121c4b99e 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -191,9 +191,9 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
 
 	fprintf(f, " nat %s %s/%d %s %s", sel->flags & TCA_NAT_FLAG_EGRESS ?
 					  "egress" : "ingress",
-		format_host(AF_INET, 4, &sel->old_addr, buf1, sizeof(buf1)),
+		format_host_r(AF_INET, 4, &sel->old_addr, buf1, sizeof(buf1)),
 		len,
-		format_host(AF_INET, 4, &sel->new_addr, buf2, sizeof(buf2)),
+		format_host_r(AF_INET, 4, &sel->new_addr, buf2, sizeof(buf2)),
 		action_n2a(sel->action, buf3, sizeof(buf3)));
 
 	if (show_stats) {
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH 6/7] lib/utils: introduce rt_addr_n2a_rta()
From: Phil Sutter @ 2016-03-22 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1458671719-14361-1-git-send-email-phil@nwl.cc>

This simple macro eases calling rt_addr_n2a() with data from an rt_attr
pointer.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/utils.h       |  2 ++
 ip/iplink_bond.c      |  4 +---
 ip/ipmroute.c         |  8 ++------
 ip/ipprefix.c         | 14 +++-----------
 ip/iproute.c          | 20 +++++++-------------
 ip/iproute_lwtunnel.c | 19 ++++---------------
 ip/iprule.c           | 16 ++++++----------
 ip/link_ip6tnl.c      |  8 ++------
 tc/f_flower.c         |  8 ++------
 9 files changed, 29 insertions(+), 70 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index ebb80c9c20b6d..ef81d00f3d70d 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -130,6 +130,8 @@ const char *format_host(int af, int lne, const void *addr);
 const char *rt_addr_n2a_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
 const char *rt_addr_n2a(int af, int len, const void *addr);
+#define rt_addr_n2a_rta(af, rta) \
+	rt_addr_n2a(af, RTA_PAYLOAD(rta), RTA_DATA(rta))
 
 int read_family(const char *name);
 const char *family_name(int family);
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index 7da58e4556c07..fe83479a091a8 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -422,9 +422,7 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
 			if (iptb[i])
 				fprintf(f, "%s",
-					rt_addr_n2a(AF_INET,
-						    RTA_PAYLOAD(iptb[i]),
-						    RTA_DATA(iptb[i])));
+					rt_addr_n2a_rta(AF_INET, iptb[i]));
 			if (i < BOND_MAX_ARP_TARGETS-1 && iptb[i+1])
 				fprintf(f, ",");
 		}
diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index 2b9f892a62630..c33cdcbbde21b 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -123,16 +123,12 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[RTA_SRC])
 		len = snprintf(obuf, sizeof(obuf),
-			       "(%s, ", rt_addr_n2a(family,
-						    RTA_PAYLOAD(tb[RTA_SRC]),
-						    RTA_DATA(tb[RTA_SRC])));
+			       "(%s, ", rt_addr_n2a_rta(family, tb[RTA_SRC]));
 	else
 		len = sprintf(obuf, "(unknown, ");
 	if (tb[RTA_DST])
 		snprintf(obuf + len, sizeof(obuf) - len,
-			 "%s)", rt_addr_n2a(family,
-					    RTA_PAYLOAD(tb[RTA_DST]),
-					    RTA_DATA(tb[RTA_DST])));
+			 "%s)", rt_addr_n2a_rta(family, tb[RTA_DST]));
 	else
 		snprintf(obuf + len, sizeof(obuf) - len, "unknown) ");
 
diff --git a/ip/ipprefix.c b/ip/ipprefix.c
index 4d986dbc1a5d1..a833efcf67c4a 100644
--- a/ip/ipprefix.c
+++ b/ip/ipprefix.c
@@ -71,19 +71,11 @@ int print_prefix(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	parse_rtattr(tb, RTA_MAX, RTM_RTA(prefix), len);
 
-	fprintf(fp, "prefix ");
-
 	if (tb[PREFIX_ADDRESS]) {
-		struct in6_addr *pfx;
-
-		pfx = (struct in6_addr *)RTA_DATA(tb[PREFIX_ADDRESS]);
-
-		fprintf(fp, "%s", rt_addr_n2a(family,
-					      RTA_PAYLOAD(tb[PREFIX_ADDRESS]),
-					      pfx));
+		fprintf(fp, "prefix %s/%u",
+		        rt_addr_n2a_rta(family, tb[PREFIX_ADDRESS]),
+			prefix->prefix_len);
 	}
-	fprintf(fp, "/%u ", prefix->prefix_len);
-
 	fprintf(fp, "dev %s ", ll_index_to_name(prefix->prefix_ifindex));
 
 	if (prefix->prefix_flags & IF_PREFIX_ONLINK)
diff --git a/ip/iproute.c b/ip/iproute.c
index 67d551b54d00c..8224d7ffa94bf 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -370,11 +370,9 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[RTA_DST]) {
 		if (r->rtm_dst_len != host_len) {
-			fprintf(fp, "%s/%u ", rt_addr_n2a(r->rtm_family,
-						       RTA_PAYLOAD(tb[RTA_DST]),
-						       RTA_DATA(tb[RTA_DST])),
-				r->rtm_dst_len
-				);
+			fprintf(fp, "%s/%u ",
+			        rt_addr_n2a_rta(r->rtm_family, tb[RTA_DST]),
+			        r->rtm_dst_len);
 		} else {
 			fprintf(fp, "%s ",
 			        format_host_rta(r->rtm_family, tb[RTA_DST]));
@@ -386,11 +384,9 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	}
 	if (tb[RTA_SRC]) {
 		if (r->rtm_src_len != host_len) {
-			fprintf(fp, "from %s/%u ", rt_addr_n2a(r->rtm_family,
-						       RTA_PAYLOAD(tb[RTA_SRC]),
-						       RTA_DATA(tb[RTA_SRC])),
-				r->rtm_src_len
-				);
+			fprintf(fp, "from %s/%u ",
+			        rt_addr_n2a_rta(r->rtm_family, tb[RTA_SRC]),
+			        r->rtm_src_len);
 		} else {
 			fprintf(fp, "from %s ",
 			        format_host_rta(r->rtm_family, tb[RTA_SRC]));
@@ -439,9 +435,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		   and symbolic name will not be useful.
 		 */
 		fprintf(fp, " src %s ",
-			rt_addr_n2a(r->rtm_family,
-				    RTA_PAYLOAD(tb[RTA_PREFSRC]),
-				    RTA_DATA(tb[RTA_PREFSRC])));
+			rt_addr_n2a_rta(r->rtm_family, tb[RTA_PREFSRC]));
 	}
 	if (tb[RTA_PRIORITY])
 		fprintf(fp, " metric %u ", rta_getattr_u32(tb[RTA_PRIORITY]));
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 56af9e4e92ecd..3baac77208168 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -77,15 +77,11 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap)
 
 	if (tb[LWTUNNEL_IP_SRC])
 		fprintf(fp, "src %s ",
-			rt_addr_n2a(AF_INET,
-				    RTA_PAYLOAD(tb[LWTUNNEL_IP_SRC]),
-				    RTA_DATA(tb[LWTUNNEL_IP_SRC])));
+			rt_addr_n2a_rta(AF_INET, tb[LWTUNNEL_IP_SRC]));
 
 	if (tb[LWTUNNEL_IP_DST])
 		fprintf(fp, "dst %s ",
-			rt_addr_n2a(AF_INET,
-				    RTA_PAYLOAD(tb[LWTUNNEL_IP_DST]),
-				    RTA_DATA(tb[LWTUNNEL_IP_DST])));
+			rt_addr_n2a_rta(AF_INET, tb[LWTUNNEL_IP_DST]));
 
 	if (tb[LWTUNNEL_IP_TTL])
 		fprintf(fp, "ttl %d ", rta_getattr_u8(tb[LWTUNNEL_IP_TTL]));
@@ -112,7 +108,6 @@ static void print_encap_ila(FILE *fp, struct rtattr *encap)
 static void print_encap_ip6(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[LWTUNNEL_IP6_MAX+1];
-	char abuf[256];
 
 	parse_rtattr_nested(tb, LWTUNNEL_IP6_MAX, encap);
 
@@ -121,17 +116,11 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap)
 
 	if (tb[LWTUNNEL_IP6_SRC])
 		fprintf(fp, "src %s ",
-			rt_addr_n2a(AF_INET6,
-				    RTA_PAYLOAD(tb[LWTUNNEL_IP6_SRC]),
-				    RTA_DATA(tb[LWTUNNEL_IP6_SRC]),
-				    abuf, sizeof(abuf)));
+			rt_addr_n2a_rta(AF_INET6, tb[LWTUNNEL_IP6_SRC]));
 
 	if (tb[LWTUNNEL_IP6_DST])
 		fprintf(fp, "dst %s ",
-			rt_addr_n2a(AF_INET6,
-				    RTA_PAYLOAD(tb[LWTUNNEL_IP6_DST]),
-				    RTA_DATA(tb[LWTUNNEL_IP6_DST]),
-				    abuf, sizeof(abuf)));
+			rt_addr_n2a_rta(AF_INET6, tb[LWTUNNEL_IP6_DST]));
 
 	if (tb[LWTUNNEL_IP6_HOPLIMIT])
 		fprintf(fp, "hoplimit %d ", rta_getattr_u8(tb[LWTUNNEL_IP6_HOPLIMIT]));
diff --git a/ip/iprule.c b/ip/iprule.c
index ac570440d6633..7cb19e4d5ebd0 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -83,11 +83,9 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[FRA_SRC]) {
 		if (r->rtm_src_len != host_len) {
-			fprintf(fp, "from %s/%u ", rt_addr_n2a(r->rtm_family,
-						       RTA_PAYLOAD(tb[FRA_SRC]),
-						       RTA_DATA(tb[FRA_SRC])),
-				r->rtm_src_len
-				);
+			fprintf(fp, "from %s/%u ",
+			        rt_addr_n2a_rta(r->rtm_family, tb[FRA_SRC]),
+			        r->rtm_src_len);
 		} else {
 			fprintf(fp, "from %s ",
 			        format_host_rta(r->rtm_family, tb[FRA_SRC]));
@@ -100,11 +98,9 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[FRA_DST]) {
 		if (r->rtm_dst_len != host_len) {
-			fprintf(fp, "to %s/%u ", rt_addr_n2a(r->rtm_family,
-						       RTA_PAYLOAD(tb[FRA_DST]),
-						       RTA_DATA(tb[FRA_DST])),
-				r->rtm_dst_len
-				);
+			fprintf(fp, "to %s/%u ",
+			        rt_addr_n2a_rta(r->rtm_family, tb[FRA_DST]),
+			        r->rtm_dst_len);
 		} else {
 			fprintf(fp, "to %s ",
 			        format_host_rta(r->rtm_family, tb[FRA_DST]));
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 4b32fe5aa8088..8a31d0dcd1883 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -289,16 +289,12 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 
 	if (tb[IFLA_IPTUN_REMOTE]) {
 		fprintf(f, "remote %s ",
-			rt_addr_n2a(AF_INET6,
-				    RTA_PAYLOAD(tb[IFLA_IPTUN_REMOTE]),
-				    RTA_DATA(tb[IFLA_IPTUN_REMOTE])));
+			rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_REMOTE]));
 	}
 
 	if (tb[IFLA_IPTUN_LOCAL]) {
 		fprintf(f, "local %s ",
-			rt_addr_n2a(AF_INET6,
-				    RTA_PAYLOAD(tb[IFLA_IPTUN_LOCAL]),
-				    RTA_DATA(tb[IFLA_IPTUN_LOCAL])));
+			rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_LOCAL]));
 	}
 
 	if (tb[IFLA_IPTUN_LINK] && rta_getattr_u32(tb[IFLA_IPTUN_LINK])) {
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 40f8c595b2eec..306f056c1b662 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -415,16 +415,12 @@ static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
 	}
 	if (!addr_attr || RTA_PAYLOAD(addr_attr) != len)
 		return;
-	fprintf(f, "\n  %s %s", name, rt_addr_n2a(family,
-						  RTA_PAYLOAD(addr_attr),
-						  RTA_DATA(addr_attr)));
+	fprintf(f, "\n  %s %s", name, rt_addr_n2a_rta(family, addr_attr));
 	if (!mask_attr || RTA_PAYLOAD(mask_attr) != len)
 		return;
 	bits = __mask_bits(RTA_DATA(mask_attr), len);
 	if (bits < 0)
-		fprintf(f, "/%s", rt_addr_n2a(family,
-					      RTA_PAYLOAD(mask_attr),
-					      RTA_DATA(mask_attr)));
+		fprintf(f, "/%s", rt_addr_n2a_rta(family, mask_attr));
 	else if (bits < len * 8)
 		fprintf(f, "/%d", bits);
 }
-- 
2.7.2

^ permalink raw reply related

* [iproute PATCH 5/7] lib/utils: introduce format_host_rta()
From: Phil Sutter @ 2016-03-22 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <1458671719-14361-1-git-send-email-phil@nwl.cc>

This simple macro eases calling format_host() with data from an rt_attr
pointer.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/utils.h       |  2 ++
 ip/ipaddress.c        | 20 ++++++++------------
 ip/ipaddrlabel.c      |  5 ++---
 ip/ipneigh.c          |  4 +---
 ip/iproute.c          | 33 +++++++++++----------------------
 ip/iproute_lwtunnel.c |  5 ++---
 ip/iprule.c           | 16 ++++++----------
 ip/iptoken.c          |  9 +++------
 8 files changed, 35 insertions(+), 59 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index bc2cbce0cc303..ebb80c9c20b6d 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -125,6 +125,8 @@ int af_byte_len(int af);
 const char *format_host_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
 const char *format_host(int af, int lne, const void *addr);
+#define format_host_rta(af, rta) \
+	format_host(af, RTA_PAYLOAD(rta), RTA_DATA(rta))
 const char *rt_addr_n2a_r(int af, int len, const void *addr,
 			       char *buf, int buflen);
 const char *rt_addr_n2a(int af, int len, const void *addr);
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 03c8c03cd4a17..3998d8cec4ab2 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1058,18 +1058,16 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	if (rta_tb[IFA_LOCAL]) {
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s",
-		              format_host(ifa->ifa_family,
-		                          RTA_PAYLOAD(rta_tb[IFA_LOCAL]),
-		                          RTA_DATA(rta_tb[IFA_LOCAL])));
+		              format_host_rta(ifa->ifa_family,
+		                              rta_tb[IFA_LOCAL]));
 		if (rta_tb[IFA_ADDRESS] &&
 		    memcmp(RTA_DATA(rta_tb[IFA_ADDRESS]),
 		           RTA_DATA(rta_tb[IFA_LOCAL]),
 		           ifa->ifa_family == AF_INET ? 4 : 16)) {
 			fprintf(fp, " peer ");
 			color_fprintf(fp, ifa_family_color(ifa->ifa_family),
-			              "%s", format_host(ifa->ifa_family,
-			              RTA_PAYLOAD(rta_tb[IFA_ADDRESS]),
-			              RTA_DATA(rta_tb[IFA_ADDRESS])));
+			              "%s", format_host_rta(ifa->ifa_family,
+			              rta_tb[IFA_ADDRESS]));
 		}
 		fprintf(fp, "/%d ", ifa->ifa_prefixlen);
 	}
@@ -1080,16 +1078,14 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	if (rta_tb[IFA_BROADCAST]) {
 		fprintf(fp, "brd ");
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s ",
-			format_host(ifa->ifa_family,
-				    RTA_PAYLOAD(rta_tb[IFA_BROADCAST]),
-				    RTA_DATA(rta_tb[IFA_BROADCAST])));
+		              format_host_rta(ifa->ifa_family,
+		                              rta_tb[IFA_BROADCAST]));
 	}
 	if (rta_tb[IFA_ANYCAST]) {
 		fprintf(fp, "any ");
 		color_fprintf(fp, ifa_family_color(ifa->ifa_family), "%s ",
-			format_host(ifa->ifa_family,
-				    RTA_PAYLOAD(rta_tb[IFA_ANYCAST]),
-				    RTA_DATA(rta_tb[IFA_ANYCAST])));
+		              format_host_rta(ifa->ifa_family,
+		                              rta_tb[IFA_ANYCAST]));
 	}
 	fprintf(fp, "scope %s ", rtnl_rtscope_n2a(ifa->ifa_scope, b1, sizeof(b1)));
 	if (ifa_flags & IFA_F_SECONDARY) {
diff --git a/ip/ipaddrlabel.c b/ip/ipaddrlabel.c
index 6076bb952297f..b4cd784094719 100644
--- a/ip/ipaddrlabel.c
+++ b/ip/ipaddrlabel.c
@@ -75,9 +75,8 @@ int print_addrlabel(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg
 
 	if (tb[IFAL_ADDRESS]) {
 		fprintf(fp, "prefix %s/%u ",
-			format_host(ifal->ifal_family,
-				    RTA_PAYLOAD(tb[IFAL_ADDRESS]),
-				    RTA_DATA(tb[IFAL_ADDRESS])),
+			format_host_rta(ifal->ifal_family,
+		                        tb[IFAL_ADDRESS]),
 			ifal->ifal_prefixlen);
 	}
 
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 583aad30dc820..c49fb4e7f7b58 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -278,9 +278,7 @@ int print_neigh(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "miss ");
 	if (tb[NDA_DST]) {
 		fprintf(fp, "%s ",
-			format_host(r->ndm_family,
-				    RTA_PAYLOAD(tb[NDA_DST]),
-				    RTA_DATA(tb[NDA_DST])));
+			format_host_rta(r->ndm_family, tb[NDA_DST]));
 	}
 	if (!filter.index && r->ndm_ifindex)
 		fprintf(fp, "dev %s ", ll_index_to_name(r->ndm_ifindex));
diff --git a/ip/iproute.c b/ip/iproute.c
index 1ec9294290b2a..67d551b54d00c 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -376,10 +376,8 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				r->rtm_dst_len
 				);
 		} else {
-			fprintf(fp, "%s ", format_host(r->rtm_family,
-						       RTA_PAYLOAD(tb[RTA_DST]),
-						       RTA_DATA(tb[RTA_DST]))
-				);
+			fprintf(fp, "%s ",
+			        format_host_rta(r->rtm_family, tb[RTA_DST]));
 		}
 	} else if (r->rtm_dst_len) {
 		fprintf(fp, "0/%d ", r->rtm_dst_len);
@@ -394,19 +392,15 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				r->rtm_src_len
 				);
 		} else {
-			fprintf(fp, "from %s ", format_host(r->rtm_family,
-						       RTA_PAYLOAD(tb[RTA_SRC]),
-						       RTA_DATA(tb[RTA_SRC]))
-				);
+			fprintf(fp, "from %s ",
+			        format_host_rta(r->rtm_family, tb[RTA_SRC]));
 		}
 	} else if (r->rtm_src_len) {
 		fprintf(fp, "from 0/%u ", r->rtm_src_len);
 	}
 	if (tb[RTA_NEWDST]) {
-		fprintf(fp, "as to %s ", format_host(r->rtm_family,
-						  RTA_PAYLOAD(tb[RTA_NEWDST]),
-						  RTA_DATA(tb[RTA_NEWDST]))
-			);
+		fprintf(fp, "as to %s ",
+		        format_host_rta(r->rtm_family, tb[RTA_NEWDST]));
 	}
 
 	if (tb[RTA_ENCAP])
@@ -419,9 +413,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[RTA_GATEWAY] && filter.rvia.bitlen != host_len) {
 		fprintf(fp, "via %s ",
-			format_host(r->rtm_family,
-				    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-				    RTA_DATA(tb[RTA_GATEWAY])));
+		        format_host_rta(r->rtm_family, tb[RTA_GATEWAY]));
 	}
 	if (tb[RTA_VIA]) {
 		size_t len = RTA_PAYLOAD(tb[RTA_VIA]) - 2;
@@ -653,16 +645,13 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 							tb[RTA_ENCAP]);
 				if (tb[RTA_NEWDST]) {
 					fprintf(fp, " as to %s ",
-						format_host(r->rtm_family,
-						RTA_PAYLOAD(tb[RTA_NEWDST]),
-						RTA_DATA(tb[RTA_NEWDST]),
-						abuf, sizeof(abuf)));
+						format_host_rta(r->rtm_family,
+								tb[RTA_NEWDST]));
 				}
 				if (tb[RTA_GATEWAY]) {
 					fprintf(fp, " via %s ",
-						format_host(r->rtm_family,
-							    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-							    RTA_DATA(tb[RTA_GATEWAY])));
+						format_host_rta(r->rtm_family,
+								tb[RTA_GATEWAY]));
 				}
 				if (tb[RTA_VIA]) {
 					size_t len = RTA_PAYLOAD(tb[RTA_VIA]) - 2;
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 53d3ad4e6ea8e..56af9e4e92ecd 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -62,9 +62,8 @@ static void print_encap_mpls(FILE *fp, struct rtattr *encap)
 	parse_rtattr_nested(tb, MPLS_IPTUNNEL_MAX, encap);
 
 	if (tb[MPLS_IPTUNNEL_DST])
-		fprintf(fp, " %s ", format_host(AF_MPLS,
-			RTA_PAYLOAD(tb[MPLS_IPTUNNEL_DST]),
-			RTA_DATA(tb[MPLS_IPTUNNEL_DST])));
+		fprintf(fp, " %s ",
+		        format_host_rta(AF_MPLS, tb[MPLS_IPTUNNEL_DST]));
 }
 
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
diff --git a/ip/iprule.c b/ip/iprule.c
index 3fd510efe5e1b..ac570440d6633 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -89,10 +89,8 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				r->rtm_src_len
 				);
 		} else {
-			fprintf(fp, "from %s ", format_host(r->rtm_family,
-						       RTA_PAYLOAD(tb[FRA_SRC]),
-						       RTA_DATA(tb[FRA_SRC]))
-				);
+			fprintf(fp, "from %s ",
+			        format_host_rta(r->rtm_family, tb[FRA_SRC]));
 		}
 	} else if (r->rtm_src_len) {
 		fprintf(fp, "from 0/%d ", r->rtm_src_len);
@@ -108,9 +106,8 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				r->rtm_dst_len
 				);
 		} else {
-			fprintf(fp, "to %s ", format_host(r->rtm_family,
-						       RTA_PAYLOAD(tb[FRA_DST]),
-						       RTA_DATA(tb[FRA_DST])));
+			fprintf(fp, "to %s ",
+			        format_host_rta(r->rtm_family, tb[FRA_DST]));
 		}
 	} else if (r->rtm_dst_len) {
 		fprintf(fp, "to 0/%d ", r->rtm_dst_len);
@@ -183,9 +180,8 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	if (r->rtm_type == RTN_NAT) {
 		if (tb[RTA_GATEWAY]) {
 			fprintf(fp, "map-to %s ",
-				format_host(r->rtm_family,
-					    RTA_PAYLOAD(tb[RTA_GATEWAY]),
-					    RTA_DATA(tb[RTA_GATEWAY])));
+			        format_host_rta(r->rtm_family,
+			                        tb[RTA_GATEWAY]));
 		} else
 			fprintf(fp, "masquerade");
 	} else if (r->rtm_type == FR_ACT_GOTO) {
diff --git a/ip/iptoken.c b/ip/iptoken.c
index 02fe98e249526..6e1a1ab7f36e7 100644
--- a/ip/iptoken.c
+++ b/ip/iptoken.c
@@ -78,12 +78,9 @@ static int print_token(const struct sockaddr_nl *who, struct nlmsghdr *n, void *
 		return -1;
 	}
 
-	fprintf(fp, "token %s ",
-		format_host(ifi->ifi_family,
-			    RTA_PAYLOAD(ltb[IFLA_INET6_TOKEN]),
-			    RTA_DATA(ltb[IFLA_INET6_TOKEN])));
-	fprintf(fp, "dev %s ", ll_index_to_name(ifi->ifi_index));
-	fprintf(fp, "\n");
+	fprintf(fp, "token %s dev %s\n",
+	        format_host_rta(ifi->ifi_family, ltb[IFLA_INET6_TOKEN]),
+	        ll_index_to_name(ifi->ifi_index));
 	fflush(fp);
 
 	return 0;
-- 
2.7.2

^ permalink raw reply related

* Re: [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
From: David Ahern @ 2016-03-22 18:29 UTC (permalink / raw)
  To: Lance Richardson, netdev
In-Reply-To: <1458660679-7279-1-git-send-email-lrichard@redhat.com>

On 3/22/16 9:31 AM, Lance Richardson wrote:
> Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
> before calling fib_lookup(), which means fib_table_lookup() is
> using non-deterministic data at this line:
>
> 	if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
>
> Fix by initializing fl4.flowi4_flags to zero.
>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>
> ---
>   net/ipv4/fib_frontend.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 21add55..896844a 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
>   		fl4.flowi4_scope = scope;
>   		fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
>   		fl4.flowi4_tun_key.tun_id = 0;
> +		fl4.flowi4_flags = 0;
>   		if (!fib_lookup(net, &fl4, &res, 0))
>   			return FIB_RES_PREFSRC(net, res);
>   	} else {
>

Fixes: 58189ca7b2741 ("net: Fix vti use case with oif in dst lookups")

I think a more robust solution is to move fl4 to this if case and init 
when it is declared:

	struct flowi4 fl4 = {
		.flowi4_iif = LOOPBACK_IFINDEX,
		.daddr = ip_hdr(skb)->saddr,
		.flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
		.flowi4_scope = scope,
		.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0,
	};

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Cong Wang @ 2016-03-22 18:19 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Wei Wang, Wei Wang, Eric Dumazet, David Miller, Eric Dumazet,
	Linux Kernel Network Developers
In-Reply-To: <20160322173939.GA53936@kafai-mba.local>

On Tue, Mar 22, 2016 at 10:39 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Tue, Mar 22, 2016 at 09:53:35AM -0700, Cong Wang wrote:
>> On Mon, Mar 21, 2016 at 11:02 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>> > In term of difference, AFAICT, the current patch is an optimization in the
>> > sense that the update_pmtu() code path does not have to do a dst_check to
>> > discover its sk->sk_dst_cache is invalid, and then do a relookup to find out
>> > that the just created RTF_CACHE clone should be used.  To get this, it may
>> > make more sense to remove all the relookup code together during update_pmtu().
>> > Even if this slow path was to be optimized, should it be put in a
>> > separate patch where net-next is a better candidate?
>> >
>>
>> Speaking of RTF_CACHE, I am curious why you didn't use FIB next hop exception
>> as what ipv4 does to cache exceptions? This makes IPv6 has more gap with IPv4.
>> This is (almost) irrelevant to this patch.
> There are a few differences between IPv6 and IPv4.  Both in terms of
> data structure and functionality.  The last 'RTF_CACHE on exception' patchset is one
> step toward this direction. More patches are needed and are welcomed ;)

Sure, I will take a look at this once net-next is re-open.


>
>>
>>
>> > I think fixing it in __udp6_lib_err() or what Cong Wang is suggesting makes
>> > more sense for a net branch fix.  If there is logic specific to connected-udp,
>> > I would do it in the __udp6_lib_err() instead.  After looking at
>> > udpv6_sendmsg() and how it calls ip6_dst_store(), may also need to be careful
>> > what daddr and saddr should be passed to ip6_dst_store(), or at least a commit
>> > message.  The first patch is essentially passing NULL to daddr and saddr
>> > while the second patch seems passing something else.
>>
>> Raw socket needs to fix too, we can't just fix __udp6_lib_err(), this is also
>> why fixing ip6_sk_update_pmtu() is better, its call path is better.
> I don't see rawv6 socket is storing the dst.  I probably have overlooked it.  Can
> you point it out?


I thought sk->sk_dst_cache is generic to all sockets, but it is up to
each kind of
socket to decide to use it or not, and you are right, raw socket doesn't seem to
care about it even though it calls *_sk_update_pmtu().

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Martin KaFai Lau @ 2016-03-22 18:16 UTC (permalink / raw)
  To: Wei Wang
  Cc: Cong Wang, Wei Wang, Eric Dumazet, David Miller, Eric Dumazet,
	Linux Kernel Network Developers
In-Reply-To: <CAC15z3jDRvJR4uZ97mC+beaMaPQvAvTHB9OLKCdQ4hcwRu07vw@mail.gmail.com>

On Tue, Mar 22, 2016 at 11:03:29AM -0700, Wei Wang wrote:
> But one thing here, we will have to generate the same flowi6 in both
> ip6_sk_update_pmtu() as well as ip6_update_pmtu(). Is this considered
> as a not clean enough fix?
If they share common codes to build flowi6, can the common codes
be factored out?

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Wei Wang @ 2016-03-22 18:03 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Cong Wang, Wei Wang, Eric Dumazet, David Miller, Eric Dumazet,
	Linux Kernel Network Developers
In-Reply-To: <20160322173939.GA53936@kafai-mba.local>

Thanks Martin and Cong.

I guess then we are going with the following fix in ip6_sk_update_pmtu():
1. call ip6_upate_pmtu() as it is
2. do a dst_check()
3. re-lookup() if it is invalid
4. and then do a ip6_dst_store()/dst_set

But one thing here, we will have to generate the same flowi6 in both
ip6_sk_update_pmtu() as well as ip6_update_pmtu(). Is this considered
as a not clean enough fix?


On Tue, Mar 22, 2016 at 10:39 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Tue, Mar 22, 2016 at 09:53:35AM -0700, Cong Wang wrote:
>> On Mon, Mar 21, 2016 at 11:02 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>> > In term of difference, AFAICT, the current patch is an optimization in the
>> > sense that the update_pmtu() code path does not have to do a dst_check to
>> > discover its sk->sk_dst_cache is invalid, and then do a relookup to find out
>> > that the just created RTF_CACHE clone should be used.  To get this, it may
>> > make more sense to remove all the relookup code together during update_pmtu().
>> > Even if this slow path was to be optimized, should it be put in a
>> > separate patch where net-next is a better candidate?
>> >
>>
>> Speaking of RTF_CACHE, I am curious why you didn't use FIB next hop exception
>> as what ipv4 does to cache exceptions? This makes IPv6 has more gap with IPv4.
>> This is (almost) irrelevant to this patch.
> There are a few differences between IPv6 and IPv4.  Both in terms of
> data structure and functionality.  The last 'RTF_CACHE on exception' patchset is one
> step toward this direction. More patches are needed and are welcomed ;)
>
>>
>>
>> > I think fixing it in __udp6_lib_err() or what Cong Wang is suggesting makes
>> > more sense for a net branch fix.  If there is logic specific to connected-udp,
>> > I would do it in the __udp6_lib_err() instead.  After looking at
>> > udpv6_sendmsg() and how it calls ip6_dst_store(), may also need to be careful
>> > what daddr and saddr should be passed to ip6_dst_store(), or at least a commit
>> > message.  The first patch is essentially passing NULL to daddr and saddr
>> > while the second patch seems passing something else.
>>
>> Raw socket needs to fix too, we can't just fix __udp6_lib_err(), this is also
>> why fixing ip6_sk_update_pmtu() is better, its call path is better.
> I don't see rawv6 socket is storing the dst.  I probably have overlooked it.  Can
> you point it out?
>
> Having said that, I don't feel strongly on any of the two places.  I think only
> implementation can tell.

^ permalink raw reply

* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-22 17:47 UTC (permalink / raw)
  To: Edward Cree; +Cc: Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F17A13.8090701@solarflare.com>

On Tue, Mar 22, 2016 at 10:00 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 18/03/16 23:25, Alexander Duyck wrote:
>> This patch adds support for something I am referring to as GSO partial.
>> The basic idea is that we can support a broader range of devices for
>> segmentation if we use fixed outer headers and have the hardware only
>> really deal with segmenting the inner header.  The idea behind the naming
>> is due to the fact that everything before csum_start will be fixed headers,
>> and everything after will be the region that is handled by hardware.
>>
>> With the current implementation it allows us to add support for the
>> following GSO types with an inner TSO or TSO6 offload:
>> NETIF_F_GSO_GRE
>> NETIF_F_GSO_GRE_CSUM
>> NETIF_F_UDP_TUNNEL
>> NETIF_F_UDP_TUNNEL_CSUM
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
> If I'm correctly understanding what you're doing, you're building a large
> TCP segment, feeding it through the encapsulation drivers as normal, then
> at GSO time you're fixing up length fields, checksums etc. in the headers.
> I think we can do this more simply, by making it so that at the time when
> we _generate_ the TCP segment, we give it headers saying it's one MSS big,
> but have several MSS of data.  Similarly when adding the encap headers,
> they all need to get their lengths from what the layer below tells them,
> rather than the current length of data in the SKB.  Then at GSO time all
> the headers already have the right things in, and you don't need to call
> any per-protocol GSO callbacks for them.

One issue I have to deal with here is that we have no way of knowing
what the underlying hardware can support at the time of segment being
created.  You have to keep in mind that what we have access to is the
tunnel dev in many cases, not the underlying dev so we don't know if
things can be offloaded to hardware or not.  By pushing this logic
into the GSO code we can actually implement it without much overhead
since we either segment it into an MSS multiple, or into single MSS
sized chunks.  This way we defer the decision until the very last
moment when we actually know if we can offload some portion of this in
hardware or not.

> Any protocol that noticed it was putting something non-copyable in its
> headers (e.g. GRE with the Counter field, or an outer IP layer without DF
> set needing real IPIDs) would set a flag in the SKB to indicate that we
> really do need to call through the per-protocol GSO stuff.  (Ideally, if
> we had a separate skb->gso_start field rather than piggybacking on
> csum_start, we could reset it to point just before us, so that any further
> headers outside us still can be copied rather than taking callbacks.  But
> I'm not sure whether that's worth using up sk_buff real estate for.)

The idea behind piggybacking on csum_start was due to the fact that we
cannot perform GSO/TSO unless CHECKSUM_PARTIAL is set.  As far as I
know in the case of TCP offloads this always ends up being the
inner-most L4 header so it works out in that it actually reduces code
path as we were having to deal with all the skb->encapsulation checks.
It was a relationship that already existed, I just decided to make use
of it since it simplifies things pretty significantly.

As far as retreating I don't really see how that would work. In most
cases it is an all-or-nothing proposition to setup these outer
headers.  Either we can segment the frame with the outer headers
replicated or we cannot.  I suspect it would end up being a common
case where the hardware will update the outer IP and inner TCP
headers, but I think the outer L4 and inner IP headers will be the
ones that most likely always end up being static.  Also we already
have code paths in place in the GRE driver for instance that prevent
us from using GSO in the case of TUNNEL_SEQ being enabled.

> (It might still be necessary to put the true length in the TCP header, if
> hardware is using that as an input to segmentation.  I think sfc hardware
> just uses 'total length of all payload DMA descriptors', but others might
> behave differently.)

That is what most drivers do.  The way I kind of retained that is that
the TCP header doesn't include an actual length field, but I left the
pseudo-header using the full length of all data.  My thought was to
end up using something like the ixgbe approach for most devices.  What
I did there was replicate the tunnel headers and inner IPv4 or IPv6
header.  In the case of ixgbe and i40e I can throw away the checksum
and length values for the outer IP header, one thing I was curious
about is if I really needed to retain the full packet size for those.

> However, I haven't yet had the time to attempt to implement this, so there
> might be some obvious reason I'm missing why this is impossible.
> Also, it's possible that I've completely misunderstood your patch and it's
> orthogonal to and can coexist with what I'm suggesting.

The one piece I could really use would be an understanding of what
inputs your hardware is expecting in order for us to extend TSO to
support this kind of approach.  Then I could start tailoring the
output generated so that we had something that would work with more
devices.  I was thinking the approach I have taken is fairly generic
since essentially it allows us to get away with TSO as long as we are
allowed to provide the offsets for the IP header and the TCP header.
>From what I can tell it looks like the Solarflare drivers do something
similar so you might even try making changes similar to what I did for
ixgbe to see if you can get a proof of concept working for sfc.

- Alex

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Martin KaFai Lau @ 2016-03-22 17:39 UTC (permalink / raw)
  To: Cong Wang
  Cc: Wei Wang, Wei Wang, Eric Dumazet, David Miller, Eric Dumazet,
	Linux Kernel Network Developers
In-Reply-To: <CAM_iQpVfofoThoUS8UYkh-2KR_Co42tfxz00mCXx+JMWpwgg8A@mail.gmail.com>

On Tue, Mar 22, 2016 at 09:53:35AM -0700, Cong Wang wrote:
> On Mon, Mar 21, 2016 at 11:02 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> > In term of difference, AFAICT, the current patch is an optimization in the
> > sense that the update_pmtu() code path does not have to do a dst_check to
> > discover its sk->sk_dst_cache is invalid, and then do a relookup to find out
> > that the just created RTF_CACHE clone should be used.  To get this, it may
> > make more sense to remove all the relookup code together during update_pmtu().
> > Even if this slow path was to be optimized, should it be put in a
> > separate patch where net-next is a better candidate?
> >
>
> Speaking of RTF_CACHE, I am curious why you didn't use FIB next hop exception
> as what ipv4 does to cache exceptions? This makes IPv6 has more gap with IPv4.
> This is (almost) irrelevant to this patch.
There are a few differences between IPv6 and IPv4.  Both in terms of
data structure and functionality.  The last 'RTF_CACHE on exception' patchset is one
step toward this direction. More patches are needed and are welcomed ;)

>
>
> > I think fixing it in __udp6_lib_err() or what Cong Wang is suggesting makes
> > more sense for a net branch fix.  If there is logic specific to connected-udp,
> > I would do it in the __udp6_lib_err() instead.  After looking at
> > udpv6_sendmsg() and how it calls ip6_dst_store(), may also need to be careful
> > what daddr and saddr should be passed to ip6_dst_store(), or at least a commit
> > message.  The first patch is essentially passing NULL to daddr and saddr
> > while the second patch seems passing something else.
>
> Raw socket needs to fix too, we can't just fix __udp6_lib_err(), this is also
> why fixing ip6_sk_update_pmtu() is better, its call path is better.
I don't see rawv6 socket is storing the dst.  I probably have overlooked it.  Can
you point it out?

Having said that, I don't feel strongly on any of the two places.  I think only
implementation can tell.

^ permalink raw reply

* Re: [PATCH] ath9k: fix checkpatch.pl identation and sapce errors.
From: Joe Perches @ 2016-03-22 17:36 UTC (permalink / raw)
  To: Ashwini Singh, ath9k-devel, kvalo
  Cc: linux-wireless, ath9k-devel, netdev, linux-kernel, Rajeev_Kumar
In-Reply-To: <1458629628-6264-1-git-send-email-Ashwini_Singh@mentor.com>

On Tue, 2016-03-22 at 12:23 +0530, Ashwini Singh wrote:
> Signed-off-by: Ashwini Singh <Ashwini_Singh@mentor.com>

It might be better to break this into a few different patches.

o Remove trailing whitespace
o Fix vertical line / brace location
o Multi-line statement parenthesis alignment
o Space to tab conversions
o Whitespace addition / removal

^ permalink raw reply

* Re: ip-token: unable to remove a token & multi-token handling & concurrent use w/ EUI64/privacy
From: Daniel Borkmann @ 2016-03-22 17:03 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: netdev, hannes
In-Reply-To: <robbat2-20160319T175636-156562343Z@orbis-terrarum.net>

Hi Robin,

On 03/19/2016 07:53 PM, Robin H. Johnson wrote:
[...]
> Playing around with IPv6 tokens, I ran into a problem:
> Once you have a token set on an interface, it's impossible to remove it!
>
> # ip token set :: dev eth0
> RTNETLINK answers: Invalid argument

I'll have a look into a fix, I think this was intentional, but I currently
fail to recall a reason why (should have put a note into the commit log). ;)
The draft is pretty terse in any case, it seems as we only invalidate other
tokenized addresses, it should be okay to just remove it.

> This is a side-effect of rejecting ipv6_addr_any in inet6_set_iftoken.
>
> While this gets fixed, I have two related feature requests for this:
> - Please make it possible to configure multiple tokens on an interface:
>    Use case: Deploying local services on well-known addresses inside a
>    network without explicit prefix configuration.
> - Adding a token causes other address generation methods to be disabled,
>    this is problematic if you wish to prefer privacy addresses for
>    outbound connections.
>
> Design suggestion:
> Convert from using a single token to using a list of tokens, with an
> explicit default IPv6-any-addr (::) in the list, to represent that
> other address generation should ALSO take place (EUI64/privacy).
> Deletion of the any-addr from the list should disable EUI64/privacy
> addresses.

Seems you already have some patches, please feel free to send them. ;)

Thanks for the feedback!
Daniel

^ permalink raw reply

* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Edward Cree @ 2016-03-22 17:00 UTC (permalink / raw)
  To: Alexander Duyck, netdev, davem, alexander.duyck, tom
In-Reply-To: <20160318232522.14955.13475.stgit@localhost.localdomain>

On 18/03/16 23:25, Alexander Duyck wrote:
> This patch adds support for something I am referring to as GSO partial.
> The basic idea is that we can support a broader range of devices for
> segmentation if we use fixed outer headers and have the hardware only
> really deal with segmenting the inner header.  The idea behind the naming
> is due to the fact that everything before csum_start will be fixed headers,
> and everything after will be the region that is handled by hardware.
>
> With the current implementation it allows us to add support for the
> following GSO types with an inner TSO or TSO6 offload:
> NETIF_F_GSO_GRE
> NETIF_F_GSO_GRE_CSUM
> NETIF_F_UDP_TUNNEL
> NETIF_F_UDP_TUNNEL_CSUM
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
If I'm correctly understanding what you're doing, you're building a large
TCP segment, feeding it through the encapsulation drivers as normal, then
at GSO time you're fixing up length fields, checksums etc. in the headers.
I think we can do this more simply, by making it so that at the time when
we _generate_ the TCP segment, we give it headers saying it's one MSS big,
but have several MSS of data.  Similarly when adding the encap headers,
they all need to get their lengths from what the layer below tells them,
rather than the current length of data in the SKB.  Then at GSO time all
the headers already have the right things in, and you don't need to call
any per-protocol GSO callbacks for them.
Any protocol that noticed it was putting something non-copyable in its
headers (e.g. GRE with the Counter field, or an outer IP layer without DF
set needing real IPIDs) would set a flag in the SKB to indicate that we
really do need to call through the per-protocol GSO stuff.  (Ideally, if
we had a separate skb->gso_start field rather than piggybacking on
csum_start, we could reset it to point just before us, so that any further
headers outside us still can be copied rather than taking callbacks.  But
I'm not sure whether that's worth using up sk_buff real estate for.)
(It might still be necessary to put the true length in the TCP header, if
hardware is using that as an input to segmentation.  I think sfc hardware
just uses 'total length of all payload DMA descriptors', but others might
behave differently.)
However, I haven't yet had the time to attempt to implement this, so there
might be some obvious reason I'm missing why this is impossible.
Also, it's possible that I've completely misunderstood your patch and it's
orthogonal to and can coexist with what I'm suggesting.
-Ed

^ permalink raw reply

* Re: [PATCH 3/3] libceph: use KMEM_CACHE macro
From: Ilya Dryomov @ 2016-03-22 16:58 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Yan, Zheng, Sage Weil, David S. Miller, Ceph Development, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <a9c3e96ba6bf321ca13d9445d2770781fb7033cd.1457853190.git.geliangtang@163.com>

On Sun, Mar 13, 2016 at 8:18 AM, Geliang Tang <geliangtang@163.com> wrote:
> Use KMEM_CACHE() instead of kmem_cache_create() to simplify the code.
>
> Signed-off-by: Geliang Tang <geliangtang@163.com>
> ---
>  net/ceph/messenger.c  | 10 ++--------
>  net/ceph/osd_client.c |  5 +----
>  2 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index 9382619..32c997e 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -235,18 +235,12 @@ static struct workqueue_struct *ceph_msgr_wq;
>  static int ceph_msgr_slab_init(void)
>  {
>         BUG_ON(ceph_msg_cache);
> -       ceph_msg_cache = kmem_cache_create("ceph_msg",
> -                                       sizeof (struct ceph_msg),
> -                                       __alignof__(struct ceph_msg), 0, NULL);
> -
> +       ceph_msg_cache = KMEM_CACHE(ceph_msg, 0);
>         if (!ceph_msg_cache)
>                 return -ENOMEM;
>
>         BUG_ON(ceph_msg_data_cache);
> -       ceph_msg_data_cache = kmem_cache_create("ceph_msg_data",
> -                                       sizeof (struct ceph_msg_data),
> -                                       __alignof__(struct ceph_msg_data),
> -                                       0, NULL);
> +       ceph_msg_data_cache = KMEM_CACHE(ceph_msg_data, 0);
>         if (ceph_msg_data_cache)
>                 return 0;
>
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 5bc0537..7558855 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -2783,10 +2783,7 @@ EXPORT_SYMBOL(ceph_osdc_writepages);
>  int ceph_osdc_setup(void)
>  {
>         BUG_ON(ceph_osd_request_cache);
> -       ceph_osd_request_cache = kmem_cache_create("ceph_osd_request",
> -                                       sizeof (struct ceph_osd_request),
> -                                       __alignof__(struct ceph_osd_request),
> -                                       0, NULL);
> +       ceph_osd_request_cache = KMEM_CACHE(ceph_osd_request, 0);
>
>         return ceph_osd_request_cache ? 0 : -ENOMEM;
>  }

Applied, with osd_client.c hunk dropped.  ceph_osd_request cache
objects are not longer sizeof(struct ceph_osd_request).

Thanks,

                Ilya

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Cong Wang @ 2016-03-22 16:53 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Wei Wang, Wei Wang, Eric Dumazet, David Miller, Eric Dumazet,
	Linux Kernel Network Developers
In-Reply-To: <20160322060220.GA50824@kafai-mba.local>

On Mon, Mar 21, 2016 at 11:02 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> I think Cong Wang is suggesting, in ip6_sk_update_pmtu():
> 1. call ip6_upate_pmtu() as it is
> 2. do a dst_check()
> 3. re-lookup() if it is invalid
> 4. and then do a ip6_dst_store()/dst_set
>
> The above is exactly what inet6_csk_update_pmtu(), which was also used in the
> first patch, is doing.

Well, the reasons why I suggest to fix ip6_sk_update_pmtu() instead of anything
else (no matter __udp6_lib_err() in v1 or ip6_update_pmtu() in v2) are that:

1) Catch up with ipv4 part, which is ipv4_sk_update_pmtu()
2) Straight-forward, (ideally) no need to bother other irrelevant
callers like xfrm.

Functionally all the solutions should work to fix Wei's problem, we just need to
find which one is more elegant.


>
> In term of difference, AFAICT, the current patch is an optimization in the
> sense that the update_pmtu() code path does not have to do a dst_check to
> discover its sk->sk_dst_cache is invalid, and then do a relookup to find out
> that the just created RTF_CACHE clone should be used.  To get this, it may
> make more sense to remove all the relookup code together during update_pmtu().
> Even if this slow path was to be optimized, should it be put in a
> separate patch where net-next is a better candidate?
>

Speaking of RTF_CACHE, I am curious why you didn't use FIB next hop exception
as what ipv4 does to cache exceptions? This makes IPv6 has more gap with IPv4.
This is (almost) irrelevant to this patch.


> I think fixing it in __udp6_lib_err() or what Cong Wang is suggesting makes
> more sense for a net branch fix.  If there is logic specific to connected-udp,
> I would do it in the __udp6_lib_err() instead.  After looking at
> udpv6_sendmsg() and how it calls ip6_dst_store(), may also need to be careful
> what daddr and saddr should be passed to ip6_dst_store(), or at least a commit
> message.  The first patch is essentially passing NULL to daddr and saddr
> while the second patch seems passing something else.

Raw socket needs to fix too, we can't just fix __udp6_lib_err(), this is also
why fixing ip6_sk_update_pmtu() is better, its call path is better.

^ permalink raw reply

* Re: [PATCH v5 2/4] Documentation: Bindings: Add STM32 DWMAC glue
From: Chen-Yu Tsai @ 2016-03-22 16:27 UTC (permalink / raw)
  To: Giuseppe CAVALLARO
  Cc: Alexandre Torgue, Chen-Yu Tsai, Maxime Coquelin, netdev,
	devicetree, Joachim Eastwood, linux-kernel, linux-arm-kernel
In-Reply-To: <56EFFEEC.1010407-qxv4g6HH51o@public.gmane.org>

On Mon, Mar 21, 2016 at 10:02 PM, Giuseppe CAVALLARO
<peppe.cavallaro-qxv4g6HH51o@public.gmane.org> wrote:
> On 3/21/2016 11:45 AM, Alexandre Torgue wrote:
>>
>> Hi,
>>
>> 2016-03-18 17:00 GMT+01:00 Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>:
>>>
>>> Hi,
>>>
>>> On Fri, Mar 18, 2016 at 11:37 PM, Alexandre TORGUE
>>> <alexandre.torgue-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>> Signed-off-by: Alexandre TORGUE <alexandre.torgue-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> new file mode 100644
>>>> index 0000000..ada2aa4
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>>> @@ -0,0 +1,32 @@
>>>> +STMicroelectronics STM32 / MCU DWMAC glue layer controller
>>>> +
>>>> +This file documents platform glue layer for stmmac.
>>>> +Please see stmmac.txt for the other unchanged properties.
>>>> +
>>>> +The device node has following properties.
>>>> +
>>>> +Required properties:
>>>> +- compatible:  Should be "st,stm32-dwmac" to select glue, and
>>>> +              "snps,dwmac-3.50a" to select IP vesrion.
>
>
> Almost all the synp gmac chips have the HW capability register that is
> used for setting all the parameters at probe time.
> This will override fields passed from DT. In theory, it is not
> necessary to pass: "snps,dwmac-3.50a" from device tree at least there
> is either no HW cap reg or the glue has some w/a for a specific chip
> revision.
> To be honest, I like to see the "snps,dwmac-3.50a" as compatibility
> to also have a better readability (that's my personal view ;-) ).

I agree having the versioned strings is good for informational purposes,
and to signal hardware capability. It is not so good for directly
binding drivers in the implementation though.

Unfortunately, as Joachim pointed out, exynos5440 uses it so we cannot
change it.

ChenYu

> Peppe
>
>
>>>
>>> If you need have sort of hardware glue, then it is not compatible.
>>>
>>
>> We could have the case where the glue is set by a bootloader.
>> In this case, we will select IP version in compatible and we will use
>> generic dwmac glue to probe stmmac driver.
>>
>> Regards
>>
>> Alex.
>>
>>> ChenYu
>>>
>>>> +- clocks: Must contain a phandle for each entry in clock-names.
>>>> +- clock-names: Should be "stmmaceth" for the host clock.
>>>> +              Should be "tx-clk" for the MAC TX clock.
>>>> +              Should be "rx-clk" for the MAC RX clock.
>>>> +- st,syscon : Should be phandle/offset pair. The phandle to the syscon
>>>> node which
>>>> +             encompases the glue register, and the offset of the
>>>> control register.
>>>> +Example:
>>>> +
>>>> +               ethernet0: dwmac@40028000 {
>>>> +                       compatible = "st,stm32-dwmac",
>>>> "snps,dwmac-3.50a";
>>>> +                       status = "disabled";
>>>> +                       reg = <0x40028000 0x8000>;
>>>> +                       reg-names = "stmmaceth";
>>>> +                       interrupts = <0 61 0>, <0 62 0>;
>>>> +                       interrupt-names = "macirq", "eth_wake_irq";
>>>> +                       clock-names = "stmmaceth", "tx-clk", "rx-clk";
>>>> +                       clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
>>>> +                       st,syscon = <&syscfg 0x4>;
>>>> +                       snps,pbl = <8>;
>>>> +                       snps,mixed-burst;
>>>> +                       dma-ranges;
>>>> +               };
>>>> --
>>>> 1.9.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 2/4] Documentation: Bindings: Add STM32 DWMAC glue
From: Alexandre Torgue @ 2016-03-22 16:11 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: Rob Herring, Chen-Yu Tsai, Maxime Coquelin, Giuseppe Cavallaro,
	netdev, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <CAGhQ9VwS=YwgfeUuK+LzGjy3f9btVGV7bN=TATGHwQs+JW+iGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi guys,

I will fix typo issues (s/vesrion/version and ethernet @).

Concerning compatible string. For sure "snps,dwmac-3.50a" string is
not used inside glue driver.
I perfere to keep it for information but if you really want that I
remove it I will not block ;)

2016-03-21 16:36 GMT+01:00 Joachim  Eastwood <manabian-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> On 21 March 2016 at 13:40, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Sat, Mar 19, 2016 at 12:00:22AM +0800, Chen-Yu Tsai wrote:
>>> Hi,
>>>
>>> On Fri, Mar 18, 2016 at 11:37 PM, Alexandre TORGUE
>>> <alexandre.torgue-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> > +- clocks: Must contain a phandle for each entry in clock-names.
>>> > +- clock-names: Should be "stmmaceth" for the host clock.
>>
We can remove host clock (stmmac eth) entry here and refer to
stmmac.txt binding for common entry

>> This doesn't sound like the clock input signal name...
>>
>>> > +              Should be "tx-clk" for the MAC TX clock.
>>> > +              Should be "rx-clk" for the MAC RX clock.
>>
>> How can other DWMAC blocks not have these clocks? The glue can't really
>> add these clocks. It could combine them into one or a new version of
>> DWMAC could have a different number of clock inputs. So if there is
>> variation here, then some of the bindings are probably wrong. I guess
>> the only change I'm suggesting is possibly moving these into common
>> binding doc.
>
> The LPC18xx implementation probably have these clocks as well but the
> LPC1850 user manual only documents the main clock. Someone with access
> to the IP block doc from Synopsys should be able to check which clocks
> the MAC really needs.
>
> Rockchip bindings have two clocks named "mac_clk_rx" and "mac_clk_tx".
> These are probably the same as stm32 needs so maybe use these names
> and move them into the main doc and update the rockchip binding.
>
I think we can use same name. But I have a doubt on moving it in a
common bindings (maybe I don't well understood). When you say "common
binding file" is it "stmmac.txt" binding ? If yes does it mean that we
have to control it inside stmmac driver (no more in glue) ? In this
case those clocks will become "required" for stm32 and rockship but
not for others chip. It could create confusion?

Best regards

Alex

>
> regards,
> Joachim Eastwood
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 2/2] AF_VSOCK: Shrink the area influenced by prepare_to_wait
From: Claudio Imbrenda @ 2016-03-22 16:05 UTC (permalink / raw)
  To: davem; +Cc: labbott, netdev, linux-kernel
In-Reply-To: <1458662752-2415-1-git-send-email-imbrenda@linux.vnet.ibm.com>

When a thread is prepared for waiting by calling prepare_to_wait, sleeping
is not allowed until either the wait has taken place or finish_wait has
been called.  The existing code in af_vsock imposed unnecessary no-sleep
assumptions to a broad list of backend functions.
This patch shrinks the influence of prepare_to_wait to the area where it
is strictly needed, therefore relaxing the no-sleep restriction there.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 net/vmw_vsock/af_vsock.c | 158 +++++++++++++++++++++++++----------------------
 1 file changed, 85 insertions(+), 73 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 7fd1220..3dce53e 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1209,10 +1209,14 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
 
 		if (signal_pending(current)) {
 			err = sock_intr_errno(timeout);
-			goto out_wait_error;
+			sk->sk_state = SS_UNCONNECTED;
+			sock->state = SS_UNCONNECTED;
+			goto out_wait;
 		} else if (timeout == 0) {
 			err = -ETIMEDOUT;
-			goto out_wait_error;
+			sk->sk_state = SS_UNCONNECTED;
+			sock->state = SS_UNCONNECTED;
+			goto out_wait;
 		}
 
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
@@ -1220,20 +1224,17 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
 
 	if (sk->sk_err) {
 		err = -sk->sk_err;
-		goto out_wait_error;
-	} else
+		sk->sk_state = SS_UNCONNECTED;
+		sock->state = SS_UNCONNECTED;
+	} else {
 		err = 0;
+	}
 
 out_wait:
 	finish_wait(sk_sleep(sk), &wait);
 out:
 	release_sock(sk);
 	return err;
-
-out_wait_error:
-	sk->sk_state = SS_UNCONNECTED;
-	sock->state = SS_UNCONNECTED;
-	goto out_wait;
 }
 
 static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
@@ -1270,18 +1271,20 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
 	       listener->sk_err == 0) {
 		release_sock(listener);
 		timeout = schedule_timeout(timeout);
+		finish_wait(sk_sleep(listener), &wait);
 		lock_sock(listener);
 
 		if (signal_pending(current)) {
 			err = sock_intr_errno(timeout);
-			goto out_wait;
+			goto out;
 		} else if (timeout == 0) {
 			err = -EAGAIN;
-			goto out_wait;
+			goto out;
 		}
 
 		prepare_to_wait(sk_sleep(listener), &wait, TASK_INTERRUPTIBLE);
 	}
+	finish_wait(sk_sleep(listener), &wait);
 
 	if (listener->sk_err)
 		err = -listener->sk_err;
@@ -1301,19 +1304,15 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
 		 */
 		if (err) {
 			vconnected->rejected = true;
-			release_sock(connected);
-			sock_put(connected);
-			goto out_wait;
+		} else {
+			newsock->state = SS_CONNECTED;
+			sock_graft(connected, newsock);
 		}
 
-		newsock->state = SS_CONNECTED;
-		sock_graft(connected, newsock);
 		release_sock(connected);
 		sock_put(connected);
 	}
 
-out_wait:
-	finish_wait(sk_sleep(listener), &wait);
 out:
 	release_sock(listener);
 	return err;
@@ -1557,11 +1556,11 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (err < 0)
 		goto out;
 
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 
 	while (total_written < len) {
 		ssize_t written;
 
+		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		while (vsock_stream_has_space(vsk) == 0 &&
 		       sk->sk_err == 0 &&
 		       !(sk->sk_shutdown & SEND_SHUTDOWN) &&
@@ -1570,27 +1569,33 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 			/* Don't wait for non-blocking sockets. */
 			if (timeout == 0) {
 				err = -EAGAIN;
-				goto out_wait;
+				finish_wait(sk_sleep(sk), &wait);
+				goto out_err;
 			}
 
 			err = transport->notify_send_pre_block(vsk, &send_data);
-			if (err < 0)
-				goto out_wait;
+			if (err < 0) {
+				finish_wait(sk_sleep(sk), &wait);
+				goto out_err;
+			}
 
 			release_sock(sk);
 			timeout = schedule_timeout(timeout);
 			lock_sock(sk);
 			if (signal_pending(current)) {
 				err = sock_intr_errno(timeout);
-				goto out_wait;
+				finish_wait(sk_sleep(sk), &wait);
+				goto out_err;
 			} else if (timeout == 0) {
 				err = -EAGAIN;
-				goto out_wait;
+				finish_wait(sk_sleep(sk), &wait);
+				goto out_err;
 			}
 
 			prepare_to_wait(sk_sleep(sk), &wait,
 					TASK_INTERRUPTIBLE);
 		}
+		finish_wait(sk_sleep(sk), &wait);
 
 		/* These checks occur both as part of and after the loop
 		 * conditional since we need to check before and after
@@ -1598,16 +1603,16 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		 */
 		if (sk->sk_err) {
 			err = -sk->sk_err;
-			goto out_wait;
+			goto out_err;
 		} else if ((sk->sk_shutdown & SEND_SHUTDOWN) ||
 			   (vsk->peer_shutdown & RCV_SHUTDOWN)) {
 			err = -EPIPE;
-			goto out_wait;
+			goto out_err;
 		}
 
 		err = transport->notify_send_pre_enqueue(vsk, &send_data);
 		if (err < 0)
-			goto out_wait;
+			goto out_err;
 
 		/* Note that enqueue will only write as many bytes as are free
 		 * in the produce queue, so we don't need to ensure len is
@@ -1620,7 +1625,7 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 				len - total_written);
 		if (written < 0) {
 			err = -ENOMEM;
-			goto out_wait;
+			goto out_err;
 		}
 
 		total_written += written;
@@ -1628,14 +1633,13 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 		err = transport->notify_send_post_enqueue(
 				vsk, written, &send_data);
 		if (err < 0)
-			goto out_wait;
+			goto out_err;
 
 	}
 
-out_wait:
+out_err:
 	if (total_written > 0)
 		err = total_written;
-	finish_wait(sk_sleep(sk), &wait);
 out:
 	release_sock(sk);
 	return err;
@@ -1716,21 +1720,61 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	if (err < 0)
 		goto out;
 
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 
 	while (1) {
-		s64 ready = vsock_stream_has_data(vsk);
+		s64 ready;
 
-		if (ready < 0) {
-			/* Invalid queue pair content. XXX This should be
-			 * changed to a connection reset in a later change.
-			 */
+		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+		ready = vsock_stream_has_data(vsk);
 
-			err = -ENOMEM;
-			goto out_wait;
-		} else if (ready > 0) {
+		if (ready == 0) {
+			if (sk->sk_err != 0 ||
+			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
+			    (vsk->peer_shutdown & SEND_SHUTDOWN)) {
+				finish_wait(sk_sleep(sk), &wait);
+				break;
+			}
+			/* Don't wait for non-blocking sockets. */
+			if (timeout == 0) {
+				err = -EAGAIN;
+				finish_wait(sk_sleep(sk), &wait);
+				break;
+			}
+
+			err = transport->notify_recv_pre_block(
+					vsk, target, &recv_data);
+			if (err < 0) {
+				finish_wait(sk_sleep(sk), &wait);
+				break;
+			}
+			release_sock(sk);
+			timeout = schedule_timeout(timeout);
+			lock_sock(sk);
+
+			if (signal_pending(current)) {
+				err = sock_intr_errno(timeout);
+				finish_wait(sk_sleep(sk), &wait);
+				break;
+			} else if (timeout == 0) {
+				err = -EAGAIN;
+				finish_wait(sk_sleep(sk), &wait);
+				break;
+			}
+		} else {
 			ssize_t read;
 
+			finish_wait(sk_sleep(sk), &wait);
+
+			if (ready < 0) {
+				/* Invalid queue pair content. XXX This should
+				* be changed to a connection reset in a later
+				* change.
+				*/
+
+				err = -ENOMEM;
+				goto out;
+			}
+
 			err = transport->notify_recv_pre_dequeue(
 					vsk, target, &recv_data);
 			if (err < 0)
@@ -1750,42 +1794,12 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 					vsk, target, read,
 					!(flags & MSG_PEEK), &recv_data);
 			if (err < 0)
-				goto out_wait;
+				goto out;
 
 			if (read >= target || flags & MSG_PEEK)
 				break;
 
 			target -= read;
-		} else {
-			if (sk->sk_err != 0 || (sk->sk_shutdown & RCV_SHUTDOWN)
-			    || (vsk->peer_shutdown & SEND_SHUTDOWN)) {
-				break;
-			}
-			/* Don't wait for non-blocking sockets. */
-			if (timeout == 0) {
-				err = -EAGAIN;
-				break;
-			}
-
-			err = transport->notify_recv_pre_block(
-					vsk, target, &recv_data);
-			if (err < 0)
-				break;
-
-			release_sock(sk);
-			timeout = schedule_timeout(timeout);
-			lock_sock(sk);
-
-			if (signal_pending(current)) {
-				err = sock_intr_errno(timeout);
-				break;
-			} else if (timeout == 0) {
-				err = -EAGAIN;
-				break;
-			}
-
-			prepare_to_wait(sk_sleep(sk), &wait,
-					TASK_INTERRUPTIBLE);
 		}
 	}
 
@@ -1816,8 +1830,6 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		err = copied;
 	}
 
-out_wait:
-	finish_wait(sk_sleep(sk), &wait);
 out:
 	release_sock(sk);
 	return err;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3 1/2] Revert "vsock: Fix blocking ops call in prepare_to_wait"
From: Claudio Imbrenda @ 2016-03-22 16:05 UTC (permalink / raw)
  To: davem; +Cc: labbott, netdev, linux-kernel
In-Reply-To: <1458662752-2415-1-git-send-email-imbrenda@linux.vnet.ibm.com>

This reverts commit 5988818008257ca42010d6b43a3e0e48afec9898 ("vsock: Fix
blocking ops call in prepare_to_wait")

The commit reverted with this patch caused us to potentially miss wakeups.
Since the condition is not checked between the prepare_to_wait and the
schedule(), if a wakeup happens after the condition is checked but before
the sleep happens, we will miss it. ( A description of the problem can be
found here: http://www.makelinux.net/ldd3/chp-6-sect-2 ).

By reverting the patch, the behaviour is still incorrect (since we
shouldn't sleep between the prepare_to_wait and the schedule) but at least
it will not miss wakeups.

The next patch in the series actually fixes the behaviour.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 net/vmw_vsock/af_vsock.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index bbe65dc..7fd1220 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1557,6 +1557,8 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (err < 0)
 		goto out;
 
+	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+
 	while (total_written < len) {
 		ssize_t written;
 
@@ -1576,9 +1578,7 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 				goto out_wait;
 
 			release_sock(sk);
-			prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 			timeout = schedule_timeout(timeout);
-			finish_wait(sk_sleep(sk), &wait);
 			lock_sock(sk);
 			if (signal_pending(current)) {
 				err = sock_intr_errno(timeout);
@@ -1588,6 +1588,8 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 				goto out_wait;
 			}
 
+			prepare_to_wait(sk_sleep(sk), &wait,
+					TASK_INTERRUPTIBLE);
 		}
 
 		/* These checks occur both as part of and after the loop
@@ -1633,6 +1635,7 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 out_wait:
 	if (total_written > 0)
 		err = total_written;
+	finish_wait(sk_sleep(sk), &wait);
 out:
 	release_sock(sk);
 	return err;
@@ -1713,6 +1716,7 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	if (err < 0)
 		goto out;
 
+	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 
 	while (1) {
 		s64 ready = vsock_stream_has_data(vsk);
@@ -1723,7 +1727,7 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 			 */
 
 			err = -ENOMEM;
-			goto out;
+			goto out_wait;
 		} else if (ready > 0) {
 			ssize_t read;
 
@@ -1746,7 +1750,7 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 					vsk, target, read,
 					!(flags & MSG_PEEK), &recv_data);
 			if (err < 0)
-				goto out;
+				goto out_wait;
 
 			if (read >= target || flags & MSG_PEEK)
 				break;
@@ -1769,9 +1773,7 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 				break;
 
 			release_sock(sk);
-			prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 			timeout = schedule_timeout(timeout);
-			finish_wait(sk_sleep(sk), &wait);
 			lock_sock(sk);
 
 			if (signal_pending(current)) {
@@ -1781,6 +1783,9 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 				err = -EAGAIN;
 				break;
 			}
+
+			prepare_to_wait(sk_sleep(sk), &wait,
+					TASK_INTERRUPTIBLE);
 		}
 	}
 
@@ -1811,6 +1816,8 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 		err = copied;
 	}
 
+out_wait:
+	finish_wait(sk_sleep(sk), &wait);
 out:
 	release_sock(sk);
 	return err;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v3 0/2] AF_VSOCK: Shrink the area influenced by prepare_to_wait
From: Claudio Imbrenda @ 2016-03-22 16:05 UTC (permalink / raw)
  To: davem; +Cc: labbott, netdev, linux-kernel

This patchset applies on net-next.

I think I found a problem with the patch submitted by Laura Abbott
( https://lkml.org/lkml/2016/2/4/711 ): we might miss wakeups.
Since the condition is not checked between the prepare_to_wait and the
schedule(), if a wakeup happens after the condition is checked but before
the sleep happens, and we miss it. ( A description of the problem can be
found here: http://www.makelinux.net/ldd3/chp-6-sect-2 ).

The first patch reverts the previous broken patch, while the second patch
properly fixes the sleep-while-waiting issue.


Claudio Imbrenda (2):
  Revert "vsock: Fix blocking ops call in prepare_to_wait"
  AF_VSOCK: Shrink the area influenced by prepare_to_wait

 net/vmw_vsock/af_vsock.c | 155 ++++++++++++++++++++++++++---------------------
 1 file changed, 87 insertions(+), 68 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [iproute PATCH v3] Use ARRAY_SIZE macro everywhere
From: Phil Sutter @ 2016-03-22 15:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20160321115905.78a2b3e1@xeon-e3>

This patch was generated by the following semantic patch (a trimmed down
version of what is shipped with Linux sources):

@@
type T;
T[] E;
@@
(
- (sizeof(E)/sizeof(*E))
+ ARRAY_SIZE(E)
|
- (sizeof(E)/sizeof(E[...]))
+ ARRAY_SIZE(E)
|
- (sizeof(E)/sizeof(T))
+ ARRAY_SIZE(E)
)

The only manual adjustment was to include utils.h in misc/nstat.c to make
the macro known there.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Rebased onto current master to avoid merge conflicts.

Changes since v2:
- Patch recreated from scratch.
---
 bridge/link.c | 2 +-
 misc/nstat.c  | 2 +-
 misc/ss.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index 353e1c3da45db..b347040ccf91d 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -319,7 +319,7 @@ static int brlink_modify(int argc, char **argv)
 		} else if (strcmp(*argv, "state") == 0) {
 			NEXT_ARG();
 			char *endptr;
-			size_t nstates = sizeof(port_states) / sizeof(*port_states);
+			size_t nstates = ARRAY_SIZE(port_states);
 
 			state = strtol(*argv, &endptr, 10);
 			if (!(**argv != '\0' && *endptr == '\0')) {
diff --git a/misc/nstat.c b/misc/nstat.c
index a9e0f20789e3c..4f3863ff99121 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -95,7 +95,7 @@ static int useless_number(const char *id)
 {
 	int i;
 
-	for (i = 0; i < sizeof(useless_numbers)/sizeof(*useless_numbers); i++)
+	for (i = 0; i < ARRAY_SIZE(useless_numbers); i++)
 		if (strcmp(id, useless_numbers[i]) == 0)
 			return 1;
 	return 0;
diff --git a/misc/ss.c b/misc/ss.c
index 192389cc82371..449c391579af1 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -666,7 +666,7 @@ static int get_slabstat(struct slabstat *s)
 	while (fgets(buf, sizeof(buf), fp) != NULL) {
 		int i;
 
-		for (i = 0; i < sizeof(slabstat_ids)/sizeof(slabstat_ids[0]); i++) {
+		for (i = 0; i < ARRAY_SIZE(slabstat_ids); i++) {
 			if (memcmp(buf, slabstat_ids[i], strlen(slabstat_ids[i])) == 0) {
 				sscanf(buf, "%*s%d", ((int *)s) + i);
 				cnt--;
-- 
2.7.2

^ permalink raw reply related

* [PATCH net] ipv4: initialize flowi4_flags before calling fib_lookup()
From: Lance Richardson @ 2016-03-22 15:31 UTC (permalink / raw)
  To: netdev

Field fl4.flowi4_flags is not initialized in fib_compute_spec_dst()
before calling fib_lookup(), which means fib_table_lookup() is
using non-deterministic data at this line:

	if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {

Fix by initializing fl4.flowi4_flags to zero.

Signed-off-by: Lance Richardson <lrichard@redhat.com>
---
 net/ipv4/fib_frontend.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 21add55..896844a 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -304,6 +304,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
 		fl4.flowi4_scope = scope;
 		fl4.flowi4_mark = IN_DEV_SRC_VMARK(in_dev) ? skb->mark : 0;
 		fl4.flowi4_tun_key.tun_id = 0;
+		fl4.flowi4_flags = 0;
 		if (!fib_lookup(net, &fl4, &res, 0))
 			return FIB_RES_PREFSRC(net, res);
 	} else {
-- 
2.5.0

^ permalink raw reply related

* Re: [PATCH] lan78xx: Protect runtime_auto check by #ifdef CONFIG_PM
From: Oliver Neukum @ 2016-03-22 15:29 UTC (permalink / raw)
  To: Alan Stern
  Cc: David S. Miller, Geert Uytterhoeven,
	Microchip Linux Driver Support, Woojung Huh, Rafael J. Wysocki,
	Guenter Roeck, linux-kernel, linux-pm, linux-usb, netdev
In-Reply-To: <Pine.LNX.4.44L0.1603221111040.1793-100000@iolanthe.rowland.org>

On Tue, 2016-03-22 at 11:13 -0400, Alan Stern wrote:
> > Indeed. In that case the point is moot. But it is correct to ask
> > the core whether the device is autosuspended at that point rather
> > than keep a private flag if you can.
> 
> That's why we have pm_runtime_status_suspended().

I guess we are in violent agreement though we were unaware of being
in that state.

	Regards
		Oliver

^ permalink raw reply

* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Marcelo Ricardo Leitner @ 2016-03-22 15:28 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Baozeng Ding, vyasevich, nhorman, davem, linux-sctp, netdev,
	linux-kernel
In-Reply-To: <1458660088.10868.19.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Mar 22, 2016 at 08:21:28AM -0700, Eric Dumazet wrote:
> On Tue, 2016-03-22 at 23:08 +0800, Baozeng Ding wrote:
> > Hi all,
> > 
> > The following program triggers an out-of-bounds bug in
> > sctp_getsockopt. The kernel version is 4.5 (on Mar 16
> > commit 09fd671ccb2475436bd5f597f751ca4a7d177aea). 
> > 
> > ==================================================================
> > BUG: KASAN: stack-out-of-bounds in string+0x1ef/0x200 at addr
> > ffff88003ae679e0
> > Read of size 1 by task syz-executor/19753
> > page:ffffea0000eb99c0 count:0 mapcount:0 mapping:          (null)
> > index:0x0
> > flags: 0x1fffc0000000000()
> > page dumped because: kasan: bad access detected
> > CPU: 3 PID: 19753 Comm: syz-executor Not tainted 4.5.0+ #8
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> >  0000000000000003 ffff88003ae67578 ffffffff82945051 ffff88003ae67608
> >  ffff88003ae679e0 0000000000000096 dffffc0000000000 ffff88003ae675f8
> >  ffffffff81709f88 000000000000030d 0000000000000000 0000000000000286
> > Call Trace:
> >  [<     inline     >] __dump_stack lib/dump_stack.c:15
> >  [<ffffffff82945051>] dump_stack+0xb3/0x112 lib/dump_stack.c:51
> >  [<     inline     >] print_address_description mm/kasan/report.c:150
> >  [<ffffffff81709f88>] kasan_report_error+0x4f8/0x530 mm/kasan/report.c:236
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<     inline     >] kasan_report mm/kasan/report.c:259
> >  [<ffffffff81709ffe>] __asan_report_load1_noabort+0x3e/0x40 mm/kasan/report.c:277
> >  [<ffffffff8296613f>] ? string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296613f>] string+0x1ef/0x200 lib/vsprintf.c:591
> >  [<ffffffff8296f103>] vsnprintf+0xb83/0x1900 lib/vsprintf.c:2049
> >  [<ffffffff8296e580>] ? pointer+0xab0/0xab0 lib/vsprintf.c:1584
> >  [<ffffffff813456f2>] __request_module+0x132/0x6b0 kernel/kmod.c:146
> >  [<ffffffff814056b0>] ? mark_held_locks+0xd0/0x130 kernel/locking/lockdep.c:2552
> >  [<ffffffff813455c0>] ? call_usermodehelper_setup+0x2b0/0x2b0 kernel/kmod.c:530
> >  [<ffffffff85da47b0>] ? mutex_lock_interruptible_nested+0x980/0x980
> >  [<ffffffff8168fed4>] ? __might_fault+0xe4/0x1d0 mm/memory.c:3833
> >  [<ffffffff8538f74c>] find_inlist_lock.constprop.17+0x10c/0x210 net/bridge/netfilter/ebtables.c:347
> >  [<     inline     >] find_table_lock net/bridge/netfilter/ebtables.c:356
> >  [<ffffffff853904ab>] do_ebt_get_ctl+0x13b/0x540 net/bridge/netfilter/ebtables.c:1524
> >  [<ffffffff85390370>] ? copy_everything_to_user+0x600/0x600 net/bridge/netfilter/ebtables.c:1455
> >  [<     inline     >] ? __mutex_unlock_common_slowpath kernel/locking/mutex.c:751
> >  [<ffffffff85da6799>] ? __mutex_unlock_slowpath+0x239/0x3f0 kernel/locking/mutex.c:762
> >  [<ffffffff85da6959>] ? mutex_unlock+0x9/0x10 kernel/locking/mutex.c:437
> >  [<ffffffff84dea126>] ? nf_sockopt_find+0x1a6/0x220 net/netfilter/nf_sockopt.c:87
> >  [<     inline     >] nf_sockopt net/netfilter/nf_sockopt.c:103
> >  [<ffffffff84dea20d>] nf_getsockopt+0x6d/0xc0 net/netfilter/nf_sockopt.c:121
> >  [<ffffffff84fadf05>] ip_getsockopt+0x135/0x190 net/ipv4/ip_sockglue.c:1523
> >  [<ffffffff84faddd0>] ? do_ip_getsockopt+0x1520/0x1520 net/ipv4/ip_sockglue.c:1353
> >  [<     inline     >] ? wake_up_process kernel/sched/core.c:2024
> >  [<ffffffff8138bcc2>] ? wake_up_q+0x82/0xe0 kernel/sched/core.c:416
> >  [<     inline     >] ? atomic_dec_and_test /arch/x86/include/asm/atomic.h:117
> >  [<     inline     >] ? mmdrop include/linux/sched.h:2611
> >  [<ffffffff814a3310>] ? drop_futex_key_refs.isra.13+0x70/0xe0 kernel/futex.c:444
> >  [<ffffffff8583a4dd>] sctp_getsockopt+0x18d/0x3f40 net/sctp/socket.c:5964
> >  [<ffffffff8140785b>] ? __lock_acquire+0x15fb/0x5dd0 kernel/locking/lockdep.c:3226
> >  [<ffffffff8583a350>] ? sctp_do_peeloff+0x2b0/0x2b0 net/sctp/socket.c:4434
> >  [<ffffffff81406260>] ? debug_check_no_locks_freed+0x290/0x290 kernel/locking/lockdep.c:4104
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:922
> >  [<ffffffff817b398c>] ? __fget+0x20c/0x3b0 fs/file.c:712
> >  [<     inline     >] ? rcu_lock_release include/linux/rcupdate.h:491
> >  [<     inline     >] ? rcu_read_unlock include/linux/rcupdate.h:926
> >  [<ffffffff817b39b5>] ? __fget+0x235/0x3b0 fs/file.c:712
> >  [<ffffffff817b37c7>] ? __fget+0x47/0x3b0 fs/file.c:696
> >  [<ffffffff817b3c11>] ? __fget_light+0xa1/0x1f0 fs/file.c:759
> >  [<ffffffff84c3a695>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2579
> >  [<     inline     >] SYSC_getsockopt net/socket.c:1783
> >  [<ffffffff84c37e12>] SyS_getsockopt+0x142/0x230 net/socket.c:1765
> >  [<ffffffff84c37cd0>] ? SyS_setsockopt+0x240/0x240 net/socket.c:1752
> >  [<ffffffff85dab922>] ? entry_SYSCALL_64_fastpath+0x5/0xc1 arch/x86/entry/entry_64.S:191
> >  [<ffffffff81003017>] ? trace_hardirqs_on_thunk+0x17/0x19 arch/x86/entry/thunk_64.S:39
> >  [<ffffffff85dab940>] entry_SYSCALL_64_fastpath+0x23/0xc1 arch/x86/entry/entry_64.S:207
> > Memory state around the buggy address:
> >  ffff88003ae67880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67900: 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00 00
> > >ffff88003ae67980: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f3 f3 f3
> >                                                        ^
> >  ffff88003ae67a00: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88003ae67a80: f1 f1 f1 f1 04 f4 f4 f4 f3 f3 f3 f3 00 00 00 00
> > ==================================================================
> > 
> > #include <unistd.h>
> > #include <sys/syscall.h>
> > #include <netinet/in.h>
> > #include <string.h>
> > #include <stdint.h>
> > #include <sys/mman.h>
> > #include <sys/socket.h>
> > 
> > int main()
> > {
> >         int sock = 0;
> >         int sock_dup = 0;
> >         mmap((void *)0x20000000ul, 0x5000ul, PROT_READ|PROT_WRITE,
> > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0);
> >         sock = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP);
> >         sock_dup = dup(sock);
> >         memcpy((void*)0x20000bf3,"\xac\x71\x93\x68\x02\xb3\xd1\x86\x52\xf1\xf0\x18\x09\x56\xc6\x98\x6f\x8e\x74\xb7\x17\xd4\x3a\x64\x51\x68\x13\x2d\x25\xba\x6d\x3f\x74\x68\x84\x89\x04\xd1\xa6\xe2\x7d\xaf\xfa\xd9\xce\x52\xbe\x6f\xb6\xe3\xff\x92\x35\xa1\x88\x4a\x68\x27\xaa\x25\xf8\xc1\xd5\x3b\xe5\x69\x11\x4f\x75\x4c\xe9\xff\x8b\x86\x53\x20\xb7\x10\xa2\x62\xcc\xc3\x06\x85\xde\x3e\x1c\x5a\x62\x3a\x2d\x0d\x0b\x0c\xb2\xac\x75\x42\x4d\x82\x3f\x7b\xf7\x28\xea\x2d\xff\x42\xa8\xdf\xb3\x49\x1a\xfd\xae\x2c\xd4\x35\x8e\x96\xb3\xe1\x0a\x92\x56\xb7\xde\xe8\x9e\xc3\x9e\x88\x79\xc4\x71\x46\x27\xf4\x9e\x85\xf4\x8f\x1f\x9a\xe5\x7e\x02\x09\x34\x80\x1e\x87\xa8\x9a\xce\xac\xfb\x43\x07\xdf\x15\xe8\x71\x9a\xa3\x80\x18\x1b\x15\xbd\x57\xb6\xc1\x73\x6e\xb1\x28\x3a\x01\xd5\x8e\x15\x85\xbd\x52\xdf\xfa\x64\xaa\x13\x
 0e\x2f\x64\x05\x11\xce\x79\x8b\xa8\x02\x29\x7f\x72\x0f\x37\x89\xb4\x54\x0b\x09\x02\x75\xc2\x8e\xd7\xcd\x7e\xfb\x4f\x72\xf1\x47\xea\xa2\x2a\xc3\xc4\xe9\x70\xfe\xa5\x80\x88\x21\x33\xcf\x13\x66\!
>  x98\x23\x
>  10\x5c\xa4\xbd\xee\xc0\xb4\xdd\xfb\xff\xf2\x38\xab\xca\x36\x62\x35\x84\xe4\x73\x5c\xc7\x3e\x72\x2e\x17\x43\x6f\x85\x45\x4f\x82\x62\x0d\x77\xae\xcb\xe1\x8f\xe8\xf0\x84\x3e\x62\x8b\x70\x2b\x55\xb5\xa7\x13\xcf\xa1\x78\x77\x82\xe2\xb7\x1c\x65\x7f\xb5\x79\x73\x01\x07\xd1\x9f\x45\x6a\xbb\x3d\xbf\xc8\x71\x5b\x9f\x30\xc7\xb9\xb8\x53\x9f\xe1\xba\xb6\x78\x9e\x05\x75\xa3\x55\xb1\x26\x96\xa9\xb2\x82\xce\x81\x5c\x8a\x18\xb3\x4b\x0c\x18\x8c\xf2\x7c\x09\xde\xcb\xcf\x78\x22\x58\xf6\x15\xf6\xf7\x48\xda\x08\x75\xd4\xc1\x20\xc3\x18\x2e\x89\xe8\x5b\x48\xd9\xbc\x1f\xbb\xed\x31\xaf\x12\x4d\xcd\x46\x60\xa0\xef\x0e\x2e\x21\x1d\x2b\x68\x75\xb9\x42\x5e\xd7\xae\x35\x46\xe9\x06\x63\x1d\x3c\xd6\x9c\x14\x3b\x09\x29\x49\x70\xb9\xe1\xe0\x09\x45\x41\x62\x0c\xff\x5a\x77\xbe\x31\xa6\x03\x94\x92\xde\x41\x99\xfa\x68\x99\x7
 4\xbb\x0a\x3d\xac\x9c\x7e\x00\x6b\xcd\xc1\x83\xa7\xc5\x63\xdd\x10\xea\x59\x27\xdc\x02\x98\xd6\x43\x20\x24\x4e\xc0\xdc\xa2\x98\xdf\x3e\xaf\x61\x35\xa0\x95\x3f\x9a\xaa\x7d\xe9\xe9\x0d\xe5\x97\x!
>  66\x1a\x9
>  f\xbf\x56\xc8\x37\x84\x18\x2b\xd2\xcd\xd6\xb3\x19\xd8\x4a\x30\x6e\xcb\x99\x1c\xe9\x0f\xdb\xca\x30\xe1\xe2\x90\xba\xb9\x61\x00\xbf\xeb\xad\x6a\xc8\x52\xea\x1a\x92\x05\x0c\x3b\x78\x82\x01\xac\xfd\x88\x6c\xca\xe2\xfb\xe7\x0f\xcc\x75\x9c\x98\x12\x26\xcf\xa6\x80\x02\x35\xdf\x6e\xe1\x11\x1d\xa7\x30\x17\x38\x41\xd9\x81\x55\x1a\x1e\xd1\xfe\x60\xbf\xef\x09\x25\xc0\xdb\x9f\xc4\xc6\x54\x1a\x85\x36\x85\x05\xb3\x9f\x2c\xc5\xcd\x12\x51\xef\xbe\x10\x79\xbf\x11\x00\x47\x0d\x9c\x14\x43\x1a\x46\xea\xd1\x34\x2e\x10\x6b\xa4\x3c\x25\x21\xe3\xb9\x15\x78\x6c\x40\x87\x90\xf7\x93\x5a\x66\x5f\x0a\x76\xff\xc2\xe2\x14\x35\x88\x47\xa1\x33\x5b\x8f\x3d\xc5\x89\xb7\xf9\x8a\x40\xf0\x1e\xc9\x30\xcd\xd8\x96\x41\x78\x58\x97\x49\xc8\x50\x61\x36\x8f\x7e\x44\x41\xc0\x84\xbb\x35\xf0\x63\xa9\xc2\x2a\xbd\xcc\x4b\xab\x8b\x16\x33
 \xc0\x66\xbf\x47\x62\x9b\xc4\x47\x2d\x68\x83\xca\xe3\x52\x79\xd7\xe0\x61\x80\x15\xf1\x90\x83\xa2\xbb\x4c\xe5\x8b\x50\xc8\x1b\x68\x7b\xee\x57\xdc\x54\xfa\x90\xf1\xf5\xec\x7d\x93\xe0\x80\x74\x0!
>  6\xbe\xac
>  \xc8\x85\x4d\xe8\xbf\xd3\xdd\x34\x55\xc4\xbf\x2f\x24\x19\xad\x86\x1e\x69\x2b\x6c\x3f\x00\xe8\x4b\xbb\x99\xcf\x17\x99\x00\x9d\x6c\x70\x57\xcc\x35\xee\x07\x87\x25\x8c\x0c\x8b\x9b\x38\x15\xcc\x05\x6f\xf8\x16\x78\x0b\x41\xfa\x23\x96\xc0\x79\xf8\xb7\xf0\x2b\x60\x7e\x98\xe3\x7b\xab\x80\x1f\x0d\xbf\xf6\x7e\x37\x06\xf1\x11\x42\x38\x2a\x70\xdf\xa4\xca\xf5\xf3\xf4\x7d\xca\x10\x0c\xd5\xe2\x90\xa0\x15\xde\xc2\x61\xa2\x88\xea\x32\x37\x97\x83\xd0\x4c\xad\xe2\xae\x9b\x53\xa2\xc2\x54\x0c\xbd\xe1\x50\x3b\x15\xd4\xb1\xa9\x41\x6e\x18\x2e\x30\x3f\x91\x03\x81\x86\x8c\x5c\x1f\x76\x51\x92\xf5\xb5\xb2\xc3\x16\x01\xef\xe3\x9e\xb1\x92\x0e\x0e\xcb\x20\x7f\x10\x29\x08\x6e\x15\x3d\x1e\x7c\x70\xf5\xb5\x3c\x56\x15\x3c\x59\xe6\xe7\x9e\x16\xcd\xfc\x8e\xfa\x12\x99\xbb\x07\xaa\xd7\x1c\xd0\xae\x93\x4c\xba\x16\x5d\x0c\xed\
 x1d\x02\x87\xcd\x38\x31\xc6\x10\x42\xe1\x46\x4e\xa3\xae\xb6\xda\xb6\xb0\x49\x55\x89\x57\xe6\xac\xe3\xbf\xb5\x5c\x59\x93\x0d\x21\x35\xdd\x57\x8c\x04\x15\x91\x05\x69\x4a\xdb\x5e\xcb\x4d\xa3\x5d!
>  \xa8\x7e\
>  x95\x9e\x9d\x95\x61\xc9\x1c\xdd\x66\x0a\x76\x18\xbb\x59\x6a\xa5\xc0\xf2\xb8\x2f\xa9\x4c\xa8\xb3\x2b\xa3\x8a\xbf\x5c\xe8\x18\x3d\x7f\x0e\x2f\xe9\x06\xf9\xb6\xcc\x60\xcc\x38\x6c\x9a\x78\xa7\x7c\x61",
> > 1037);
> >         getsockopt(sock_dup, IPPROTO_IP, 0x81,  (void *)0x20000bf3ul,
> > (socklen_t *)0x20003000ul);        
> >         return 0;
> > }
> > 
> > Best Regards,
> > 
> > Baozeng Ding
> 
> More likely a netfilter bug in net/bridge/netfilter/ebtables.c

+1. sctp diverted it as the option level is IPPROTO_IP and not
SOL_SCTP.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox