[PATCH v2 0/5] Add RDMA support to the pNFS file+flexfiles data channels

[PATCH v2 0/5] Add RDMA support to the pNFS file+flexfiles data channels

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

Add support for connecting to the pNFS files/flexfiles data servers
through RDMA, assuming that the GETDEVICEINFO call advertises that
support.

v2: Fix layoutstats encoding for pNFS/flexfiles.

Trond Myklebust (5):
  SUNRPC: xprt_load_transport() needs to support the netid "rdma6"
  NFSv4/pNFS: Use connections to a DS that are all of the same protocol
    family
  NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr
  pNFS/flexfiles: Fix up layoutstats reporting for non-TCP transports
  pNFS: Clean up open coded kmemdup_nul()

 fs/nfs/flexfilelayout/flexfilelayout.c | 34 +++++++++--
 fs/nfs/pnfs.h                          |  1 +
 fs/nfs/pnfs_nfs.c                      | 80 +++++++++++++++++++-------
 net/sunrpc/xprtrdma/module.c           |  2 +
 4 files changed, 91 insertions(+), 26 deletions(-)

-- 
2.28.0

[PATCH v2 1/5] SUNRPC: xprt_load_transport() needs to support the netid "rdma6"

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

According to RFC5666, the correct netid for an IPv6 addressed RDMA
transport is "rdma6", which we've supported as a mount option since
Linux-4.7. The problem is when we try to load the module "xprtrdma6",
that will fail, since there is no modulealias of that name.

Fixes: 181342c5ebe8 ("xprtrdma: Add rdma6 option to support NFS/RDMA IPv6")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 net/sunrpc/xprtrdma/module.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sunrpc/xprtrdma/module.c b/net/sunrpc/xprtrdma/module.c
index 620327c01302..fb55983628b4 100644
--- a/net/sunrpc/xprtrdma/module.c
+++ b/net/sunrpc/xprtrdma/module.c
@@ -23,7 +23,9 @@ MODULE_AUTHOR("Open Grid Computing and Network Appliance, Inc.");
 MODULE_DESCRIPTION("RPC/RDMA Transport");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS("svcrdma");
+MODULE_ALIAS("svcrdma6");
 MODULE_ALIAS("xprtrdma");
+MODULE_ALIAS("xprtrdma6");
 
 static void __exit rpc_rdma_cleanup(void)
 {
-- 
2.28.0

[PATCH v2 2/5] NFSv4/pNFS: Use connections to a DS that are all of the same protocol family

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

If the pNFS metadata server advertises multiple addresses for the same
data server, we should try to connect to just one protocol family and
transport type on the assumption that homogeneity will improve performance.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/pnfs_nfs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 679767ac258d..7027dac41cc7 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -860,6 +860,9 @@ static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
 				.addrlen = da->da_addrlen,
 				.servername = clp->cl_hostname,
 			};
+
+			if (da->da_addr.ss_family != clp->cl_addr.ss_family)
+				continue;
 			/* Add this address as an alias */
 			rpc_clnt_add_xprt(clp->cl_rpcclient, &xprt_args,
 					rpc_clnt_test_and_add_xprt, NULL);
@@ -920,6 +923,8 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 				.data = &xprtdata,
 			};
 
+			if (da->da_addr.ss_family != clp->cl_addr.ss_family)
+				continue;
 			/**
 			* Test this address for session trunking and
 			* add as an alias
-- 
2.28.0

[PATCH v2 3/5] NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

We want to enable RDMA and UDP as valid transport methods if a
GETDEVICEINFO call specifies it. Do so by adding a parser for the
netid that translates it to an appropriate argument for the RPC
transport layer.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/pnfs.h     |  1 +
 fs/nfs/pnfs_nfs.c | 66 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 2661c44c62db..20ee37b751e3 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -51,6 +51,7 @@ struct nfs4_pnfs_ds_addr {
 	size_t			da_addrlen;
 	struct list_head	da_node;  /* nfs4_pnfs_dev_hlist dev_dslist */
 	char			*da_remotestr;	/* human readable addr+port */
