[RFC 0/2] Sharing flex file XDR encoding between client and server

[RFC 0/2] Sharing flex file XDR encoding between client and server

[Tom Haynes]

Hi,

This builds on my prior change set for the flex file server.

I'm not too hot on putting this much code in a header, but
I can't think of a cleaner way without leaking too much
between the client and server.

Any guidance on how this is typically approached?

Thanks,
Tom


Tom Haynes (2):
  nfs: Encoding a netaddr is common to client and server
  nfsd: Encode a netaddr correctly

 fs/nfs/flexfilelayout/flexfilelayout.c |  93 +-----------------------------
 fs/nfs/flexfilelayout/flexfilelayout.h |   5 +-
 fs/nfsd/flexfilelayout.c               |  26 +--------
 fs/nfsd/flexfilelayoutxdr.c            |  34 +++++------
 fs/nfsd/flexfilelayoutxdr.h            |  15 +----
 include/linux/nfs4_ff.h                | 100 +++++++++++++++++++++++++++++++++
 6 files changed, 120 insertions(+), 153 deletions(-)
 create mode 100644 include/linux/nfs4_ff.h

-- 
2.5.5

[RFC 1/2] nfs: Encoding a netaddr is common to client and server

[Tom Haynes]

Signed-off-by: Tom Haynes <loghyr@primarydata.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 93 +-------------------------------
 fs/nfs/flexfilelayout/flexfilelayout.h |  5 +-
 include/linux/nfs4_ff.h                | 96 ++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 95 deletions(-)
 create mode 100644 include/linux/nfs4_ff.h

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 0e8018b..1973aea 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -9,6 +9,7 @@
 #include <linux/nfs_fs.h>
 #include <linux/nfs_page.h>
 #include <linux/module.h>
+#include <linux/nfs4_ff.h>
 
 #include <linux/sunrpc/metrics.h>
 
@@ -2024,96 +2025,6 @@ ff_layout_encode_layoutreturn(struct pnfs_layout_hdr *lo,
 	dprintk("%s: Return\n", __func__);
 }
 
-static int
-ff_layout_ntop4(const struct sockaddr *sap, char *buf, const size_t buflen)
-{
-	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
-
-	return snprintf(buf, buflen, "%pI4", &sin->sin_addr);
-}
-
-static size_t
-ff_layout_ntop6_noscopeid(const struct sockaddr *sap, char *buf,
-			  const int buflen)
-{
-	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
-	const struct in6_addr *addr = &sin6->sin6_addr;
-
-	/*
-	 * RFC 4291, Section 2.2.2
-	 *
-	 * Shorthanded ANY address
-	 */
-	if (ipv6_addr_any(addr))
-		return snprintf(buf, buflen, "::");
-
-	/*
-	 * RFC 4291, Section 2.2.2
-	 *
-	 * Shorthanded loopback address
-	 */
-	if (ipv6_addr_loopback(addr))
-		return snprintf(buf, buflen, "::1");
-
-	/*
-	 * RFC 4291, Section 2.2.3
-	 *
-	 * Special presentation address format for mapped v4
-	 * addresses.
-	 */
-	if (ipv6_addr_v4mapped(addr))
-		return snprintf(buf, buflen, "::ffff:%pI4",
-					&addr->s6_addr32[3]);
-
-	/*
-	 * RFC 4291, Section 2.2.1
-	 */
-	return snprintf(buf, buflen, "%pI6c", addr);
-}
-
-/* Derived from rpc_sockaddr2uaddr */
-static void
-ff_layout_encode_netaddr(struct xdr_stream *xdr, struct nfs4_pnfs_ds_addr *da)
-{
-	struct sockaddr *sap = (struct sockaddr *)&da->da_addr;
-	char portbuf[RPCBIND_MAXUADDRPLEN];
-	char addrbuf[RPCBIND_MAXUADDRLEN];
-	char *netid;
-	unsigned short port;
-	int len, netid_len;
-	__be32 *p;
-
-	switch (sap->sa_family) {
-	case AF_INET:
-		if (ff_layout_ntop4(sap, addrbuf, sizeof(addrbuf)) == 0)
-			return;
-		port = ntohs(((struct sockaddr_in *)sap)->sin_port);
-		netid = "tcp";
-		netid_len = 3;
-		break;
-	case AF_INET6:
-		if (ff_layout_ntop6_noscopeid(sap, addrbuf, sizeof(addrbuf)) == 0)
-			return;
-		port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
-		netid = "tcp6";
-		netid_len = 4;
-		break;
-	default:
-		/* we only support tcp and tcp6 */
-		WARN_ON_ONCE(1);
-		return;
-	}
-
-	snprintf(portbuf, sizeof(portbuf), ".%u.%u", port >> 8, port & 0xff);
-	len = strlcat(addrbuf, portbuf, sizeof(addrbuf));
-
-	p = xdr_reserve_space(xdr, 4 + netid_len);
-	xdr_encode_opaque(p, netid, netid_len);
-
-	p = xdr_reserve_space(xdr, 4 + len);
-	xdr_encode_opaque(p, addrbuf, len);
-}
-
 static void
 ff_layout_encode_nfstime(struct xdr_stream *xdr,
 			 ktime_t t)
