NFSD IPv6 patches

The existing logic in ip_map_parse() can not currently parse shorthanded IPv6 addresses (anything with a double colon), nor can it parse an IPv6 presentation address with a scope ID. An IPv6-enabled mountd can pass down both.

Signed-off-by Chuck Lever <[email protected]>

---

 net/sunrpc/svcauth_unix.c |   47 +++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 21 deletions(-)

net/sunrpc/svcauth_unix.c +26 -21

index d8c0411..97f0e9e 100644
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #define RPCDBG_FACILITY	RPCDBG_AUTH
 
+#include <linux/sunrpc/clnt.h>
 
 /*
  * AUTHUNIX and AUTHNULL credentials are both handled here.

@@ -187,10 +188,13 @@ static int ip_map_parse(struct cache_detail *cd,
 	 * for scratch: */
 	char *buf = mesg;
 	int len;
-	int b1, b2, b3, b4, b5, b6, b7, b8;
-	char c;
 	char class[8];
-	struct in6_addr addr;
+	union {
+		struct sockaddr		sa;
+		struct sockaddr_in	s4;
+		struct sockaddr_in6	s6;
+	} address;
+	struct sockaddr_in6 sin6;
 	int err;
 
 	struct ip_map *ipmp;

@@ -209,24 +213,24 @@ static int ip_map_parse(struct cache_detail *cd,
 	len = qword_get(&mesg, buf, mlen);
 	if (len <= 0) return -EINVAL;
 
-	if (sscanf(buf, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) == 4) {
-		addr.s6_addr32[0] = 0;
-		addr.s6_addr32[1] = 0;
-		addr.s6_addr32[2] = htonl(0xffff);
-		addr.s6_addr32[3] =
-			htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
-       } else if (sscanf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x%c",
-			&b1, &b2, &b3, &b4, &b5, &b6, &b7, &b8, &c) == 8) {
-		addr.s6_addr16[0] = htons(b1);
-		addr.s6_addr16[1] = htons(b2);
-		addr.s6_addr16[2] = htons(b3);
-		addr.s6_addr16[3] = htons(b4);
-		addr.s6_addr16[4] = htons(b5);
-		addr.s6_addr16[5] = htons(b6);
-		addr.s6_addr16[6] = htons(b7);
-		addr.s6_addr16[7] = htons(b8);
-       } else
+	if (rpc_pton(buf, len, &address.sa, sizeof(address)) == 0)
 		return -EINVAL;
+	switch (address.sa.sa_family) {
+	case AF_INET:
+		/* Form a mapped IPv4 address in sin6 */
+		memset(&sin6, 0, sizeof(sin6));
+		sin6.sin6_family = AF_INET6;
+		sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
+		sin6.sin6_addr.s6_addr32[3] = address.s4.sin_addr.s_addr;
+		break;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	case AF_INET6:
+		memcpy(&sin6, &address.s6, sizeof(sin6));
+		break;
+#endif
+	default:
+		return -EINVAL;
+	}
 
 	expiry = get_expiry(&mesg);
 	if (expiry ==0)

@@ -243,7 +247,8 @@ static int ip_map_parse(struct cache_detail *cd,
 	} else
 		dom = NULL;
 
