[PATCH 00/23] Clean ups and bug fixes for 2.6.30

[PATCH 00/23] Clean ups and bug fixes for 2.6.30

[Chuck Lever]

Hi Bruce-

These patches are ready for consideration for 2.6.30.  They've seen
only very light testing.  Many of these you've seen before.

---

Chuck Lever (23):
      NFSD: Prevent buffer overflow in write_recoverydir()
      NFSD: Prevent buffer overflow in write_leasetime()
      NFSD: Prevent buffer overflow in write_maxblksize()
      NFSD: Prevent buffer overflow in write_versions()
      NFSD: Prevent buffer overflow in write_threads()
      NFSD: Support IPv6 addresses in write_failover_ip()
      NFS: Move NFS client's IP address parser to nfs_common/
      SUNRPC: Clean up one_sock_name()
      SUNRPC: Support PF_INET6 in one_sock_name()
      SUNRPC: Switch one_sock_name() to use snprintf()
      SUNRPC: pass buffer size to svc_sock_names()
      SUNRPC: pass buffer size to svc_addsock()
      NFSD: Prevent a buffer overflow in svc_xprt_names()
      NFSD: move lockd_up() before svc_addsock()
      NFSD: Finish refactoring __write_ports()
      NFSD: Note an additional requirement when passing TCP sockets to portlist
      NFSD: Refactor socket creation out of __write_ports()
      NFSD: Refactor portlist socket closing into a helper
      NFSD: Refactor transport addition out of __write_ports()
      NFSD: Refactor transport removal out of __write_ports()
      SUNRPC: Clean up svc_find_xprt() calling sequence
      SUNRPC: Clean up static inline functions in svc_xprt.h
      SUNRPC: Fix return type of svc_addr_len()


 fs/nfs/internal.h                        |    3 
 fs/nfs/nfs4namespace.c                   |    2 
 fs/nfs/super.c                           |  121 ------------
 fs/nfs_common/Makefile                   |    1 
 fs/nfs_common/nfs_addr_parse.c           |  151 +++++++++++++++
 fs/nfsd/nfsctl.c                         |  298 +++++++++++++++++-------------
 include/linux/nfs_addr_parse.h           |   29 +++
 include/linux/sunrpc/svc_xprt.h          |   54 +++--
 include/linux/sunrpc/svcsock.h           |    7 +
 net/sunrpc/svc_xprt.c                    |   75 +++++---
 net/sunrpc/svcsock.c                     |   80 ++++++--
 net/sunrpc/xprtrdma/svc_rdma_transport.c |   16 +-
 12 files changed, 521 insertions(+), 316 deletions(-)
 create mode 100644 fs/nfs_common/nfs_addr_parse.c
 create mode 100644 include/linux/nfs_addr_parse.h

-- 
Chuck Lever

[PATCH 01/23] SUNRPC: Fix return type of svc_addr_len()

[Chuck Lever]

The svc_addr_len() helper function can return a negative errno value,
but its return type is size_t, which is unsigned.

The RDMA transport code passes this return value directly to memset(),
without checking first if it's negative.  This could cause memset() to
clobber a large piece of memory if svc_addr_len() has returned an
error.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/svc_xprt.h          |    3 ++-
 net/sunrpc/xprtrdma/svc_rdma_transport.c |   16 ++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 879c701..088d81f 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -114,7 +114,7 @@ static inline unsigned short svc_addr_port(struct sockaddr *sa)
 	return ret;
 }
 
-static inline size_t svc_addr_len(struct sockaddr *sa)
+static inline int svc_addr_len(const struct sockaddr *sa)
 {
 	switch (sa->sa_family) {
 	case AF_INET:
@@ -122,6 +122,7 @@ static inline size_t svc_addr_len(struct sockaddr *sa)
 	case AF_INET6:
 		return sizeof(struct sockaddr_in6);
 	}
+
 	return -EAFNOSUPPORT;
 }
 
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index 3d810e7..d1ec6f9 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -546,6 +546,7 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id, size_t client_ird)
 	struct svcxprt_rdma *listen_xprt = new_cma_id->context;
 	struct svcxprt_rdma *newxprt;
 	struct sockaddr *sa;
+	int len;
 
 	/* Create a new transport */
 	newxprt = rdma_create_xprt(listen_xprt->sc_xprt.xpt_server, 0);
@@ -563,9 +564,20 @@ static void handle_connect_req(struct rdma_cm_id *new_cma_id, size_t client_ird)
 
 	/* Set the local and remote addresses in the transport */
 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.dst_addr;
-	svc_xprt_set_remote(&newxprt->sc_xprt, sa, svc_addr_len(sa));
+	len = svc_addr_len(sa);
+	if (len < 0) {
+		dprintk("svcrdma: dst_addr has a bad address family\n");
+		return;
+	}
+	svc_xprt_set_remote(&newxprt->sc_xprt, sa, len);
+
 	sa = (struct sockaddr *)&newxprt->sc_cm_id->route.addr.src_addr;
-	svc_xprt_set_local(&newxprt->sc_xprt, sa, svc_addr_len(sa));
+	len = svc_addr_len(sa);
+	if (len < 0) {
+		dprintk("svcrdma: src_addr has a bad address family\n");
+		return;
+	}
+	svc_xprt_set_local(&newxprt->sc_xprt, sa, len);
 
 	/*
 	 * Enqueue the new transport on the accept queue of the listening

[PATCH 02/23] SUNRPC: Clean up static inline functions in svc_xprt.h

[Chuck Lever]

Clean up:  Enable the use of const arguments in higher level svc_ APIs
by adding const to the arguments of the helper functions in svc_xprt.h

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/svc_xprt.h |   46 ++++++++++++++++++++++-----------------
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 088d81f..53fbcdb 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -89,29 +89,32 @@ static inline void svc_xprt_get(struct svc_xprt *xprt)
 	kref_get(&xprt->xpt_ref);
 }
 static inline void svc_xprt_set_local(struct svc_xprt *xprt,
-				      struct sockaddr *sa, int salen)
+				      const struct sockaddr *sa,
+				      const size_t salen)
 {
 	memcpy(&xprt->xpt_local, sa, salen);
 	xprt->xpt_locallen = salen;
 }
 static inline void svc_xprt_set_remote(struct svc_xprt *xprt,
-				       struct sockaddr *sa, int salen)
+				       const struct sockaddr *sa,
+				       const size_t salen)
 {
 	memcpy(&xprt->xpt_remote, sa, salen);
 	xprt->xpt_remotelen = salen;
 }
-static inline unsigned short svc_addr_port(struct sockaddr *sa)
+static inline unsigned short svc_addr_port(const struct sockaddr *sa)
 {
-	unsigned short ret = 0;
+	const struct sockaddr_in *sin = (const struct sockaddr_in *)sa;
+	const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)sa;
+
 	switch (sa->sa_family) {
 	case AF_INET:
-		ret = ntohs(((struct sockaddr_in *)sa)->sin_port);
-		break;
+		return ntohs(sin->sin_port);
 	case AF_INET6:
-		ret = ntohs(((struct sockaddr_in6 *)sa)->sin6_port);
-		break;
+		return ntohs(sin6->sin6_port);
 	}
-	return ret;
+
+	return 0;
 }
 
 static inline int svc_addr_len(const struct sockaddr *sa)
@@ -126,36 +129,39 @@ static inline int svc_addr_len(const struct sockaddr *sa)
 	return -EAFNOSUPPORT;
 }
 
-static inline unsigned short svc_xprt_local_port(struct svc_xprt *xprt)
+static inline unsigned short svc_xprt_local_port(const struct svc_xprt *xprt)
 {
-	return svc_addr_port((struct sockaddr *)&xprt->xpt_local);
+	return svc_addr_port((const struct sockaddr *)&xprt->xpt_local);
 }
 
-static inline unsigned short svc_xprt_remote_port(struct svc_xprt *xprt)
+static inline unsigned short svc_xprt_remote_port(const struct svc_xprt *xprt)
 {
-	return svc_addr_port((struct sockaddr *)&xprt->xpt_remote);
+	return svc_addr_port((const struct sockaddr *)&xprt->xpt_remote);
 }
 
-static inline char *__svc_print_addr(struct sockaddr *addr,
-				     char *buf, size_t len)
+static inline char *__svc_print_addr(const struct sockaddr *addr,
+				     char *buf, const size_t len)
 {
+	const struct sockaddr_in *sin = (const struct sockaddr_in *)addr;
+	const struct sockaddr_in6 *sin6 = (const struct sockaddr_in6 *)addr;
+
 	switch (addr->sa_family) {
 	case AF_INET:
-		snprintf(buf, len, "%pI4, port=%u",
-			&((struct sockaddr_in *)addr)->sin_addr,
-			ntohs(((struct sockaddr_in *) addr)->sin_port));
+		snprintf(buf, len, "%pI4, port=%u", &sin->sin_addr,
+			ntohs(sin->sin_port));
 		break;
 
 	case AF_INET6:
 		snprintf(buf, len, "%pI6, port=%u",
-			 &((struct sockaddr_in6 *)addr)->sin6_addr,
-			ntohs(((struct sockaddr_in6 *) addr)->sin6_port));
+			 &sin6->sin6_addr,
+			ntohs(sin6->sin6_port));
 		break;
 
 	default:
 		snprintf(buf, len, "unknown address type: %d", addr->sa_family);
 		break;
 	}
+
 	return buf;
 }
 #endif /* SUNRPC_SVC_XPRT_H */

[PATCH 03/23] SUNRPC: Clean up svc_find_xprt() calling sequence

[Chuck Lever]

Clean up: add documentating comment and use appropriate data types for
svc_find_xprt()'s arguments.

This also eliminates a mixed sign comparison: @port was an int, while
the return value of svc_xprt_local_port() is an unsigned short.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/svc_xprt.h |    3 ++-
 net/sunrpc/svc_xprt.c           |   19 +++++++++++++------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 53fbcdb..819257c 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -81,7 +81,8 @@ void	svc_close_xprt(struct svc_xprt *xprt);
 void	svc_delete_xprt(struct svc_xprt *xprt);
 int	svc_port_is_privileged(struct sockaddr *sin);
 int	svc_print_xprts(char *buf, int maxlen);
-struct	svc_xprt *svc_find_xprt(struct svc_serv *, char *, int, int);
+struct	svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
+			const sa_family_t family, const unsigned short port);
 int	svc_xprt_names(struct svc_serv *serv, char *buf, int buflen);
 
 static inline void svc_xprt_get(struct svc_xprt *xprt)
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 3772081..9d28051 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1036,7 +1036,13 @@ static struct svc_deferred_req *svc_deferred_dequeue(struct svc_xprt *xprt)
 	return dr;
 }
 
-/*
+/**
+ * svc_find_xprt - find an RPC transport instance
+ * @serv: pointer to svc_serv to search
+ * @xcl_name: C string containing transport's class name
+ * @family: Address family of transport's local address
+ * @port: transport's IP port number
+ *
  * Return the transport instance pointer for the endpoint accepting
  * connections/peer traffic from the specified transport class,
  * address family and port.
@@ -1045,23 +1051,24 @@ static struct svc_deferred_req *svc_deferred_dequeue(struct svc_xprt *xprt)
  * wild-card, and will result in matching the first transport in the
  * service's list that has a matching class name.
  */
-struct svc_xprt *svc_find_xprt(struct svc_serv *serv, char *xcl_name,
-			       int af, int port)
+struct svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
+			       const sa_family_t family,
+			       const unsigned short port)
 {
 	struct svc_xprt *xprt;
 	struct svc_xprt *found = NULL;
 
 	/* Sanity check the args */
-	if (!serv || !xcl_name)
+	if (serv == NULL || xcl_name == NULL)
 		return found;
 
 	spin_lock_bh(&serv->sv_lock);
 	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
 		if (strcmp(xprt->xpt_class->xcl_name, xcl_name))
 			continue;
-		if (af != AF_UNSPEC && af != xprt->xpt_local.ss_family)
+		if (family != AF_UNSPEC && family != xprt->xpt_local.ss_family)
 			continue;
-		if (port && port != svc_xprt_local_port(xprt))
+		if (port != 0 && port != svc_xprt_local_port(xprt))
 			continue;
 		found = xprt;
 		svc_xprt_get(xprt);

[PATCH 04/23] NFSD: Refactor transport removal out of __write_ports()

[Chuck Lever]

Clean up: Refactor transport removal out of __write_ports() to make it
easier to understand and maintain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 817999e..c56f50f 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -879,8 +879,32 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
 	return rv;
 }
 
+/*
+ * A transport listener is removed by writing a "-", it's transport
+ * name, and it's port number
+ */
+static ssize_t __write_ports_delxprt(const char *transport,
+				     const unsigned short port)
+{
+	struct svc_xprt *xprt;
+
+	if (port == 0 || nfsd_serv == NULL)
+		return -EINVAL;
+
+	xprt = svc_find_xprt(nfsd_serv, transport, AF_UNSPEC, port);
+	if (xprt == NULL)
+		return -ENOTCONN;
+
+	svc_close_xprt(xprt);
+	svc_xprt_put(xprt);
+	return 0;
+}
+
 static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 {
+	char transport[16];
+	int port;
+
 	if (size == 0) {
 		int len = 0;
 
@@ -951,30 +975,11 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 			return err < 0 ? err : 0;
 		}
 	}
