[RFC Patch net-next 1/5] net: introduce generic union inet_addr

[RFC Patch net-next 1/5] net: introduce generic union inet_addr

[Cong Wang]

Cc: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
 Documentation/printk-formats.txt |    9 +++++
 drivers/net/netconsole.c         |   22 ++++++-------
 include/linux/netpoll.h          |    9 +-----
 include/net/inet_addr.h          |   62 ++++++++++++++++++++++++++++++++++++++
 lib/vsprintf.c                   |   18 ++++++++++-
 net/core/netpoll.c               |   52 ++++++++++++++-----------------
 6 files changed, 122 insertions(+), 50 deletions(-)
 create mode 100644 include/net/inet_addr.h

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 3af5ae6..26ff336 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -121,6 +121,15 @@ IPv6 addresses:
 	print a compressed IPv6 address as described by
 	http://tools.ietf.org/html/rfc5952
 
+Generic IP addresses:
+
+	%pIg
+	%pig
+
+	For printing genernic IP address, which has union inet_addr type.
+	The 'Ig' specifier is same with 'I4' (for IPv4 addresses) or 'I6'
+	(for IPv6 address), 'ig' is same with 'i4' or 'i6'.
+
 UUID/GUID addresses:
 
 	%pUb	00010203-0405-0607-0809-0a0b0c0d0e0f
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 1d1d0a1..02d7389 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -269,18 +269,12 @@ static ssize_t show_remote_port(struct netconsole_target *nt, char *buf)
 
 static ssize_t show_local_ip(struct netconsole_target *nt, char *buf)
 {
-	if (nt->np.ipv6)
-		return snprintf(buf, PAGE_SIZE, "%pI6c\n", &nt->np.local_ip.in6);
-	else
-		return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.local_ip);
+	return snprintf(buf, PAGE_SIZE, "%pIg\n", &nt->np.local_ip);
 }
 
 static ssize_t show_remote_ip(struct netconsole_target *nt, char *buf)
 {
-	if (nt->np.ipv6)
-		return snprintf(buf, PAGE_SIZE, "%pI6c\n", &nt->np.remote_ip.in6);
-	else
-		return snprintf(buf, PAGE_SIZE, "%pI4\n", &nt->np.remote_ip);
+	return snprintf(buf, PAGE_SIZE, "%pIg\n", &nt->np.remote_ip);
 }
 
 static ssize_t show_local_mac(struct netconsole_target *nt, char *buf)