-	ipmp = ip_map_lookup(class, &addr);
+	/* IPv6 scope IDs are ignored for now */
+	ipmp = ip_map_lookup(class, &sin6.sin6_addr);
 	if (ipmp) {
 		err = ip_map_update(ipmp,
 			     container_of(dom, struct unix_domain, h),

Quoted 4 lines · Chuck Lever

The existing logic in ip_map_parse() can not currently parse shorthanded IPv6 addresses (anything with a double colon), nor can it parse an IPv6 presentation address with a scope ID. An IPv6-enabled mountd can pass down both.

Why does mountd need to be able to do that?

Quoted 85 lines

Signed-off-by Chuck Lever <[email protected]>

---

net/sunrpc/svcauth_unix.c | 47 ++++++++++++++++++++++++
+--------------------

1 files changed, 26 insertions(+), 21 deletions(-)

net/sunrpc/svcauth_unix.c +7 -3

index d8c0411..97f0e9e 100644
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #define RPCDBG_FACILITY	RPCDBG_AUTH
 
+#include <linux/sunrpc/clnt.h>
 
 /*
 * AUTHUNIX and AUTHNULL credentials are both handled here.

@@ -187,10 +188,13 @@ static int ip_map_parse(struct cache_detail  
 *cd,
 	 * for scratch: */
 	char *buf = mesg;
 	int len;
-	int b1, b2, b3, b4, b5, b6, b7, b8;
-	char c;
 	char class[8];
-	struct in6_addr addr;
+	union {
+		struct sockaddr		sa;
+		struct sockaddr_in	s4;
+		struct sockaddr_in6	s6;
+	} address;
+	struct sockaddr_in6 sin6;
 	int err;
 

	struct ip_map *ipmp;

net/sunrpc/svcauth_unix.c +17 -17

@@ -209,24 +213,24 @@ static int ip_map_parse(struct cache_detail  
 *cd,
 	len = qword_get(&mesg, buf, mlen);
 	if (len <= 0) return -EINVAL;
 
-	if (sscanf(buf, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) == 4) {
-		addr.s6_addr32[0] = 0;
-		addr.s6_addr32[1] = 0;
-		addr.s6_addr32[2] = htonl(0xffff);
-		addr.s6_addr32[3] =
-			htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
-       } else if (sscanf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x: 
 %04x%c",
-			&b1, &b2, &b3, &b4, &b5, &b6, &b7, &b8, &c) == 8) {
-		addr.s6_addr16[0] = htons(b1);
-		addr.s6_addr16[1] = htons(b2);
-		addr.s6_addr16[2] = htons(b3);
-		addr.s6_addr16[3] = htons(b4);
-		addr.s6_addr16[4] = htons(b5);
-		addr.s6_addr16[5] = htons(b6);
-		addr.s6_addr16[6] = htons(b7);
-		addr.s6_addr16[7] = htons(b8);
-       } else
+	if (rpc_pton(buf, len, &address.sa, sizeof(address)) == 0)
 		return -EINVAL;
+	switch (address.sa.sa_family) {
+	case AF_INET:
+		/* Form a mapped IPv4 address in sin6 */
+		memset(&sin6, 0, sizeof(sin6));
+		sin6.sin6_family = AF_INET6;
+		sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
+		sin6.sin6_addr.s6_addr32[3] = address.s4.sin_addr.s_addr;
+		break;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	case AF_INET6:
+		memcpy(&sin6, &address.s6, sizeof(sin6));
+		break;
+#endif
+	default:
+		return -EINVAL;
+	}
 

	expiry = get_expiry(&mesg);
	if (expiry ==0)

net/sunrpc/svcauth_unix.c +0 -0

@@ -243,7 +247,8 @@ static int ip_map_parse(struct cache_detail *cd,
 	} else
 		dom = NULL;
 

net/sunrpc/svcauth_unix.c static int ip_map_parse(struct cache_detail *cd, · line 254

Quoted 7 lines

net/sunrpc/svcauth_unix.c +2 -1

-	ipmp = ip_map_lookup(class, &addr);
+	/* IPv6 scope IDs are ignored for now */
+	ipmp = ip_map_lookup(class, &sin6.sin6_addr);
 	if (ipmp) {
 		err = ip_map_update(ipmp,
 			     container_of(dom, struct unix_domain, h),

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · J. Bruce Fields

Quoted 4 lines · Chuck Lever

The existing logic in ip_map_parse() can not currently parse shorthanded IPv6 addresses (anything with a double colon), nor can it parse an IPv6 presentation address with a scope ID. An IPv6-enabled mountd can pass down both.

Quoted 1 line

Why does mountd need to be able to do that?

AFAICT, the glibc APIs (like inet_ntop(3) and getnameinfo(3)) provide no way to disable shorthanding.

Since we have kernel support for it now, it makes sense to me to allow the kernel to accept a broader range of valid presentation address formats.

Quoted 86 lines

Quoted 86 lines

Signed-off-by Chuck Lever <[email protected]>

---

net/sunrpc/svcauth_unix.c | 47 ++++++++++++++++++++++++
+--------------------

1 files changed, 26 insertions(+), 21 deletions(-)

net/sunrpc/svcauth_unix.c +7 -3

index d8c0411..97f0e9e 100644
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #define RPCDBG_FACILITY	RPCDBG_AUTH
 
+#include <linux/sunrpc/clnt.h>
 
 /*
 * AUTHUNIX and AUTHNULL credentials are both handled here.

@@ -187,10 +188,13 @@ static int ip_map_parse(struct cache_detail  
 *cd,
 	 * for scratch: */
 	char *buf = mesg;
 	int len;
-	int b1, b2, b3, b4, b5, b6, b7, b8;
-	char c;
 	char class[8];
-	struct in6_addr addr;
+	union {
+		struct sockaddr		sa;
+		struct sockaddr_in	s4;
+		struct sockaddr_in6	s6;
+	} address;
+	struct sockaddr_in6 sin6;
 	int err;
 

	struct ip_map *ipmp;

net/sunrpc/svcauth_unix.c +17 -17

@@ -209,24 +213,24 @@ static int ip_map_parse(struct cache_detail  
 *cd,
 	len = qword_get(&mesg, buf, mlen);
 	if (len <= 0) return -EINVAL;
 
-	if (sscanf(buf, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) == 4) {
-		addr.s6_addr32[0] = 0;
-		addr.s6_addr32[1] = 0;
-		addr.s6_addr32[2] = htonl(0xffff);
-		addr.s6_addr32[3] =
-			htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
-       } else if (sscanf(buf, "%04x:%04x:%04x:%04x:%04x:%04x:%04x: 
 %04x%c",
-			&b1, &b2, &b3, &b4, &b5, &b6, &b7, &b8, &c) == 8) {
-		addr.s6_addr16[0] = htons(b1);
-		addr.s6_addr16[1] = htons(b2);
-		addr.s6_addr16[2] = htons(b3);
-		addr.s6_addr16[3] = htons(b4);
-		addr.s6_addr16[4] = htons(b5);
-		addr.s6_addr16[5] = htons(b6);
-		addr.s6_addr16[6] = htons(b7);
-		addr.s6_addr16[7] = htons(b8);
-       } else
+	if (rpc_pton(buf, len, &address.sa, sizeof(address)) == 0)
 		return -EINVAL;
+	switch (address.sa.sa_family) {
+	case AF_INET:
+		/* Form a mapped IPv4 address in sin6 */
+		memset(&sin6, 0, sizeof(sin6));
+		sin6.sin6_family = AF_INET6;
+		sin6.sin6_addr.s6_addr32[2] = htonl(0xffff);
+		sin6.sin6_addr.s6_addr32[3] = address.s4.sin_addr.s_addr;
+		break;
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	case AF_INET6:
+		memcpy(&sin6, &address.s6, sizeof(sin6));
+		break;
+#endif
+	default:
+		return -EINVAL;
+	}
 

	expiry = get_expiry(&mesg);
	if (expiry ==0)

net/sunrpc/svcauth_unix.c +0 -0

@@ -243,7 +247,8 @@ static int ip_map_parse(struct cache_detail *cd,
 	} else
 		dom = NULL;
 

Quoted 12 lines

net/sunrpc/svcauth_unix.c static int ip_map_parse(struct cache_detail *cd, · line 254

Quoted 7 lines

net/sunrpc/svcauth_unix.c +2 -1

-	ipmp = ip_map_lookup(class, &addr);
+	/* IPv6 scope IDs are ignored for now */
+	ipmp = ip_map_lookup(class, &sin6.sin6_addr);
 	if (ipmp) {
 		err = ip_map_update(ipmp,
 			     container_of(dom, struct unix_domain, h),

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

 fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
 fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +4 -0

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)

Quoted 29 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +11 -0

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {

fs/nfsd/nfsctl.c static ssize_t __write_ports_delfd(char *buf) · line 997

Quoted 3 lines

fs/nfsd/nfsctl.c +3 -0

+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;

I realize you're just moving this code, not writing it, so this is a separate question, but: it looks like it's svc_create_xprt() itself that chooses ENOENT for this case. Is there any reason it shouldn't just use EPROTONOSUPPORT from the start?

Quoted 32 lines · J. Bruce Fields

Quoted 31 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +13 -0

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)

Quoted 8 lines

fs/nfsd/nfsctl.c static ssize_t __write_ports_delfd(char *buf) · line 997

Quoted 1 line

fs/nfsd/nfsctl.c +1 -0

+			err = -EPROTONOSUPPORT;

I realize you're just moving this code, not writing it, so this is a separate question, but: it looks like it's svc_create_xprt() itself that
chooses ENOENT for this case. Is there any reason it shouldn't just use
EPROTONOSUPPORT from the start?

I guess this little transformation is for the benefit of rpc.nfsd? I don't know of a reason offhand why you couldn't change svc_create_xprt(). I don't immediately see any of svc_create_xprt's callers that might depend on getting ENOENT, but I didn't look very hard.

Quoted 44 lines · Chuck Lever

Quoted 40 lines · J. Bruce Fields

Quoted 22 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +4 -0

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)

fs/nfsd/nfsctl.c static ssize_t __write_ports_delfd(char *buf) · line 997

Quoted 10 lines

fs/nfsd/nfsctl.c +10 -0

+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;

I realize you're just moving this code, not writing it, so this is a separate question, but: it looks like it's svc_create_xprt() itself that
chooses ENOENT for this case. Is there any reason it shouldn't just use
EPROTONOSUPPORT from the start?

Quoted 5 lines

I guess this little transformation is for the benefit of rpc.nfsd? I don't know of a reason offhand why you couldn't change svc_create_xprt(). I don't immediately see any of svc_create_xprt's callers that might depend on getting ENOENT, but I didn't look very hard.

Could I talk you into writing a patch that does that, as long as you're touching this code?

Quoted 51 lines · J. Bruce Fields

Quoted 49 lines · Chuck Lever

Quoted 32 lines · J. Bruce Fields

Quoted 31 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +13 -0

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)

Quoted 8 lines

fs/nfsd/nfsctl.c static ssize_t __write_ports_delfd(char *buf) · line 997

Quoted 1 line

fs/nfsd/nfsctl.c +1 -0

+			err = -EPROTONOSUPPORT;

I realize you're just moving this code, not writing it, so this is a separate question, but: it looks like it's svc_create_xprt() itself that
chooses ENOENT for this case. Is there any reason it shouldn't just use
EPROTONOSUPPORT from the start?

I guess this little transformation is for the benefit of rpc.nfsd? I don't know of a reason offhand why you couldn't change svc_create_xprt(). I don't immediately see any of svc_create_xprt's callers that might depend on getting ENOENT, but I didn't look very hard.

Quoted 3 lines

Could I talk you into writing a patch that does that, as long as you're
touching this code?

Since we have a little time before 2.6.34, I'll look into it.

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

 fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
 fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +20 -6

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;
+		return err;
+	}
+
+	return 0;
+}
+
 /*
  * A transport listener is added by writing it's transport name and
  * a port number.

@@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_create_xprt(nfsd_serv, transport,
-				PF_INET, port, SVC_SOCK_ANONYMOUS);
-	if (err < 0) {
-		/* Give a reasonable perror msg for bad transport string */
-		if (err == -ENOENT)
-			err = -EPROTONOSUPPORT;

fs/nfsd/nfsctl.c +1 -0

+
 	return 0;
 }
 

fs/nfsd/nfssvc.c +22 -5

index 171699e..7af14c1 100644
@@ -275,14 +275,32 @@ int nfsd_create_serv(void)
 	return err;
 }
 
-static int nfsd_init_socks(int port)
+static int nfsd_create_xprt(const char *transport, const unsigned short port)
+{
+	int ret;
+
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0)
+		return ret;
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET6, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0 && ret != -EAFNOSUPPORT)
+		return ret;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
+
+	return 0;
+}
+
+static int nfsd_init_socks(const unsigned short port)
 {
 	int error;
 	if (!list_empty(&nfsd_serv->sv_permsocks))
 		return 0;
 
-	error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("udp", port);
 	if (error < 0)
 		return error;
 

@@ -290,8 +308,7 @@ static int nfsd_init_socks(int port)
 	if (error < 0)
 		return error;
 
-	error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("tcp", port);
 	if (error < 0)
 		return error;
 

Quoted 61 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +25 -7

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;
+		return err;
+	}
+
+	return 0;
+}
+
 /*
 * A transport listener is added by writing it's transport name and
 * a port number.

@@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char  
 *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_create_xprt(nfsd_serv, transport,
-				PF_INET, port, SVC_SOCK_ANONYMOUS);
-	if (err < 0) {
-		/* Give a reasonable perror msg for bad transport string */
-		if (err == -ENOENT)
-			err = -EPROTONOSUPPORT;
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET,  
 port);