-	/*
-	 * Remove a transport by writing it's transport name and port number
-	 */
-	if (buf[0] == '-' && isalpha(buf[1])) {
-		struct svc_xprt *xprt;
-		int err = -EINVAL;
-		char transport[16];
-		int port;
-		if (sscanf(&buf[1], "%15s %4d", transport, &port) == 2) {
-			if (port == 0)
-				return -EINVAL;
-			if (nfsd_serv) {
-				xprt = svc_find_xprt(nfsd_serv, transport,
-						     AF_UNSPEC, port);
-				if (xprt) {
-					svc_close_xprt(xprt);
-					svc_xprt_put(xprt);
-					err = 0;
-				} else
-					err = -ENOTCONN;
-			}
-			return err < 0 ? err : 0;
-		}
-	}
+
+	if (buf[0] == '-' && isalpha(buf[1]))
+		if (sscanf(&buf[1], "%15s %4u", transport, &port) == 2)
+			return __write_ports_delxprt(transport, port);
+
 	return -EINVAL;
 }
 

[PATCH 05/23] NFSD: Refactor transport addition out of __write_ports()

[Chuck Lever]

Clean up: Refactor transport addition out of __write_ports() to make
it easier to understand and maintain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   49 ++++++++++++++++++++++++++++---------------------
 1 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index c56f50f..72af950 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -880,6 +880,30 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
 }
 
 /*
+ * A transport listener is added by writing it's transport name and
+ * a port number
+ */
+static ssize_t __write_ports_addxprt(const char *transport,
+				     const unsigned short port)
+{
+	int err;
+
+	err = nfsd_create_serv();
+	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;
+		return err;
+	}
+	return 0;
+}
+
+/*
  * A transport listener is removed by writing a "-", it's transport
  * name, and it's port number
  */
@@ -954,27 +978,10 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 		kfree(toclose);
 		return len;
 	}
-	/*
-	 * Add a transport listener by writing it's transport name
-	 */
-	if (isalpha(buf[0])) {
-		int err;
-		char transport[16];
-		int port;
-		if (sscanf(buf, "%15s %4d", transport, &port) == 2) {
-			err = nfsd_create_serv();
-			if (!err) {
-				err = svc_create_xprt(nfsd_serv,
-						      transport, PF_INET, port,
-						      SVC_SOCK_ANONYMOUS);
-				if (err == -ENOENT)
-					/* Give a reasonable perror msg for
-					 * bad transport string */
-					err = -EPROTONOSUPPORT;
-			}
-			return err < 0 ? err : 0;
-		}
-	}
+
+	if (isalpha(buf[0]))
+		if (sscanf(buf, "%15s %4u", transport, &port) == 2)
+			return __write_ports_addxprt(transport, port);
 
 	if (buf[0] == '-' && isalpha(buf[1]))
 		if (sscanf(&buf[1], "%15s %4u", transport, &port) == 2)

[PATCH 06/23] NFSD: Refactor portlist socket closing into a helper

[Chuck Lever]

Clean up: Refactor the socket closing logic out of __write_ports() to
make it easier to understand and maintain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   36 ++++++++++++++++++++++++------------
 1 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 72af950..5b99a99 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -880,6 +880,27 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
 }
 
 /*
+ * A '-' followed by the 'name' of a socket means we close the socket.
+ */
+static ssize_t __write_ports_delfd(char *buf)
+{
+	char *toclose;
+	int len = 0;
+
+	toclose = kstrdup(buf + 1, GFP_KERNEL);
+	if (toclose == NULL)
+		return -ENOMEM;
+
+	if (nfsd_serv != NULL)
+		len = svc_sock_names(buf, nfsd_serv, toclose);
+	if (len >= 0)
+		lockd_down();
+
+	kfree(toclose);
+	return len;
+}
+
+/*
  * A transport listener is added by writing it's transport name and
  * a port number
  */
@@ -966,18 +987,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 		}
 		return err < 0 ? err : 0;
 	}
-	if (buf[0] == '-' && isdigit(buf[1])) {
-		char *toclose = kstrdup(buf+1, GFP_KERNEL);
-		int len = 0;
-		if (!toclose)
-			return -ENOMEM;
-		if (nfsd_serv)
-			len = svc_sock_names(buf, nfsd_serv, toclose);
-		if (len >= 0)
-			lockd_down();
-		kfree(toclose);
-		return len;
-	}
+
+	if (buf[0] == '-' && isdigit(buf[1]))
+		return __write_ports_delfd(buf);
 
 	if (isalpha(buf[0]))
 		if (sscanf(buf, "%15s %4u", transport, &port) == 2)

[PATCH 07/23] NFSD: Refactor socket creation out of __write_ports()

[Chuck Lever]

Clean up: Refactor the socket creation logic out of __write_ports() to
make it easier to understand and maintain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   64 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 5b99a99..0d3efa4 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -880,6 +880,37 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
 }
 
 /*
+ * A single 'fd' number was written, in which case it must be for
+ * a socket of a supported family/protocol, and we use it as an
+ * nfsd listener.
+ */
+static ssize_t __write_ports_addfd(char *buf)
+{
+	char *mesg = buf;
+	int fd, err;
+
+	err = get_int(&mesg, &fd);
+	if (err != 0 || fd < 0)
+		return -EINVAL;
+
+	err = nfsd_create_serv();
+	if (err != 0)
+		return err;
+
+	err = svc_addsock(nfsd_serv, fd, buf);
+	if (err >= 0) {
+		err = lockd_up();
+		if (err < 0)
+			svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf);
+
+		/* Decrease the count, but don't shut down the service */
+		nfsd_serv->sv_nrthreads--;
+	}
+
+	return err < 0 ? err : 0;
+}
+
+/*
  * A '-' followed by the 'name' of a socket means we close the socket.
  */
 static ssize_t __write_ports_delfd(char *buf)
@@ -957,36 +988,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 			len = svc_xprt_names(nfsd_serv, buf, 0);
 		return len;
 	}
-	/* Either a single 'fd' number is written, in which
-	 * case it must be for a socket of a supported family/protocol,
-	 * and we use it as an nfsd socket, or
-	 * A '-' followed by the 'name' of a socket in which case
-	 * we close the socket.
-	 */
-	if (isdigit(buf[0])) {
-		char *mesg = buf;
-		int fd;
-		int err;
-		err = get_int(&mesg, &fd);
-		if (err)
-			return -EINVAL;
-		if (fd < 0)
-			return -EINVAL;
-		err = nfsd_create_serv();
-		if (!err) {
-			err = svc_addsock(nfsd_serv, fd, buf);
-			if (err >= 0) {
-				err = lockd_up();
-				if (err < 0)
-					svc_sock_names(buf+strlen(buf)+1, nfsd_serv, buf);
-			}
-			/* Decrease the count, but don't shutdown the
-			 * the service
-			 */
-			nfsd_serv->sv_nrthreads--;
-		}
-		return err < 0 ? err : 0;
-	}
+
+	if (isdigit(buf[0]))
+		return __write_ports_addfd(buf);
 
 	if (buf[0] == '-' && isdigit(buf[1]))
 		return __write_ports_delfd(buf);

[PATCH 08/23] NFSD: Note an additional requirement when passing TCP sockets to portlist

[Chuck Lever]

User space must call listen(3) on SOCK_STREAM sockets passed into
/proc/fs/nfsd/portlist, otherwise that listener is ignored.  Document
this.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 0d3efa4..441fbfd 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1025,7 +1025,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
  *			buf:		C string containing an unsigned
  *					integer value representing a bound
  *					but unconnected socket that is to be
- *					used as an NFSD listener
+ *					used as an NFSD listener; listen(3)
+ *					must be called for a SOCK_STREAM
+ *					socket, otherwise it is ignored
  *			size:		non-zero length of C string in @buf
  * Output:
  *	On success:	NFS service is started;

[PATCH 09/23] NFSD: Finish refactoring __write_ports()

[Chuck Lever]

Clean up: Refactor transport name listing out of __write_ports() to
make it easier to understand and maintain.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 441fbfd..80f773b 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -880,6 +880,17 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
 }
 
 /*
+ * Zero-length write.  Return a list of NFSD's current listener
+ * transports.
+ */
+static ssize_t __write_ports_names(char *buf)
+{
+	if (nfsd_serv == NULL)
+		return 0;
+	return svc_xprt_names(nfsd_serv, buf, 0);
+}
+
+/*
  * A single 'fd' number was written, in which case it must be for
  * a socket of a supported family/protocol, and we use it as an
  * nfsd listener.
@@ -981,13 +992,8 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size)
 	char transport[16];
 	int port;
 
-	if (size == 0) {
-		int len = 0;
-
-		if (nfsd_serv)
-			len = svc_xprt_names(nfsd_serv, buf, 0);
-		return len;
-	}
+	if (size == 0)
+		return __write_ports_names(buf);
 
 	if (isdigit(buf[0]))
 		return __write_ports_addfd(buf);

[PATCH 10/23] NFSD: move lockd_up() before svc_addsock()

[Chuck Lever]

Clean up.

A couple of years ago, a series of commits, finishing with commit
5680c446, swapped the order of the lockd_up() and svc_addsock() calls
in __write_ports().  At that time lockd_up() needed to know the
transport protocol of the passed-in socket to start a listener on the
same transport protocol.

These days, lockd_up() doesn't take a protocol argument; it always
starts both a UDP and TCP listener.  It's now more straightforward to
try the lockd_up() first, then do a lockd_down() if the svc_addsock()
fails.

Careful review of this code shows that the svc_sock_names() call is
used only to close the just-opened socket in case lockd_up() fails.
So it is no longer needed if lockd_up() is done first.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 80f773b..aa60191 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -908,17 +908,18 @@ static ssize_t __write_ports_addfd(char *buf)
 	if (err != 0)
 		return err;
 
-	err = svc_addsock(nfsd_serv, fd, buf);
-	if (err >= 0) {
-		err = lockd_up();
-		if (err < 0)
-			svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf);
+	err = lockd_up();
+	if (err != 0)
+		goto out;
 
-		/* Decrease the count, but don't shut down the service */
-		nfsd_serv->sv_nrthreads--;
-	}
+	err = svc_addsock(nfsd_serv, fd, buf);
+	if (err != 0)
+		lockd_down();
 
-	return err < 0 ? err : 0;
+out:
+	/* Decrease the count, but don't shut down the service */
+	nfsd_serv->sv_nrthreads--;
+	return err;
 }
 
 /*

[PATCH 11/23] NFSD: Prevent a buffer overflow in svc_xprt_names()

[Chuck Lever]

The svc_xprt_names() function can overflow its buffer if it's so near
the end of the passed in buffer that the "name too long" string still
doesn't fit.  Of course, it could never tell if it was near the end
of the passed in buffer, since its only caller passes in zero as the
buffer length.

Let's make this API a little safer.

Change svc_xprt_names() so it *always* checks for a buffer overflow,
and change its only caller to pass in the correct buffer length.

If svc_xprt_names() does overflow its buffer, just fail with a
reasonable errno code, instead of trying to write a message at the end
of the buffer.  I don't like this much, but I can't figure out a clean
way that's always safe to return some of the names, *and* an
indication that the buffer was not long enough.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c                |    2 +
 include/linux/sunrpc/svc_xprt.h |    2 +
 net/sunrpc/svc_xprt.c           |   56 +++++++++++++++++++++++++++------------
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index aa60191..b0f7300 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -887,7 +887,7 @@ static ssize_t __write_ports_names(char *buf)
 {
 	if (nfsd_serv == NULL)
 		return 0;
-	return svc_xprt_names(nfsd_serv, buf, 0);
+	return svc_xprt_names(nfsd_serv, buf, SIMPLE_TRANSACTION_LIMIT);
 }
 
 /*
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 819257c..aab050b 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -83,7 +83,7 @@ int	svc_port_is_privileged(struct sockaddr *sin);
 int	svc_print_xprts(char *buf, int maxlen);
 struct	svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
 			const sa_family_t family, const unsigned short port);
-int	svc_xprt_names(struct svc_serv *serv, char *buf, int buflen);
+int	svc_xprt_names(struct svc_serv *serv, char *buf, const int buflen);
 
 static inline void svc_xprt_get(struct svc_xprt *xprt)
 {
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 9d28051..da64e1a 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1079,36 +1079,58 @@ struct svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
 }
 EXPORT_SYMBOL_GPL(svc_find_xprt);
 
-/*
- * Format a buffer with a list of the active transports. A zero for
- * the buflen parameter disables target buffer overflow checking.
+static int svc_one_xprt_name(const struct svc_xprt *xprt,
+			     char *pos, int remaining)
+{
+	int len;
+
+	len = snprintf(pos, remaining, "%s %u\n",
+			xprt->xpt_class->xcl_name,
+			svc_xprt_local_port(xprt));
+	if (len >= remaining)
+		return -ENAMETOOLONG;
+	return len;
+}
+
+/**
+ * svc_xprt_names - format a buffer with a list of transport names
+ * @serv: pointer to an RPC service
+ * @buf: pointer to a buffer to be filled in
+ * @buflen: length of buffer to be filled in
+ *
+ * Fills in @buf with a string containing a list of transport names,
+ * each name terminated with '\n'.
+ *
+ * Returns positive length of the filled-in string on success; otherwise
+ * a negative errno value is returned if an error occurs.
  */