@@ -418,17 +412,19 @@ static ssize_t store_local_ip(struct netconsole_target *nt,
 
 	if (strnchr(buf, count, ':')) {
 		const char *end;
-		if (in6_pton(buf, count, nt->np.local_ip.in6.s6_addr, -1, &end) > 0) {
+		if (in6_pton(buf, count, nt->np.local_ip.sin6.sin6_addr.s6_addr, -1, &end) > 0) {
 			if (*end && *end != '\n') {
 				printk(KERN_ERR "netconsole: invalid IPv6 address at: <%c>\n", *end);
 				return -EINVAL;
 			}
+			nt->np.local_ip.sa.sa_family = AF_INET6;
 			nt->np.ipv6 = true;
 		} else
 			return -EINVAL;
 	} else {
 		if (!nt->np.ipv6) {
-			nt->np.local_ip.ip = in_aton(buf);
+			nt->np.local_ip.sin.sin_addr.s_addr = in_aton(buf);
+			nt->np.local_ip.sa.sa_family = AF_INET;
 		} else
 			return -EINVAL;
 	}
@@ -449,17 +445,19 @@ static ssize_t store_remote_ip(struct netconsole_target *nt,
 
 	if (strnchr(buf, count, ':')) {
 		const char *end;
-		if (in6_pton(buf, count, nt->np.remote_ip.in6.s6_addr, -1, &end) > 0) {
+		if (in6_pton(buf, count, nt->np.remote_ip.sin6.sin6_addr.s6_addr, -1, &end) > 0) {
 			if (*end && *end != '\n') {
 				printk(KERN_ERR "netconsole: invalid IPv6 address at: <%c>\n", *end);
 				return -EINVAL;
 			}
+			nt->np.remote_ip.sa.sa_family = AF_INET6;
 			nt->np.ipv6 = true;
 		} else
 			return -EINVAL;
 	} else {
 		if (!nt->np.ipv6) {
-			nt->np.remote_ip.ip = in_aton(buf);
+			nt->np.remote_ip.sin.sin_addr.s_addr = in_aton(buf);
+			nt->np.remote_ip.sa.sa_family = AF_INET;
 		} else
 			return -EINVAL;
 	}
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index f3c7c24..3884834 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -11,14 +11,7 @@
 #include <linux/interrupt.h>
 #include <linux/rcupdate.h>
 #include <linux/list.h>
-
-union inet_addr {
-	__u32		all[4];
-	__be32		ip;
-	__be32		ip6[4];
-	struct in_addr	in;
-	struct in6_addr	in6;
-};
+#include <net/inet_addr.h>
 
 struct netpoll {
 	struct net_device *dev;
diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
new file mode 100644
index 0000000..66a16fe
--- /dev/null
+++ b/include/net/inet_addr.h
@@ -0,0 +1,62 @@
+#ifndef _INET_ADDR_H
+#define _INET_ADDR_H
+
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <linux/socket.h>
+#include <net/addrconf.h>
+
+union inet_addr {
+	struct sockaddr_in sin;
+	struct sockaddr_in6 sin6;
+	struct sockaddr sa;
+};
+
+#if IS_ENABLED(CONFIG_IPV6)
+static inline
+bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
+{
+	if (a->sa.sa_family != b->sa.sa_family)
+		return false;
+	if (a->sa.sa_family == AF_INET6)
+		return ipv6_addr_equal(&a->sin6.sin6_addr, &b->sin6.sin6_addr);
+	else
+		return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
+}
+
+static inline bool inet_addr_any(const union inet_addr *ipa)
+{
+	if (ipa->sa.sa_family == AF_INET6)
+		return ipv6_addr_any(&ipa->sin6.sin6_addr);
+	else
+		return ipa->sin.sin_addr.s_addr == htonl(INADDR_ANY);
+}
+
+static inline bool inet_addr_multicast(const union inet_addr *ipa)
+{
+	if (ipa->sa.sa_family == AF_INET6)
+		return ipv6_addr_is_multicast(&ipa->sin6.sin6_addr);
+	else
+		return IN_MULTICAST(ntohl(ipa->sin.sin_addr.s_addr));
+}
+
+#else /* !CONFIG_IPV6 */
+
+static inline
+bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
+{
+	return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
+}
+
+static inline bool inet_addr_any(const union inet_addr *ipa)
+{
+	return ipa->sin.sin_addr.s_addr == htonl(INADDR_ANY);
+}
+
+static inline bool inet_addr_multicast(const union inet_addr *ipa)
+{
+	return IN_MULTICAST(ntohl(ipa->sin.sin_addr.s_addr));
+}
+#endif
+
+#endif
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index e149c64..6bcc7b9 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -27,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/ioport.h>
 #include <net/addrconf.h>
+#include <net/inet_addr.h>
 
 #include <asm/page.h>		/* for PAGE_SIZE */
 #include <asm/sections.h>	/* for dereference_function_descriptor() */
@@ -1004,10 +1005,10 @@ int kptr_restrict __read_mostly;
  * - 'MF' For a 6-byte MAC FDDI address, it prints the address
  *       with a dash-separated hex notation
  * - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
- * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
+ * - 'I' [46g] for IPv4/IPv6 addresses printed in the usual way
  *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
  *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
- * - 'i' [46] for 'raw' IPv4/IPv6 addresses
+ * - 'i' [46g] for 'raw' IPv4/IPv6 addresses
  *       IPv6 omits the colons (01020304...0f)
  *       IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
  * - '[Ii]4[hnbl]' IPv4 addresses in host, network, big or little endian order
@@ -1093,6 +1094,17 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 			return ip6_addr_string(buf, end, ptr, spec, fmt);
 		case '4':
 			return ip4_addr_string(buf, end, ptr, spec, fmt);
+		case 'g': {
+				union inet_addr *ia = ptr;
+
+				if (ia->sa.sa_family == AF_INET6) {
+					ptr = &ia->sin6.sin6_addr;
+					return ip6_addr_string(buf, end, ptr, spec, fmt);
+				} else {
+					ptr = &ia->sin.sin_addr.s_addr;
+					return ip4_addr_string(buf, end, ptr, spec, fmt);
+				}
+			}
 		}
 		break;
 	case 'U':
@@ -1370,6 +1382,8 @@ qualifier:
  * %pI6 print an IPv6 address with colons
  * %pi6 print an IPv6 address without colons
  * %pI6c print an IPv6 address as specified by RFC 5952
+ * %pIg print generic IP address of union inet_addr, either %pI4 or %pI6
+ * %pig print generic IP address of union inet_addr, either %pi4 or %pi6
  * %pU[bBlL] print a UUID/GUID in big or little endian using lower or upper
  *   case.
  * %*ph[CDN] a variable-length hex string with a separator (supports up to 64
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 03c8ec3..c300358 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -455,8 +455,8 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
 
 	if (np->ipv6) {
 		udph->check = 0;
-		udph->check = csum_ipv6_magic(&np->local_ip.in6,
-					      &np->remote_ip.in6,
+		udph->check = csum_ipv6_magic(&np->local_ip.sin6.sin6_addr,
+					      &np->remote_ip.sin6.sin6_addr,
 					      udp_len, IPPROTO_UDP,
 					      csum_partial(udph, udp_len, 0));
 		if (udph->check == 0)
@@ -475,16 +475,16 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
 		ip6h->payload_len = htons(sizeof(struct udphdr) + len);
 		ip6h->nexthdr = IPPROTO_UDP;
 		ip6h->hop_limit = 32;
-		ip6h->saddr = np->local_ip.in6;
-		ip6h->daddr = np->remote_ip.in6;
+		ip6h->saddr = np->local_ip.sin6.sin6_addr;
+		ip6h->daddr = np->remote_ip.sin6.sin6_addr;
 
 		eth = (struct ethhdr *) skb_push(skb, ETH_HLEN);
 		skb_reset_mac_header(skb);
 		skb->protocol = eth->h_proto = htons(ETH_P_IPV6);
 	} else {
 		udph->check = 0;
-		udph->check = csum_tcpudp_magic(np->local_ip.ip,
-						np->remote_ip.ip,
+		udph->check = csum_tcpudp_magic(np->local_ip.sin.sin_addr.s_addr,
+						np->remote_ip.sin.sin_addr.s_addr,
 						udp_len, IPPROTO_UDP,
 						csum_partial(udph, udp_len, 0));
 		if (udph->check == 0)
@@ -503,8 +503,8 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
 		iph->ttl      = 64;
 		iph->protocol = IPPROTO_UDP;
 		iph->check    = 0;
-		put_unaligned(np->local_ip.ip, &(iph->saddr));
-		put_unaligned(np->remote_ip.ip, &(iph->daddr));
+		put_unaligned(np->local_ip.sin.sin_addr.s_addr, &(iph->saddr));
+		put_unaligned(np->remote_ip.sin.sin_addr.s_addr, &(iph->daddr));
 		iph->check    = ip_fast_csum((unsigned char *)iph, iph->ihl);
 
 		eth = (struct ethhdr *) skb_push(skb, ETH_HLEN);
@@ -588,7 +588,7 @@ static void netpoll_neigh_reply(struct sk_buff *skb, struct netpoll_info *npinfo
 
 		spin_lock_irqsave(&npinfo->rx_lock, flags);
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (tip != np->local_ip.ip)
+			if (tip != np->local_ip.sin.sin_addr.s_addr)
 				continue;
 
 			hlen = LL_RESERVED_SPACE(np->dev);
@@ -676,7 +676,7 @@ static void netpoll_neigh_reply(struct sk_buff *skb, struct netpoll_info *npinfo
 
 		spin_lock_irqsave(&npinfo->rx_lock, flags);
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (!ipv6_addr_equal(daddr, &np->local_ip.in6))
+			if (!ipv6_addr_equal(daddr, &np->local_ip.sin6.sin6_addr))
 				continue;
 
 			hlen = LL_RESERVED_SPACE(np->dev);
@@ -826,9 +826,11 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
 		if (checksum_udp(skb, uh, ulen, iph->saddr, iph->daddr))
 			goto out;
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (np->local_ip.ip && np->local_ip.ip != iph->daddr)
+			__be32 daddr = np->local_ip.sin.sin_addr.s_addr;
+			__be32 saddr = np->remote_ip.sin.sin_addr.s_addr;
+			if (daddr && daddr != iph->daddr)
 				continue;
-			if (np->remote_ip.ip && np->remote_ip.ip != iph->saddr)
+			if (saddr && saddr != iph->saddr)
 				continue;
 			if (np->local_port && np->local_port != ntohs(uh->dest))
 				continue;
@@ -864,9 +866,9 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
 		if (udp6_csum_init(skb, uh, IPPROTO_UDP))
 			goto out;
 		list_for_each_entry_safe(np, tmp, &npinfo->rx_np, rx) {
-			if (!ipv6_addr_equal(&np->local_ip.in6, &ip6h->daddr))
+			if (!ipv6_addr_equal(&np->local_ip.sin6.sin6_addr, &ip6h->daddr))
 				continue;
-			if (!ipv6_addr_equal(&np->remote_ip.in6, &ip6h->saddr))
+			if (!ipv6_addr_equal(&np->remote_ip.sin6.sin6_addr, &ip6h->saddr))
 				continue;
 			if (np->local_port && np->local_port != ntohs(uh->dest))
 				continue;
@@ -897,16 +899,10 @@ out:
 void netpoll_print_options(struct netpoll *np)
 {
 	np_info(np, "local port %d\n", np->local_port);
-	if (np->ipv6)
-		np_info(np, "local IPv6 address %pI6c\n", &np->local_ip.in6);
-	else
-		np_info(np, "local IPv4 address %pI4\n", &np->local_ip.ip);
+	np_info(np, "local IPv6 address %pIg\n", &np->local_ip);
 	np_info(np, "interface '%s'\n", np->dev_name);
 	np_info(np, "remote port %d\n", np->remote_port);
-	if (np->ipv6)
-		np_info(np, "remote IPv6 address %pI6c\n", &np->remote_ip.in6);
-	else
-		np_info(np, "remote IPv4 address %pI4\n", &np->remote_ip.ip);
+	np_info(np, "remote IPv6 address %pIg\n", &np->remote_ip);
 	np_info(np, "remote ethernet address %pM\n", np->remote_mac);
 }
 EXPORT_SYMBOL(netpoll_print_options);
@@ -920,7 +916,7 @@ static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
 		if (!*end)
 			return 0;
 	}
-	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
+	if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
 #if IS_ENABLED(CONFIG_IPV6)
 		if (!*end)
 			return 1;
@@ -1139,7 +1135,7 @@ int netpoll_setup(struct netpoll *np)
 		rtnl_lock();
 	}
 
-	if (!np->local_ip.ip) {
+	if (!np->local_ip.sin.sin_addr.s_addr) {
 		if (!np->ipv6) {
 			in_dev = __in_dev_get_rtnl(ndev);
 
@@ -1150,8 +1146,8 @@ int netpoll_setup(struct netpoll *np)
 				goto put;
 			}
 
-			np->local_ip.ip = in_dev->ifa_list->ifa_local;
-			np_info(np, "local IP %pI4\n", &np->local_ip.ip);
+			np->local_ip.sin.sin_addr.s_addr = in_dev->ifa_list->ifa_local;
+			np_info(np, "local IP %pI4\n", &np->local_ip.sin.sin_addr.s_addr);
 		} else {
 #if IS_ENABLED(CONFIG_IPV6)
 			struct inet6_dev *idev;
@@ -1165,7 +1161,7 @@ int netpoll_setup(struct netpoll *np)
 				list_for_each_entry(ifp, &idev->addr_list, if_list) {
 					if (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)
 						continue;
-					np->local_ip.in6 = ifp->addr;
+					np->local_ip.sin6.sin6_addr = ifp->addr;
 					err = 0;
 					break;
 				}
@@ -1176,7 +1172,7 @@ int netpoll_setup(struct netpoll *np)
 				       np->dev_name);
 				goto put;
 			} else
-				np_info(np, "local IPv6 %pI6c\n", &np->local_ip.in6);
+				np_info(np, "local IPv6 %pI6c\n", &np->local_ip.sin6.sin6_addr);
 #else
 			np_err(np, "IPv6 is not supported %s, aborting\n",
 			       np->dev_name);
-- 
1.7.7.6

Re: [RFC Patch net-next 1/5] net: introduce generic union inet_addr

[Andy Shevchenko]

On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote: 
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>

I was about to answer for the Daniel's patch about %pig.

Daniel, could you resend your patch series to the LKML, since it touches
lib/vsprintf.c.
Also, regarding to your patch 2/2, could it be possible to split it to
two parts: first substitutes SCTP macros not related to IP addresses and
second one is explicitly targeting against SCTP_DEBUG_PRINTK_IPADDR ?
By the way, in some places in 2/2 you didn't change open coded function
name to the '"%s: ...", __func__, ...'.

Cong, I don't think is a good idea to update lib/ code and net/ code in
one patch, since that are logically a bit different. lib/ code sounds
more common, it's better if it leads separately this series.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [RFC Patch net-next 1/5] net: introduce generic union inet_addr

[Daniel Borkmann]

On 06/27/2013 09:42 AM, Andy Shevchenko wrote:
> On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
>> Cc: Daniel Borkmann <dborkman@redhat.com>
>> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> I was about to answer for the Daniel's patch about %pig.
>
> Daniel, could you resend your patch series to the LKML, since it touches
> lib/vsprintf.c.

Agreed. Will put lkml into CC for the vsprintf change.

> Also, regarding to your patch 2/2, could it be possible to split it to
> two parts: first substitutes SCTP macros not related to IP addresses and
> second one is explicitly targeting against SCTP_DEBUG_PRINTK_IPADDR ?

Well, as Vlad, one of the SCTP maintainers, already went through this patch
and I've already applied the few changes he requested, I'd like not to put
too much burden on yet another round of review, as the rest of the code
stays as is.

> By the way, in some places in 2/2 you didn't change open coded function
> name to the '"%s: ...", __func__, ...'.

Right, but those messages are rather more of generic nature. I went through
all of this in detail, and I think it's okay like that.

> Cong, I don't think is a good idea to update lib/ code and net/ code in
> one patch, since that are logically a bit different. lib/ code sounds
> more common, it's better if it leads separately this series.

[RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Cong Wang]

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 include/net/inet_addr.h |   20 ++++++++++++++++++++
 net/core/netpoll.c      |   24 ++----------------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
index 66a16fe..1379287 100644
--- a/include/net/inet_addr.h
+++ b/include/net/inet_addr.h
@@ -4,6 +4,7 @@
 #include <linux/in.h>
 #include <linux/in6.h>
 #include <linux/socket.h>
+#include <linux/inet.h>
 #include <net/addrconf.h>
 
 union inet_addr {
@@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
 }
 #endif
 
+static inline int inet_pton(const char *str, union inet_addr *addr)
+{
+	const char *end;
+
+	if (!strchr(str, ':') &&
+	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
+		if (!*end)
+			return 0;
+	}
+	if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
+#if IS_ENABLED(CONFIG_IPV6)
+		if (!*end)
+			return 1;
+#else
+		return -1;
+#endif
+	}
+	return -1;
+}
 #endif
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index c300358..6631b5e 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -907,26 +907,6 @@ void netpoll_print_options(struct netpoll *np)
 }
 EXPORT_SYMBOL(netpoll_print_options);
 
-static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
-{
-	const char *end;
-
-	if (!strchr(str, ':') &&
-	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
-		if (!*end)
-			return 0;
-	}
-	if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
-#if IS_ENABLED(CONFIG_IPV6)
-		if (!*end)
-			return 1;
-#else
-		return -1;
-#endif
-	}
-	return -1;
-}
-
 int netpoll_parse_options(struct netpoll *np, char *opt)
 {
 	char *cur=opt, *delim;
@@ -946,7 +926,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 		if ((delim = strchr(cur, '/')) == NULL)
 			goto parse_failed;
 		*delim = 0;
-		ipv6 = netpoll_parse_ip_addr(cur, &np->local_ip);
+		ipv6 = inet_pton(cur, &np->local_ip);
 		if (ipv6 < 0)
 			goto parse_failed;
 		else
@@ -982,7 +962,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
 	if ((delim = strchr(cur, '/')) == NULL)
 		goto parse_failed;
 	*delim = 0;
-	ipv6 = netpoll_parse_ip_addr(cur, &np->remote_ip);
+	ipv6 = inet_pton(cur, &np->remote_ip);
 	if (ipv6 < 0)
 		goto parse_failed;
 	else if (np->ipv6 != (bool)ipv6)
-- 
1.7.7.6

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Sergei Shtylyov]

Hello.

On 27-06-2013 10:43, Cong Wang wrote:

> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>   include/net/inet_addr.h |   20 ++++++++++++++++++++
>   net/core/netpoll.c      |   24 ++----------------------
>   2 files changed, 22 insertions(+), 22 deletions(-)

> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
>   #include <linux/in.h>
>   #include <linux/in6.h>
>   #include <linux/socket.h>
> +#include <linux/inet.h>
>   #include <net/addrconf.h>
>
>   union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
>   }
>   #endif
>
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
> +	const char *end;
> +
> +	if (!strchr(str, ':') &&
> +	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> +		if (!*end)
> +			return 0;
> +	}
> +	if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
> +#if IS_ENABLED(CONFIG_IPV6)
> +		if (!*end)

    How about:

		if (IS_ENABLED(CONFIG_IPV6) && !*end)

> +			return 1;
> +#else
> +		return -1;
> +#endif
> +	}
> +	return -1;
> +}
>   #endif

WBR, Sergei

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Sergei Shtylyov]

On 06/27/2013 06:18 PM, Sergei Shtylyov wrote:

>> Signed-off-by: Cong Wang <amwang@redhat.com>
>> ---
>>   include/net/inet_addr.h |   20 ++++++++++++++++++++
>>   net/core/netpoll.c      |   24 ++----------------------
>>   2 files changed, 22 insertions(+), 22 deletions(-)

>> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
>> index 66a16fe..1379287 100644
>> --- a/include/net/inet_addr.h
>> +++ b/include/net/inet_addr.h
[...]
>> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union
>> inet_addr *ipa)
>>   }
>>   #endif
>>
>> +static inline int inet_pton(const char *str, union inet_addr *addr)
>> +{
>> +    const char *end;
>> +
>> +    if (!strchr(str, ':') &&
>> +        in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
>> +        if (!*end)
>> +            return 0;
>> +    }
>> +    if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +        if (!*end)

>     How about:

>          if (IS_ENABLED(CONFIG_IPV6) && !*end)

>> +            return 1;
>> +#else

    Sorry, managed to miss #else... #if could be avoided anyway, tho 
this could require an extra patch.

>> +        return -1;
>> +#endif
>> +    }
>> +    return -1;
>> +}
>>   #endif