+	if (err < 0)
 		return err;
-	}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET6,  
 port);

fs/nfsd/nfsctl.c static ssize_t __write_ports_addxprt(char *buf) · line 1031

Quoted 3 lines

fs/nfsd/nfsctl.c +3 -0

+	if (err < 0 && err != -EAFNOSUPPORT)
+		return err;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */

Does every caller of __write_ports_create_xprt() with PF_INET6 end up being under this ifdef? Could we instead just move the ifdef inside it (or inside __svc_xpo_create())? (As usual I'd rather keep any necessary hidden away to the extent possible.)

--b.

Quoted 49 lines

fs/nfsd/nfsctl.c +1 -0

+

	return 0;
}

fs/nfsd/nfssvc.c +21 -3

index 171699e..7af14c1 100644
@@ -275,14 +275,32 @@ int nfsd_create_serv(void)
 	return err;
 }
 
-static int nfsd_init_socks(int port)
+static int nfsd_create_xprt(const char *transport, const unsigned  
 short port)
+{
+	int ret;
+
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0)
+		return ret;
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET6, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0 && ret != -EAFNOSUPPORT)
+		return ret;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
+
+	return 0;
+}
+
+static int nfsd_init_socks(const unsigned short port)
 {
 	int error;
 	if (!list_empty(&nfsd_serv->sv_permsocks))
 		return 0;
 
-	error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("udp", port);
 	if (error < 0)
 		return error;

@@ -290,8 +308,7 @@ static int nfsd_init_socks(int port)
 	if (error < 0)
 		return error;
 

fs/nfsd/nfssvc.c static int nfsd_init_socks(int port) · line 314

Quoted 7 lines

fs/nfsd/nfssvc.c +1 -2

-	error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("tcp", port);
 	if (error < 0)
 		return error;
 

Quoted 64 lines · J. Bruce Fields

Quoted 63 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +27 -7

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;
+		return err;
+	}
+
+	return 0;
+}
+
 /*
 * A transport listener is added by writing it's transport name and
 * a port number.

@@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char  
 *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_create_xprt(nfsd_serv, transport,
-				PF_INET, port, SVC_SOCK_ANONYMOUS);
-	if (err < 0) {
-		/* Give a reasonable perror msg for bad transport string */
-		if (err == -ENOENT)
-			err = -EPROTONOSUPPORT;
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET,  
 port);