-int svc_xprt_names(struct svc_serv *serv, char *buf, int buflen)
+int svc_xprt_names(struct svc_serv *serv, char *buf, const int buflen)
 {
 	struct svc_xprt *xprt;
-	char xprt_str[64];
-	int totlen = 0;
-	int len;
+	int len, totlen;
+	char *pos;
 
 	/* Sanity check args */
 	if (!serv)
 		return 0;
 
 	spin_lock_bh(&serv->sv_lock);
+
+	pos = buf;
+	totlen = 0;
 	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
-		len = snprintf(xprt_str, sizeof(xprt_str),
-			       "%s %d\n", xprt->xpt_class->xcl_name,
-			       svc_xprt_local_port(xprt));
-		/* If the string was truncated, replace with error string */
-		if (len >= sizeof(xprt_str))
-			strcpy(xprt_str, "name-too-long\n");
-		/* Don't overflow buffer */
-		len = strlen(xprt_str);
-		if (buflen && (len + totlen >= buflen))
+		len = svc_one_xprt_name(xprt, pos, buflen - totlen);
+		if (len < 0) {
+			*buf = '\0';
+			totlen = len;
+		}
+		if (len <= 0)
 			break;
-		strcpy(buf+totlen, xprt_str);
+
+		pos += len;
 		totlen += len;
 	}
+
 	spin_unlock_bh(&serv->sv_lock);
 	return totlen;
 }

[PATCH 12/23] SUNRPC: pass buffer size to svc_addsock()

[Chuck Lever]

Adjust the synopsis of svc_addsock() to pass in the size of the output
buffer.  Add a documenting comment.

This is a cosmetic change for now.  A subsequent patch will make sure
the buffer length is passed to one_sock_name(), where the length will
actually be useful.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c               |    2 +-
 include/linux/sunrpc/svcsock.h |    3 ++-
 net/sunrpc/svcsock.c           |   16 +++++++++++++---
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index b0f7300..917578f 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -912,7 +912,7 @@ static ssize_t __write_ports_addfd(char *buf)
 	if (err != 0)
 		goto out;
 
-	err = svc_addsock(nfsd_serv, fd, buf);
+	err = svc_addsock(nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT);
 	if (err != 0)
 		lockd_down();
 
diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
index 483e103..e23241c 100644
--- a/include/linux/sunrpc/svcsock.h
+++ b/include/linux/sunrpc/svcsock.h
@@ -39,7 +39,8 @@ int		svc_send(struct svc_rqst *);
 void		svc_drop(struct svc_rqst *);
 void		svc_sock_update_bufs(struct svc_serv *serv);
 int		svc_sock_names(char *buf, struct svc_serv *serv, char *toclose);
-int		svc_addsock(struct svc_serv *serv, int fd, char *name_return);
+int		svc_addsock(struct svc_serv *serv, const int fd,
+					char *name_return, const size_t len);
 void		svc_init_xprt_sock(void);
 void		svc_cleanup_xprt_sock(void);
 
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index ac6cd65..61ca2dd 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1159,9 +1159,19 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
 	return svsk;
 }
 
-int svc_addsock(struct svc_serv *serv,
-		int fd,
-		char *name_return)
+/**
+ * svc_addsock - add a listener socket to an RPC service
+ * @serv: pointer to RPC service to which to add a new listener
+ * @fd: file descriptor of the new listener
+ * @name_return: pointer to buffer to fill in with name of listener
+ * @len: size of the buffer
+ *
+ * Fills in socket name and returns positive length of name if successful.
+ * Name is terminated with '\n'.  On error, returns a negative errno
+ * value.
+ */
+int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
+		const size_t len)
 {
 	int err = 0;
 	struct socket *so = sockfd_lookup(fd, &err);

[PATCH 13/23] SUNRPC: pass buffer size to svc_sock_names()

[Chuck Lever]

Adjust the synopsis of svc_sock_names() to pass in the size of the
output buffer.  Add a documenting comment.

This is a cosmetic change for now.  A subsequent patch will make sure
the buffer length is passed to one_sock_name(), where the length will
actually be useful.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c               |    3 ++-
 include/linux/sunrpc/svcsock.h |    4 +++-
 net/sunrpc/svcsock.c           |   19 +++++++++++++++++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 917578f..69c77ad 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -935,7 +935,8 @@ static ssize_t __write_ports_delfd(char *buf)
 		return -ENOMEM;
 
 	if (nfsd_serv != NULL)
-		len = svc_sock_names(buf, nfsd_serv, toclose);
+		len = svc_sock_names(nfsd_serv, buf,
+					SIMPLE_TRANSACTION_LIMIT, toclose);
 	if (len >= 0)
 		lockd_down();
 
diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
index e23241c..8271631 100644
--- a/include/linux/sunrpc/svcsock.h
+++ b/include/linux/sunrpc/svcsock.h
@@ -38,7 +38,9 @@ int		svc_recv(struct svc_rqst *, long);
 int		svc_send(struct svc_rqst *);
 void		svc_drop(struct svc_rqst *);
 void		svc_sock_update_bufs(struct svc_serv *serv);
-int		svc_sock_names(char *buf, struct svc_serv *serv, char *toclose);
+int		svc_sock_names(struct svc_serv *serv, char *buf,
+					const size_t buflen,
+					const char *toclose);
 int		svc_addsock(struct svc_serv *serv, const int fd,
 					char *name_return, const size_t len);
 void		svc_init_xprt_sock(void);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 61ca2dd..107b887 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -259,8 +259,23 @@ static int one_sock_name(char *buf, struct svc_sock *svsk)
 	return len;
 }
 