WBR, Sergei

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Ben Hutchings]

On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  include/net/inet_addr.h |   20 ++++++++++++++++++++
>  net/core/netpoll.c      |   24 ++----------------------
>  2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
>  #include <linux/in.h>
>  #include <linux/in6.h>
>  #include <linux/socket.h>
> +#include <linux/inet.h>
>  #include <net/addrconf.h>
>  
>  union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
>  }
>  #endif
>  
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
> +	const char *end;
> +
> +	if (!strchr(str, ':') &&
> +	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> +		if (!*end)
> +			return 0;
> +	}
> +	if (in6_pton(str, -1, addr->sin6.sin6_addr.s6_addr, -1, &end) > 0) {
> +#if IS_ENABLED(CONFIG_IPV6)
> +		if (!*end)
> +			return 1;
> +#else
> +		return -1;
> +#endif
> +	}
> +	return -1;
> +}
[...]

The return values for this are awful.  I think the return value should
be 0 for success or -EINVAL on error.

The caller can then check addr->sa.sa_family if they want to know the
address family immediately.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Cong Wang]

On Thu, 2013-06-27 at 16:32 +0100, Ben Hutchings wrote:
> The return values for this are awful.  I think the return value should
> be 0 for success or -EINVAL on error.
> 
> The caller can then check addr->sa.sa_family if they want to know the
> address family immediately. 

Indeed, I will fix it.

Thanks!

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Stephen Hemminger]

On Thu, 27 Jun 2013 14:43:15 +0800
Cong Wang <amwang@redhat.com> wrote:

> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  include/net/inet_addr.h |   20 ++++++++++++++++++++
>  net/core/netpoll.c      |   24 ++----------------------
>  2 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
> index 66a16fe..1379287 100644
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -4,6 +4,7 @@
>  #include <linux/in.h>
>  #include <linux/in6.h>
>  #include <linux/socket.h>
> +#include <linux/inet.h>
>  #include <net/addrconf.h>
>  
>  union inet_addr {
> @@ -59,4 +60,23 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
>  }
>  #endif
>  
> +static inline int inet_pton(const char *str, union inet_addr *addr)
> +{
>

A couple of comments:
1. No reason for this to be inline
2. If function has same name as userspace it must have same arguments
   and return value. Either:
    a. rename it to kinet_pton or some other name
    b. make it work the same.

Re: [RFC Patch net-next 2/5] net: introduce generic inet_pton()

[Cong Wang]

On Thu, 2013-06-27 at 14:51 -0700, Stephen Hemminger wrote:
> >  
> > +static inline int inet_pton(const char *str, union inet_addr *addr)
> > +{
> >
> 
> A couple of comments:
> 1. No reason for this to be inline

Okay, I will move them into net/core/utils.c.

> 2. If function has same name as userspace it must have same arguments
>    and return value. Either:
>     a. rename it to kinet_pton or some other name
>     b. make it work the same.
> 

Makes sense too me, I will try your option b) first, if it is over-kill
I will fall back to option a).