+	if (err < 0)
 		return err;
-	}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET6,  
 port);
+	if (err < 0 && err != -EAFNOSUPPORT)
+		return err;

Quoted 8 lines

fs/nfsd/nfsctl.c static ssize_t __write_ports_addxprt(char · line 1034

Quoted 1 line

fs/nfsd/nfsctl.c +1 -0

+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */

Does every caller of __write_ports_create_xprt() with PF_INET6 end up being under this ifdef? Could we instead just move the ifdef inside it
(or inside __svc_xpo_create())? (As usual I'd rather keep any necessary
hidden away to the extent possible.)

The only two call sites are right here: one with PF_INET and one with PF_INET6.

When IPv6 support is not enabled, we want only the first call (with PF_INET). When IPv6 support is enabled, we want two calls; one with PF_INET and then one with PF_INET6.

This particular interface is for creating a named transport. We don't name our IPv6 transports separately from IPv4 transports, so this is a simple way to get IPv6 enabled listeners. It's actually been a while since I sat down and thought through this.

Quoted 51 lines

--b.

Quoted 49 lines

fs/nfsd/nfsctl.c +1 -0

+

	return 0;
}

fs/nfsd/nfssvc.c +21 -3

index 171699e..7af14c1 100644
@@ -275,14 +275,32 @@ int nfsd_create_serv(void)
 	return err;
 }
 