-int
-svc_sock_names(char *buf, struct svc_serv *serv, char *toclose)
+/**
+ * svc_sock_names - construct a list of listener names in a string
+ * @serv: pointer to RPC service
+ * @buf: pointer to a buffer to fill in with socket names
+ * @buflen: size of the buffer to be filled
+ * @toclose: pointer to '\0'-terminated C string containing the name
+ *		of a listener to be closed
+ *
+ * Fills in @buf with a '\n'-separated list of names of listener
+ * sockets.  If @toclose is not NULL, the socket named by @toclose
+ * is closed, and is not included in the output list.
+ *
+ * Returns positive length of the socket name string, or a negative
+ * errno value on error.
+ */
+int svc_sock_names(struct svc_serv *serv, char *buf, const size_t buflen,
+		   const char *toclose)
 {
 	struct svc_sock *svsk, *closesk = NULL;
 	int len = 0;

[PATCH 14/23] SUNRPC: Switch one_sock_name() to use snprintf()

[Chuck Lever]

Use snprintf() in one_sock_name() to prevent overflowing the output
buffer.  If the name doesn't fit in the buffer, the buffer is filled
in with an empty string, and -ENAMETOOLONG is returned.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/svcsock.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 107b887..a1bf142 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -240,22 +240,27 @@ out:
 /*
  * Report socket names for nfsdfs
  */
-static int one_sock_name(char *buf, struct svc_sock *svsk)
+static int svc_one_sock_name(struct svc_sock *svsk, char *buf, int remaining)
 {
 	int len;
 
 	switch(svsk->sk_sk->sk_family) {
-	case AF_INET:
-		len = sprintf(buf, "ipv4 %s %pI4 %d\n",
+	case PF_INET:
+		len = snprintf(buf, remaining, "ipv4 %s %pI4 %d\n",
 			      svsk->sk_sk->sk_protocol == IPPROTO_UDP ?
 			      "udp" : "tcp",
 			      &inet_sk(svsk->sk_sk)->rcv_saddr,
 			      inet_sk(svsk->sk_sk)->num);
 		break;
 	default:
-		len = sprintf(buf, "*unknown-%d*\n",
+		len = snprintf(buf, remaining, "*unknown-%d*\n",
 			       svsk->sk_sk->sk_family);
 	}
+
+	if (len >= remaining) {
+		*buf = '\0';
+		return -ENAMETOOLONG;
+	}
 	return len;
 }
 
@@ -282,15 +287,21 @@ int svc_sock_names(struct svc_serv *serv, char *buf, const size_t buflen,
 
 	if (!serv)
 		return 0;
+
 	spin_lock_bh(&serv->sv_lock);
 	list_for_each_entry(svsk, &serv->sv_permsocks, sk_xprt.xpt_list) {
-		int onelen = one_sock_name(buf+len, svsk);
-		if (toclose && strcmp(toclose, buf+len) == 0)
+		int onelen = svc_one_sock_name(svsk, buf + len, buflen - len);
+		if (onelen < 0) {
+			len = onelen;
+			break;
+		}
+		if (toclose && strcmp(toclose, buf + len) == 0)
 			closesk = svsk;
 		else
 			len += onelen;
 	}
 	spin_unlock_bh(&serv->sv_lock);
+
 	if (closesk)
 		/* Should unregister with portmap, but you cannot
 		 * unregister just one protocol...
@@ -1226,7 +1237,7 @@ int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
 		sockfd_put(so);
 		return err;
 	}
-	return one_sock_name(name_return, svsk);
+	return svc_one_sock_name(svsk, name_return, len);
 }
 EXPORT_SYMBOL_GPL(svc_addsock);
 

[PATCH 15/23] SUNRPC: Support PF_INET6 in one_sock_name()

[Chuck Lever]

Add an arm to the switch statement in svc_one_sock_name() so it can
construct the name of PF_INET6 sockets properly.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Cc: Aime Le Rouzic <aime.le-rouzic@bull.net>
---

 net/sunrpc/svcsock.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index a1bf142..6d19ef3 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -252,6 +252,13 @@ static int svc_one_sock_name(struct svc_sock *svsk, char *buf, int remaining)
 			      &inet_sk(svsk->sk_sk)->rcv_saddr,
 			      inet_sk(svsk->sk_sk)->num);
 		break;
+	case PF_INET6:
+		len = snprintf(buf, remaining, "ipv6 %s %pI6 %d\n",
+				svsk->sk_sk->sk_protocol == IPPROTO_UDP ?
+				"udp" : "tcp",
+				&inet6_sk(svsk->sk_sk)->rcv_saddr,
+				inet_sk(svsk->sk_sk)->num);
+		break;
 	default:
 		len = snprintf(buf, remaining, "*unknown-%d*\n",
 			       svsk->sk_sk->sk_family);

[PATCH 16/23] SUNRPC: Clean up one_sock_name()

[Chuck Lever]

Clean up svc_one_sock_name() by setting up automatic variables for
frequently used expressions.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/svcsock.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 6d19ef3..2033dfa 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -242,26 +242,27 @@ out:
  */
 static int svc_one_sock_name(struct svc_sock *svsk, char *buf, int remaining)
 {
+	const struct sock *sk = svsk->sk_sk;
+	const char *proto_name = sk->sk_protocol == IPPROTO_UDP ?
+							"udp" : "tcp";
 	int len;
 
-	switch(svsk->sk_sk->sk_family) {
+	switch (sk->sk_family) {
 	case PF_INET:
 		len = snprintf(buf, remaining, "ipv4 %s %pI4 %d\n",
-			      svsk->sk_sk->sk_protocol == IPPROTO_UDP ?
-			      "udp" : "tcp",
-			      &inet_sk(svsk->sk_sk)->rcv_saddr,
-			      inet_sk(svsk->sk_sk)->num);
+				proto_name,
+				&inet_sk(sk)->rcv_saddr,
+				inet_sk(sk)->num);
 		break;
 	case PF_INET6:
 		len = snprintf(buf, remaining, "ipv6 %s %pI6 %d\n",
-				svsk->sk_sk->sk_protocol == IPPROTO_UDP ?
-				"udp" : "tcp",
-				&inet6_sk(svsk->sk_sk)->rcv_saddr,
-				inet_sk(svsk->sk_sk)->num);
+				proto_name,
+				&inet6_sk(sk)->rcv_saddr,
+				inet_sk(sk)->num);
 		break;
 	default:
 		len = snprintf(buf, remaining, "*unknown-%d*\n",
-			       svsk->sk_sk->sk_family);
+				sk->sk_family);
 	}
 
 	if (len >= remaining) {

[PATCH 17/23] NFS: Move NFS client's IP address parser to nfs_common/

[Chuck Lever]

The kernel's NFS server also needs to parse presentation format IP
addresses.  Since both the client and the server can reside in their own
modules, move the parser logic to nfs_common, which can be used by either.

I expect other common IPv4/IPv6 helpers to be moved in here at some
later point.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/internal.h              |    3 -
 fs/nfs/nfs4namespace.c         |    2 +
 fs/nfs/super.c                 |  121 --------------------------------
 fs/nfs_common/Makefile         |    1 
 fs/nfs_common/nfs_addr_parse.c |  151 ++++++++++++++++++++++++++++++++++++++++
 include/linux/nfs_addr_parse.h |   29 ++++++++
 6 files changed, 184 insertions(+), 123 deletions(-)
 create mode 100644 fs/nfs_common/nfs_addr_parse.c
 create mode 100644 include/linux/nfs_addr_parse.h

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 4da6bbd..82d899f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -167,7 +167,6 @@ extern void nfs4_clear_inode(struct inode *);
 void nfs_zap_acl_cache(struct inode *inode);
 
 /* super.c */
-void nfs_parse_ip_address(char *, size_t, struct sockaddr *, size_t *);
 extern struct file_system_type nfs_xdev_fs_type;
 #ifdef CONFIG_NFS_V4
 extern struct file_system_type nfs4_xdev_fs_type;
@@ -291,8 +290,6 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
 		PAGE_SIZE - 1) >> PAGE_SHIFT;
 }
 
-#define IPV6_SCOPE_DELIMITER	'%'
-
 /*
  * Set the port number in an address.  Be agnostic about the address
  * family.
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 30befc3..660fc36 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -15,6 +15,8 @@
 #include <linux/sunrpc/clnt.h>
 #include <linux/vfs.h>
 #include <linux/inet.h>
+#include <linux/nfs_addr_parse.h>
+
 #include "internal.h"
 #include "nfs4_fs.h"
 
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 520fe6f..bf55e23 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -51,6 +51,7 @@
 #include <linux/nfs_xdr.h>
 #include <linux/magic.h>
 #include <linux/parser.h>
+#include <linux/nfs_addr_parse.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -697,126 +698,6 @@ static int nfs_verify_server_address(struct sockaddr *addr)
 	return 0;
 }
 
-static void nfs_parse_ipv4_address(char *string, size_t str_len,
-				   struct sockaddr *sap, size_t *addr_len)
-{
-	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
-
-	if (str_len <= INET_ADDRSTRLEN) {
-		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
-				(int)str_len, string);
-
-		sin->sin_family = AF_INET;
-		*addr_len = sizeof(*sin);
-		if (in4_pton(string, str_len, addr, '\0', NULL))
-			return;
-	}
-
-	sap->sa_family = AF_UNSPEC;
-	*addr_len = 0;
-}
-
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
-				   const char *delim,
-				   struct sockaddr_in6 *sin6)
-{
-	char *p;
-	size_t len;
-
-	if ((string + str_len) == delim)
-		return 1;
-
-	if (*delim != IPV6_SCOPE_DELIMITER)
-		return 0;
-
-	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
-		return 0;
-
-	len = (string + str_len) - delim - 1;
-	p = kstrndup(delim + 1, len, GFP_KERNEL);
-	if (p) {
-		unsigned long scope_id = 0;
-		struct net_device *dev;
-
-		dev = dev_get_by_name(&init_net, p);
-		if (dev != NULL) {
-			scope_id = dev->ifindex;
-			dev_put(dev);
-		} else {
-			if (strict_strtoul(p, 10, &scope_id) == 0) {
-				kfree(p);
-				return 0;
-			}
-		}
-
-		kfree(p);
-
-		sin6->sin6_scope_id = scope_id;
-		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
-		return 1;
-	}
-
-	return 0;
-}
-
-static void nfs_parse_ipv6_address(char *string, size_t str_len,
-				   struct sockaddr *sap, size_t *addr_len)
-{
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
-	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
-	const char *delim;
-
-	if (str_len <= INET6_ADDRSTRLEN) {
-		dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
-				(int)str_len, string);
-
-		sin6->sin6_family = AF_INET6;
-		*addr_len = sizeof(*sin6);
-		if (in6_pton(string, str_len, addr,
-					IPV6_SCOPE_DELIMITER, &delim) != 0) {
-			if (nfs_parse_ipv6_scope_id(string, str_len,
-							delim, sin6) != 0)
-				return;
-		}
-	}
-
-	sap->sa_family = AF_UNSPEC;
-	*addr_len = 0;
-}
-#else
-static void nfs_parse_ipv6_address(char *string, size_t str_len,
-				   struct sockaddr *sap, size_t *addr_len)
-{
-	sap->sa_family = AF_UNSPEC;
-	*addr_len = 0;
-}
-#endif
-
-/*
- * Construct a sockaddr based on the contents of a string that contains
- * an IP address in presentation format.
- *
- * If there is a problem constructing the new sockaddr, set the address
- * family to AF_UNSPEC.
- */
-void nfs_parse_ip_address(char *string, size_t str_len,
-				 struct sockaddr *sap, size_t *addr_len)
-{
-	unsigned int i, colons;
-
-	colons = 0;
-	for (i = 0; i < str_len; i++)
-		if (string[i] == ':')
-			colons++;
-
-	if (colons >= 2)
-		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
-	else
-		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
-}
-
 /*
  * Sanity check the NFS transport protocol.
  *
diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
index f689ed8..032b4c5 100644
--- a/fs/nfs_common/Makefile
+++ b/fs/nfs_common/Makefile
@@ -2,6 +2,7 @@
 # Makefile for Linux filesystem routines that are shared by client and server.
 #
 
+obj-$(CONFIG_NFS_COMMON) += nfs_addr_parse.o
 obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
 
 nfs_acl-objs := nfsacl.o
diff --git a/fs/nfs_common/nfs_addr_parse.c b/fs/nfs_common/nfs_addr_parse.c
new file mode 100644
index 0000000..529b407
--- /dev/null
+++ b/fs/nfs_common/nfs_addr_parse.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/unistd.h>
+
+#include <linux/inet.h>
+#include <linux/in6.h>
+#include <net/ipv6.h>
+#include <linux/netdevice.h>
+
+#include <linux/nfs_addr_parse.h>
+
+static void nfs_parse_ipv4_address(const char *buf, const size_t buflen,
+				   struct sockaddr *sap, size_t *salenp)
+{
+	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
+
+	if (buflen <= INET_ADDRSTRLEN) {
+		sin->sin_family = AF_INET;
+		*salenp = sizeof(*sin);
+		if (in4_pton(buf, buflen, addr, '\0', NULL))
+			return;
+	}
+
+	sap->sa_family = AF_UNSPEC;
+	*salenp = 0;
+}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+static int nfs_parse_ipv6_scope_id(const char *buf, const size_t buflen,
+				   const char *delim,
+				   struct sockaddr_in6 *sin6)
+{
+	char *p;
+	size_t len;
+
+	if ((buf + buflen) == delim)
+		return 1;
+
+	if (*delim != IPV6_SCOPE_DELIMITER)
+		return 0;
+
+	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
+		return 0;
+
+	len = (buf + buflen) - delim - 1;
+	p = kstrndup(delim + 1, len, GFP_KERNEL);
+	if (p) {
+		unsigned long scope_id = 0;
+		struct net_device *dev;
+
+		dev = dev_get_by_name(&init_net, p);
+		if (dev != NULL) {
+			scope_id = dev->ifindex;
+			dev_put(dev);
+		} else {
+			if (strict_strtoul(p, 10, &scope_id) == 0) {
+				kfree(p);
+				return 0;
+			}
+		}
+
+		kfree(p);
+
+		sin6->sin6_scope_id = scope_id;
+		return 1;
+	}
+
+	return 0;
+}
+
+static void nfs_parse_ipv6_address(const char *buf, const size_t buflen,
+				   struct sockaddr *sap, size_t *salenp)
+{
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
+	const char *delim;
+
+	if (buflen <= INET6_ADDRSTRLEN) {
+		sin6->sin6_family = AF_INET6;
+		*salenp = sizeof(*sin6);
+		if (in6_pton(buf, buflen, addr,
+					IPV6_SCOPE_DELIMITER, &delim) != 0) {
+			if (nfs_parse_ipv6_scope_id(buf, buflen,
+							delim, sin6) != 0)
+				return;
+		}
+	}
+
+	sap->sa_family = AF_UNSPEC;
+	*salenp = 0;
+}
+#else	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
+static void nfs_parse_ipv6_address(const char *buf, const size_t buflen,
+				   struct sockaddr *sap, size_t *salenp)
+{
+	sap->sa_family = AF_UNSPEC;
+	*salenp = 0;
+}
+#endif	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
+
+/**
+ * nfs_parse_ip_address - convert IP address string to sockaddr
+ * @buf: C string containing presentation format IP address
+ * @buflen: length of @buf in bytes
+ * @sap: pointer to buffer in which to construct sockaddr
+ * @salenp: IN: length of buffer in bytes
+ *	      OUT: size of address in bytes
+ *
+ * Construct a sockaddr based on the contents of a string that contains
+ * an IP address in presentation format.
+ *
+ * If there is a problem constructing the new sockaddr, set the address
+ * family to AF_UNSPEC.
+ */
+void nfs_parse_ip_address(const char *buf, const size_t buflen,
+			  struct sockaddr *sap, size_t *salenp)
+{
+	unsigned int i, colons;
+
+	colons = 0;
+	for (i = 0; i < buflen; i++)
+		if (buf[i] == ':')
+			colons++;
+
+	if (colons >= 2)
+		nfs_parse_ipv6_address(buf, buflen, sap, salenp);
+	else
+		nfs_parse_ipv4_address(buf, buflen, sap, salenp);
+}
+EXPORT_SYMBOL_GPL(nfs_parse_ip_address);
diff --git a/include/linux/nfs_addr_parse.h b/include/linux/nfs_addr_parse.h
new file mode 100644
index 0000000..193c71c
--- /dev/null
+++ b/include/linux/nfs_addr_parse.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ *
+ */
+
+#ifndef __LINUX_NFS_ADDR_PARSE_H
+#define __LINUX_NFS_ADDR_PARSE_H
+
+#define IPV6_SCOPE_DELIMITER	'%'
+
+void	nfs_parse_ip_address(const char *buf, const size_t buflen,
+				struct sockaddr *sap, size_t *salenp);
+
+#endif	/* !__LINUX_NFS_ADDR_PARSE_H */

[PATCH 18/23] NFSD: Support IPv6 addresses in write_failover_ip()

[Chuck Lever]

In write_failover_ip(), replace the sscanf() with a call to the common
NFS presentation address parser.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 69c77ad..ca42e81 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -38,6 +38,7 @@
 #include <linux/nfsd/xdr.h>
 #include <linux/nfsd/syscall.h>
 #include <linux/lockd/lockd.h>
+#include <linux/nfs_addr_parse.h>
 
 #include <asm/uaccess.h>
 #include <net/ipv6.h>
@@ -476,22 +477,18 @@ static ssize_t write_getfd(struct file *file, char *buf, size_t size)
  *
  * Input:
  *			buf:	'\n'-terminated C string containing a
- *				presentation format IPv4 address
+ *				presentation format IP address
  *			size:	length of C string in @buf
  * Output:
  *	On success:	returns zero if all specified locks were released;
  *			returns one if one or more locks were not released
  *	On error:	return code is negative errno value
- *
- * Note: Only AF_INET client addresses are passed in
  */
 static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
 {
-	struct sockaddr_in sin = {
-		.sin_family	= AF_INET,
-	};
-	int b1, b2, b3, b4;
-	char c;
+	struct sockaddr_storage address;
+	struct sockaddr *sap = (struct sockaddr *)&address;
+	size_t salen = sizeof(address);
 	char *fo_path;
 
 	/* sanity check */
@@ -505,14 +502,11 @@ static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
 	if (qword_get(&buf, fo_path, size) < 0)
 		return -EINVAL;
 
-	/* get ipv4 address */
-	if (sscanf(fo_path, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) != 4)
-		return -EINVAL;
-	if (b1 > 255 || b2 > 255 || b3 > 255 || b4 > 255)
+	nfs_parse_ip_address(fo_path, size, sap, &salen);
+	if (sap->sa_family == AF_UNSPEC)
 		return -EINVAL;
-	sin.sin_addr.s_addr = htonl((b1 << 24) | (b2 << 16) | (b3 << 8) | b4);
 
-	return nlmsvc_unlock_all_by_ip((struct sockaddr *)&sin);
+	return nlmsvc_unlock_all_by_ip(sap);
 }
 
 /**

[PATCH 19/23] NFSD: Prevent buffer overflow in write_threads()

[Chuck Lever]

Fix up write_threads() to eliminate the small possibility of
overflowing its output buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index ca42e81..64083b5 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -666,9 +666,9 @@ static ssize_t write_filehandle(struct file *file, char *buf, size_t size)
 static ssize_t write_threads(struct file *file, char *buf, size_t size)
 {
 	char *mesg = buf;
-	int rv;
+
 	if (size > 0) {
-		int newthreads;
+		int rv, newthreads;
 		rv = get_int(&mesg, &newthreads);
 		if (rv)
 			return rv;
@@ -678,8 +678,9 @@ static ssize_t write_threads(struct file *file, char *buf, size_t size)
 		if (rv)
 			return rv;
 	}
-	sprintf(buf, "%d\n", nfsd_nrthreads());
-	return strlen(buf);
+
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n",
+							nfsd_nrthreads());
 }
 
 /**

[PATCH 20/23] NFSD: Prevent buffer overflow in write_versions()

[Chuck Lever]

Fix up write_versions() to eliminate the possibility of overflowing
its output buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 64083b5..cb786bf 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -777,7 +777,7 @@ static ssize_t __write_versions(struct file *file, char *buf, size_t size)
 {
 	char *mesg = buf;
 	char *vers, sign;
-	int len, num;
+	int len, num, remaining;
 	ssize_t tlen = 0;
 	char *sep;
 
@@ -818,18 +818,28 @@ static ssize_t __write_versions(struct file *file, char *buf, size_t size)
 		 */
 		nfsd_reset_versions();
 	}
+
 	/* Now write current state into reply buffer */
-	len = 0;
+	tlen = 0;
 	sep = "";
-	for (num=2 ; num <= 4 ; num++)
+	remaining = SIMPLE_TRANSACTION_LIMIT;
+	for (num = 2 ; num <= 4 ; num++)
 		if (nfsd_vers(num, NFSD_AVAIL)) {
-			len += sprintf(buf+len, "%s%c%d", sep,
-				       nfsd_vers(num, NFSD_TEST)?'+':'-',
-				       num);
+			len = snprintf(buf, remaining, "%s%c%d", sep,
+					nfsd_vers(num, NFSD_TEST) ?  '+' : '-',
+					num);
 			sep = " ";
+
+			if (len >= remaining - 1)
+				break;
+
+			remaining -= len;
+			buf += len;
+			tlen += len;
 		}
-	len += sprintf(buf+len, "\n");
-	return len;
+	snprintf(buf, remaining, "\n");
+
+	return tlen;
 }
 
 /**

[PATCH 21/23] NFSD: Prevent buffer overflow in write_maxblksize()

[Chuck Lever]

Fix up write_maxblksize() to eliminate the slight possibility of
overflowing its output buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index cb786bf..67a0b91 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1127,6 +1127,7 @@ int nfsd_max_blksize;
 static ssize_t write_maxblksize(struct file *file, char *buf, size_t size)
 {
 	char *mesg = buf;
+
 	if (size > 0) {
 		int bsize;
 		int rv = get_int(&mesg, &bsize);
@@ -1148,7 +1149,9 @@ static ssize_t write_maxblksize(struct file *file, char *buf, size_t size)
 		nfsd_max_blksize = bsize;
 		mutex_unlock(&nfsd_mutex);
 	}
-	return sprintf(buf, "%d\n", nfsd_max_blksize);
+
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n",
+							nfsd_max_blksize);
 }
 
 #ifdef CONFIG_NFSD_V4

[PATCH 22/23] NFSD: Prevent buffer overflow in write_leasetime()

[Chuck Lever]

Fix up write_leasetime() to eliminate the small possibility of
overflowing its output buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 67a0b91..235295c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1175,8 +1175,9 @@ static ssize_t __write_leasetime(struct file *file, char *buf, size_t size)
 			return -EINVAL;
 		nfs4_reset_lease(lease);
 	}
-	sprintf(buf, "%ld\n", nfs4_lease_time());
-	return strlen(buf);
+
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%ld\n",
+							nfs4_lease_time());
 }
 
 /**

[PATCH 23/23] NFSD: Prevent buffer overflow in write_recoverydir()

[Chuck Lever]

Fix up write_recoverydir() to eliminate the possibility of overflowing
its output buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsctl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 235295c..ee7dc72 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1233,8 +1233,9 @@ static ssize_t __write_recoverydir(struct file *file, char *buf, size_t size)
 
 		status = nfs4_reset_recoverydir(recdir);
 	}
-	sprintf(buf, "%s\n", nfs4_recoverydir());
-	return strlen(buf);
+
+	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%s\n",
+							nfs4_recoverydir());
 }
 
 /**

Re: [PATCH 17/23] NFS: Move NFS client's IP address parser to nfs_common/

[Jeff Layton]

On Thu, 05 Mar 2009 16:33:31 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:

> The kernel's NFS server also needs to parse presentation format IP
> addresses.  Since both the client and the server can reside in their own
> modules, move the parser logic to nfs_common, which can be used by either.
> 
> I expect other common IPv4/IPv6 helpers to be moved in here at some
> later point.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfs/internal.h              |    3 -
>  fs/nfs/nfs4namespace.c         |    2 +
>  fs/nfs/super.c                 |  121 --------------------------------
>  fs/nfs_common/Makefile         |    1 
>  fs/nfs_common/nfs_addr_parse.c |  151 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/nfs_addr_parse.h |   29 ++++++++
>  6 files changed, 184 insertions(+), 123 deletions(-)
>  create mode 100644 fs/nfs_common/nfs_addr_parse.c
>  create mode 100644 include/linux/nfs_addr_parse.h
> 

I like the basic idea here -- moving these helpers into common code.
That said, I don't like the fact that this common code ends up in the
base kernel. IMO, a better solution would be a new module
(nfs_common.ko maybe?) that contains this code and any other common
code that's needed in the future...

> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 4da6bbd..82d899f 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -167,7 +167,6 @@ extern void nfs4_clear_inode(struct inode *);
>  void nfs_zap_acl_cache(struct inode *inode);
>  
>  /* super.c */
> -void nfs_parse_ip_address(char *, size_t, struct sockaddr *, size_t *);
>  extern struct file_system_type nfs_xdev_fs_type;
>  #ifdef CONFIG_NFS_V4
>  extern struct file_system_type nfs4_xdev_fs_type;
> @@ -291,8 +290,6 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
>  		PAGE_SIZE - 1) >> PAGE_SHIFT;
>  }
>  
> -#define IPV6_SCOPE_DELIMITER	'%'
> -
>  /*
>   * Set the port number in an address.  Be agnostic about the address
>   * family.
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index 30befc3..660fc36 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -15,6 +15,8 @@
>  #include <linux/sunrpc/clnt.h>
>  #include <linux/vfs.h>
>  #include <linux/inet.h>
> +#include <linux/nfs_addr_parse.h>
> +
>  #include "internal.h"
>  #include "nfs4_fs.h"
>  
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 520fe6f..bf55e23 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -51,6 +51,7 @@
>  #include <linux/nfs_xdr.h>
>  #include <linux/magic.h>
>  #include <linux/parser.h>
> +#include <linux/nfs_addr_parse.h>
>  
>  #include <asm/system.h>
>  #include <asm/uaccess.h>
> @@ -697,126 +698,6 @@ static int nfs_verify_server_address(struct sockaddr *addr)
>  	return 0;
>  }
>  
> -static void nfs_parse_ipv4_address(char *string, size_t str_len,
> -				   struct sockaddr *sap, size_t *addr_len)
> -{
> -	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> -	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> -
> -	if (str_len <= INET_ADDRSTRLEN) {
> -		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> -				(int)str_len, string);
> -
> -		sin->sin_family = AF_INET;
> -		*addr_len = sizeof(*sin);
> -		if (in4_pton(string, str_len, addr, '\0', NULL))
> -			return;
> -	}
> -
> -	sap->sa_family = AF_UNSPEC;
> -	*addr_len = 0;
> -}
> -
> -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> -static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
> -				   const char *delim,
> -				   struct sockaddr_in6 *sin6)
> -{
> -	char *p;
> -	size_t len;
> -
> -	if ((string + str_len) == delim)
> -		return 1;
> -
> -	if (*delim != IPV6_SCOPE_DELIMITER)
> -		return 0;
> -
> -	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
> -		return 0;
> -
> -	len = (string + str_len) - delim - 1;
> -	p = kstrndup(delim + 1, len, GFP_KERNEL);
> -	if (p) {
> -		unsigned long scope_id = 0;
> -		struct net_device *dev;
> -
> -		dev = dev_get_by_name(&init_net, p);
> -		if (dev != NULL) {
> -			scope_id = dev->ifindex;
> -			dev_put(dev);
> -		} else {
> -			if (strict_strtoul(p, 10, &scope_id) == 0) {
> -				kfree(p);
> -				return 0;
> -			}
> -		}
> -
> -		kfree(p);
> -
> -		sin6->sin6_scope_id = scope_id;
> -		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
> -		return 1;
> -	}
> -
> -	return 0;
> -}
> -
> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
> -				   struct sockaddr *sap, size_t *addr_len)
> -{
> -	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
> -	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
> -	const char *delim;
> -
> -	if (str_len <= INET6_ADDRSTRLEN) {
> -		dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
> -				(int)str_len, string);
> -
> -		sin6->sin6_family = AF_INET6;
> -		*addr_len = sizeof(*sin6);
> -		if (in6_pton(string, str_len, addr,
> -					IPV6_SCOPE_DELIMITER, &delim) != 0) {
> -			if (nfs_parse_ipv6_scope_id(string, str_len,
> -							delim, sin6) != 0)
> -				return;
> -		}
> -	}
> -
> -	sap->sa_family = AF_UNSPEC;
> -	*addr_len = 0;
> -}
> -#else
> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
> -				   struct sockaddr *sap, size_t *addr_len)
> -{
> -	sap->sa_family = AF_UNSPEC;
> -	*addr_len = 0;
> -}
> -#endif
> -
> -/*
> - * Construct a sockaddr based on the contents of a string that contains
> - * an IP address in presentation format.
> - *
> - * If there is a problem constructing the new sockaddr, set the address
> - * family to AF_UNSPEC.
> - */
> -void nfs_parse_ip_address(char *string, size_t str_len,
> -				 struct sockaddr *sap, size_t *addr_len)
> -{
> -	unsigned int i, colons;
> -
> -	colons = 0;
> -	for (i = 0; i < str_len; i++)
> -		if (string[i] == ':')
> -			colons++;
> -
> -	if (colons >= 2)
> -		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
> -	else
> -		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
> -}
> -
>  /*
>   * Sanity check the NFS transport protocol.
>   *
> diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
> index f689ed8..032b4c5 100644
> --- a/fs/nfs_common/Makefile
> +++ b/fs/nfs_common/Makefile
> @@ -2,6 +2,7 @@
>  # Makefile for Linux filesystem routines that are shared by client and server.
>  #
>  
> +obj-$(CONFIG_NFS_COMMON) += nfs_addr_parse.o
>  obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
>  
>  nfs_acl-objs := nfsacl.o
> diff --git a/fs/nfs_common/nfs_addr_parse.c b/fs/nfs_common/nfs_addr_parse.c
> new file mode 100644
> index 0000000..529b407
> --- /dev/null
> +++ b/fs/nfs_common/nfs_addr_parse.c
> @@ -0,0 +1,151 @@
> +/*
> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/errno.h>
> +#include <linux/unistd.h>
> +
> +#include <linux/inet.h>
> +#include <linux/in6.h>
> +#include <net/ipv6.h>
> +#include <linux/netdevice.h>
> +
> +#include <linux/nfs_addr_parse.h>
> +
> +static void nfs_parse_ipv4_address(const char *buf, const size_t buflen,
> +				   struct sockaddr *sap, size_t *salenp)
> +{
> +	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> +	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> +
> +	if (buflen <= INET_ADDRSTRLEN) {
> +		sin->sin_family = AF_INET;
> +		*salenp = sizeof(*sin);
> +		if (in4_pton(buf, buflen, addr, '\0', NULL))
> +			return;
> +	}
> +
> +	sap->sa_family = AF_UNSPEC;
> +	*salenp = 0;
> +}
> +
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> +static int nfs_parse_ipv6_scope_id(const char *buf, const size_t buflen,
> +				   const char *delim,
> +				   struct sockaddr_in6 *sin6)
> +{
> +	char *p;
> +	size_t len;
> +
> +	if ((buf + buflen) == delim)
> +		return 1;
> +
> +	if (*delim != IPV6_SCOPE_DELIMITER)
> +		return 0;
> +
> +	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
> +		return 0;
> +
> +	len = (buf + buflen) - delim - 1;
> +	p = kstrndup(delim + 1, len, GFP_KERNEL);
> +	if (p) {
> +		unsigned long scope_id = 0;
> +		struct net_device *dev;
> +
> +		dev = dev_get_by_name(&init_net, p);
> +		if (dev != NULL) {
> +			scope_id = dev->ifindex;
> +			dev_put(dev);
> +		} else {
> +			if (strict_strtoul(p, 10, &scope_id) == 0) {
> +				kfree(p);
> +				return 0;
> +			}
> +		}
> +
> +		kfree(p);
> +
> +		sin6->sin6_scope_id = scope_id;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void nfs_parse_ipv6_address(const char *buf, const size_t buflen,
> +				   struct sockaddr *sap, size_t *salenp)
> +{
> +	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
> +	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
> +	const char *delim;
> +
> +	if (buflen <= INET6_ADDRSTRLEN) {
> +		sin6->sin6_family = AF_INET6;
> +		*salenp = sizeof(*sin6);
> +		if (in6_pton(buf, buflen, addr,
> +					IPV6_SCOPE_DELIMITER, &delim) != 0) {
> +			if (nfs_parse_ipv6_scope_id(buf, buflen,
> +							delim, sin6) != 0)
> +				return;
> +		}
> +	}
> +
> +	sap->sa_family = AF_UNSPEC;
> +	*salenp = 0;
> +}
> +#else	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
> +static void nfs_parse_ipv6_address(const char *buf, const size_t buflen,
> +				   struct sockaddr *sap, size_t *salenp)
> +{
> +	sap->sa_family = AF_UNSPEC;
> +	*salenp = 0;
> +}
> +#endif	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
> +
> +/**
> + * nfs_parse_ip_address - convert IP address string to sockaddr
> + * @buf: C string containing presentation format IP address
> + * @buflen: length of @buf in bytes
> + * @sap: pointer to buffer in which to construct sockaddr
> + * @salenp: IN: length of buffer in bytes
> + *	      OUT: size of address in bytes
> + *
> + * Construct a sockaddr based on the contents of a string that contains
> + * an IP address in presentation format.
> + *
> + * If there is a problem constructing the new sockaddr, set the address
> + * family to AF_UNSPEC.
> + */
> +void nfs_parse_ip_address(const char *buf, const size_t buflen,
> +			  struct sockaddr *sap, size_t *salenp)
> +{
> +	unsigned int i, colons;
> +
> +	colons = 0;
> +	for (i = 0; i < buflen; i++)
> +		if (buf[i] == ':')
> +			colons++;
> +
> +	if (colons >= 2)
> +		nfs_parse_ipv6_address(buf, buflen, sap, salenp);
> +	else
> +		nfs_parse_ipv4_address(buf, buflen, sap, salenp);
> +}
> +EXPORT_SYMBOL_GPL(nfs_parse_ip_address);
> diff --git a/include/linux/nfs_addr_parse.h b/include/linux/nfs_addr_parse.h
> new file mode 100644
> index 0000000..193c71c
> --- /dev/null
> +++ b/include/linux/nfs_addr_parse.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + *
> + */
> +
> +#ifndef __LINUX_NFS_ADDR_PARSE_H
> +#define __LINUX_NFS_ADDR_PARSE_H
> +
> +#define IPV6_SCOPE_DELIMITER	'%'
> +
> +void	nfs_parse_ip_address(const char *buf, const size_t buflen,
> +				struct sockaddr *sap, size_t *salenp);
> +
> +#endif	/* !__LINUX_NFS_ADDR_PARSE_H */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 17/23] NFS: Move NFS client's IP address parser to nfs_common/

