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

[PATCH iproute2-next 0/8] ip: Introduce and use get_addr_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.

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.

Thanks,
Serhii

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

 include/utils.h  |    1 +
 ip/ipaddress.c   |   29 +++++++++----------
 ip/ipl2tp.c      |   37 +++++++++++-------------
 ip/ipmroute.c    |   18 ++++++++----
 ip/ipneigh.c     |   14 +++++-----
 ip/iprule.c      |   40 +++++++++++++++-----------
 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      |   82 +++++++++++++++++++++++++++++++++++++++++-------------
 16 files changed, 209 insertions(+), 226 deletions(-)

-- 
1.7.10.4

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

[Serhey Popovych]

It 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.

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

diff --git a/include/utils.h b/include/utils.h
index f562547..b618663 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);
diff --git a/lib/utils.c b/lib/utils.c
index e20b60e..7bbb1eb 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) {
-- 
1.7.10.4

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

[Serhey Popovych]

Setting ->family in @inet_prefix seems pointless:
inet_addr_match() does not consider it's value:
better is to force get_addr_rta() to use address
family from filter to ensure we get correct one.

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

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index ba60125..38d994e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1523,19 +1523,20 @@ 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 (filter.pfx.family && rta_tb[IFA_LOCAL]) {
+		struct rtattr *rta = rta_tb[IFA_LOCAL];
+		inet_prefix dst, *f_pfx = &filter.pfx;
+
+		if (get_addr_rta(&dst, rta, f_pfx->family))
+			return 0;
+		if (inet_addr_match(&dst, f_pfx, f_pfx->bitlen))
+			return 0;
+	}
+
 	if (filter.flushb) {
 		struct nlmsghdr *fn;
 
@@ -1893,12 +1894,12 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
 					tb[IFA_LOCAL] = tb[IFA_ADDRESS];
 
 				if (filter.pfx.family && tb[IFA_LOCAL]) {
-					inet_prefix dst = {
-						.family = ifa->ifa_family
-					};
+					struct rtattr *rta = tb[IFA_LOCAL];
+					inet_prefix dst, *f_pfx = &filter.pfx;
 
-					memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
-					if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
+					if (get_addr_rta(&dst, rta, f_pfx->family))
+						continue;
+					if (inet_addr_match(&dst, f_pfx, f_pfx->bitlen))
 						continue;
 				}
 				if (filter.label) {
-- 
1.7.10.4

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

[Serhey Popovych]

Setting ->family in @inet_prefix seems pointless:
inet_addr_match() does not consider it's value:
better is to force get_addr_rta() to use address
family from filter to ensure we get correct one.

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

diff --git a/ip/iprule.c b/ip/iprule.c
index 96695d8..2da18f6 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,25 +88,35 @@ 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))
+		struct rtattr *rta = tb[FRA_SRC];
+		inet_prefix src, *f_src = &filter.src;
+
+		if (f_src->family != r->rtm_family ||
+		    f_src->bitlen > r->rtm_src_len)
 			return false;
+
+		if (rta) {
+			if (get_addr_rta(&src, rta, f_src->family))
+				return false;
+			if (inet_addr_match(&src, f_src, f_src->bitlen))
+				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))
+		struct rtattr *rta = tb[FRA_DST];
+		inet_prefix dst, *f_dst = &filter.dst;
+
+		if (f_dst->family != r->rtm_family ||
+		    f_dst->bitlen > r->rtm_dst_len)
 			return false;
+
+		if (rta) {
+			if (get_addr_rta(&dst, rta, f_dst->family))
+				return false;
+			if (inet_addr_match(&dst, f_dst, f_dst->bitlen))
+				return false;
+		}
 	}
 
 	if (filter.tosmask && filter.tos ^ r->rtm_tos)
-- 
1.7.10.4

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

[Serhey Popovych]

Setting ->family in @inet_prefix seems pointless:
inet_addr_match() does not consider it's value:
better is to force get_addr_rta() to use address
family from filter to ensure we get correct one.

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

diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index 0450ea9..d568d00 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -96,18 +96,22 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		return 0;
 
 	if (tb[RTA_DST] && filter.mdst.bitlen > 0) {
-		inet_prefix dst = { .family = r->rtm_family };
+		struct rtattr *rta = tb[RTA_DST];
+		inet_prefix dst, *f_mdst = &filter.mdst;
 
-		memcpy(&dst.data, RTA_DATA(tb[RTA_DST]), RTA_PAYLOAD(tb[RTA_DST]));
-		if (inet_addr_match(&dst, &filter.mdst, filter.mdst.bitlen))
+		if (get_addr_rta(&dst, rta, f_mdst->family))
+			return 0;
+		if (inet_addr_match(&dst, f_mdst, f_mdst->bitlen))
 			return 0;
 	}
 
 	if (tb[RTA_SRC] && filter.msrc.bitlen > 0) {
-		inet_prefix src = { .family = r->rtm_family };
+		struct rtattr *rta = tb[RTA_SRC];
+		inet_prefix src, *f_msrc = &filter.msrc;
 
-		memcpy(&src.data, RTA_DATA(tb[RTA_SRC]), RTA_PAYLOAD(tb[RTA_SRC]));
-		if (inet_addr_match(&src, &filter.msrc, filter.msrc.bitlen))
+		if (get_addr_rta(&src, rta, f_msrc->family))
+			return 0;
+		if (inet_addr_match(&src, f_msrc, f_msrc->bitlen))
 			return 0;
 	}
 
@@ -213,6 +217,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;
-- 
1.7.10.4

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

[Serhey Popovych]

Setting ->family in @inet_prefix seems pointless:
inet_addr_match() does not consider it's value:
better is to force get_addr_rta() to use address
family from filter to ensure we get correct one.

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

diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 11f280d..d9de935 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -234,14 +234,14 @@ 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 (filter.pfx.family && tb[NDA_DST]) {
+		struct rtattr *rta = tb[NDA_DST];
+		inet_prefix dst, *f_pfx = &filter.pfx;
 
-			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 (get_addr_rta(&dst, rta, f_pfx->family))
+			return 0;
+		if (inet_addr_match(&dst, f_pfx, f_pfx->bitlen))
+			return 0;
 	}
 	if (filter.unused_only && tb[NDA_CACHEINFO]) {
 		struct nda_cacheinfo *ci = RTA_DATA(tb[NDA_CACHEINFO]);
-- 
1.7.10.4

[PATCH iproute2-next 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 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 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 674603b..61b7516 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 a0eeb5a..e3db87f 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 2/8] ipaddress: Use get_addr_rta()

[David Ahern]

On 1/22/18 10:26 AM, Serhey Popovych wrote:
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index ba60125..38d994e 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -1523,19 +1523,20 @@ 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 (filter.pfx.family && rta_tb[IFA_LOCAL]) {
> +		struct rtattr *rta = rta_tb[IFA_LOCAL];
> +		inet_prefix dst, *f_pfx = &filter.pfx;
> +
> +		if (get_addr_rta(&dst, rta, f_pfx->family))
> +			return 0;
> +		if (inet_addr_match(&dst, f_pfx, f_pfx->bitlen))
> +			return 0;
> +	}
> +


Those 2 checks are a consistent them throughout the patches. Given that
why not create a helper that takes an rta and an inet_prefix and returns
 the result of both checks?

Re: [PATCH iproute2-next 2/8] ipaddress: Use get_addr_rta()

[Serhey Popovych]

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

David Ahern wrote:
> On 1/22/18 10:26 AM, Serhey Popovych wrote:
>> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> index ba60125..38d994e 100644
>> --- a/ip/ipaddress.c
>> +++ b/ip/ipaddress.c
>> @@ -1523,19 +1523,20 @@ 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 (filter.pfx.family && rta_tb[IFA_LOCAL]) {
>> +		struct rtattr *rta = rta_tb[IFA_LOCAL];
>> +		inet_prefix dst, *f_pfx = &filter.pfx;
>> +
>> +		if (get_addr_rta(&dst, rta, f_pfx->family))
>> +			return 0;
>> +		if (inet_addr_match(&dst, f_pfx, f_pfx->bitlen))
>> +			return 0;
>> +	}
>> +
> 
> 
> Those 2 checks are a consistent them throughout the patches. Given that
> why not create a helper that takes an rta and an inet_prefix and returns
>  the result of both checks?

Good point, will address this in v2. Thanks.

> 



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