[PATCH iproute2-next v2 0/8] ip: Introduce and use get_addr_rta()/inet_addr_match_rta()

[PATCH iproute2-next v2 0/8] ip: Introduce and use get_addr_rta()/inet_addr_match_rta()

[Serhey Popovych]

Now we enhance get_addr() to return additional
information about address (e.g. if it unspecified
or multicast) we want to have same functionality
for attributes in netlink message.

Introduce and use get_addr_rta() that parses
given netlink attribute into @inet_prefix
data structure in the same way similar get_addr()
parses address from it's string representation.

Use attribute length to guess address family:
force it by giving non AF_UNSPEC @family to
get_addr_rta() to ensure address is of expected
family.

Introduce and use inet_addr_match_rta() to further
simplify and unify code where get_addr_rta()
intended to be used together with inet_addr_match().

This is next step in ipv4 and ipv6 modules
unification to prepare for merge in the future.

Any comments, suggestions and criticism as always
welcome.

v2
  Introduce and use inet_addr_match_rta() as suggested
  by David Ahern.

  Check for result from get_prefix() in places where
  inet_addr_match_rta() being used.

Thanks,
Serhii

Serhey Popovych (8):
  utils: Introduce get_addr_rta() and inet_addr_match_rta()
  ipaddress: Use inet_addr_match_rta()
  iprule: Use inet_addr_match_rta()
  ipmroute: Use inet_addr_match_rta()
  ipneigh: Use inet_addr_match_rta()
  ipl2tp: Use get_addr_rta()
  tcp_metric: Use get_addr_rta()
  ip/tunnel: Unify local/remote endpoint address printing

 include/utils.h  |    2 ++
 ip/ipaddress.c   |   29 ++++++-----------
 ip/ipl2tp.c      |   37 +++++++++------------
 ip/ipmroute.c    |   26 ++++++---------
 ip/ipneigh.c     |   13 +++-----
 ip/iprule.c      |   36 ++++++++++-----------
 ip/link_gre.c    |   21 ++----------
 ip/link_gre6.c   |   26 ++-------------
 ip/link_ip6tnl.c |   15 ++-------
 ip/link_iptnl.c  |   21 ++----------
 ip/link_vti.c    |   21 ++----------
 ip/link_vti6.c   |   21 ++----------
 ip/tcp_metrics.c |   60 +++++++++++++++++-----------------
 ip/tunnel.c      |   27 ++++++++++++++++
 ip/tunnel.h      |    2 ++
 lib/utils.c      |   95 +++++++++++++++++++++++++++++++++++++++++++-----------
 16 files changed, 206 insertions(+), 246 deletions(-)

-- 
1.7.10.4

[PATCH iproute2-next v2 1/8] utils: Introduce get_addr_rta() and inet_addr_match_rta()

[Serhey Popovych]

First is used to get address from netlink attribute to
inet_prefix data structure. Use memcpy() with constant
value to let complier optimize by replacing a call by
inlining load/store instructions.

Second is used to match address in given netlink attribute
with one given as reference. It matches successfully if
no attribute is given (@rta is NULL), reference address
family is AF_UNSPEC or it's length isn't given; fails if
get_attr_rta() can't get attribute or it's family does
not match reference; calls inet_addr_match() to get final
verdict.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/utils.h |    2 ++
 lib/utils.c     |   95 ++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index f562547..0394268 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -120,6 +120,7 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family);
 int get_addr(inet_prefix *dst, const char *arg, int family);
 int get_prefix(inet_prefix *dst, char *arg, int family);
 int mask2bits(__u32 netmask);
+int get_addr_rta(inet_prefix *dst, const struct rtattr *rta, int family);
 int get_addr_ila(__u64 *val, const char *arg);
 
 int read_prop(const char *dev, char *prop, long *value);
@@ -174,6 +175,7 @@ int check_ifname(const char *);
 int get_ifname(char *, const char *);
 int matches(const char *arg, const char *pattern);
 int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits);
+int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rta);
 
 const char *dnet_ntop(int af, const void *addr, char *str, size_t len);
 int dnet_pton(int af, const char *src, void *addr);
diff --git a/lib/utils.c b/lib/utils.c
index e20b60e..8e15625 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -534,6 +534,28 @@ int get_addr64(__u64 *ap, const char *cp)
 	return 1;
 }
 