[Chuck Lever]

Hi Jeff-

On Mar 5, 2009, at Mar 5, 2009, 9:10 PM, Jeff Layton wrote:
> On Thu, 05 Mar 2009 16:33:31 -0500
> Chuck Lever <chuck.lever@oracle.com> wrote:
>
>> The kernel's NFS server also needs to parse presentation format IP
>> addresses.  Since both the client and the server can reside in  
>> their own
>> modules, move the parser logic to nfs_common, which can be used by  
>> either.
>>
>> I expect other common IPv4/IPv6 helpers to be moved in here at some
>> later point.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>> fs/nfs/internal.h              |    3 -
>> fs/nfs/nfs4namespace.c         |    2 +
>> fs/nfs/super.c                 |  121  
>> --------------------------------
>> fs/nfs_common/Makefile         |    1
>> fs/nfs_common/nfs_addr_parse.c |  151 ++++++++++++++++++++++++++++++ 
>> ++++++++++
>> include/linux/nfs_addr_parse.h |   29 ++++++++
>> 6 files changed, 184 insertions(+), 123 deletions(-)
>> create mode 100644 fs/nfs_common/nfs_addr_parse.c
>> create mode 100644 include/linux/nfs_addr_parse.h
>
> I like the basic idea here -- moving these helpers into common code.
> That said, I don't like the fact that this common code ends up in the
> base kernel.