-static int nfsd_init_socks(int port)
+static int nfsd_create_xprt(const char *transport, const unsigned  
 short port)
+{
+	int ret;
+
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0)
+		return ret;
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET6, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0 && ret != -EAFNOSUPPORT)
+		return ret;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
+
+	return 0;
+}
+
+static int nfsd_init_socks(const unsigned short port)
 {
 	int error;
 	if (!list_empty(&nfsd_serv->sv_permsocks))
 		return 0;
 
-	error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("udp", port);
 	if (error < 0)
 		return error;

@@ -290,8 +308,7 @@ static int nfsd_init_socks(int port)
 	if (error < 0)
 		return error;
 

Quoted 7 lines

fs/nfsd/nfssvc.c static int nfsd_init_socks(int port) · line 314

Quoted 7 lines

fs/nfsd/nfssvc.c +1 -2

-	error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("tcp", port);
 	if (error < 0)
 		return error;
 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Quoted 76 lines · Chuck Lever

Quoted 64 lines · J. Bruce Fields

Quoted 63 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +27 -7

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;
+		return err;
+	}
+
+	return 0;
+}
+
 /*
 * A transport listener is added by writing it's transport name and
 * a port number.

@@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char
 *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_create_xprt(nfsd_serv, transport,
-				PF_INET, port, SVC_SOCK_ANONYMOUS);
-	if (err < 0) {
-		/* Give a reasonable perror msg for bad transport string */
-		if (err == -ENOENT)
-			err = -EPROTONOSUPPORT;
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET,
 port);