+static void set_address_type(inet_prefix *addr)
+{
+	switch (addr->family) {
+	case AF_INET:
+		if (!addr->data[0])
+			addr->flags |= ADDRTYPE_INET_UNSPEC;
+		else if (IN_MULTICAST(ntohl(addr->data[0])))
+			addr->flags |= ADDRTYPE_INET_MULTI;
+		else
+			addr->flags |= ADDRTYPE_INET;
+		break;
+	case AF_INET6:
+		if (IN6_IS_ADDR_UNSPECIFIED(addr->data))
+			addr->flags |= ADDRTYPE_INET_UNSPEC;
+		else if (IN6_IS_ADDR_MULTICAST(addr->data))
+			addr->flags |= ADDRTYPE_INET_MULTI;
+		else
+			addr->flags |= ADDRTYPE_INET;
+		break;
+	}
+}
+
 static int __get_addr_1(inet_prefix *addr, const char *name, int family)
 {
 	memset(addr, 0, sizeof(*addr));
@@ -627,25 +649,7 @@ int get_addr_1(inet_prefix *addr, const char *name, int family)
 	if (ret)
 		return ret;
 
-	switch (addr->family) {
-	case AF_INET:
-		if (!addr->data[0])
-			addr->flags |= ADDRTYPE_INET_UNSPEC;
-		else if (IN_MULTICAST(ntohl(addr->data[0])))
-			addr->flags |= ADDRTYPE_INET_MULTI;
-		else
-			addr->flags |= ADDRTYPE_INET;
-		break;
-	case AF_INET6:
-		if (IN6_IS_ADDR_UNSPECIFIED(addr->data))
-			addr->flags |= ADDRTYPE_INET_UNSPEC;
-		else if (IN6_IS_ADDR_MULTICAST(addr->data))
-			addr->flags |= ADDRTYPE_INET_MULTI;
-		else
-			addr->flags |= ADDRTYPE_INET;
-		break;
-	}
-
+	set_address_type(addr);
 	return 0;
 }
 
@@ -734,6 +738,46 @@ int get_addr(inet_prefix *dst, const char *arg, int family)
 	return 0;
 }
 
+int get_addr_rta(inet_prefix *dst, const struct rtattr *rta, int family)
+{
+	const int len = RTA_PAYLOAD(rta);
+	const void *data = RTA_DATA(rta);
+
+	switch (len) {
+	case 4:
+		dst->family = AF_INET;
+		dst->bytelen = 4;
+		memcpy(dst->data, data, 4);
+		break;
+	case 16:
+		dst->family = AF_INET6;
+		dst->bytelen = 16;
+		memcpy(dst->data, data, 16);
+		break;
+	case 2:
+		dst->family = AF_DECnet;
+		dst->bytelen = 2;
+		memcpy(dst->data, data, 2);
+		break;
+	case 10:
+		dst->family = AF_IPX;
+		dst->bytelen = 10;
+		memcpy(dst->data, data, 10);
+		break;
+	default:
+		return -1;
+	}
+
+	if (family != AF_UNSPEC && family != dst->family)
+		return -2;
+
+	dst->bitlen = -1;
+	dst->flags = 0;
+
+	set_address_type(dst);
+	return 0;
+}
+
 int get_prefix(inet_prefix *dst, char *arg, int family)
 {
 	if (family == AF_PACKET) {
@@ -864,6 +908,19 @@ int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits)
 	return 0;
 }
 
+int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rta)
+{
+	inet_prefix dst;
+
+	if (!rta || m->family == AF_UNSPEC || m->bitlen <= 0)
+		return 0;
+
+	if (get_addr_rta(&dst, rta, m->family))
+		return -1;
+
+	return inet_addr_match(&dst, m, m->bitlen);
+}
+
 int __iproute2_hz_internal;
 
 int __get_hz(void)
-- 
1.7.10.4

[PATCH iproute2-next v2 2/8] ipaddress: Use inet_addr_match_rta()

[Serhey Popovych]

While there check return from get_prefix() for filter address.

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index ba60125..44894e6 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1523,19 +1523,13 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		if (fnmatch(filter.label, label, 0) != 0)
 			return 0;
 	}
-	if (filter.pfx.family) {
-		if (rta_tb[IFA_LOCAL]) {
-			inet_prefix dst = { .family = ifa->ifa_family };
-
-			memcpy(&dst.data, RTA_DATA(rta_tb[IFA_LOCAL]), RTA_PAYLOAD(rta_tb[IFA_LOCAL]));
-			if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
-				return 0;
-		}
-	}
 
 	if (filter.family && filter.family != ifa->ifa_family)
 		return 0;
 