Can you explain why?  The only reason you've stated in the past is  
because it is slightly inconvenient for those who update to a kernel  
with this code, but build NFS in modules.  In that one case you have  
to do a full kernel build to pick up these functions.

Really, this stuff should be in some common place, like net/core/  
(which is built-in), so these can be shared with, say, NLM, and CIFS,  
as well.

> IMO, a better solution would be a new module
> (nfs_common.ko maybe?) that contains this code and any other common
> code that's needed in the future...

Who would load it in that case?  I think putting it in a module would  
just add unneeded complexity in each of the callers.  Until the amount  
of code in nfs_common.c is larger than the NFS, NLM, and NFSD logic to  
"load the common module" you'll need, you don't gain anything.

Since we are close to the 2.6.30 merge window, I'd prefer to worry  
about that kind of micro-optimization later.

>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>> index 4da6bbd..82d899f 100644
>> --- a/fs/nfs/internal.h
>> +++ b/fs/nfs/internal.h
>> @@ -167,7 +167,6 @@ extern void nfs4_clear_inode(struct inode *);
>> void nfs_zap_acl_cache(struct inode *inode);
>>
>> /* super.c */
>> -void nfs_parse_ip_address(char *, size_t, struct sockaddr *,  
>> size_t *);
>> extern struct file_system_type nfs_xdev_fs_type;
>> #ifdef CONFIG_NFS_V4
>> extern struct file_system_type nfs4_xdev_fs_type;
>> @@ -291,8 +290,6 @@ unsigned int nfs_page_array_len(unsigned int  
>> base, size_t len)
>> 		PAGE_SIZE - 1) >> PAGE_SHIFT;
>> }
>>
>> -#define IPV6_SCOPE_DELIMITER	'%'
>> -
>> /*
>>  * Set the port number in an address.  Be agnostic about the address
>>  * family.
>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>> index 30befc3..660fc36 100644
>> --- a/fs/nfs/nfs4namespace.c
>> +++ b/fs/nfs/nfs4namespace.c
>> @@ -15,6 +15,8 @@
>> #include <linux/sunrpc/clnt.h>
>> #include <linux/vfs.h>
>> #include <linux/inet.h>
>> +#include <linux/nfs_addr_parse.h>
>> +
>> #include "internal.h"
>> #include "nfs4_fs.h"
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index 520fe6f..bf55e23 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -51,6 +51,7 @@
>> #include <linux/nfs_xdr.h>
>> #include <linux/magic.h>
>> #include <linux/parser.h>
>> +#include <linux/nfs_addr_parse.h>
>>
>> #include <asm/system.h>
>> #include <asm/uaccess.h>
>> @@ -697,126 +698,6 @@ static int nfs_verify_server_address(struct  
>> sockaddr *addr)
>> 	return 0;
>> }
>>
>> -static void nfs_parse_ipv4_address(char *string, size_t str_len,
>> -				   struct sockaddr *sap, size_t *addr_len)
>> -{
>> -	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
>> -	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
>> -
>> -	if (str_len <= INET_ADDRSTRLEN) {
>> -		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
>> -				(int)str_len, string);
>> -
>> -		sin->sin_family = AF_INET;
>> -		*addr_len = sizeof(*sin);
>> -		if (in4_pton(string, str_len, addr, '\0', NULL))
>> -			return;
>> -	}
>> -
>> -	sap->sa_family = AF_UNSPEC;
>> -	*addr_len = 0;
>> -}
>> -
>> -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>> -static int nfs_parse_ipv6_scope_id(const char *string, const  
>> size_t str_len,
>> -				   const char *delim,
>> -				   struct sockaddr_in6 *sin6)
>> -{
>> -	char *p;
>> -	size_t len;
>> -
>> -	if ((string + str_len) == delim)
>> -		return 1;
>> -
>> -	if (*delim != IPV6_SCOPE_DELIMITER)
>> -		return 0;
>> -
>> -	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
>> -		return 0;
>> -
>> -	len = (string + str_len) - delim - 1;
>> -	p = kstrndup(delim + 1, len, GFP_KERNEL);
>> -	if (p) {
>> -		unsigned long scope_id = 0;
>> -		struct net_device *dev;
>> -
>> -		dev = dev_get_by_name(&init_net, p);
>> -		if (dev != NULL) {
>> -			scope_id = dev->ifindex;
>> -			dev_put(dev);
>> -		} else {
>> -			if (strict_strtoul(p, 10, &scope_id) == 0) {
>> -				kfree(p);
>> -				return 0;
>> -			}
>> -		}
>> -
>> -		kfree(p);
>> -
>> -		sin6->sin6_scope_id = scope_id;
>> -		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
>> -		return 1;
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
>> -				   struct sockaddr *sap, size_t *addr_len)
>> -{
>> -	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
>> -	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
>> -	const char *delim;
>> -
>> -	if (str_len <= INET6_ADDRSTRLEN) {
>> -		dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
>> -				(int)str_len, string);
>> -
>> -		sin6->sin6_family = AF_INET6;
>> -		*addr_len = sizeof(*sin6);
>> -		if (in6_pton(string, str_len, addr,
>> -					IPV6_SCOPE_DELIMITER, &delim) != 0) {
>> -			if (nfs_parse_ipv6_scope_id(string, str_len,
>> -							delim, sin6) != 0)
>> -				return;
>> -		}
>> -	}
>> -
>> -	sap->sa_family = AF_UNSPEC;
>> -	*addr_len = 0;
>> -}
>> -#else
>> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
>> -				   struct sockaddr *sap, size_t *addr_len)
>> -{
>> -	sap->sa_family = AF_UNSPEC;
>> -	*addr_len = 0;
>> -}
>> -#endif
>> -
>> -/*
>> - * Construct a sockaddr based on the contents of a string that  
>> contains
>> - * an IP address in presentation format.
>> - *
>> - * If there is a problem constructing the new sockaddr, set the  
>> address
>> - * family to AF_UNSPEC.
>> - */
>> -void nfs_parse_ip_address(char *string, size_t str_len,
>> -				 struct sockaddr *sap, size_t *addr_len)
>> -{
>> -	unsigned int i, colons;
>> -
>> -	colons = 0;
>> -	for (i = 0; i < str_len; i++)
>> -		if (string[i] == ':')
>> -			colons++;
>> -
>> -	if (colons >= 2)
>> -		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
>> -	else
>> -		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
>> -}
>> -
>> /*
>>  * Sanity check the NFS transport protocol.
>>  *
>> diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
>> index f689ed8..032b4c5 100644
>> --- a/fs/nfs_common/Makefile
>> +++ b/fs/nfs_common/Makefile
>> @@ -2,6 +2,7 @@
>> # Makefile for Linux filesystem routines that are shared by client  
>> and server.
>> #
>>
>> +obj-$(CONFIG_NFS_COMMON) += nfs_addr_parse.o
>> obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
>>
>> nfs_acl-objs := nfsacl.o
>> diff --git a/fs/nfs_common/nfs_addr_parse.c b/fs/nfs_common/ 
>> nfs_addr_parse.c
>> new file mode 100644
>> index 0000000..529b407
>> --- /dev/null
>> +++ b/fs/nfs_common/nfs_addr_parse.c
>> @@ -0,0 +1,151 @@
>> +/*
>> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; if not, write to the
>> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> + * Boston, MA 021110-1307, USA.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/string.h>
>> +#include <linux/errno.h>
>> +#include <linux/unistd.h>
>> +
>> +#include <linux/inet.h>
>> +#include <linux/in6.h>
>> +#include <net/ipv6.h>
>> +#include <linux/netdevice.h>
>> +
>> +#include <linux/nfs_addr_parse.h>
>> +
>> +static void nfs_parse_ipv4_address(const char *buf, const size_t  
>> buflen,
>> +				   struct sockaddr *sap, size_t *salenp)
>> +{
>> +	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
>> +	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
>> +
>> +	if (buflen <= INET_ADDRSTRLEN) {
>> +		sin->sin_family = AF_INET;
>> +		*salenp = sizeof(*sin);
>> +		if (in4_pton(buf, buflen, addr, '\0', NULL))
>> +			return;
>> +	}
>> +
>> +	sap->sa_family = AF_UNSPEC;
>> +	*salenp = 0;
>> +}
>> +
>> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>> +static int nfs_parse_ipv6_scope_id(const char *buf, const size_t  
>> buflen,
>> +				   const char *delim,
>> +				   struct sockaddr_in6 *sin6)
>> +{
>> +	char *p;
>> +	size_t len;
>> +
>> +	if ((buf + buflen) == delim)
>> +		return 1;
>> +
>> +	if (*delim != IPV6_SCOPE_DELIMITER)
>> +		return 0;
>> +
>> +	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
>> +		return 0;
>> +
>> +	len = (buf + buflen) - delim - 1;
>> +	p = kstrndup(delim + 1, len, GFP_KERNEL);
>> +	if (p) {
>> +		unsigned long scope_id = 0;
>> +		struct net_device *dev;
>> +
>> +		dev = dev_get_by_name(&init_net, p);
>> +		if (dev != NULL) {
>> +			scope_id = dev->ifindex;
>> +			dev_put(dev);
>> +		} else {
>> +			if (strict_strtoul(p, 10, &scope_id) == 0) {
>> +				kfree(p);
>> +				return 0;
>> +			}
>> +		}
>> +
>> +		kfree(p);
>> +
>> +		sin6->sin6_scope_id = scope_id;
>> +		return 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void nfs_parse_ipv6_address(const char *buf, const size_t  
>> buflen,
>> +				   struct sockaddr *sap, size_t *salenp)
>> +{
>> +	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
>> +	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
>> +	const char *delim;
>> +
>> +	if (buflen <= INET6_ADDRSTRLEN) {
>> +		sin6->sin6_family = AF_INET6;
>> +		*salenp = sizeof(*sin6);
>> +		if (in6_pton(buf, buflen, addr,
>> +					IPV6_SCOPE_DELIMITER, &delim) != 0) {
>> +			if (nfs_parse_ipv6_scope_id(buf, buflen,
>> +							delim, sin6) != 0)
>> +				return;
>> +		}
>> +	}
>> +
>> +	sap->sa_family = AF_UNSPEC;
>> +	*salenp = 0;
>> +}
>> +#else	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
>> +static void nfs_parse_ipv6_address(const char *buf, const size_t  
>> buflen,
>> +				   struct sockaddr *sap, size_t *salenp)
>> +{
>> +	sap->sa_family = AF_UNSPEC;
>> +	*salenp = 0;
>> +}
>> +#endif	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
>> +
>> +/**
>> + * nfs_parse_ip_address - convert IP address string to sockaddr
>> + * @buf: C string containing presentation format IP address
>> + * @buflen: length of @buf in bytes
>> + * @sap: pointer to buffer in which to construct sockaddr
>> + * @salenp: IN: length of buffer in bytes
>> + *	      OUT: size of address in bytes
>> + *
>> + * Construct a sockaddr based on the contents of a string that  
>> contains
>> + * an IP address in presentation format.
>> + *
>> + * If there is a problem constructing the new sockaddr, set the  
>> address
>> + * family to AF_UNSPEC.
>> + */
>> +void nfs_parse_ip_address(const char *buf, const size_t buflen,
>> +			  struct sockaddr *sap, size_t *salenp)
>> +{
>> +	unsigned int i, colons;
>> +
>> +	colons = 0;
>> +	for (i = 0; i < buflen; i++)
>> +		if (buf[i] == ':')
>> +			colons++;
>> +
>> +	if (colons >= 2)
>> +		nfs_parse_ipv6_address(buf, buflen, sap, salenp);
>> +	else
>> +		nfs_parse_ipv4_address(buf, buflen, sap, salenp);
>> +}
>> +EXPORT_SYMBOL_GPL(nfs_parse_ip_address);
>> diff --git a/include/linux/nfs_addr_parse.h b/include/linux/ 
>> nfs_addr_parse.h
>> new file mode 100644
>> index 0000000..193c71c
>> --- /dev/null
>> +++ b/include/linux/nfs_addr_parse.h
>> @@ -0,0 +1,29 @@
>> +/*
>> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this program; if not, write to the
>> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
>> + * Boston, MA 021110-1307, USA.
>> + *
>> + */
>> +
>> +#ifndef __LINUX_NFS_ADDR_PARSE_H
>> +#define __LINUX_NFS_ADDR_PARSE_H
>> +
>> +#define IPV6_SCOPE_DELIMITER	'%'
>> +
>> +void	nfs_parse_ip_address(const char *buf, const size_t buflen,
>> +				struct sockaddr *sap, size_t *salenp);
>> +
>> +#endif	/* !__LINUX_NFS_ADDR_PARSE_H */
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux- 
>> nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
> -- 
> Jeff Layton <jlayton@redhat.com>

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

