2020-11-06 23:07:07

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 2/3] NFSv4/pNFS: Use connections to a DS that are all of the same protocol family

From: Trond Myklebust <[email protected]>

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 <[email protected]>
---
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


2020-11-06 23:07:58

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 3/3] NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr

From: Trond Myklebust <[email protected]>

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 <[email protected]>
---
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

2020-11-10 16:27:26

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: Re: [PATCH 3/3] NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr

Hi Trond,

this is what I get when trying to build your testing branch:


CC [M] fs/nfs/pnfs_nfs.o
fs/nfs/pnfs_nfs.c: In function ‘nfs4_decode_mp_ds_addr’:
fs/nfs/pnfs_nfs.c:1088:22: error: passing argument 1 of ‘kmemdup_nul’ from incompatible pointer type [-Werror=incompa
tible-pointer-types]
1088 | netid = kmemdup_nul(p, nlen, gfp_flags);
| ^
| |
| __be32 * {aka unsigned int *}
In file included from ./include/linux/bitmap.h:9,
from ./include/linux/cpumask.h:12,
from ./arch/x86/include/asm/cpumask.h:5,
from ./arch/x86/include/asm/msr.h:11,
from ./arch/x86/include/asm/processor.h:22,
from ./arch/x86/include/asm/cpufeature.h:5,
from ./arch/x86/include/asm/thread_info.h:53,
from ./include/linux/thread_info.h:38,
from ./include/linux/uio.h:9,
from ./include/linux/socket.h:8, from ./include/uapi/linux/in.h:24,
from ./include/linux/in.h:19,
from ./include/linux/nfs_fs.h:22,
from fs/nfs/pnfs_nfs.c:11:
./include/linux/string.h:180:38: note: expected ‘const char *’ but argument is of type ‘__be32 *’ {aka ‘unsigned int
*’}
180 | extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
| ~~~~~~~~~~~~^
fs/nfs/pnfs_nfs.c:1108:20: error: passing argument 1 of ‘kmemdup_nul’ from incompatible pointer type [-We



Regards,
Tigran.

----- Original Message -----
> From: "trondmy" <[email protected]>
> To: "linux-nfs" <[email protected]>
> Sent: Friday, 6 November, 2020 23:55:27
> Subject: [PATCH 3/3] NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr

> From: Trond Myklebust <[email protected]>
>
> 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 <[email protected]>
> ---
> 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