+	int			da_transport;
 };
 
 struct nfs4_pnfs_ds {
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 7027dac41cc7..d00750743100 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -854,13 +854,15 @@ static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
 
 		if (!IS_ERR(clp)) {
 			struct xprt_create xprt_args = {
-				.ident = XPRT_TRANSPORT_TCP,
+				.ident = da->da_transport,
 				.net = clp->cl_net,
 				.dstaddr = (struct sockaddr *)&da->da_addr,
 				.addrlen = da->da_addrlen,
 				.servername = clp->cl_hostname,
 			};
 
+			if (da->da_transport != clp->cl_proto)
+				continue;
 			if (da->da_addr.ss_family != clp->cl_addr.ss_family)
 				continue;
 			/* Add this address as an alias */
@@ -870,7 +872,7 @@ static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
 		}
 		clp = get_v3_ds_connect(mds_srv,
 				(struct sockaddr *)&da->da_addr,
-				da->da_addrlen, IPPROTO_TCP,
+				da->da_addrlen, da->da_transport,
 				timeo, retrans);
 		if (IS_ERR(clp))
 			continue;
@@ -908,7 +910,7 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 
 		if (!IS_ERR(clp) && clp->cl_mvops->session_trunk) {
 			struct xprt_create xprt_args = {
-				.ident = XPRT_TRANSPORT_TCP,
+				.ident = da->da_transport,
 				.net = clp->cl_net,
 				.dstaddr = (struct sockaddr *)&da->da_addr,
 				.addrlen = da->da_addrlen,
@@ -923,6 +925,8 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 				.data = &xprtdata,
 			};
 
+			if (da->da_transport != clp->cl_proto)
+				continue;
 			if (da->da_addr.ss_family != clp->cl_addr.ss_family)
 				continue;
 			/**
@@ -937,8 +941,9 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
 		} else {
 			clp = nfs4_set_ds_client(mds_srv,
 						(struct sockaddr *)&da->da_addr,
-						da->da_addrlen, IPPROTO_TCP,
-						timeo, retrans, minor_version);
+						da->da_addrlen,
+						da->da_transport, timeo,
+						retrans, minor_version);
 			if (IS_ERR(clp))
 				continue;
 
@@ -1017,6 +1022,41 @@ int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
 }
 EXPORT_SYMBOL_GPL(nfs4_pnfs_ds_connect);
 
+struct xprt_lookup_transport {
+	const char *name;
+	sa_family_t protofamily;
+	int type;
+};
+
+static const struct xprt_lookup_transport xl_xprt[] = {
+	{ "tcp", AF_INET, XPRT_TRANSPORT_TCP },
+	{ "tcp6", AF_INET6, XPRT_TRANSPORT_TCP },
+	{ "rdma", AF_INET, XPRT_TRANSPORT_RDMA },
+	{ "rdma6", AF_INET6, XPRT_TRANSPORT_RDMA },
+	{ "udp", AF_INET, XPRT_TRANSPORT_UDP },
+	{ "udp6", AF_INET6, XPRT_TRANSPORT_UDP },
+	{ NULL, 0, -1 },
+};
+
+static int
+nfs4_decode_transport(const char *str, size_t len, sa_family_t protofamily)
+{
+	const struct xprt_lookup_transport *p;
+
+	for (p = xl_xprt; p->name; p++) {
+		if (p->protofamily != protofamily)
+			continue;
+		if (strlen(p->name) != len)
+			continue;
+		if (memcmp(p->name, str, len) != 0)
+			continue;
+		if (xprt_load_transport(p->name) != 0)
+			continue;
+		return p->type;
+	}
+	return -1;
+}
+
 /*
  * Currently only supports ipv4, ipv6 and one multi-path address.
  */
@@ -1029,8 +1069,8 @@ nfs4_decode_mp_ds_addr(struct net *net, struct xdr_stream *xdr, gfp_t gfp_flags)
 	int nlen, rlen;
 	int tmp[2];
 	__be32 *p;
-	char *netid, *match_netid;
-	size_t len, match_netid_len;
+	char *netid;
+	size_t len;
 	char *startsep = "";
 	char *endsep = "";
 
@@ -1114,15 +1154,11 @@ nfs4_decode_mp_ds_addr(struct net *net, struct xdr_stream *xdr, gfp_t gfp_flags)
 	case AF_INET:
 		((struct sockaddr_in *)&da->da_addr)->sin_port = port;
 		da->da_addrlen = sizeof(struct sockaddr_in);
-		match_netid = "tcp";
-		match_netid_len = 3;
 		break;
 
 	case AF_INET6:
 		((struct sockaddr_in6 *)&da->da_addr)->sin6_port = port;
 		da->da_addrlen = sizeof(struct sockaddr_in6);
-		match_netid = "tcp6";
-		match_netid_len = 4;
 		startsep = "[";
 		endsep = "]";
 		break;
@@ -1133,9 +1169,11 @@ nfs4_decode_mp_ds_addr(struct net *net, struct xdr_stream *xdr, gfp_t gfp_flags)
 		goto out_free_da;
 	}
 
-	if (nlen != match_netid_len || strncmp(netid, match_netid, nlen)) {
-		dprintk("%s: ERROR: r_netid \"%s\" != \"%s\"\n",
-			__func__, netid, match_netid);
+	da->da_transport = nfs4_decode_transport(netid, nlen,
+						 da->da_addr.ss_family);
+	if (da->da_transport == -1) {
+		dprintk("%s: ERROR: unknown r_netid \"%*s\"\n",
+			__func__, nlen, netid);
 		goto out_free_da;
 	}
 
-- 
2.28.0

[PATCH v2 4/5] pNFS/flexfiles: Fix up layoutstats reporting for non-TCP transports

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

Ensure that we report the correct netid when using UDP or RDMA
transports to the DSes.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 34 ++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index a163533446fa..c760238ba649 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -2279,18 +2279,41 @@ ff_layout_encode_netaddr(struct xdr_stream *xdr, struct nfs4_pnfs_ds_addr *da)
 		if (ff_layout_ntop4(sap, addrbuf, sizeof(addrbuf)) == 0)
 			return;
 		port = ntohs(((struct sockaddr_in *)sap)->sin_port);
-		netid = "tcp";
-		netid_len = 3;
+		switch (da->da_transport) {
+		case XPRT_TRANSPORT_TCP:
+			netid = "tcp";
+			break;
+		case XPRT_TRANSPORT_RDMA:
+			netid = "rdma";
+			break;
+		case XPRT_TRANSPORT_UDP:
+			netid = "udp";
+			break;
+		default:
+			WARN_ON_ONCE(1);
+			return;
+		}
 		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;
+		switch (da->da_transport) {
+		case XPRT_TRANSPORT_TCP:
+			netid = "tcp6";
+			break;
+		case XPRT_TRANSPORT_RDMA:
+			netid = "rdma6";
+			break;
+		case XPRT_TRANSPORT_UDP:
+			netid = "udp6";
+			break;
+		default:
+			WARN_ON_ONCE(1);
+			return;
+		}
 		break;
 	default:
-		/* we only support tcp and tcp6 */
 		WARN_ON_ONCE(1);
 		return;
 	}
@@ -2298,6 +2321,7 @@ ff_layout_encode_netaddr(struct xdr_stream *xdr, struct nfs4_pnfs_ds_addr *da)
 	snprintf(portbuf, sizeof(portbuf), ".%u.%u", port >> 8, port & 0xff);
 	len = strlcat(addrbuf, portbuf, sizeof(addrbuf));
 
+	netid_len = strlen(netid);
 	p = xdr_reserve_space(xdr, 4 + netid_len);
 	xdr_encode_opaque(p, netid, netid_len);
 
-- 
2.28.0

[PATCH v2 5/5] pNFS: Clean up open coded kmemdup_nul()

[trondmy]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 fs/nfs/pnfs_nfs.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index d00750743100..5acddd14ffe8 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -1085,13 +1085,10 @@ nfs4_decode_mp_ds_addr(struct net *net, struct xdr_stream *xdr, gfp_t gfp_flags)
 	if (unlikely(!p))
 		goto out_err;
 
-	netid = kmalloc(nlen+1, gfp_flags);
+	netid = kmemdup_nul(p, nlen, gfp_flags);
 	if (unlikely(!netid))
 		goto out_err;
 
-	netid[nlen] = '\0';
-	memcpy(netid, p, nlen);
-
 	/* r_addr: ip/ip6addr with port in dec octets - see RFC 5665 */
 	p = xdr_inline_decode(xdr, 4);
 	if (unlikely(!p))
@@ -1108,13 +1105,11 @@ nfs4_decode_mp_ds_addr(struct net *net, struct xdr_stream *xdr, gfp_t gfp_flags)
 			rlen);
 		goto out_free_netid;
 	}