+	if (err < 0)
 		return err;
-	}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET6,
 port);
+	if (err < 0 && err != -EAFNOSUPPORT)
+		return err;

Quoted 8 lines

fs/nfsd/nfsctl.c static ssize_t __write_ports_addxprt(char · line 1034

Quoted 1 line

fs/nfsd/nfsctl.c +1 -0

+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */

Does every caller of __write_ports_create_xprt() with PF_INET6 end up being under this ifdef? Could we instead just move the ifdef inside it
(or inside __svc_xpo_create())? (As usual I'd rather keep any necessary
hidden away to the extent possible.)

Quoted 2 lines

The only two call sites are right here: one with PF_INET and one with PF_INET6.

OK, but svc_create_xprt has a few more.

Quoted 4 lines

When IPv6 support is not enabled, we want only the first call (with PF_INET). When IPv6 support is enabled, we want two calls; one with PF_INET and then one with PF_INET6.

Right, I understand, but we could equally well do that by calling svc_create_xprt twice, and depending on it (or __svc_xpo_create(), or someone else there) to return -EAFNOSUPPORT in the !defined(CONFIG_IPV6) && !defined(CONFIG_IPV6_MODULE case. Right?

No change in behavior, I just want the #ifdef's buried a little further if it's possible.

--b.

Quoted 71 lines

This particular interface is for creating a named transport. We don't name our IPv6 transports separately from IPv4 transports, so this is a simple way to get IPv6 enabled listeners. It's actually been a while since I sat down and thought through this.

Quoted 51 lines

--b.

Quoted 49 lines

fs/nfsd/nfsctl.c +1 -0

+

	return 0;
}

fs/nfsd/nfssvc.c +21 -3

index 171699e..7af14c1 100644
@@ -275,14 +275,32 @@ int nfsd_create_serv(void)
 	return err;
 }
 
-static int nfsd_init_socks(int port)
+static int nfsd_create_xprt(const char *transport, const unsigned
 short port)