@@ -2160,7 +2071,7 @@ ff_layout_encode_layoutstats(struct xdr_stream *xdr,
 	/* layoutupdate length */
 	start = xdr_reserve_space(xdr, 4);
 	/* netaddr4 */
-	ff_layout_encode_netaddr(xdr, da);
+	nfs4_encode_netaddr(xdr, (struct sockaddr *)&da->da_addr);
 	/* nfs_fh4 */
 	p = xdr_reserve_space(xdr, 4 + fh->size);
 	xdr_encode_opaque(p, fh->data, fh->size);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 1bcdb15..d71998e 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -9,10 +9,7 @@
 #ifndef FS_NFS_NFS4FLEXFILELAYOUT_H
 #define FS_NFS_NFS4FLEXFILELAYOUT_H
 
-#define FF_FLAGS_NO_LAYOUTCOMMIT 1
-#define FF_FLAGS_NO_IO_THRU_MDS  2
-#define FF_FLAGS_NO_READ_IO      4
-
+#include <linux/nfs4_ff.h>
 #include "../pnfs.h"
 
 /* XXX: Let's filter out insanely large mirror count for now to avoid oom
diff --git a/include/linux/nfs4_ff.h b/include/linux/nfs4_ff.h
new file mode 100644
index 0000000..869eb1a
--- /dev/null
+++ b/include/linux/nfs4_ff.h
@@ -0,0 +1,96 @@
+#ifndef _LINUX_NFS4_FF_H
+#define _LINUX_NFS4_FF_H
+
+/* Flex file layout hints on I/O */
+#define FF_FLAGS_NO_LAYOUTCOMMIT	1
+#define FF_FLAGS_NO_IO_THRU_MDS		2
+#define FF_FLAGS_NO_READ_IO		4
+
+static inline size_t
+nfs4_ntop6_noscopeid(const struct sockaddr *sap, char *buf, const int buflen)
+{
+	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+	const struct in6_addr *addr = &sin6->sin6_addr;
+
+	/*
+	 * RFC 4291, Section 2.2.2
+	 *
+	 * Shorthanded ANY address
+	 */
+	if (ipv6_addr_any(addr))
+		return snprintf(buf, buflen, "::");
+
+	/*
+	 * RFC 4291, Section 2.2.2
+	 *
+	 * Shorthanded loopback address
+	 */
+	if (ipv6_addr_loopback(addr))
+		return snprintf(buf, buflen, "::1");
+
+	/*
+	 * RFC 4291, Section 2.2.3
+	 *
+	 * Special presentation address format for mapped v4
+	 * addresses.
+	 */
+	if (ipv6_addr_v4mapped(addr))
+		return snprintf(buf, buflen, "::ffff:%pI4",
+					&addr->s6_addr32[3]);
+
+	/*
+	 * RFC 4291, Section 2.2.1
+	 */
+	return snprintf(buf, buflen, "%pI6c", addr);
+}
+
+static inline int
+nfs4_ntop4(const struct sockaddr *sap, char *buf, const size_t buflen)
+{
+	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+
+	return snprintf(buf, buflen, "%pI4", &sin->sin_addr);
+}
+
+/* Derived from rpc_sockaddr2uaddr */
+static inline void
+nfs4_encode_netaddr(struct xdr_stream *xdr, struct sockaddr *sap)
+{
+	char portbuf[RPCBIND_MAXUADDRPLEN];
+	char addrbuf[RPCBIND_MAXUADDRLEN];
+	char *netid;
+	unsigned short port;
+	int len, netid_len;
+	__be32 *p;
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		if (nfs4_ntop4(sap, addrbuf, sizeof(addrbuf)) == 0)
+			return;
+		port = ntohs(((struct sockaddr_in *)sap)->sin_port);
+		netid = "tcp";
+		netid_len = 3;
+		break;
+	case AF_INET6:
+		if (nfs4_ntop6_noscopeid(sap, addrbuf, sizeof(addrbuf)) == 0)
+			return;
+		port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
+		netid = "tcp6";
+		netid_len = 4;
+		break;
+	default:
+		/* we only support tcp and tcp6 */
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	snprintf(portbuf, sizeof(portbuf), ".%u.%u", port >> 8, port & 0xff);
+	len = strlcat(addrbuf, portbuf, sizeof(addrbuf));
+
+	p = xdr_reserve_space(xdr, 4 + netid_len);
+	xdr_encode_opaque(p, netid, netid_len);
+
+	p = xdr_reserve_space(xdr, 4 + len);
+	xdr_encode_opaque(p, addrbuf, len);
+}
+#endif
-- 
2.5.5

[RFC 2/2] nfsd: Encode a netaddr correctly

[Tom Haynes]

Signed-off-by: Tom Haynes <loghyr@primarydata.com>
---
 fs/nfsd/flexfilelayout.c    | 26 ++------------------------
 fs/nfsd/flexfilelayoutxdr.c | 34 ++++++++++++++--------------------
 fs/nfsd/flexfilelayoutxdr.h | 15 +--------------
 include/linux/nfs4_ff.h     |  4 ++++
 4 files changed, 21 insertions(+), 58 deletions(-)

diff --git a/fs/nfsd/flexfilelayout.c b/fs/nfsd/flexfilelayout.c
index df880e9..9ba4d4f 100644
--- a/fs/nfsd/flexfilelayout.c
+++ b/fs/nfsd/flexfilelayout.c
@@ -11,6 +11,7 @@
 #include <linux/nfsd/debug.h>
 
 #include <linux/sunrpc/addr.h>
+#include <linux/nfs4_ff.h>
 
 #include "flexfilelayoutxdr.h"
 #include "pnfs.h"
@@ -81,9 +82,6 @@ nfsd4_ff_proc_getdeviceinfo(struct super_block *sb, struct svc_rqst *rqstp,
 {
 	struct pnfs_ff_device_addr *da;
 
-	u16 port;
-	char addr[INET6_ADDRSTRLEN];
-
 	da = kzalloc(sizeof(struct pnfs_ff_device_addr), GFP_KERNEL);
 	if (!da)
 		return nfserrno(-ENOMEM);
@@ -96,27 +94,7 @@ nfsd4_ff_proc_getdeviceinfo(struct super_block *sb, struct svc_rqst *rqstp,
 	da->rsize = svc_max_payload(rqstp);
 	da->wsize = da->rsize;
 
-	rpc_ntop((struct sockaddr *)&rqstp->rq_daddr,
-		 addr, INET6_ADDRSTRLEN);
-	if (rqstp->rq_daddr.ss_family == AF_INET) {
-		struct sockaddr_in *sin;
-
-		sin = (struct sockaddr_in *)&rqstp->rq_daddr;
-		port = ntohs(sin->sin_port);
-		snprintf(da->netaddr.netid, FF_NETID_LEN + 1, "tcp");
-		da->netaddr.netid_len = 3;
-	} else {
-		struct sockaddr_in6 *sin6;
-
-		sin6 = (struct sockaddr_in6 *)&rqstp->rq_daddr;
-		port = ntohs(sin6->sin6_port);
-		snprintf(da->netaddr.netid, FF_NETID_LEN + 1, "tcp6");
-		da->netaddr.netid_len = 4;
-	}
-
-	da->netaddr.addr_len =
-		snprintf(da->netaddr.addr, FF_ADDR_LEN + 1,
-			 "%s.%hhu.%hhu", addr, port >> 8, port & 0xff);
+	memcpy(&da->daddr, &rqstp->rq_daddr, rqstp->rq_daddrlen);
 
 	da->tightly_coupled = false;
 
diff --git a/fs/nfsd/flexfilelayoutxdr.c b/fs/nfsd/flexfilelayoutxdr.c
index a241f29..83a1a56 100644
--- a/fs/nfsd/flexfilelayoutxdr.c
+++ b/fs/nfsd/flexfilelayoutxdr.c
@@ -3,6 +3,7 @@
  */
 #include <linux/sunrpc/svc.h>
 #include <linux/nfs4.h>
+#include <linux/nfs4_ff.h>
 
 #include "nfsd.h"
 #include "flexfilelayoutxdr.h"
@@ -79,37 +80,30 @@ nfsd4_ff_encode_getdeviceinfo(struct xdr_stream *xdr,
 		struct nfsd4_getdeviceinfo *gdp)
 {
 	struct pnfs_ff_device_addr *da = gdp->gd_device;
-	int len;
-	int ver_len;
-	int addr_len;
-	__be32 *p;
-
-	/* len + padding for two strings */
-	addr_len = 16 + da->netaddr.netid_len + da->netaddr.addr_len;
-	ver_len = 20;
+	__be32 *p, *start;
 
-	len = 4 + ver_len + 4 + addr_len;
+	start = xdr_reserve_space(xdr, 4);
+	if (!start)
+		return nfserr_resource;
 
-	p = xdr_reserve_space(xdr, len + sizeof(__be32));
+	p = xdr_reserve_space(xdr, 4);
 	if (!p)
 		return nfserr_resource;
 
-	/*
-	 * Fill in the overall length and number of volumes at the beginning
-	 * of the layout.
-	 */
-	*p++ = cpu_to_be32(len);
-	*p++ = cpu_to_be32(1);			/* 1 netaddr */
-	p = xdr_encode_opaque(p, da->netaddr.netid, da->netaddr.netid_len);
-	p = xdr_encode_opaque(p, da->netaddr.addr, da->netaddr.addr_len);
-
-	*p++ = cpu_to_be32(1);			/* 1 versions */
+	/* We only send 1 netaddr */
+	*p++ = cpu_to_be32(1);
+	nfs4_encode_netaddr(xdr, (struct sockaddr *)&da->daddr);
 
+	/* We only send 1 version */
+	p = xdr_reserve_space(xdr, 24);
+	*p++ = cpu_to_be32(1);
 	*p++ = cpu_to_be32(da->version);
 	*p++ = cpu_to_be32(da->minor_version);
 	*p++ = cpu_to_be32(da->rsize);
 	*p++ = cpu_to_be32(da->wsize);
 	*p++ = cpu_to_be32(da->tightly_coupled);
 
+	*start = cpu_to_be32((xdr->p - start - 1) * 4);
+
 	return 0;
 }
diff --git a/fs/nfsd/flexfilelayoutxdr.h b/fs/nfsd/flexfilelayoutxdr.h
index 467defd..1fe2bb8 100644
--- a/fs/nfsd/flexfilelayoutxdr.h
+++ b/fs/nfsd/flexfilelayoutxdr.h
@@ -7,23 +7,10 @@
 #include <linux/inet.h>
 #include "xdr4.h"
 
-#define FF_FLAGS_NO_LAYOUTCOMMIT 1
-#define FF_FLAGS_NO_IO_THRU_MDS  2
-#define FF_FLAGS_NO_READ_IO      4
-
 struct xdr_stream;
 
-#define FF_NETID_LEN		(4)
-#define FF_ADDR_LEN		(INET6_ADDRSTRLEN + 8)
-struct pnfs_ff_netaddr {
-	char				netid[FF_NETID_LEN + 1];
-	char				addr[FF_ADDR_LEN + 1];
-	u32				netid_len;
-	u32				addr_len;
-};
-
 struct pnfs_ff_device_addr {
-	struct pnfs_ff_netaddr		netaddr;
+	struct sockaddr_storage		daddr;
 	u32				version;
 	u32				minor_version;
 	u32				rsize;
diff --git a/include/linux/nfs4_ff.h b/include/linux/nfs4_ff.h
index 869eb1a..634fbfa1 100644
--- a/include/linux/nfs4_ff.h
+++ b/include/linux/nfs4_ff.h
@@ -1,6 +1,10 @@
 #ifndef _LINUX_NFS4_FF_H
 #define _LINUX_NFS4_FF_H
 
+#include <linux/sunrpc/debug.h>
+#include <linux/sunrpc/auth.h>
+#include <linux/sunrpc/clnt.h>
+
 /* Flex file layout hints on I/O */
 #define FF_FLAGS_NO_LAYOUTCOMMIT	1
 #define FF_FLAGS_NO_IO_THRU_MDS		2
-- 
2.5.5

Re: [RFC 0/2] Sharing flex file XDR encoding between client and server

[J. Bruce Fields]

On Tue, Jun 07, 2016 at 04:34:44PM -0700, Tom Haynes wrote:
> This builds on my prior change set for the flex file server.
> 
> I'm not too hot on putting this much code in a header, but
> I can't think of a cleaner way without leaking too much
> between the client and server.
> 
> Any guidance on how this is typically approached?

Looks like there's some cut-and-paste from net/sunrpc/addr.c.  Any
reason not to just use those functions and/or add any necessary new
stuff there?

--b.

> 
> Thanks,
> Tom
> 
> 
> Tom Haynes (2):
>   nfs: Encoding a netaddr is common to client and server
>   nfsd: Encode a netaddr correctly
> 
>  fs/nfs/flexfilelayout/flexfilelayout.c |  93 +-----------------------------
>  fs/nfs/flexfilelayout/flexfilelayout.h |   5 +-
>  fs/nfsd/flexfilelayout.c               |  26 +--------
>  fs/nfsd/flexfilelayoutxdr.c            |  34 +++++------
>  fs/nfsd/flexfilelayoutxdr.h            |  15 +----
>  include/linux/nfs4_ff.h                | 100 +++++++++++++++++++++++++++++++++
>  6 files changed, 120 insertions(+), 153 deletions(-)
>  create mode 100644 include/linux/nfs4_ff.h
> 
> -- 
> 2.5.5