-	buf = kmalloc(rlen + 1, gfp_flags);
+	buf = kmemdup_nul(p, rlen, gfp_flags);
 	if (!buf) {
 		dprintk("%s: Not enough memory\n", __func__);
 		goto out_free_netid;
 	}
-	buf[rlen] = '\0';
-	memcpy(buf, p, rlen);
 
 	/* replace port '.' with '-' */
 	portstr = strrchr(buf, '.');
-- 
2.28.0

Re: [PATCH v2 1/5] SUNRPC: xprt_load_transport() needs to support the netid "rdma6"

[Chuck Lever]

> On Nov 9, 2020, at 4:10 PM, trondmy@gmail.com wrote:
> 
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> According to RFC5666, the correct netid for an IPv6 addressed RDMA
> transport is "rdma6", which we've supported as a mount option since
> Linux-4.7. The problem is when we try to load the module "xprtrdma6",
> that will fail, since there is no modulealias of that name.

Trying to wrap my head around this. Who is forming the legacy names
"xprtrdma6" and "svcrdma6" ?

The module name these days is "rpcrdma". Seems like you should fix
the code that is trying to load these by the wrong name rather than
adding more legacy names.


> Fixes: 181342c5ebe8 ("xprtrdma: Add rdma6 option to support NFS/RDMA IPv6")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> net/sunrpc/xprtrdma/module.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/net/sunrpc/xprtrdma/module.c b/net/sunrpc/xprtrdma/module.c
> index 620327c01302..fb55983628b4 100644
> --- a/net/sunrpc/xprtrdma/module.c
> +++ b/net/sunrpc/xprtrdma/module.c
> @@ -23,7 +23,9 @@ MODULE_AUTHOR("Open Grid Computing and Network Appliance, Inc.");
> MODULE_DESCRIPTION("RPC/RDMA Transport");
> MODULE_LICENSE("Dual BSD/GPL");
> MODULE_ALIAS("svcrdma");
> +MODULE_ALIAS("svcrdma6");
> MODULE_ALIAS("xprtrdma");
> +MODULE_ALIAS("xprtrdma6");
> 
> static void __exit rpc_rdma_cleanup(void)
> {
> -- 
> 2.28.0
> 

--
Chuck Lever

Re: [PATCH v2 1/5] SUNRPC: xprt_load_transport() needs to support the netid "rdma6"

[Trond Myklebust]

On Mon, 2020-11-09 at 16:50 -0500, Chuck Lever wrote:
> 
> 
> > On Nov 9, 2020, at 4:10 PM, trondmy@gmail.com wrote:
> > 
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > According to RFC5666, the correct netid for an IPv6 addressed RDMA
> > transport is "rdma6", which we've supported as a mount option since
> > Linux-4.7. The problem is when we try to load the module
> > "xprtrdma6",
> > that will fail, since there is no modulealias of that name.
> 
> Trying to wrap my head around this. Who is forming the legacy names
> "xprtrdma6" and "svcrdma6" ?

I don't care about "svcrdma6", because nothing uses that name, AFAICS,
because __write_ports_addxprt() appears to use the "transport name",
whatever that is.

> The module name these days is "rpcrdma". Seems like you should fix
> the code that is trying to load these by the wrong name rather than
> adding more legacy names.

No, I'm not going to do that.

The intention was always that xprt_load_transport() should take the
netid as its argument. Furthermore, it makes no sense for either the
NFS or generic RPC layers to have to figure out how by themselves how
to translate netids into transport module names.

> 
> > Fixes: 181342c5ebe8 ("xprtrdma: Add rdma6 option to support
> > NFS/RDMA IPv6")
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> > net/sunrpc/xprtrdma/module.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/sunrpc/xprtrdma/module.c
> > b/net/sunrpc/xprtrdma/module.c
> > index 620327c01302..fb55983628b4 100644
> > --- a/net/sunrpc/xprtrdma/module.c
> > +++ b/net/sunrpc/xprtrdma/module.c
> > @@ -23,7 +23,9 @@ MODULE_AUTHOR("Open Grid Computing and Network
> > Appliance, Inc.");
> > MODULE_DESCRIPTION("RPC/RDMA Transport");
> > MODULE_LICENSE("Dual BSD/GPL");
> > MODULE_ALIAS("svcrdma");
> > +MODULE_ALIAS("svcrdma6");
> > MODULE_ALIAS("xprtrdma");
> > +MODULE_ALIAS("xprtrdma6");
> > 
> > static void __exit rpc_rdma_cleanup(void)
> > {
> > -- 
> > 2.28.0
> > 
> 
> --
> Chuck Lever
> 
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com