+	if (inet_addr_match_rta(&filter.pfx, rta_tb[IFA_LOCAL]))
+		return 0;
+
 	if (filter.flushb) {
 		struct nlmsghdr *fn;
 
@@ -1889,18 +1883,12 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
 			if ((filter.flags ^ ifa_flags) & filter.flagmask)
 				continue;
 			if (filter.pfx.family || filter.label) {
-				if (!tb[IFA_LOCAL])
-					tb[IFA_LOCAL] = tb[IFA_ADDRESS];
+				struct rtattr *rta =
+					tb[IFA_LOCAL] ? : tb[IFA_ADDRESS];
 
-				if (filter.pfx.family && tb[IFA_LOCAL]) {
-					inet_prefix dst = {
-						.family = ifa->ifa_family
-					};
+				if (inet_addr_match_rta(&filter.pfx, rta))
+					continue;
 
-					memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
-					if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
-						continue;
-				}
 				if (filter.label) {
 					SPRINT_BUF(b1);
 					const char *label;
@@ -2072,7 +2060,8 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
 	while (argc > 0) {
 		if (strcmp(*argv, "to") == 0) {
 			NEXT_ARG();
-			get_prefix(&filter.pfx, *argv, filter.family);
+			if (get_prefix(&filter.pfx, *argv, filter.family))
+				invarg("invalid \"to\"\n", *argv);
 			if (filter.family == AF_UNSPEC)
 				filter.family = filter.pfx.family;
 		} else if (strcmp(*argv, "scope") == 0) {
-- 
1.7.10.4

[PATCH iproute2-next v2 3/8] iprule: Use inet_addr_match_rta()

[Serhey Popovych]

While there check return from get_prefix() for filter address.

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

diff --git a/ip/iprule.c b/ip/iprule.c
index 96695d8..a3abf2f 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -76,8 +76,6 @@ static struct
 static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 {
 	struct rtmsg *r = NLMSG_DATA(n);
-	inet_prefix src = { .family = r->rtm_family };
-	inet_prefix dst = { .family = r->rtm_family };
 	__u32 table;
 
 	if (preferred_family != AF_UNSPEC && r->rtm_family != preferred_family)
@@ -90,24 +88,24 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		return false;
 
 	if (filter.src.family) {
-		if (tb[FRA_SRC]) {
-			memcpy(&src.data, RTA_DATA(tb[FRA_SRC]),
-			       (r->rtm_src_len + 7) / 8);
-		}
-		if (filter.src.family != r->rtm_family ||
-		    filter.src.bitlen > r->rtm_src_len ||
-		    inet_addr_match(&src, &filter.src, filter.src.bitlen))
+		inet_prefix *f_src = &filter.src;
+
+		if (f_src->family != r->rtm_family ||
+		    f_src->bitlen > r->rtm_src_len)
+			return false;
+
+		if (inet_addr_match_rta(f_src, tb[FRA_SRC]))
 			return false;
 	}
 
 	if (filter.dst.family) {
-		if (tb[FRA_DST]) {
-			memcpy(&dst.data, RTA_DATA(tb[FRA_DST]),
-			       (r->rtm_dst_len + 7) / 8);
-		}
-		if (filter.dst.family != r->rtm_family ||
-		    filter.dst.bitlen > r->rtm_dst_len ||
-		    inet_addr_match(&dst, &filter.dst, filter.dst.bitlen))
+		inet_prefix *f_dst = &filter.dst;
+
+		if (f_dst->family != r->rtm_family ||
+		    f_dst->bitlen > r->rtm_dst_len)
+			return false;
+
+		if (inet_addr_match_rta(f_dst, tb[FRA_DST]))
 			return false;
 	}
 
@@ -500,13 +498,15 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
 		} else if (matches(*argv, "from") == 0 ||
 			   matches(*argv, "src") == 0) {
 			NEXT_ARG();
-			get_prefix(&filter.src, *argv, af);
+			if (get_prefix(&filter.src, *argv, af))
+				invarg("from value is invalid\n", *argv);
 		} else {
 			if (matches(*argv, "dst") == 0 ||
 			    matches(*argv, "to") == 0) {
 				NEXT_ARG();
 			}
-			get_prefix(&filter.dst, *argv, af);
+			if (get_prefix(&filter.dst, *argv, af))
+				invarg("to value is invalid\n", *argv);
 		}
 		argc--; argv++;
 	}
-- 
1.7.10.4

[PATCH iproute2-next v2 4/8] ipmroute: Use inet_addr_match_rta()

[Serhey Popovych]

While there check return from get_prefix() for filter address.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ipmroute.c |   26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index 0450ea9..aa5029b 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -95,21 +95,11 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	if (filter.af && filter.af != r->rtm_family)
 		return 0;
 
-	if (tb[RTA_DST] && filter.mdst.bitlen > 0) {
-		inet_prefix dst = { .family = r->rtm_family };
-
-		memcpy(&dst.data, RTA_DATA(tb[RTA_DST]), RTA_PAYLOAD(tb[RTA_DST]));
-		if (inet_addr_match(&dst, &filter.mdst, filter.mdst.bitlen))
-			return 0;
-	}
-
-	if (tb[RTA_SRC] && filter.msrc.bitlen > 0) {
-		inet_prefix src = { .family = r->rtm_family };
+	if (inet_addr_match_rta(&filter.mdst, tb[RTA_DST]))
+		return 0;
 
-		memcpy(&src.data, RTA_DATA(tb[RTA_SRC]), RTA_PAYLOAD(tb[RTA_SRC]));
-		if (inet_addr_match(&src, &filter.msrc, filter.msrc.bitlen))
-			return 0;
-	}
+	if (inet_addr_match_rta(&filter.msrc, tb[RTA_SRC]))
+		return 0;
 
 	family = get_real_family(r->rtm_type, r->rtm_family);
 
@@ -213,6 +203,8 @@ static int mroute_list(int argc, char **argv)
 	} else
 		filter.af = RTNL_FAMILY_IP6MR;
 
+	filter.msrc.family = filter.mdst.family = family;
+
 	while (argc > 0) {
 		if (matches(*argv, "table") == 0) {
 			__u32 tid;
@@ -233,14 +225,16 @@ static int mroute_list(int argc, char **argv)
 			id = *argv;
 		} else if (matches(*argv, "from") == 0) {
 			NEXT_ARG();
-			get_prefix(&filter.msrc, *argv, family);
+			if (get_prefix(&filter.msrc, *argv, family))
+				invarg("from value is invalid\n", *argv);
 		} else {
 			if (strcmp(*argv, "to") == 0) {
 				NEXT_ARG();
 			}
 			if (matches(*argv, "help") == 0)
 				usage();
-			get_prefix(&filter.mdst, *argv, family);
+			if (get_prefix(&filter.mdst, *argv, family))
+				invarg("to value is invalid\n", *argv);
 		}
 		argc--; argv++;
 	}
-- 
1.7.10.4

[PATCH iproute2-next v2 5/8] ipneigh: Use inet_addr_match_rta()

[Serhey Popovych]

While there check return from get_prefix() for filter address.

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

diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 11f280d..0735424 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -234,15 +234,9 @@ int print_neigh(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	parse_rtattr(tb, NDA_MAX, NDA_RTA(r), n->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
 
-	if (tb[NDA_DST]) {
-		if (filter.pfx.family) {
-			inet_prefix dst = { .family = r->ndm_family };
+	if (inet_addr_match_rta(&filter.pfx, tb[NDA_DST]))
+		return 0;
 
-			memcpy(&dst.data, RTA_DATA(tb[NDA_DST]), RTA_PAYLOAD(tb[NDA_DST]));
-			if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
-				return 0;
-		}
-	}
 	if (filter.unused_only && tb[NDA_CACHEINFO]) {
 		struct nda_cacheinfo *ci = RTA_DATA(tb[NDA_CACHEINFO]);
 
@@ -418,7 +412,8 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 			}
 			if (matches(*argv, "help") == 0)
 				usage();
-			get_prefix(&filter.pfx, *argv, filter.family);
+			if (get_prefix(&filter.pfx, *argv, filter.family))
+				invarg("to value is invalid\n", *argv);
 			if (filter.family == AF_UNSPEC)
 				filter.family = filter.pfx.family;
 		}
-- 
1.7.10.4

[PATCH iproute2-next v2 6/8] ipl2tp: Use get_addr_rta()

[Serhey Popovych]

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ipl2tp.c |   37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
index 7c5ed31..8aaee74 100644
--- a/ip/ipl2tp.c
+++ b/ip/ipl2tp.c
@@ -296,7 +296,7 @@ static int get_response(struct nlmsghdr *n, void *arg)
 	struct l2tp_data *data = arg;
 	struct l2tp_parm *p = &data->config;
 	struct rtattr *attrs[L2TP_ATTR_MAX + 1];
-	struct rtattr *nla_stats;
+	struct rtattr *nla_stats, *rta;
 	int len;
 
 	/* Validate message and parse attributes */
@@ -352,30 +352,25 @@ static int get_response(struct nlmsghdr *n, void *arg)
 
 	if (attrs[L2TP_ATTR_RECV_TIMEOUT])
 		p->reorder_timeout = rta_getattr_u64(attrs[L2TP_ATTR_RECV_TIMEOUT]);
-	if (attrs[L2TP_ATTR_IP_SADDR]) {
-		p->local_ip.family = AF_INET;
-		p->local_ip.data[0] = rta_getattr_u32(attrs[L2TP_ATTR_IP_SADDR]);
-		p->local_ip.bytelen = 4;
-		p->local_ip.bitlen = -1;
-	}
-	if (attrs[L2TP_ATTR_IP_DADDR]) {
-		p->peer_ip.family = AF_INET;
-		p->peer_ip.data[0] = rta_getattr_u32(attrs[L2TP_ATTR_IP_DADDR]);
-		p->peer_ip.bytelen = 4;
-		p->peer_ip.bitlen = -1;
-	}
-	if (attrs[L2TP_ATTR_IP6_SADDR]) {
+
+	rta = attrs[L2TP_ATTR_IP_SADDR];
+	p->local_ip.family = AF_INET;
+	if (!rta) {
+		rta = attrs[L2TP_ATTR_IP6_SADDR];
 		p->local_ip.family = AF_INET6;
-		memcpy(&p->local_ip.data, RTA_DATA(attrs[L2TP_ATTR_IP6_SADDR]),
-			p->local_ip.bytelen = 16);
-		p->local_ip.bitlen = -1;
 	}
-	if (attrs[L2TP_ATTR_IP6_DADDR]) {
+	if (rta && get_addr_rta(&p->local_ip, rta, p->local_ip.family))
+		return -1;
+
+	rta = attrs[L2TP_ATTR_IP_DADDR];
+	p->peer_ip.family = AF_INET;
+	if (!rta) {
+		rta = attrs[L2TP_ATTR_IP6_DADDR];
 		p->peer_ip.family = AF_INET6;
-		memcpy(&p->peer_ip.data, RTA_DATA(attrs[L2TP_ATTR_IP6_DADDR]),
-			p->peer_ip.bytelen = 16);
-		p->peer_ip.bitlen = -1;
 	}
+	if (rta && get_addr_rta(&p->peer_ip, rta, p->peer_ip.family))
+		return -1;
+
 	if (attrs[L2TP_ATTR_UDP_SPORT])
 		p->local_udp_port = rta_getattr_u16(attrs[L2TP_ATTR_UDP_SPORT]);
 	if (attrs[L2TP_ATTR_UDP_DPORT])
-- 
1.7.10.4

[PATCH iproute2-next v2 7/8] tcp_metric: Use get_addr_rta()

[Serhey Popovych]

While there remove & from inet_prefix.data when
since it is array.

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

diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c
index 3f9790e..7e2d9eb 100644
--- a/ip/tcp_metrics.c
+++ b/ip/tcp_metrics.c
@@ -96,7 +96,7 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	struct rtattr *attrs[TCP_METRICS_ATTR_MAX + 1], *a;
 	int len = n->nlmsg_len;
 	inet_prefix daddr, saddr;
-	int family, i, atype, stype, dlen = 0, slen = 0;
+	int i, atype, stype;
 
 	if (n->nlmsg_type != genl_family)
 		return -1;
@@ -116,61 +116,61 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		if (f.daddr.family && f.daddr.family != AF_INET)
 			return 0;
 		a = attrs[TCP_METRICS_ATTR_ADDR_IPV4];
-		memcpy(&daddr.data, RTA_DATA(a), 4);
-		daddr.bytelen = 4;
-		family = AF_INET;
+		daddr.family = AF_INET;
 		atype = TCP_METRICS_ATTR_ADDR_IPV4;
-		dlen = RTA_PAYLOAD(a);
 	} else if (attrs[TCP_METRICS_ATTR_ADDR_IPV6]) {
 		if (f.daddr.family && f.daddr.family != AF_INET6)
 			return 0;
 		a = attrs[TCP_METRICS_ATTR_ADDR_IPV6];
-		memcpy(&daddr.data, RTA_DATA(a), 16);
-		daddr.bytelen = 16;
-		family = AF_INET6;
+		daddr.family = AF_INET6;
 		atype = TCP_METRICS_ATTR_ADDR_IPV6;
-		dlen = RTA_PAYLOAD(a);
 	} else {
 		return 0;
 	}
 
+	if (get_addr_rta(&daddr, a, daddr.family))
+		return 0;
+
+	if (f.daddr.family && f.daddr.bitlen >= 0 &&
+	    inet_addr_match(&daddr, &f.daddr, f.daddr.bitlen))
+		return 0;
+
 	if (attrs[TCP_METRICS_ATTR_SADDR_IPV4]) {
 		if (f.saddr.family && f.saddr.family != AF_INET)
 			return 0;
 		a = attrs[TCP_METRICS_ATTR_SADDR_IPV4];
-		memcpy(&saddr.data, RTA_DATA(a), 4);
-		saddr.bytelen = 4;
+		saddr.family = AF_INET;
 		stype = TCP_METRICS_ATTR_SADDR_IPV4;
-		slen = RTA_PAYLOAD(a);
 	} else if (attrs[TCP_METRICS_ATTR_SADDR_IPV6]) {
 		if (f.saddr.family && f.saddr.family != AF_INET6)
 			return 0;
 		a = attrs[TCP_METRICS_ATTR_SADDR_IPV6];
-		memcpy(&saddr.data, RTA_DATA(a), 16);
-		saddr.bytelen = 16;
+		saddr.family = AF_INET6;
 		stype = TCP_METRICS_ATTR_SADDR_IPV6;
-		slen = RTA_PAYLOAD(a);
+	} else {
+		saddr.family = AF_UNSPEC;
+		stype = 0;
 	}
 
-	if (f.daddr.family && f.daddr.bitlen >= 0 &&
-	    inet_addr_match(&daddr, &f.daddr, f.daddr.bitlen))
-	       return 0;
-	/* Only check for the source-address if the kernel supports it,
-	 * meaning slen != 0.
-	 */
-	if (slen && f.saddr.family && f.saddr.bitlen >= 0 &&
-	    inet_addr_match(&saddr, &f.saddr, f.saddr.bitlen))
-		return 0;
+	/* Only get/check for the source-address if the kernel supports it. */
+	if (saddr.family) {
+		if (get_addr_rta(&saddr, a, saddr.family))
+			return 0;
+
+		if (f.saddr.family && f.saddr.bitlen >= 0 &&
+		    inet_addr_match(&saddr, &f.saddr, f.saddr.bitlen))
+			return 0;
+	}
 
 	if (f.flushb) {
 		struct nlmsghdr *fn;
 
 		TCPM_REQUEST(req2, 128, TCP_METRICS_CMD_DEL, NLM_F_REQUEST);
 
-		addattr_l(&req2.n, sizeof(req2), atype, &daddr.data,
+		addattr_l(&req2.n, sizeof(req2), atype, daddr.data,
 			  daddr.bytelen);
-		if (slen)
-			addattr_l(&req2.n, sizeof(req2), stype, &saddr.data,
+		if (saddr.family)
+			addattr_l(&req2.n, sizeof(req2), stype, saddr.data,
 				  saddr.bytelen);
 
 		if (NLMSG_ALIGN(f.flushp) + req2.n.nlmsg_len > f.flushe) {
@@ -190,7 +190,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));
+		format_host(daddr.family, daddr.bytelen, daddr.data));
 
 	a = attrs[TCP_METRICS_ATTR_AGE];
 	if (a) {
@@ -292,9 +292,9 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		fprintf(fp, " fo_cookie %s", cookie);
 	}
 
-	if (slen) {
+	if (saddr.family) {
 		fprintf(fp, " source %s",
-			format_host(family, slen, &saddr.data));
+			format_host(saddr.family, saddr.bytelen, saddr.data));
 	}
 
 	fprintf(fp, "\n");
-- 
1.7.10.4

[PATCH iproute2-next v2 8/8] ip/tunnel: Unify local/remote endpoint address printing

[Serhey Popovych]

Introduce and use tnl_print_endpoint() helper to print
of tunnel endpoint address.

Note that for AF_INET and AF_INET6 inet_ntop(3) is used
that may return NULL in case of failure and while unlikely
format_host_rta() might return NULL too. Handle this case
when passing local/remote to print_string().

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |   21 ++-------------------
 ip/link_gre6.c   |   26 ++------------------------
 ip/link_ip6tnl.c |   15 ++-------------
 ip/link_iptnl.c  |   21 ++-------------------
 ip/link_vti.c    |   21 ++-------------------
 ip/link_vti6.c   |   21 ++-------------------
 ip/tunnel.c      |   27 +++++++++++++++++++++++++++
 ip/tunnel.h      |    2 ++
 8 files changed, 41 insertions(+), 113 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index a430ab8..6b2c49d 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -398,30 +398,13 @@ get_failed:
 static void gre_print_direct_opt(FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
-	const char *local = "any";
-	const char *remote = "any";
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	__u8 ttl = 0;
 	__u8 tos = 0;
 
-	if (tb[IFLA_GRE_REMOTE]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_REMOTE]);
-
-		if (addr)
-			remote = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "remote", "remote %s ", remote);
-
-	if (tb[IFLA_GRE_LOCAL]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_GRE_LOCAL]);
-
-		if (addr)
-			local = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "local", "local %s ", local);
+	tnl_print_endpoint("remote", tb[IFLA_GRE_REMOTE], AF_INET);
+	tnl_print_endpoint("local", tb[IFLA_GRE_LOCAL], AF_INET);
 
 	if (tb[IFLA_GRE_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 5f465fe..78e7a91 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -425,13 +425,10 @@ get_failed:
 static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
-	const char *local = "any";
-	const char *remote = "any";
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
 	__u32 flowinfo = 0;
-	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
 	__u8 ttl = 0;
 
 	if (!tb)
@@ -448,27 +445,8 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (tb[IFLA_GRE_FLOWINFO])
 		flowinfo = rta_getattr_u32(tb[IFLA_GRE_FLOWINFO]);
 
-	if (tb[IFLA_GRE_REMOTE]) {
-		struct in6_addr addr;
-
-		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);
-	}
-
-	print_string(PRINT_ANY, "remote", "remote %s ", remote);
-
-	if (tb[IFLA_GRE_LOCAL]) {
-		struct in6_addr addr;
-
-		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);
-	}
-
-	print_string(PRINT_ANY, "local", "local %s ", local);
+	tnl_print_endpoint("remote", tb[IFLA_GRE_REMOTE], AF_INET6);
+	tnl_print_endpoint("local", tb[IFLA_GRE_LOCAL], AF_INET6);
 
 	if (tb[IFLA_GRE_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_GRE_LINK]);
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8f5c9bd..562ef7d 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -364,19 +364,8 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (tb[IFLA_IPTUN_REMOTE]) {
-		print_string(PRINT_ANY,
-			     "remote",
-			     "remote %s ",
-			     rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_REMOTE]));
-	}
-
-	if (tb[IFLA_IPTUN_LOCAL]) {
-		print_string(PRINT_ANY,
-			     "local",
-			     "local %s ",
-			     rt_addr_n2a_rta(AF_INET6, tb[IFLA_IPTUN_LOCAL]));
-	}
+	tnl_print_endpoint("remote", tb[IFLA_IPTUN_REMOTE], AF_INET6);
+	tnl_print_endpoint("local", tb[IFLA_IPTUN_LOCAL], AF_INET6);
 
 	if (tb[IFLA_IPTUN_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index ce3855c..eb0a9f4 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -361,8 +361,6 @@ get_failed:
 static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
-	const char *local = "any";
-	const char *remote = "any";
 	__u16 prefixlen;
 	__u8 ttl = 0;
 	__u8 tos = 0;
@@ -390,23 +388,8 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 		}
 	}
 
-	if (tb[IFLA_IPTUN_REMOTE]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_REMOTE]);
-
-		if (addr)
-			remote = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "remote", "remote %s ", remote);
-
-	if (tb[IFLA_IPTUN_LOCAL]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_IPTUN_LOCAL]);
-
-		if (addr)
-			local = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "local", "local %s ", local);
+	tnl_print_endpoint("remote", tb[IFLA_IPTUN_REMOTE], AF_INET);
+	tnl_print_endpoint("local", tb[IFLA_IPTUN_LOCAL], AF_INET);
 
 	if (tb[IFLA_IPTUN_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_IPTUN_LINK]);
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 1439e53..f756140 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -165,30 +165,13 @@ get_failed:
 
 static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	const char *local = "any";