Re: [PATCH 17/23] NFS: Move NFS client's IP address parser to nfs_common/

[Jeff Layton]

On Fri, 6 Mar 2009 11:16:44 -0500
Chuck Lever <chuck.lever@oracle.com> wrote:

> Hi Jeff-
> 
> On Mar 5, 2009, at Mar 5, 2009, 9:10 PM, Jeff Layton wrote:
> > On Thu, 05 Mar 2009 16:33:31 -0500
> > Chuck Lever <chuck.lever@oracle.com> wrote:
> >
> >> The kernel's NFS server also needs to parse presentation format IP
> >> addresses.  Since both the client and the server can reside in  
> >> their own
> >> modules, move the parser logic to nfs_common, which can be used by  
> >> either.
> >>
> >> I expect other common IPv4/IPv6 helpers to be moved in here at some
> >> later point.
> >>
> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> >> ---
> >>
> >> fs/nfs/internal.h              |    3 -
> >> fs/nfs/nfs4namespace.c         |    2 +
> >> fs/nfs/super.c                 |  121  
> >> --------------------------------
> >> fs/nfs_common/Makefile         |    1
> >> fs/nfs_common/nfs_addr_parse.c |  151 ++++++++++++++++++++++++++++++ 
> >> ++++++++++
> >> include/linux/nfs_addr_parse.h |   29 ++++++++
> >> 6 files changed, 184 insertions(+), 123 deletions(-)
> >> create mode 100644 fs/nfs_common/nfs_addr_parse.c
> >> create mode 100644 include/linux/nfs_addr_parse.h
> >
> > I like the basic idea here -- moving these helpers into common code.
> > That said, I don't like the fact that this common code ends up in the
> > base kernel.
> 
> Can you explain why?  The only reason you've stated in the past is  
> because it is slightly inconvenient for those who update to a kernel  
> with this code, but build NFS in modules.  In that one case you have  
> to do a full kernel build to pick up these functions.
>

Sure, that's inconvenient but I can work around it and it's not a
big deal. My main objection is because it doesn't seem like
nfs-specific routines ought to be in core kernel code. They should be
in modules.
 
> Really, this stuff should be in some common place, like net/core/  
> (which is built-in), so these can be shared with, say, NLM, and CIFS,  
> as well.
>

Ok, I'll buy that. But if you're going to make them generic routines
it would be preferable to give them generic names and fix the current
callers. Still though, for those that aren't using networked filesystems
and such, putting this in the core kernel seems like a waste (albeit a
small one).

> > IMO, a better solution would be a new module
> > (nfs_common.ko maybe?) that contains this code and any other common
> > code that's needed in the future...
> 
> Who would load it in that case?  I think putting it in a module would  
> just add unneeded complexity in each of the callers.  Until the amount  
> of code in nfs_common.c is larger than the NFS, NLM, and NFSD logic to  
> "load the common module" you'll need, you don't gain anything.
> 

nfs.ko and nfsd.ko would load it in that case via module dependencies.
If you really wanted to make this generic library-style code, you could
call it libnetfs.ko or something. Then CIFS and other filesystems could
be changed to depend on it too.

> Since we are close to the 2.6.30 merge window, I'd prefer to worry  
> about that kind of micro-optimization later.
> 

Ok. I don't feel terribly strongly about it, but if it might be
better to wait on splitting this code out until it's closer to its
final form (in a module and/or with genericized names).

> >> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> >> index 4da6bbd..82d899f 100644
> >> --- a/fs/nfs/internal.h
> >> +++ b/fs/nfs/internal.h
> >> @@ -167,7 +167,6 @@ extern void nfs4_clear_inode(struct inode *);
> >> void nfs_zap_acl_cache(struct inode *inode);
> >>
> >> /* super.c */
> >> -void nfs_parse_ip_address(char *, size_t, struct sockaddr *,  
> >> size_t *);
> >> extern struct file_system_type nfs_xdev_fs_type;
> >> #ifdef CONFIG_NFS_V4
> >> extern struct file_system_type nfs4_xdev_fs_type;
> >> @@ -291,8 +290,6 @@ unsigned int nfs_page_array_len(unsigned int  
> >> base, size_t len)
> >> 		PAGE_SIZE - 1) >> PAGE_SHIFT;
> >> }
> >>
> >> -#define IPV6_SCOPE_DELIMITER	'%'
> >> -
> >> /*
> >>  * Set the port number in an address.  Be agnostic about the address
> >>  * family.
> >> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> >> index 30befc3..660fc36 100644
> >> --- a/fs/nfs/nfs4namespace.c
> >> +++ b/fs/nfs/nfs4namespace.c
> >> @@ -15,6 +15,8 @@
> >> #include <linux/sunrpc/clnt.h>
> >> #include <linux/vfs.h>
> >> #include <linux/inet.h>
> >> +#include <linux/nfs_addr_parse.h>
> >> +
> >> #include "internal.h"
> >> #include "nfs4_fs.h"
> >>
> >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> >> index 520fe6f..bf55e23 100644
> >> --- a/fs/nfs/super.c
> >> +++ b/fs/nfs/super.c
> >> @@ -51,6 +51,7 @@
> >> #include <linux/nfs_xdr.h>
> >> #include <linux/magic.h>
> >> #include <linux/parser.h>
> >> +#include <linux/nfs_addr_parse.h>
> >>
> >> #include <asm/system.h>
> >> #include <asm/uaccess.h>
> >> @@ -697,126 +698,6 @@ static int nfs_verify_server_address(struct  
> >> sockaddr *addr)
> >> 	return 0;
> >> }
> >>
> >> -static void nfs_parse_ipv4_address(char *string, size_t str_len,
> >> -				   struct sockaddr *sap, size_t *addr_len)
> >> -{
> >> -	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> >> -	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> >> -
> >> -	if (str_len <= INET_ADDRSTRLEN) {
> >> -		dfprintk(MOUNT, "NFS: parsing IPv4 address %*s\n",
> >> -				(int)str_len, string);
> >> -
> >> -		sin->sin_family = AF_INET;
> >> -		*addr_len = sizeof(*sin);
> >> -		if (in4_pton(string, str_len, addr, '\0', NULL))
> >> -			return;
> >> -	}
> >> -
> >> -	sap->sa_family = AF_UNSPEC;
> >> -	*addr_len = 0;
> >> -}
> >> -
> >> -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> >> -static int nfs_parse_ipv6_scope_id(const char *string, const  
> >> size_t str_len,
> >> -				   const char *delim,
> >> -				   struct sockaddr_in6 *sin6)
> >> -{
> >> -	char *p;
> >> -	size_t len;
> >> -
> >> -	if ((string + str_len) == delim)
> >> -		return 1;
> >> -
> >> -	if (*delim != IPV6_SCOPE_DELIMITER)
> >> -		return 0;
> >> -
> >> -	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
> >> -		return 0;
> >> -
> >> -	len = (string + str_len) - delim - 1;
> >> -	p = kstrndup(delim + 1, len, GFP_KERNEL);
> >> -	if (p) {
> >> -		unsigned long scope_id = 0;
> >> -		struct net_device *dev;
> >> -
> >> -		dev = dev_get_by_name(&init_net, p);
> >> -		if (dev != NULL) {
> >> -			scope_id = dev->ifindex;
> >> -			dev_put(dev);
> >> -		} else {
> >> -			if (strict_strtoul(p, 10, &scope_id) == 0) {
> >> -				kfree(p);
> >> -				return 0;
> >> -			}
> >> -		}
> >> -
> >> -		kfree(p);
> >> -
> >> -		sin6->sin6_scope_id = scope_id;
> >> -		dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
> >> -		return 1;
> >> -	}
> >> -
> >> -	return 0;
> >> -}
> >> -
> >> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
> >> -				   struct sockaddr *sap, size_t *addr_len)
> >> -{
> >> -	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
> >> -	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
> >> -	const char *delim;
> >> -
> >> -	if (str_len <= INET6_ADDRSTRLEN) {
> >> -		dfprintk(MOUNT, "NFS: parsing IPv6 address %*s\n",
> >> -				(int)str_len, string);
> >> -
> >> -		sin6->sin6_family = AF_INET6;
> >> -		*addr_len = sizeof(*sin6);
> >> -		if (in6_pton(string, str_len, addr,
> >> -					IPV6_SCOPE_DELIMITER, &delim) != 0) {
> >> -			if (nfs_parse_ipv6_scope_id(string, str_len,
> >> -							delim, sin6) != 0)
> >> -				return;
> >> -		}
> >> -	}
> >> -
> >> -	sap->sa_family = AF_UNSPEC;
> >> -	*addr_len = 0;
> >> -}
> >> -#else
> >> -static void nfs_parse_ipv6_address(char *string, size_t str_len,
> >> -				   struct sockaddr *sap, size_t *addr_len)
> >> -{
> >> -	sap->sa_family = AF_UNSPEC;
> >> -	*addr_len = 0;
> >> -}
> >> -#endif
> >> -
> >> -/*
> >> - * Construct a sockaddr based on the contents of a string that  
> >> contains
> >> - * an IP address in presentation format.
> >> - *
> >> - * If there is a problem constructing the new sockaddr, set the  
> >> address
> >> - * family to AF_UNSPEC.
> >> - */
> >> -void nfs_parse_ip_address(char *string, size_t str_len,
> >> -				 struct sockaddr *sap, size_t *addr_len)
> >> -{
> >> -	unsigned int i, colons;
> >> -
> >> -	colons = 0;
> >> -	for (i = 0; i < str_len; i++)
> >> -		if (string[i] == ':')
> >> -			colons++;
> >> -
> >> -	if (colons >= 2)
> >> -		nfs_parse_ipv6_address(string, str_len, sap, addr_len);
> >> -	else
> >> -		nfs_parse_ipv4_address(string, str_len, sap, addr_len);
> >> -}
> >> -
> >> /*
> >>  * Sanity check the NFS transport protocol.
> >>  *
> >> diff --git a/fs/nfs_common/Makefile b/fs/nfs_common/Makefile
> >> index f689ed8..032b4c5 100644
> >> --- a/fs/nfs_common/Makefile
> >> +++ b/fs/nfs_common/Makefile
> >> @@ -2,6 +2,7 @@
> >> # Makefile for Linux filesystem routines that are shared by client  
> >> and server.
> >> #
> >>
> >> +obj-$(CONFIG_NFS_COMMON) += nfs_addr_parse.o
> >> obj-$(CONFIG_NFS_ACL_SUPPORT) += nfs_acl.o
> >>
> >> nfs_acl-objs := nfsacl.o
> >> diff --git a/fs/nfs_common/nfs_addr_parse.c b/fs/nfs_common/ 
> >> nfs_addr_parse.c
> >> new file mode 100644
> >> index 0000000..529b407
> >> --- /dev/null
> >> +++ b/fs/nfs_common/nfs_addr_parse.c
> >> @@ -0,0 +1,151 @@
> >> +/*
> >> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2 of the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public
> >> + * License along with this program; if not, write to the
> >> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> >> + * Boston, MA 021110-1307, USA.
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/string.h>
> >> +#include <linux/errno.h>
> >> +#include <linux/unistd.h>
> >> +
> >> +#include <linux/inet.h>
> >> +#include <linux/in6.h>
> >> +#include <net/ipv6.h>
> >> +#include <linux/netdevice.h>
> >> +
> >> +#include <linux/nfs_addr_parse.h>
> >> +
> >> +static void nfs_parse_ipv4_address(const char *buf, const size_t  
> >> buflen,
> >> +				   struct sockaddr *sap, size_t *salenp)
> >> +{
> >> +	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> >> +	u8 *addr = (u8 *)&sin->sin_addr.s_addr;
> >> +
> >> +	if (buflen <= INET_ADDRSTRLEN) {
> >> +		sin->sin_family = AF_INET;
> >> +		*salenp = sizeof(*sin);
> >> +		if (in4_pton(buf, buflen, addr, '\0', NULL))
> >> +			return;
> >> +	}
> >> +
> >> +	sap->sa_family = AF_UNSPEC;
> >> +	*salenp = 0;
> >> +}
> >> +
> >> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> >> +static int nfs_parse_ipv6_scope_id(const char *buf, const size_t  
> >> buflen,
> >> +				   const char *delim,
> >> +				   struct sockaddr_in6 *sin6)
> >> +{
> >> +	char *p;
> >> +	size_t len;
> >> +
> >> +	if ((buf + buflen) == delim)
> >> +		return 1;
> >> +
> >> +	if (*delim != IPV6_SCOPE_DELIMITER)
> >> +		return 0;
> >> +
> >> +	if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
> >> +		return 0;
> >> +
> >> +	len = (buf + buflen) - delim - 1;
> >> +	p = kstrndup(delim + 1, len, GFP_KERNEL);
> >> +	if (p) {
> >> +		unsigned long scope_id = 0;
> >> +		struct net_device *dev;
> >> +
> >> +		dev = dev_get_by_name(&init_net, p);
> >> +		if (dev != NULL) {
> >> +			scope_id = dev->ifindex;
> >> +			dev_put(dev);
> >> +		} else {
> >> +			if (strict_strtoul(p, 10, &scope_id) == 0) {
> >> +				kfree(p);
> >> +				return 0;
> >> +			}
> >> +		}
> >> +
> >> +		kfree(p);
> >> +
> >> +		sin6->sin6_scope_id = scope_id;
> >> +		return 1;
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void nfs_parse_ipv6_address(const char *buf, const size_t  
> >> buflen,
> >> +				   struct sockaddr *sap, size_t *salenp)
> >> +{
> >> +	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
> >> +	u8 *addr = (u8 *)&sin6->sin6_addr.in6_u;
> >> +	const char *delim;
> >> +
> >> +	if (buflen <= INET6_ADDRSTRLEN) {
> >> +		sin6->sin6_family = AF_INET6;
> >> +		*salenp = sizeof(*sin6);
> >> +		if (in6_pton(buf, buflen, addr,
> >> +					IPV6_SCOPE_DELIMITER, &delim) != 0) {
> >> +			if (nfs_parse_ipv6_scope_id(buf, buflen,
> >> +							delim, sin6) != 0)
> >> +				return;
> >> +		}
> >> +	}
> >> +
> >> +	sap->sa_family = AF_UNSPEC;
> >> +	*salenp = 0;
> >> +}
> >> +#else	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
> >> +static void nfs_parse_ipv6_address(const char *buf, const size_t  
> >> buflen,
> >> +				   struct sockaddr *sap, size_t *salenp)
> >> +{
> >> +	sap->sa_family = AF_UNSPEC;
> >> +	*salenp = 0;
> >> +}
> >> +#endif	/* !(CONFIG_IPV6 || CONFIG_IPV6_MODULE) */
> >> +
> >> +/**
> >> + * nfs_parse_ip_address - convert IP address string to sockaddr
> >> + * @buf: C string containing presentation format IP address
> >> + * @buflen: length of @buf in bytes
> >> + * @sap: pointer to buffer in which to construct sockaddr
> >> + * @salenp: IN: length of buffer in bytes
> >> + *	      OUT: size of address in bytes
> >> + *
> >> + * Construct a sockaddr based on the contents of a string that  
> >> contains
> >> + * an IP address in presentation format.
> >> + *
> >> + * If there is a problem constructing the new sockaddr, set the  
> >> address
> >> + * family to AF_UNSPEC.
> >> + */
> >> +void nfs_parse_ip_address(const char *buf, const size_t buflen,
> >> +			  struct sockaddr *sap, size_t *salenp)
> >> +{
> >> +	unsigned int i, colons;
> >> +
> >> +	colons = 0;
> >> +	for (i = 0; i < buflen; i++)
> >> +		if (buf[i] == ':')
> >> +			colons++;
> >> +
> >> +	if (colons >= 2)
> >> +		nfs_parse_ipv6_address(buf, buflen, sap, salenp);
> >> +	else
> >> +		nfs_parse_ipv4_address(buf, buflen, sap, salenp);
> >> +}
> >> +EXPORT_SYMBOL_GPL(nfs_parse_ip_address);
> >> diff --git a/include/linux/nfs_addr_parse.h b/include/linux/ 
> >> nfs_addr_parse.h
> >> new file mode 100644
> >> index 0000000..193c71c
> >> --- /dev/null
> >> +++ b/include/linux/nfs_addr_parse.h
> >> @@ -0,0 +1,29 @@
> >> +/*
> >> + * Copyright (C) 2008 Oracle Corporation.  All rights reserved.
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2 of the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public
> >> + * License along with this program; if not, write to the
> >> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> >> + * Boston, MA 021110-1307, USA.
> >> + *
> >> + */
> >> +
> >> +#ifndef __LINUX_NFS_ADDR_PARSE_H
> >> +#define __LINUX_NFS_ADDR_PARSE_H
> >> +
> >> +#define IPV6_SCOPE_DELIMITER	'%'
> >> +
> >> +void	nfs_parse_ip_address(const char *buf, const size_t buflen,
> >> +				struct sockaddr *sap, size_t *salenp);
> >> +
> >> +#endif	/* !__LINUX_NFS_ADDR_PARSE_H */
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux- 
> >> nfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >
> >
> > -- 
> > Jeff Layton <jlayton@redhat.com>
> 


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 10/23] NFSD: move lockd_up() before svc_addsock()