Thanks!

[RFC Patch net-next 3/5] inetpeer: use generic union inet_addr

[Cong Wang]

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 include/net/inetpeer.h |   29 +++++----------
 net/ipv4/inetpeer.c    |   35 +++++++++++-------
 net/ipv4/tcp_metrics.c |   92 ++++++++++++++++++++----------------------------
 3 files changed, 68 insertions(+), 88 deletions(-)

diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 53f464d..7ec33fb 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -13,24 +13,13 @@
 #include <linux/spinlock.h>
 #include <linux/rtnetlink.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 #include <linux/atomic.h>
 
-struct inetpeer_addr_base {
-	union {
-		__be32			a4;
-		__be32			a6[4];
-	};
-};
-
-struct inetpeer_addr {
-	struct inetpeer_addr_base	addr;
-	__u16				family;
-};
-
 struct inet_peer {
 	/* group together avl_left,avl_right,v4daddr to speedup lookups */
 	struct inet_peer __rcu	*avl_left, *avl_right;
-	struct inetpeer_addr	daddr;
+	union inet_addr		daddr;
 	__u32			avl_height;
 
 	u32			metrics[RTAX_MAX];
@@ -133,17 +122,17 @@ static inline bool inet_metrics_new(const struct inet_peer *p)
 
 /* can be called with or without local BH being disabled */
 struct inet_peer *inet_getpeer(struct inet_peer_base *base,
-			       const struct inetpeer_addr *daddr,
+			       const union inet_addr *daddr,
 			       int create);
 
 static inline struct inet_peer *inet_getpeer_v4(struct inet_peer_base *base,
 						__be32 v4daddr,
 						int create)
 {
-	struct inetpeer_addr daddr;
+	union inet_addr daddr;
 
-	daddr.addr.a4 = v4daddr;
-	daddr.family = AF_INET;
+	daddr.sin.sin_addr.s_addr = v4daddr;
+	daddr.sa.sa_family = AF_INET;
 	return inet_getpeer(base, &daddr, create);
 }
 
@@ -151,10 +140,10 @@ static inline struct inet_peer *inet_getpeer_v6(struct inet_peer_base *base,
 						const struct in6_addr *v6daddr,
 						int create)
 {
-	struct inetpeer_addr daddr;
+	union inet_addr daddr;
 
-	*(struct in6_addr *)daddr.addr.a6 = *v6daddr;
-	daddr.family = AF_INET6;
+	daddr.sin6.sin6_addr = *v6daddr;
+	daddr.sa.sa_family = AF_INET6;
 	return inet_getpeer(base, &daddr, create);
 }
 
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 000e3d2..94a9ba7 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -197,17 +197,24 @@ void __init inet_initpeers(void)
 	INIT_DEFERRABLE_WORK(&gc_work, inetpeer_gc_worker);
 }
 