-	const char *remote = "any";
 	char s2[64];
 
 	if (!tb)
 		return;
 
-	if (tb[IFLA_VTI_REMOTE]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_VTI_REMOTE]);
-
-		if (addr)
-			remote = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "remote", "remote %s ", remote);
-
-	if (tb[IFLA_VTI_LOCAL]) {
-		unsigned int addr = rta_getattr_u32(tb[IFLA_VTI_LOCAL]);
-
-		if (addr)
-			local = format_host(AF_INET, 4, &addr);
-	}
-
-	print_string(PRINT_ANY, "local", "local %s ", local);
+	tnl_print_endpoint("remote", tb[IFLA_VTI_REMOTE], AF_INET);
+	tnl_print_endpoint("local", tb[IFLA_VTI_LOCAL], AF_INET);
 
 	if (tb[IFLA_VTI_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_VTI_LINK]);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 2a86d59..d786e8c 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -169,30 +169,13 @@ get_failed:
 
 static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
-	const char *local = "any";
-	const char *remote = "any";
-	struct in6_addr saddr;
-	struct in6_addr daddr;
 	char s2[64];
 
 	if (!tb)
 		return;
 
-	if (tb[IFLA_VTI_REMOTE]) {
-		memcpy(&daddr, RTA_DATA(tb[IFLA_VTI_REMOTE]), sizeof(daddr));
-
-		remote = format_host(AF_INET6, 16, &daddr);
-	}
-
-	print_string(PRINT_ANY, "remote", "remote %s ", remote);
-
-	if (tb[IFLA_VTI_LOCAL]) {
-		memcpy(&saddr, RTA_DATA(tb[IFLA_VTI_LOCAL]), sizeof(saddr));
-
-		local = format_host(AF_INET6, 16, &saddr);
-	}
-
-	print_string(PRINT_ANY, "local", "local %s ", local);
+	tnl_print_endpoint("remote", tb[IFLA_VTI_REMOTE], AF_INET6);
+	tnl_print_endpoint("local", tb[IFLA_VTI_LOCAL], AF_INET6);
 
 	if (tb[IFLA_VTI_LINK]) {
 		unsigned int link = rta_getattr_u32(tb[IFLA_VTI_LINK]);
diff --git a/ip/tunnel.c b/ip/tunnel.c
index 0414804..39f22df 100644
--- a/ip/tunnel.c
+++ b/ip/tunnel.c
@@ -280,6 +280,33 @@ void tnl_print_encap(struct rtattr *tb[],
 	}
 }
 