[Chuck Lever]

On Mar 5, 2009, at Mar 5, 2009, 4:32 PM, Chuck Lever wrote:

> Clean up.
>
> A couple of years ago, a series of commits, finishing with commit
> 5680c446, swapped the order of the lockd_up() and svc_addsock() calls
> in __write_ports().  At that time lockd_up() needed to know the
> transport protocol of the passed-in socket to start a listener on the
> same transport protocol.
>
> These days, lockd_up() doesn't take a protocol argument; it always
> starts both a UDP and TCP listener.  It's now more straightforward to
> try the lockd_up() first, then do a lockd_down() if the svc_addsock()
> fails.
>
> Careful review of this code shows that the svc_sock_names() call is
> used only to close the just-opened socket in case lockd_up() fails.
> So it is no longer needed if lockd_up() is done first.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>
> fs/nfsd/nfsctl.c |   19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 80f773b..aa60191 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -908,17 +908,18 @@ static ssize_t __write_ports_addfd(char *buf)
> 	if (err != 0)
> 		return err;
>
> -	err = svc_addsock(nfsd_serv, fd, buf);
> -	if (err >= 0) {
> -		err = lockd_up();
> -		if (err < 0)
> -			svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf);
> +	err = lockd_up();
> +	if (err != 0)
> +		goto out;
>
> -		/* Decrease the count, but don't shut down the service */
> -		nfsd_serv->sv_nrthreads--;
> -	}
> +	err = svc_addsock(nfsd_serv, fd, buf);
> +	if (err != 0)

Found the "lockd: no users" BUG... this line needs to be "if (err <  
0)".  Fix tested by Jeff Layton.

> +		lockd_down();
>
> -	return err < 0 ? err : 0;
> +out:
> +	/* Decrease the count, but don't shut down the service */
> +	nfsd_serv->sv_nrthreads--;
> +	return err;
> }
>
> /*

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

Re: [PATCH 10/23] NFSD: move lockd_up() before svc_addsock()

[J. Bruce Fields]

On Mon, Mar 09, 2009 at 02:15:11PM -0400, Chuck Lever wrote:
>
> On Mar 5, 2009, at Mar 5, 2009, 4:32 PM, Chuck Lever wrote:
>
>> Clean up.
>>
>> A couple of years ago, a series of commits, finishing with commit
>> 5680c446, swapped the order of the lockd_up() and svc_addsock() calls
>> in __write_ports().  At that time lockd_up() needed to know the
>> transport protocol of the passed-in socket to start a listener on the
>> same transport protocol.
>>
>> These days, lockd_up() doesn't take a protocol argument; it always
>> starts both a UDP and TCP listener.  It's now more straightforward to
>> try the lockd_up() first, then do a lockd_down() if the svc_addsock()
>> fails.
>>
>> Careful review of this code shows that the svc_sock_names() call is
>> used only to close the just-opened socket in case lockd_up() fails.
>> So it is no longer needed if lockd_up() is done first.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>> fs/nfsd/nfsctl.c |   19 ++++++++++---------
>> 1 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 80f773b..aa60191 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -908,17 +908,18 @@ static ssize_t __write_ports_addfd(char *buf)
>> 	if (err != 0)
>> 		return err;
>>
>> -	err = svc_addsock(nfsd_serv, fd, buf);
>> -	if (err >= 0) {
>> -		err = lockd_up();
>> -		if (err < 0)
>> -			svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf);
>> +	err = lockd_up();
>> +	if (err != 0)
>> +		goto out;
>>
>> -		/* Decrease the count, but don't shut down the service */
>> -		nfsd_serv->sv_nrthreads--;
>> -	}
>> +	err = svc_addsock(nfsd_serv, fd, buf);
>> +	if (err != 0)
>
> Found the "lockd: no users" BUG... this line needs to be "if (err < 0)".  
> Fix tested by Jeff Layton.

OK, good.  Will you resend that one patch at some point?  Are there any
known problems left with that series?

--b.

>
>> +		lockd_down();
>>
>> -	return err < 0 ? err : 0;
>> +out:
>> +	/* Decrease the count, but don't shut down the service */
>> +	nfsd_serv->sv_nrthreads--;
>> +	return err;
>> }
>>
>> /*
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 10/23] NFSD: move lockd_up() before svc_addsock()

[Chuck Lever]

On Mar 9, 2009, at Mar 9, 2009, 2:19 PM, J. Bruce Fields wrote:
> On Mon, Mar 09, 2009 at 02:15:11PM -0400, Chuck Lever wrote:
>>
>> On Mar 5, 2009, at Mar 5, 2009, 4:32 PM, Chuck Lever wrote:
>>
>>> Clean up.
>>>
>>> A couple of years ago, a series of commits, finishing with commit
>>> 5680c446, swapped the order of the lockd_up() and svc_addsock()  
>>> calls
>>> in __write_ports().  At that time lockd_up() needed to know the
>>> transport protocol of the passed-in socket to start a listener on  
>>> the
>>> same transport protocol.
>>>
>>> These days, lockd_up() doesn't take a protocol argument; it always
>>> starts both a UDP and TCP listener.  It's now more straightforward  
>>> to
>>> try the lockd_up() first, then do a lockd_down() if the  
>>> svc_addsock()
>>> fails.
>>>
>>> Careful review of this code shows that the svc_sock_names() call is
>>> used only to close the just-opened socket in case lockd_up() fails.
>>> So it is no longer needed if lockd_up() is done first.
>>>
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>>
>>> fs/nfsd/nfsctl.c |   19 ++++++++++---------
>>> 1 files changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>> index 80f773b..aa60191 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -908,17 +908,18 @@ static ssize_t __write_ports_addfd(char *buf)
>>> 	if (err != 0)
>>> 		return err;
>>>
>>> -	err = svc_addsock(nfsd_serv, fd, buf);
>>> -	if (err >= 0) {
>>> -		err = lockd_up();
>>> -		if (err < 0)
>>> -			svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf);
>>> +	err = lockd_up();
>>> +	if (err != 0)
>>> +		goto out;
>>>
>>> -		/* Decrease the count, but don't shut down the service */
>>> -		nfsd_serv->sv_nrthreads--;
>>> -	}
>>> +	err = svc_addsock(nfsd_serv, fd, buf);
>>> +	if (err != 0)
>>
>> Found the "lockd: no users" BUG... this line needs to be "if (err <  
>> 0)".
>> Fix tested by Jeff Layton.
>
> OK, good.  Will you resend that one patch at some point?

There is a later patch that won't apply with this change, but the fix- 
up is trivial.  It might be easier if I just send the whole set again,  
but I would like to send them with the multiple-address-family- 
listeners patches that I posted for your review last week.  I haven't  
heard your thoughts on those.

> Are there any
> known problems left with that series?

None that I have found.

> --b.
>
>>
>>> +		lockd_down();
>>>
>>> -	return err < 0 ? err : 0;
>>> +out:
>>> +	/* Decrease the count, but don't shut down the service */
>>> +	nfsd_serv->sv_nrthreads--;
>>> +	return err;
>>> }
>>>
>>> /*
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com

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