2016-06-07 23:35:18

by Thomas Haynes

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

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



2016-06-07 23:35:19

by Thomas Haynes

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

Signed-off-by: Tom Haynes <[email protected]>
---
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


2016-06-07 23:35:19

by Thomas Haynes

[permalink] [raw]
Subject: [RFC 2/2] nfsd: Encode a netaddr correctly

Signed-off-by: Tom Haynes <[email protected]>
---
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


2016-06-13 19:45:45

by J. Bruce Fields

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

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