+{
+	int ret;
+
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0)
+		return ret;
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	ret = svc_create_xprt(nfsd_serv, transport, PF_INET6, port,
+							SVC_SOCK_DEFAULTS);
+	if (ret < 0 && ret != -EAFNOSUPPORT)
+		return ret;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */
+
+	return 0;
+}
+
+static int nfsd_init_socks(const unsigned short port)
 {
 	int error;
 	if (!list_empty(&nfsd_serv->sv_permsocks))
 		return 0;
 
-	error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("udp", port);
 	if (error < 0)
 		return error;

@@ -290,8 +308,7 @@ static int nfsd_init_socks(int port)
 	if (error < 0)
 		return error;
 

Quoted 7 lines

fs/nfsd/nfssvc.c static int nfsd_init_socks(int port) · line 314

Quoted 7 lines

fs/nfsd/nfssvc.c +1 -2

-	error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
-					SVC_SOCK_DEFAULTS);
+	error = nfsd_create_xprt("tcp", port);
 	if (error < 0)
 		return error;
 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Quoted 95 lines · J. Bruce Fields

Quoted 77 lines · Chuck Lever

Quoted 66 lines · J. Bruce Fields

Quoted 54 lines · Chuck Lever

Try to use an PF_INET6 listener for NFSD if IPv6 is enabled in the kernel.

Signed-off-by Chuck Lever <[email protected]>

---

fs/nfsd/nfsctl.c |   36 +++++++++++++++++++++++++++++-------
fs/nfsd/nfssvc.c |   27 ++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 12 deletions(-)

fs/nfsd/nfsctl.c +21 -6

index 2604c3e..7ebb7a5 100644
@@ -981,6 +981,26 @@ static ssize_t __write_ports_delfd(char *buf)
 	return len;
 }
 
+static ssize_t __write_ports_create_xprt(struct svc_serv *serv,
+					 const char *transport,
+					 const int family,
+					 const unsigned short port)
+{
+	int err;
+
+	err = svc_create_xprt(serv, transport, family, port,
+						SVC_SOCK_ANONYMOUS);
+
+	if (err < 0) {
+		/* Give a reasonable perror msg for bad transport string */
+		if (err == -ENOENT)
+			err = -EPROTONOSUPPORT;
+		return err;
+	}
+
+	return 0;
+}
+
 /*
 * A transport listener is added by writing it's transport name and
 * a port number.

@@ -1000,14 +1020,16 @@ static ssize_t __write_ports_addxprt(char
 *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_create_xprt(nfsd_serv, transport,
-				PF_INET, port, SVC_SOCK_ANONYMOUS);
-	if (err < 0) {
-		/* Give a reasonable perror msg for bad transport string */
-		if (err == -ENOENT)
-			err = -EPROTONOSUPPORT;
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET,
 port);

fs/nfsd/nfsctl.c static ssize_t __write_ports_addxprt(char · line 1034

Quoted 10 lines

fs/nfsd/nfsctl.c +7 -1

+	if (err < 0)
 		return err;
-	}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	err = __write_ports_create_xprt(nfsd_serv, transport, PF_INET6,
 port);
+	if (err < 0 && err != -EAFNOSUPPORT)
+		return err;
+#endif	/* CONFIG_IPV6 || CONFIG_IPV6_MODULE */

Quoted 7 lines

Does every caller of __write_ports_create_xprt() with PF_INET6 end up
being under this ifdef? Could we instead just move the ifdef inside it
(or inside __svc_xpo_create())? (As usual I'd rather keep any necessary
hidden away to the extent possible.)

Quoted 3 lines

The only two call sites are right here: one with PF_INET and one with
PF_INET6.

OK, but svc_create_xprt has a few more.

Quoted 4 lines

When IPv6 support is not enabled, we want only the first call (with PF_INET). When IPv6 support is enabled, we want two calls; one with PF_INET and then one with PF_INET6.

Right, I understand, but we could equally well do that by calling svc_create_xprt twice, and depending on it (or __svc_xpo_create(), or someone else there) to return -EAFNOSUPPORT in the ! defined(CONFIG_IPV6)
&& !defined(CONFIG_IPV6_MODULE case. Right?

Quoted 3 lines

No change in behavior, I just want the #ifdef's buried a little further
if it's possible.

Yeah, OK. That kind of change would effect other call sites, so I would need to break this into a couple of smaller patches.