> On Jun 9, 2021, at 5:53 PM, Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote:
>
> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>
> This option will control up to how many xprts can the client
> establish to the server. This patch parses the value and sets
> up structures that keep track of max_connect.
>
> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> ---
> fs/nfs/client.c | 1 +
> fs/nfs/fs_context.c | 8 ++++++++
> fs/nfs/internal.h | 2 ++
> fs/nfs/nfs4client.c | 12 ++++++++++--
> fs/nfs/super.c | 2 ++
> include/linux/nfs_fs_sb.h | 1 +
> 6 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 330f65727c45..486dec59972b 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -179,6 +179,7 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
>
> clp->cl_proto = cl_init->proto;
> clp->cl_nconnect = cl_init->nconnect;
> + clp->cl_max_connect = cl_init->max_connect ? cl_init->max_connect : 1;
So, 1 is the default setting, meaning the "add another transport"
facility is disabled by default. Would it be less surprising for
an admin to allow some extra connections by default?
> clp->cl_net = get_net(cl_init->net);
>
> clp->cl_principal = "*";
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index d95c9a39bc70..cfbff7098f8e 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -29,6 +29,7 @@
> #endif
>
> #define NFS_MAX_CONNECTIONS 16
> +#define NFS_MAX_TRANSPORTS 128
This maximum seems excessive... again, there are diminishing
returns to adding more connections to the same server. what's
wrong with re-using NFS_MAX_CONNECTIONS for the maximum?
As always, I'm a little queasy about adding yet another mount
option. Are there real use cases where a whole-client setting
(like a sysfs attribute) would be inadequate? Is there a way
the client could figure out a reasonable maximum without a
human intervention, say, by counting the number of NICs on
the system?
> enum nfs_param {
> Opt_ac,
> @@ -60,6 +61,7 @@ enum nfs_param {
> Opt_mountvers,
> Opt_namelen,
> Opt_nconnect,
> + Opt_max_connect,
> Opt_port,
> Opt_posix,
> Opt_proto,
> @@ -158,6 +160,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
> fsparam_u32 ("mountvers", Opt_mountvers),
> fsparam_u32 ("namlen", Opt_namelen),
> fsparam_u32 ("nconnect", Opt_nconnect),
> + fsparam_u32 ("max_connect", Opt_max_connect),
> fsparam_string("nfsvers", Opt_vers),
> fsparam_u32 ("port", Opt_port),
> fsparam_flag_no("posix", Opt_posix),
> @@ -770,6 +773,11 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
> goto out_of_bounds;
> ctx->nfs_server.nconnect = result.uint_32;
> break;
> + case Opt_max_connect:
> + if (result.uint_32 < 1 || result.uint_32 > NFS_MAX_TRANSPORTS)
> + goto out_of_bounds;
> + ctx->nfs_server.max_connect = result.uint_32;
> + break;
> case Opt_lookupcache:
> switch (result.uint_32) {
> case Opt_lookupcache_all:
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index a36af04188c2..66fc936834f2 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -67,6 +67,7 @@ struct nfs_client_initdata {
> int proto;
> u32 minorversion;
> unsigned int nconnect;
> + unsigned int max_connect;
> struct net *net;
> const struct rpc_timeout *timeparms;
> const struct cred *cred;
> @@ -121,6 +122,7 @@ struct nfs_fs_context {
> int port;
> unsigned short protocol;
> unsigned short nconnect;
> + unsigned short max_connect;
> unsigned short export_path_len;
> } nfs_server;
>
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 42719384e25f..640c8235d817 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -863,6 +863,7 @@ static int nfs4_set_client(struct nfs_server *server,
> const char *ip_addr,
> int proto, const struct rpc_timeout *timeparms,
> u32 minorversion, unsigned int nconnect,
> + unsigned int max_connect,
> struct net *net)
> {
> struct nfs_client_initdata cl_init = {
> @@ -881,6 +882,8 @@ static int nfs4_set_client(struct nfs_server *server,
>
> if (minorversion == 0)
> __set_bit(NFS_CS_REUSEPORT, &cl_init.init_flags);
> + else
> + cl_init.max_connect = max_connect;
> if (proto == XPRT_TRANSPORT_TCP)
> cl_init.nconnect = nconnect;
>
> @@ -950,8 +953,10 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
> return ERR_PTR(-EINVAL);
> cl_init.hostname = buf;
>
> - if (mds_clp->cl_nconnect > 1 && ds_proto == XPRT_TRANSPORT_TCP)
> + if (mds_clp->cl_nconnect > 1 && ds_proto == XPRT_TRANSPORT_TCP) {
> cl_init.nconnect = mds_clp->cl_nconnect;
> + cl_init.max_connect = mds_clp->cl_max_connect;
> + }
>
> if (mds_srv->flags & NFS_MOUNT_NORESVPORT)
> __set_bit(NFS_CS_NORESVPORT, &cl_init.init_flags);
> @@ -1120,6 +1125,7 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
> &timeparms,
> ctx->minorversion,
> ctx->nfs_server.nconnect,
> + ctx->nfs_server.max_connect,
> fc->net_ns);
> if (error < 0)
> return error;
> @@ -1209,6 +1215,7 @@ struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
> parent_server->client->cl_timeout,
> parent_client->cl_mvops->minor_version,
> parent_client->cl_nconnect,
> + parent_client->cl_max_connect,
> parent_client->cl_net);
> if (!error)
> goto init_server;
> @@ -1224,6 +1231,7 @@ struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
> parent_server->client->cl_timeout,
> parent_client->cl_mvops->minor_version,
> parent_client->cl_nconnect,
> + parent_client->cl_max_connect,
> parent_client->cl_net);
> if (error < 0)
> goto error;
> @@ -1321,7 +1329,7 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
> error = nfs4_set_client(server, hostname, sap, salen, buf,
> clp->cl_proto, clnt->cl_timeout,
> clp->cl_minorversion,
> - clp->cl_nconnect, net);
> + clp->cl_nconnect, clp->cl_max_connect, net);
> clear_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
> if (error != 0) {
> nfs_server_insert_lists(server);
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index fe58525cfed4..e65c83494c05 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -480,6 +480,8 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
> if (clp->cl_nconnect > 0)
> seq_printf(m, ",nconnect=%u", clp->cl_nconnect);
> if (version == 4) {
> + if (clp->cl_max_connect > 1)
> + seq_printf(m, ",max_connect=%u", clp->cl_max_connect);
> if (nfss->port != NFS_PORT)
> seq_printf(m, ",port=%u", nfss->port);
> } else
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index d71a0e90faeb..2a9acbfe00f0 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -62,6 +62,7 @@ struct nfs_client {
>
> u32 cl_minorversion;/* NFSv4 minorversion */
> unsigned int cl_nconnect; /* Number of connections */
> + unsigned int cl_max_connect; /* max number of xprts allowed */
> const char * cl_principal; /* used for machine cred */
>
> #if IS_ENABLED(CONFIG_NFS_V4)
> --
> 2.27.0
>
--
Chuck Lever