+void tnl_print_endpoint(const char *name, const struct rtattr *rta, int family)
+{
+	const char *value;
+	inet_prefix dst;
+
+	if (!rta) {
+		value = "any";
+	} else if (get_addr_rta(&dst, rta, family)) {
+		value = "unknown";
+	} else if (dst.flags & ADDRTYPE_UNSPEC) {
+		value = "any";
+	} else {
+		value = format_host(family, dst.bytelen, dst.data);
+		if (!value)
+			value = "unknown";
+	}
+
+	if (is_json_context()) {
+		print_string(PRINT_JSON, name, NULL, value);
+	} else {
+		SPRINT_BUF(b1);
+
+		snprintf(b1, sizeof(b1), "%s %%s ", name);
+		print_string(PRINT_FP, NULL, b1, value);
+	}
+}
+
 /* tnl_print_stats - print tunnel statistics
  *
  * @buf - tunnel interface's line in /proc/net/dev,
diff --git a/ip/tunnel.h b/ip/tunnel.h
index a5c537c..5bd27c3 100644
--- a/ip/tunnel.h
+++ b/ip/tunnel.h
@@ -37,6 +37,8 @@ __be32 tnl_parse_key(const char *name, const char *key);
 void tnl_print_encap(struct rtattr *tb[],
 		     int encap_type, int encap_flags,
 		     int encap_sport, int encap_dport);
+void tnl_print_endpoint(const char *name,
+			const struct rtattr *rta, int family);
 void tnl_print_stats(const char *buf);
 
 #endif
-- 
1.7.10.4

Re: [PATCH iproute2-next v2 8/8] ip/tunnel: Unify local/remote endpoint address printing

[David Ahern]

On 1/23/18 12:19 PM, Serhey Popovych wrote:
> Introduce and use tnl_print_endpoint() helper to print
> of tunnel endpoint address.
> 
> Note that for AF_INET and AF_INET6 inet_ntop(3) is used
> that may return NULL in case of failure and while unlikely
> format_host_rta() might return NULL too. Handle this case
> when passing local/remote to print_string().
> 
> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
> ---
>  ip/link_gre.c    |   21 ++-------------------
>  ip/link_gre6.c   |   26 ++------------------------
>  ip/link_ip6tnl.c |   15 ++-------------
>  ip/link_iptnl.c  |   21 ++-------------------
>  ip/link_vti.c    |   21 ++-------------------
>  ip/link_vti6.c   |   21 ++-------------------
>  ip/tunnel.c      |   27 +++++++++++++++++++++++++++
>  ip/tunnel.h      |    2 ++
>  8 files changed, 41 insertions(+), 113 deletions(-)
> 
>

This one no longer applies cleanly after the latest merge to master.
Please update -- and fix the commit messages to use more than 50 columns.

Re: [PATCH iproute2-next v2 0/8] ip: Introduce and use get_addr_rta()/inet_addr_match_rta()

[David Ahern]

On 1/23/18 12:19 PM, Serhey Popovych wrote:
> Now we enhance get_addr() to return additional
> information about address (e.g. if it unspecified
> or multicast) we want to have same functionality
> for attributes in netlink message.
> 
> Introduce and use get_addr_rta() that parses
> given netlink attribute into @inet_prefix
> data structure in the same way similar get_addr()
> parses address from it's string representation.
> 
> Use attribute length to guess address family:
> force it by giving non AF_UNSPEC @family to
> get_addr_rta() to ensure address is of expected
> family.
> 
> Introduce and use inet_addr_match_rta() to further
> simplify and unify code where get_addr_rta()
> intended to be used together with inet_addr_match().
> 
> This is next step in ipv4 and ipv6 modules
> unification to prepare for merge in the future.
> 
> Any comments, suggestions and criticism as always
> welcome.

No need to resend this set, but in the future your commit messages can
use say 72 characters per line. You seem to be line wrapping between 50
and 60 characters.