-static int addr_compare(const struct inetpeer_addr *a,
-			const struct inetpeer_addr *b)
+static int addr_compare(const union inet_addr *a,
+			const union inet_addr *b)
 {
-	int i, n = (a->family == AF_INET ? 1 : 4);
+	int i;
 
-	for (i = 0; i < n; i++) {
-		if (a->addr.a6[i] == b->addr.a6[i])
-			continue;
-		if ((__force u32)a->addr.a6[i] < (__force u32)b->addr.a6[i])
+	if (a->sa.sa_family == AF_INET) {
+		if (a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr)
+			return 0;
+		if ((__force u32)a->sin.sin_addr.s_addr < (__force u32)b->sin.sin_addr.s_addr)
 			return -1;
-		return 1;
+	} else {
+		for (i = 0; i < 4; i++) {
+			if (a->sin6.sin6_addr.s6_addr32[i] == b->sin6.sin6_addr.s6_addr32[i])
+				continue;
+			if ((__force u32)a->sin6.sin6_addr.s6_addr32[i] < (__force u32)b->sin6.sin6_addr.s6_addr32[i])
+				return -1;
+			return 1;
+		}
 	}
 
 	return 0;
@@ -248,7 +255,7 @@ static int addr_compare(const struct inetpeer_addr *a,
  * But every pointer we follow is guaranteed to be valid thanks to RCU.
  * We exit from this function if number of links exceeds PEER_MAXDEPTH
  */
-static struct inet_peer *lookup_rcu(const struct inetpeer_addr *daddr,
+static struct inet_peer *lookup_rcu(const union inet_addr *daddr,
 				    struct inet_peer_base *base)
 {
 	struct inet_peer *u = rcu_dereference(base->root);
@@ -457,7 +464,7 @@ static int inet_peer_gc(struct inet_peer_base *base,
 }
 
 struct inet_peer *inet_getpeer(struct inet_peer_base *base,
-			       const struct inetpeer_addr *daddr,
+			       const union inet_addr *daddr,
 			       int create)
 {
 	struct inet_peer __rcu **stack[PEER_MAXDEPTH], ***stackptr;
@@ -465,7 +472,7 @@ struct inet_peer *inet_getpeer(struct inet_peer_base *base,
 	unsigned int sequence;
 	int invalidated, gccnt = 0;
 
-	flush_check(base, daddr->family);
+	flush_check(base, daddr->sa.sa_family);
 
 	/* Attempt a lockless lookup first.
 	 * Because of a concurrent writer, we might not find an existing entry.
@@ -505,9 +512,9 @@ relookup:
 		atomic_set(&p->refcnt, 1);
 		atomic_set(&p->rid, 0);
 		atomic_set(&p->ip_id_count,
-				(daddr->family == AF_INET) ?
-					secure_ip_id(daddr->addr.a4) :
-					secure_ipv6_id(daddr->addr.a6));
+				(daddr->sa.sa_family == AF_INET) ?
+					secure_ip_id(daddr->sin.sin_addr.s_addr) :
+					secure_ipv6_id(daddr->sin6.sin6_addr.s6_addr32));
 		p->metrics[RTAX_LOCK-1] = INETPEER_METRICS_NEW;
 		p->rate_tokens = 0;
 		/* 60*HZ is arbitrary, but chosen enough high so that the first
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index f6a005c..10b3796 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -31,7 +31,7 @@ struct tcp_fastopen_metrics {
 
 struct tcp_metrics_block {
 	struct tcp_metrics_block __rcu	*tcpm_next;
-	struct inetpeer_addr		tcpm_addr;
+	union inet_addr			tcpm_addr;
 	unsigned long			tcpm_stamp;
 	u32				tcpm_ts;
 	u32				tcpm_ts_stamp;
@@ -74,22 +74,6 @@ static void tcp_metric_set_msecs(struct tcp_metrics_block *tm,
 	tm->tcpm_vals[idx] = jiffies_to_msecs(val);
 }
 
-static bool addr_same(const struct inetpeer_addr *a,
-		      const struct inetpeer_addr *b)
-{
-	const struct in6_addr *a6, *b6;
-
-	if (a->family != b->family)
-		return false;
-	if (a->family == AF_INET)
-		return a->addr.a4 == b->addr.a4;
-
-	a6 = (const struct in6_addr *) &a->addr.a6[0];
-	b6 = (const struct in6_addr *) &b->addr.a6[0];
-
-	return ipv6_addr_equal(a6, b6);
-}
-
 struct tcpm_hash_bucket {
 	struct tcp_metrics_block __rcu	*chain;
 };
@@ -131,7 +115,7 @@ static void tcpm_suck_dst(struct tcp_metrics_block *tm, struct dst_entry *dst,
 }
 
 static struct tcp_metrics_block *tcpm_new(struct dst_entry *dst,
-					  struct inetpeer_addr *addr,
+					  union inet_addr *addr,
 					  unsigned int hash,
 					  bool reclaim)
 {
@@ -189,7 +173,7 @@ static struct tcp_metrics_block *tcp_get_encode(struct tcp_metrics_block *tm, in
 	return NULL;
 }
 
-static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *addr,
+static struct tcp_metrics_block *__tcp_get_metrics(const union inet_addr *addr,
 						   struct net *net, unsigned int hash)
 {
 	struct tcp_metrics_block *tm;
@@ -197,7 +181,7 @@ static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *a
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, addr))
+		if (inet_addr_equal(&tm->tcpm_addr, addr))
 			break;
 		depth++;
 	}
@@ -208,18 +192,18 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 						       struct dst_entry *dst)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	union inet_addr addr;
 	unsigned int hash;
 	struct net *net;
 
-	addr.family = req->rsk_ops->family;
-	switch (addr.family) {
+	addr.sa.sa_family = req->rsk_ops->family;
+	switch (addr.sa.sa_family) {
 	case AF_INET:
-		addr.addr.a4 = inet_rsk(req)->rmt_addr;
-		hash = (__force unsigned int) addr.addr.a4;
+		addr.sin.sin_addr.s_addr = inet_rsk(req)->rmt_addr;
+		hash = (__force unsigned int) addr.sin.sin_addr.s_addr;
 		break;
 	case AF_INET6:
-		*(struct in6_addr *)addr.addr.a6 = inet6_rsk(req)->rmt_addr;
+		addr.sin6.sin6_addr = inet6_rsk(req)->rmt_addr;
 		hash = ipv6_addr_hash(&inet6_rsk(req)->rmt_addr);
 		break;
 	default:
@@ -231,7 +215,7 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr))
+		if (inet_addr_equal(&tm->tcpm_addr, &addr))
 			break;
 	}
 	tcpm_check_stamp(tm, dst);
@@ -242,19 +226,19 @@ static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock
 {
 	struct inet6_timewait_sock *tw6;
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	union inet_addr addr;
 	unsigned int hash;
 	struct net *net;
 
-	addr.family = tw->tw_family;
-	switch (addr.family) {
+	addr.sa.sa_family = tw->tw_family;
+	switch (addr.sa.sa_family) {
 	case AF_INET:
-		addr.addr.a4 = tw->tw_daddr;
-		hash = (__force unsigned int) addr.addr.a4;
+		addr.sin.sin_addr.s_addr = tw->tw_daddr;
+		hash = (__force unsigned int) addr.sin.sin_addr.s_addr;
 		break;
 	case AF_INET6:
 		tw6 = inet6_twsk((struct sock *)tw);
-		*(struct in6_addr *)addr.addr.a6 = tw6->tw_v6_daddr;
+		addr.sin6.sin6_addr = tw6->tw_v6_daddr;
 		hash = ipv6_addr_hash(&tw6->tw_v6_daddr);
 		break;
 	default:
@@ -266,7 +250,7 @@ static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr))
+		if (inet_addr_equal(&tm->tcpm_addr, &addr))
 			break;
 	}
 	return tm;
@@ -277,19 +261,19 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
 						 bool create)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	union inet_addr addr;
 	unsigned int hash;
 	struct net *net;
 	bool reclaim;
 
-	addr.family = sk->sk_family;
-	switch (addr.family) {
+	addr.sa.sa_family = sk->sk_family;
+	switch (addr.sa.sa_family) {
 	case AF_INET:
-		addr.addr.a4 = inet_sk(sk)->inet_daddr;
-		hash = (__force unsigned int) addr.addr.a4;
+		addr.sin.sin_addr.s_addr = inet_sk(sk)->inet_daddr;
+		hash = (__force unsigned int) addr.sin.sin_addr.s_addr;
 		break;
 	case AF_INET6:
-		*(struct in6_addr *)addr.addr.a6 = inet6_sk(sk)->daddr;
+		addr.sin6.sin6_addr = inet6_sk(sk)->daddr;
 		hash = ipv6_addr_hash(&inet6_sk(sk)->daddr);
 		break;
 	default:
@@ -722,15 +706,15 @@ static int tcp_metrics_fill_info(struct sk_buff *msg,
 	struct nlattr *nest;
 	int i;
 
-	switch (tm->tcpm_addr.family) {
+	switch (tm->tcpm_addr.sa.sa_family) {
 	case AF_INET:
 		if (nla_put_be32(msg, TCP_METRICS_ATTR_ADDR_IPV4,
-				tm->tcpm_addr.addr.a4) < 0)
+				tm->tcpm_addr.sin.sin_addr.s_addr) < 0)
 			goto nla_put_failure;
 		break;
 	case AF_INET6:
 		if (nla_put(msg, TCP_METRICS_ATTR_ADDR_IPV6, 16,
-			    tm->tcpm_addr.addr.a6) < 0)
+			    tm->tcpm_addr.sin6.sin6_addr.s6_addr32) < 0)
 			goto nla_put_failure;
 		break;
 	default:
@@ -853,25 +837,25 @@ done:
 	return skb->len;
 }
 
-static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
+static int parse_nl_addr(struct genl_info *info, union inet_addr *addr,
 			 unsigned int *hash, int optional)
 {
 	struct nlattr *a;
 
 	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV4];
 	if (a) {
-		addr->family = AF_INET;
-		addr->addr.a4 = nla_get_be32(a);
-		*hash = (__force unsigned int) addr->addr.a4;
+		addr->sa.sa_family = AF_INET;
+		addr->sin.sin_addr.s_addr = nla_get_be32(a);
+		*hash = (__force unsigned int) addr->sin.sin_addr.s_addr;
 		return 0;
 	}
 	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV6];
 	if (a) {
 		if (nla_len(a) != sizeof(struct in6_addr))
 			return -EINVAL;
-		addr->family = AF_INET6;
-		memcpy(addr->addr.a6, nla_data(a), sizeof(addr->addr.a6));
-		*hash = ipv6_addr_hash((struct in6_addr *) addr->addr.a6);
+		addr->sa.sa_family = AF_INET6;
+		memcpy(&addr->sin6.sin6_addr, nla_data(a), sizeof(addr->sin6.sin6_addr));
+		*hash = ipv6_addr_hash(&addr->sin6.sin6_addr);
 		return 0;
 	}
 	return optional ? 1 : -EAFNOSUPPORT;
@@ -880,7 +864,7 @@ static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
 static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	union inet_addr addr;
 	unsigned int hash;
 	struct sk_buff *msg;
 	struct net *net = genl_info_net(info);
@@ -905,7 +889,7 @@ static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr)) {
+		if (inet_addr_equal(&tm->tcpm_addr, &addr)) {
 			ret = tcp_metrics_fill_info(msg, tm);
 			break;
 		}
@@ -960,7 +944,7 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	struct tcpm_hash_bucket *hb;
 	struct tcp_metrics_block *tm;
 	struct tcp_metrics_block __rcu **pp;
-	struct inetpeer_addr addr;
+	union inet_addr addr;
 	unsigned int hash;
 	struct net *net = genl_info_net(info);
 	int ret;
@@ -977,7 +961,7 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&tcp_metrics_lock);
 	for (tm = deref_locked_genl(*pp); tm;
 	     pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) {
-		if (addr_same(&tm->tcpm_addr, &addr)) {
+		if (inet_addr_equal(&tm->tcpm_addr, &addr)) {
 			*pp = tm->tcpm_next;
 			break;
 		}
-- 
1.7.7.6

Re: [RFC Patch net-next 3/5] inetpeer: use generic union inet_addr

[Eric Dumazet]

On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  include/net/inetpeer.h |   29 +++++----------
>  net/ipv4/inetpeer.c    |   35 +++++++++++-------
>  net/ipv4/tcp_metrics.c |   92 ++++++++++++++++++++----------------------------
>  3 files changed, 68 insertions(+), 88 deletions(-)
> 
> diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
> index 53f464d..7ec33fb 100644
> --- a/include/net/inetpeer.h
> +++ b/include/net/inetpeer.h
> @@ -13,24 +13,13 @@
>  #include <linux/spinlock.h>
>  #include <linux/rtnetlink.h>
>  #include <net/ipv6.h>
> +#include <net/inet_addr.h>
>  #include <linux/atomic.h>
>  
> -struct inetpeer_addr_base {
> -	union {
> -		__be32			a4;
> -		__be32			a6[4];
> -	};
> -};
> -
> -struct inetpeer_addr {
> -	struct inetpeer_addr_base	addr;
> -	__u16				family;
> -};
> -
>  struct inet_peer {
>  	/* group together avl_left,avl_right,v4daddr to speedup lookups */
>  	struct inet_peer __rcu	*avl_left, *avl_right;
> -	struct inetpeer_addr	daddr;
> +	union inet_addr		daddr;

Please compare sizeof(struct inetpeer_addr) and sizeof(union inet_addr)

If I am not mistaken, its 20 bytes instead of 28

Yes, sockaddr_in6 is a bit bloated...

Re: [RFC Patch net-next 3/5] inetpeer: use generic union inet_addr

[Cong Wang]

On Thu, 2013-06-27 at 01:01 -0700, Eric Dumazet wrote:
> On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
> >  struct inet_peer {
> >  	/* group together avl_left,avl_right,v4daddr to speedup lookups */
> >  	struct inet_peer __rcu	*avl_left, *avl_right;
> > -	struct inetpeer_addr	daddr;
> > +	union inet_addr		daddr;
> 
> Please compare sizeof(struct inetpeer_addr) and sizeof(union inet_addr)
> 
> If I am not mistaken, its 20 bytes instead of 28
> 
> Yes, sockaddr_in6 is a bit bloated...
> 

You are right.

Are you saying that I should rearrange the fields of struct inet_peer in
case of cacheline miss?

Thanks!

[RFC Patch net-next 4/5] sunrpc: use generic union inet_addr

[Cong Wang]

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 include/linux/sunrpc/addr.h |  119 +++----------------------------------------
 include/net/inet_addr.h     |   56 ++++++++++++++++++++
 2 files changed, 64 insertions(+), 111 deletions(-)

diff --git a/include/linux/sunrpc/addr.h b/include/linux/sunrpc/addr.h
index 07d8e53..4c3602e 100644
--- a/include/linux/sunrpc/addr.h
+++ b/include/linux/sunrpc/addr.h
@@ -11,6 +11,7 @@
 #include <linux/in.h>
 #include <linux/in6.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 
 size_t		rpc_ntop(const struct sockaddr *, char *, const size_t);
 size_t		rpc_pton(struct net *, const char *, const size_t,
@@ -21,137 +22,33 @@ size_t		rpc_uaddr2sockaddr(struct net *, const char *, const size_t,
 
 static inline unsigned short rpc_get_port(const struct sockaddr *sap)
 {
-	switch (sap->sa_family) {
-	case AF_INET:
-		return ntohs(((struct sockaddr_in *)sap)->sin_port);
-	case AF_INET6:
-		return ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
-	}
-	return 0;
+	return inet_addr_get_port((const union inet_addr *)sap);
 }
 
 static inline void rpc_set_port(struct sockaddr *sap,
 				const unsigned short port)
 {
-	switch (sap->sa_family) {
-	case AF_INET:
-		((struct sockaddr_in *)sap)->sin_port = htons(port);
-		break;
-	case AF_INET6:
-		((struct sockaddr_in6 *)sap)->sin6_port = htons(port);
-		break;
-	}
+	inet_addr_set_port((union inet_addr *)sap, port);
 }
 
 #define IPV6_SCOPE_DELIMITER		'%'
 #define IPV6_SCOPE_ID_LEN		sizeof("%nnnnnnnnnn")
 
-static inline bool __rpc_cmp_addr4(const struct sockaddr *sap1,
-				   const struct sockaddr *sap2)
-{
-	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sap1;
-	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sap2;
-
-	return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
-}
-
-static inline bool __rpc_copy_addr4(struct sockaddr *dst,
-				    const struct sockaddr *src)
-{
-	const struct sockaddr_in *ssin = (struct sockaddr_in *) src;
-	struct sockaddr_in *dsin = (struct sockaddr_in *) dst;
-
-	dsin->sin_family = ssin->sin_family;
-	dsin->sin_addr.s_addr = ssin->sin_addr.s_addr;
-	return true;
-}
-
-#if IS_ENABLED(CONFIG_IPV6)
-static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
-				   const struct sockaddr *sap2)
-{
-	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
-	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
-
-	if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
-		return false;
-	else if (ipv6_addr_type(&sin1->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		return sin1->sin6_scope_id == sin2->sin6_scope_id;
-
-	return true;
-}
-
-static inline bool __rpc_copy_addr6(struct sockaddr *dst,
-				    const struct sockaddr *src)
-{
-	const struct sockaddr_in6 *ssin6 = (const struct sockaddr_in6 *) src;
-	struct sockaddr_in6 *dsin6 = (struct sockaddr_in6 *) dst;
-
-	dsin6->sin6_family = ssin6->sin6_family;
-	dsin6->sin6_addr = ssin6->sin6_addr;
-	dsin6->sin6_scope_id = ssin6->sin6_scope_id;
-	return true;
-}
-#else	/* !(IS_ENABLED(CONFIG_IPV6) */
-static inline bool __rpc_cmp_addr6(const struct sockaddr *sap1,
-				   const struct sockaddr *sap2)
-{
-	return false;
-}
-
-static inline bool __rpc_copy_addr6(struct sockaddr *dst,
-				    const struct sockaddr *src)
-{
-	return false;
-}
-#endif	/* !(IS_ENABLED(CONFIG_IPV6) */
-
-/**
- * rpc_cmp_addr - compare the address portion of two sockaddrs.
- * @sap1: first sockaddr
- * @sap2: second sockaddr
- *
- * Just compares the family and address portion. Ignores port, but
- * compares the scope if it's a link-local address.
- *
- * Returns true if the addrs are equal, false if they aren't.
- */
 static inline bool rpc_cmp_addr(const struct sockaddr *sap1,
 				const struct sockaddr *sap2)
 {
-	if (sap1->sa_family == sap2->sa_family) {
-		switch (sap1->sa_family) {
-		case AF_INET:
-			return __rpc_cmp_addr4(sap1, sap2);
-		case AF_INET6:
-			return __rpc_cmp_addr6(sap1, sap2);
-		}
-	}
-	return false;
+	return inet_addr_equal((const union inet_addr *)sap1,
+			       (const union inet_addr *)sap2);
 }
 
-/**
- * rpc_copy_addr - copy the address portion of one sockaddr to another
- * @dst: destination sockaddr
- * @src: source sockaddr
- *
- * Just copies the address portion and family. Ignores port, scope, etc.
- * Caller is responsible for making certain that dst is large enough to hold
- * the address in src. Returns true if address family is supported. Returns
- * false otherwise.
- */
 static inline bool rpc_copy_addr(struct sockaddr *dst,
 				 const struct sockaddr *src)
 {
-	switch (src->sa_family) {
-	case AF_INET:
-		return __rpc_copy_addr4(dst, src);
-	case AF_INET6:
-		return __rpc_copy_addr6(dst, src);
-	}
-	return false;
+	return inet_addr_copy((union inet_addr *)dst,
+			      (const union inet_addr *)src);
 }
 
+
 /**
  * rpc_get_scope_id - return scopeid for a given sockaddr
  * @sa: sockaddr to get scopeid from
diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
index 1379287..683e904 100644
--- a/include/net/inet_addr.h
+++ b/include/net/inet_addr.h
@@ -60,6 +60,62 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
 }
 #endif
 
+/**
+ * inet_addr_copy - copy the address portion of one sockaddr to another
+ * @dst: destination sockaddr
+ * @src: source sockaddr
+ *
+ * Just copies the address portion and family. Ignores port, scope, etc.
+ * Caller is responsible for making certain that dst is large enough to hold
+ * the address in src. Returns true if address family is supported. Returns
+ * false otherwise.
+ */
+static inline bool inet_addr_copy(union inet_addr *dst,
+				  const union inet_addr *src)
+{
+	dst->sa.sa_family = src->sa.sa_family;
+
+	switch (src->sa.sa_family) {
+	case AF_INET:
+		dst->sin.sin_addr.s_addr = src->sin.sin_addr.s_addr;
+		return true;
+#if IS_ENABLED(CONFIG_IPV6)
+	case AF_INET6:
+		dst->sin6.sin6_addr = src->sin6.sin6_addr;
+		dst->sin6.sin6_scope_id = src->sin6.sin6_scope_id;
+		return true;
+#endif
+	}
+
+	return false;
+}
+
+static inline
+unsigned short inet_addr_get_port(const union inet_addr *sap)
+{
+	switch (sap->sa.sa_family) {
+	case AF_INET:
+		return ntohs(sap->sin.sin_port);
+	case AF_INET6:
+		return ntohs(sap->sin6.sin6_port);
+	}
+	return 0;
+}
+
+static inline
+void inet_addr_set_port(union inet_addr *sap,
+			const unsigned short port)
+{
+	switch (sap->sa.sa_family) {
+	case AF_INET:
+		sap->sin.sin_port = htons(port);
+		break;
+	case AF_INET6:
+		sap->sin6.sin6_port = htons(port);
+		break;
+	}
+}
+
 static inline int inet_pton(const char *str, union inet_addr *addr)
 {
 	const char *end;
-- 
1.7.7.6

Re: [RFC Patch net-next 4/5] sunrpc: use generic union inet_addr

[Ben Hutchings]

On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
[...]
> -/**
> - * rpc_cmp_addr - compare the address portion of two sockaddrs.
> - * @sap1: first sockaddr
> - * @sap2: second sockaddr
> - *
> - * Just compares the family and address portion. Ignores port, but
> - * compares the scope if it's a link-local address.

You're removing the scope comparison.

Ben.

> - * Returns true if the addrs are equal, false if they aren't.
> - */
>  static inline bool rpc_cmp_addr(const struct sockaddr *sap1,
>  				const struct sockaddr *sap2)
>  {
> -	if (sap1->sa_family == sap2->sa_family) {
> -		switch (sap1->sa_family) {
> -		case AF_INET:
> -			return __rpc_cmp_addr4(sap1, sap2);
> -		case AF_INET6:
> -			return __rpc_cmp_addr6(sap1, sap2);
> -		}
> -	}
> -	return false;
> +	return inet_addr_equal((const union inet_addr *)sap1,
> +			       (const union inet_addr *)sap2);
>  }
[...]

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC Patch net-next 4/5] sunrpc: use generic union inet_addr

[Cong Wang]

On Thu, 2013-06-27 at 16:36 +0100, Ben Hutchings wrote:
> On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
> [...]
> > -/**
> > - * rpc_cmp_addr - compare the address portion of two sockaddrs.
> > - * @sap1: first sockaddr
> > - * @sap2: second sockaddr
> > - *
> > - * Just compares the family and address portion. Ignores port, but
> > - * compares the scope if it's a link-local address.
> 
> You're removing the scope comparison.

Actually I added the following code in patch 5/5:

+               if (!ipv6_addr_equal(&a->sin6.sin6_addr,
&b->sin6.sin6_addr))
+                       return false;
+               else if
(__ipv6_addr_needs_scope_id(__ipv6_addr_type(&a->sin6.sin6_addr)))
+                       return a->sin6.sin6_scope_id ==
b->sin6.sin6_scope_id;


but I should move it to this patch, otherwise it is hard for review.

Thanks!

Re: [RFC Patch net-next 4/5] sunrpc: use generic union inet_addr

[Ben Hutchings]

On Thu, 2013-06-27 at 14:43 +0800, Cong Wang wrote:
[...]
> --- a/include/net/inet_addr.h
> +++ b/include/net/inet_addr.h
> @@ -60,6 +60,62 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
>  }
>  #endif
>  
> +/**
> + * inet_addr_copy - copy the address portion of one sockaddr to another
> + * @dst: destination sockaddr
> + * @src: source sockaddr
> + *
> + * Just copies the address portion and family. Ignores port, scope, etc.
> + * Caller is responsible for making certain that dst is large enough to hold
> + * the address in src. Returns true if address family is supported. Returns
> + * false otherwise.
> + */
> +static inline bool inet_addr_copy(union inet_addr *dst,
> +				  const union inet_addr *src)
> +{
> +	dst->sa.sa_family = src->sa.sa_family;
> +
> +	switch (src->sa.sa_family) {
> +	case AF_INET:
> +		dst->sin.sin_addr.s_addr = src->sin.sin_addr.s_addr;
> +		return true;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case AF_INET6:
> +		dst->sin6.sin6_addr = src->sin6.sin6_addr;
> +		dst->sin6.sin6_scope_id = src->sin6.sin6_scope_id;
> +		return true;
> +#endif

Most of the other functions work on IPv6 addresses without CONFIG_IPV6;
why should this be different?

Ben.

> +	}
> +
> +	return false;
> +}
[...]

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [RFC Patch net-next 4/5] sunrpc: use generic union inet_addr

[Cong Wang]

On Thu, 2013-06-27 at 16:37 +0100, Ben Hutchings wrote:
> Most of the other functions work on IPv6 addresses without
> CONFIG_IPV6;
> why should this be different? 

Save few cycles for !CONFIG_IPV6? :-/

[RFC Patch net-next 5/5] nfs,cifs: abstract generic inet_addr_equal_strict()

[Cong Wang]

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 fs/cifs/connect.c          |   38 ++++-------------
 fs/dlm/lowcomms.c          |   24 ++---------
 fs/nfs/client.c            |   94 ++-----------------------------------------
 fs/nfs/nfs4filelayoutdev.c |   37 ++---------------
 fs/nfs/super.c             |   31 +-------------
 include/net/inet_addr.h    |   32 +++++++++++++-
 6 files changed, 55 insertions(+), 201 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index e3bc39b..ca6bcf82 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -40,6 +40,7 @@
 #include <linux/module.h>
 #include <keys/user-type.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 #include <linux/parser.h>
 
 #include "cifspdu.h"
@@ -1886,17 +1887,10 @@ srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
 {
 	switch (srcaddr->sa_family) {
 	case AF_UNSPEC:
-		return (rhs->sa_family == AF_UNSPEC);
-	case AF_INET: {
-		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
-		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
-		return (saddr4->sin_addr.s_addr == vaddr4->sin_addr.s_addr);
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
-		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs;
-		return ipv6_addr_equal(&saddr6->sin6_addr, &vaddr6->sin6_addr);
-	}
+	case AF_INET:
+	case AF_INET6:
+		return inet_addr_equal((union inet_addr *)srcaddr,
+				       (union inet_addr *)rhs);
 	default:
 		WARN_ON(1);
 		return false; /* don't expect to be here */
@@ -1943,24 +1937,10 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
 	      struct sockaddr *srcaddr)
 {
 	switch (addr->sa_family) {
-	case AF_INET: {
-		struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
-		struct sockaddr_in *srv_addr4 =
-					(struct sockaddr_in *)&server->dstaddr;
-
-		if (addr4->sin_addr.s_addr != srv_addr4->sin_addr.s_addr)
-			return false;
-		break;
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
-		struct sockaddr_in6 *srv_addr6 =
-					(struct sockaddr_in6 *)&server->dstaddr;
-
-		if (!ipv6_addr_equal(&addr6->sin6_addr,
-				     &srv_addr6->sin6_addr))
-			return false;
-		if (addr6->sin6_scope_id != srv_addr6->sin6_scope_id)
+	case AF_INET:
+	case AF_INET6:
+		if (!inet_addr_equal((union inet_addr *)addr,
+				     (union inet_addr *)&server->dstaddr))
 			return false;
 		break;
 	}
diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index d0ccd2f..c27e36e 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -55,6 +55,7 @@
 #include <linux/sctp.h>
 #include <net/sctp/sctp.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 
 #include "dlm_internal.h"
 #include "lowcomms.h"
@@ -285,28 +286,13 @@ static struct dlm_node_addr *find_node_addr(int nodeid)
 static int addr_compare(struct sockaddr_storage *x, struct sockaddr_storage *y)
 {
 	switch (x->ss_family) {
-	case AF_INET: {
-		struct sockaddr_in *sinx = (struct sockaddr_in *)x;
-		struct sockaddr_in *siny = (struct sockaddr_in *)y;
-		if (sinx->sin_addr.s_addr != siny->sin_addr.s_addr)
-			return 0;
-		if (sinx->sin_port != siny->sin_port)
-			return 0;
-		break;
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *sinx = (struct sockaddr_in6 *)x;
-		struct sockaddr_in6 *siny = (struct sockaddr_in6 *)y;
-		if (!ipv6_addr_equal(&sinx->sin6_addr, &siny->sin6_addr))
-			return 0;
-		if (sinx->sin6_port != siny->sin6_port)
-			return 0;
-		break;
-	}
+	case AF_INET:
+	case AF_INET6:
+		return inet_addr_equal_strict((union inet_addr *)x,
+					      (union inet_addr *)y);
 	default:
 		return 0;
 	}
-	return 1;
 }
 
 static int nodeid_to_addr(int nodeid, struct sockaddr_storage *sas_out,
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index c513b0c..69a7561 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -38,6 +38,7 @@
 #include <linux/slab.h>
 #include <linux/idr.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 #include <linux/nfs_xdr.h>
 #include <linux/sunrpc/bc_xprt.h>
 #include <linux/nsproxy.h>
@@ -283,75 +284,6 @@ void nfs_put_client(struct nfs_client *clp)
 }
 EXPORT_SYMBOL_GPL(nfs_put_client);
 
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-/*
- * Test if two ip6 socket addresses refer to the same socket by
- * comparing relevant fields. The padding bytes specifically, are not
- * compared. sin6_flowinfo is not compared because it only affects QoS
- * and sin6_scope_id is only compared if the address is "link local"
- * because "link local" addresses need only be unique to a specific
- * link. Conversely, ordinary unicast addresses might have different
- * sin6_scope_id.
- *
- * The caller should ensure both socket addresses are AF_INET6.
- */
-static int nfs_sockaddr_match_ipaddr6(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
-	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
-
-	if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
-		return 0;
-	else if (ipv6_addr_type(&sin1->sin6_addr) & IPV6_ADDR_LINKLOCAL)
-		return sin1->sin6_scope_id == sin2->sin6_scope_id;
-
-	return 1;
-}
-#else	/* !defined(CONFIG_IPV6) && !defined(CONFIG_IPV6_MODULE) */
-static int nfs_sockaddr_match_ipaddr6(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	return 0;
-}
-#endif
-
-/*
- * Test if two ip4 socket addresses refer to the same socket, by
- * comparing relevant fields. The padding bytes specifically, are
- * not compared.
- *
- * The caller should ensure both socket addresses are AF_INET.
- */
-static int nfs_sockaddr_match_ipaddr4(const struct sockaddr *sa1,
-				      const struct sockaddr *sa2)
-{
-	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
-	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
-
-	return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
-}
-
-static int nfs_sockaddr_cmp_ip6(const struct sockaddr *sa1,
-				const struct sockaddr *sa2)
-{
-	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sa1;
-	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sa2;
-
-	return nfs_sockaddr_match_ipaddr6(sa1, sa2) &&
-		(sin1->sin6_port == sin2->sin6_port);
-}
-
-static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
-				const struct sockaddr *sa2)
-{
-	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sa1;
-	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sa2;
-
-	return nfs_sockaddr_match_ipaddr4(sa1, sa2) &&
-		(sin1->sin_port == sin2->sin_port);
-}
-
 #if defined(CONFIG_NFS_V4_1)
 /*
  * Test if two socket addresses represent the same actual socket,
@@ -360,16 +292,8 @@ static int nfs_sockaddr_cmp_ip4(const struct sockaddr *sa1,
 int nfs_sockaddr_match_ipaddr(const struct sockaddr *sa1,
 			      const struct sockaddr *sa2)
 {
-	if (sa1->sa_family != sa2->sa_family)
-		return 0;
-
-	switch (sa1->sa_family) {
-	case AF_INET:
-		return nfs_sockaddr_match_ipaddr4(sa1, sa2);
-	case AF_INET6:
-		return nfs_sockaddr_match_ipaddr6(sa1, sa2);
-	}
-	return 0;
+	return inet_addr_equal((const union inet_addr *)sa1,
+			       (const union inet_addr *)sa2);
 }
 EXPORT_SYMBOL_GPL(nfs_sockaddr_match_ipaddr);
 #endif /* CONFIG_NFS_V4_1 */
@@ -381,16 +305,8 @@ EXPORT_SYMBOL_GPL(nfs_sockaddr_match_ipaddr);
 static int nfs_sockaddr_cmp(const struct sockaddr *sa1,
 			    const struct sockaddr *sa2)
 {
-	if (sa1->sa_family != sa2->sa_family)
-		return 0;
-
-	switch (sa1->sa_family) {
-	case AF_INET:
-		return nfs_sockaddr_cmp_ip4(sa1, sa2);
-	case AF_INET6:
-		return nfs_sockaddr_cmp_ip6(sa1, sa2);
-	}
-	return 0;
+	return inet_addr_equal_strict((union inet_addr *)sa1,
+				      (union inet_addr *)sa2);
 }
 
 /*
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index 661a0f6..6509160 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -32,6 +32,7 @@
 #include <linux/vmalloc.h>
 #include <linux/module.h>
 #include <linux/sunrpc/addr.h>
+#include <net/inet_addr.h>
 
 #include "internal.h"
 #include "nfs4session.h"
@@ -74,44 +75,14 @@ print_ds(struct nfs4_pnfs_ds *ds)
 static bool
 same_sockaddr(struct sockaddr *addr1, struct sockaddr *addr2)
 {
-	struct sockaddr_in *a, *b;
-	struct sockaddr_in6 *a6, *b6;
-
-	if (addr1->sa_family != addr2->sa_family)
-		return false;
-
-	switch (addr1->sa_family) {
-	case AF_INET:
-		a = (struct sockaddr_in *)addr1;
-		b = (struct sockaddr_in *)addr2;
-
-		if (a->sin_addr.s_addr == b->sin_addr.s_addr &&
-		    a->sin_port == b->sin_port)
-			return true;
-		break;
-
-	case AF_INET6:
-		a6 = (struct sockaddr_in6 *)addr1;
-		b6 = (struct sockaddr_in6 *)addr2;
-
-		/* LINKLOCAL addresses must have matching scope_id */
-		if (ipv6_addr_scope(&a6->sin6_addr) ==
-		    IPV6_ADDR_SCOPE_LINKLOCAL &&
-		    a6->sin6_scope_id != b6->sin6_scope_id)
-			return false;
-
-		if (ipv6_addr_equal(&a6->sin6_addr, &b6->sin6_addr) &&
-		    a6->sin6_port == b6->sin6_port)
-			return true;
-		break;
-
-	default:
+	if (addr1->sa_family != AF_INET && addr1->sa_family != AF_INET6) {
 		dprintk("%s: unhandled address family: %u\n",
 			__func__, addr1->sa_family);
 		return false;
 	}
 
-	return false;
+	return inet_addr_equal_strict((union inet_addr *)addr1,
+				      (union inet_addr *)addr2);
 }
 
 static bool
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 2d7525f..eb58d8a 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -49,6 +49,7 @@
 #include <linux/in6.h>
 #include <linux/slab.h>
 #include <net/ipv6.h>
+#include <net/inet_addr.h>
 #include <linux/netdevice.h>
 #include <linux/nfs_xdr.h>
 #include <linux/magic.h>
@@ -2318,34 +2319,8 @@ static int nfs_compare_super_address(struct nfs_server *server1,
 
 	sap1 = (struct sockaddr *)&server1->nfs_client->cl_addr;
 	sap2 = (struct sockaddr *)&server2->nfs_client->cl_addr;
-
-	if (sap1->sa_family != sap2->sa_family)
-		return 0;
-
-	switch (sap1->sa_family) {
-	case AF_INET: {
-		struct sockaddr_in *sin1 = (struct sockaddr_in *)sap1;
-		struct sockaddr_in *sin2 = (struct sockaddr_in *)sap2;
-		if (sin1->sin_addr.s_addr != sin2->sin_addr.s_addr)
-			return 0;
-		if (sin1->sin_port != sin2->sin_port)
-			return 0;
-		break;
-	}
-	case AF_INET6: {
-		struct sockaddr_in6 *sin1 = (struct sockaddr_in6 *)sap1;
-		struct sockaddr_in6 *sin2 = (struct sockaddr_in6 *)sap2;
-		if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
-			return 0;
-		if (sin1->sin6_port != sin2->sin6_port)
-			return 0;
-		break;
-	}
-	default:
-		return 0;
-	}
-
-	return 1;
+	return inet_addr_equal_strict((union inet_addr *)sap1,
+				      (union inet_addr *)sap2);
 }
 
 static int nfs_compare_super(struct super_block *sb, void *data)
diff --git a/include/net/inet_addr.h b/include/net/inet_addr.h
index 683e904..9889c27 100644
--- a/include/net/inet_addr.h
+++ b/include/net/inet_addr.h
@@ -19,9 +19,15 @@ bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
 {
 	if (a->sa.sa_family != b->sa.sa_family)
 		return false;
-	if (a->sa.sa_family == AF_INET6)
-		return ipv6_addr_equal(&a->sin6.sin6_addr, &b->sin6.sin6_addr);
-	else
+	if (a->sa.sa_family == AF_UNSPEC)
+		return true;
+	else if (a->sa.sa_family == AF_INET6) {
+		if (!ipv6_addr_equal(&a->sin6.sin6_addr, &b->sin6.sin6_addr))
+			return false;
+		else if (__ipv6_addr_needs_scope_id(__ipv6_addr_type(&a->sin6.sin6_addr)))
+			return a->sin6.sin6_scope_id == b->sin6.sin6_scope_id;
+		return false;
+	} else
 		return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
 }
 
@@ -46,6 +52,8 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
 static inline
 bool inet_addr_equal(const union inet_addr *a, const union inet_addr *b)
 {
+	if (a->sa.sa_family == AF_UNSPEC)
+		return a->sa.sa_family == b->sa.sa_family;
 	return a->sin.sin_addr.s_addr == b->sin.sin_addr.s_addr;
 }
 
@@ -60,6 +68,24 @@ static inline bool inet_addr_multicast(const union inet_addr *ipa)
 }
 #endif
 
+static inline
+bool inet_addr_equal_strict(const union inet_addr *a, const union inet_addr *b)
+{
+	if (inet_addr_equal(a, b)) {
+		switch (a->sa.sa_family) {
+#if IS_ENABLED(CONFIG_IPV6)
+		case AF_INET6:
+			return a->sin6.sin6_port == b->sin6.sin6_port;
+#endif
+		case AF_INET:
+			return a->sin.sin_port == b->sin.sin_port;
+		default:
+			return true;
+		}
+	} else
+		return false;
+}
+
 /**
  * inet_addr_copy - copy the address portion of one sockaddr to another
  * @dst: destination sockaddr
-- 
1.7.7.6