2021-03-30 13:41:36

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH] NFS: Fix up the support for CONFIG_NFS_DISABLE_UDP_SUPPORT

From: Trond Myklebust <[email protected]>

Rather than removing the support in nfs_init_timeout_values(), we should
just fix up the validation checks in the mount option parsers.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/client.c | 2 --
fs/nfs/fs_context.c | 54 +++++++++++++++++++++++++++++----------------
2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 94d47be1d1f6..2aeb4e52a4f1 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -476,7 +476,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
to->to_maxval = to->to_initval;
to->to_exponential = 0;
break;
-#ifndef CONFIG_NFS_DISABLE_UDP_SUPPORT
case XPRT_TRANSPORT_UDP:
if (retrans == NFS_UNSPEC_RETRANS)
to->to_retries = NFS_DEF_UDP_RETRANS;
@@ -487,7 +486,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
to->to_maxval = NFS_MAX_UDP_TIMEOUT;
to->to_exponential = 1;
break;
-#endif
default:
BUG();
}
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 902db1262d2b..cdf32b9a6c35 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -283,20 +283,40 @@ static int nfs_verify_server_address(struct sockaddr *addr)
return 0;
}

+#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
+static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
+{
+ return true;
+}
+#else
+static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
+{
+ if (ctx->version == 4)
+ return true;
+ return false;
+}
+#endif
+
/*
* Sanity check the NFS transport protocol.
- *
*/
-static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
+static int nfs_validate_transport_protocol(struct fs_context *fc,
+ struct nfs_fs_context *ctx)
{
switch (ctx->nfs_server.protocol) {
case XPRT_TRANSPORT_UDP:
+ if (nfs_server_transport_udp_invalid(ctx))
+ goto out_invalid_transport_udp;
+ break;
case XPRT_TRANSPORT_TCP:
case XPRT_TRANSPORT_RDMA:
break;
default:
ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
}
+ return 0;
+out_invalid_transport_udp:
+ return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
}

/*
@@ -305,8 +325,6 @@ static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
*/
static void nfs_set_mount_transport_protocol(struct nfs_fs_context *ctx)
{
- nfs_validate_transport_protocol(ctx);
-
if (ctx->mount_server.protocol == XPRT_TRANSPORT_UDP ||
ctx->mount_server.protocol == XPRT_TRANSPORT_TCP)
return;
@@ -929,6 +947,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
struct nfs_fh *mntfh = ctx->mntfh;
struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
int extra_flags = NFS_MOUNT_LEGACY_INTERFACE;
+ int ret;

if (data == NULL)
goto out_no_data;
@@ -1054,6 +1073,10 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
goto generic;
}

+ ret = nfs_validate_transport_protocol(fc, ctx);
+ if (ret)
+ return ret;
+
ctx->skip_reconfig_option_check = true;
return 0;

@@ -1155,6 +1178,7 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
{
struct nfs_fs_context *ctx = nfs_fc2context(fc);
struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
+ int ret;
char *c;

if (!data) {
@@ -1227,9 +1251,9 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
ctx->acdirmin = data->acdirmin;
ctx->acdirmax = data->acdirmax;
ctx->nfs_server.protocol = data->proto;
- nfs_validate_transport_protocol(ctx);
- if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
- goto out_invalid_transport_udp;
+ ret = nfs_validate_transport_protocol(fc, ctx);
+ if (ret)
+ return ret;
done:
ctx->skip_reconfig_option_check = true;
return 0;
@@ -1240,9 +1264,6 @@ static int nfs4_parse_monolithic(struct fs_context *fc,

out_no_address:
return nfs_invalf(fc, "NFS4: mount program didn't pass remote address");
-
-out_invalid_transport_udp:
- return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
}
#endif

@@ -1307,6 +1328,10 @@ static int nfs_fs_context_validate(struct fs_context *fc)
if (!nfs_verify_server_address(sap))
goto out_no_address;

+ ret = nfs_validate_transport_protocol(fc, ctx);
+ if (ret)
+ return ret;
+
if (ctx->version == 4) {
if (IS_ENABLED(CONFIG_NFS_V4)) {
if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
@@ -1315,9 +1340,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
port = NFS_PORT;
max_namelen = NFS4_MAXNAMLEN;
max_pathlen = NFS4_MAXPATHLEN;
- nfs_validate_transport_protocol(ctx);
- if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
- goto out_invalid_transport_udp;
ctx->flags &= ~(NFS_MOUNT_NONLM | NFS_MOUNT_NOACL |
NFS_MOUNT_VER3 | NFS_MOUNT_LOCAL_FLOCK |
NFS_MOUNT_LOCAL_FCNTL);
@@ -1326,10 +1348,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
}
} else {
nfs_set_mount_transport_protocol(ctx);
-#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
- if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
- goto out_invalid_transport_udp;
-#endif
if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
port = NFS_RDMA_PORT;
}
@@ -1363,8 +1381,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
out_v4_not_compiled:
nfs_errorf(fc, "NFS: NFSv4 is not compiled into kernel");
return -EPROTONOSUPPORT;
-out_invalid_transport_udp:
- return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
out_no_address:
return nfs_invalf(fc, "NFS: mount program didn't pass remote address");
out_mountproto_mismatch:
--
2.30.2


2021-03-30 15:01:05

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix up the support for CONFIG_NFS_DISABLE_UDP_SUPPORT

On Tue, Mar 30, 2021 at 9:41 AM <[email protected]> wrote:
>
> From: Trond Myklebust <[email protected]>
>
> Rather than removing the support in nfs_init_timeout_values(), we should
> just fix up the validation checks in the mount option parsers.
>
> Signed-off-by: Trond Myklebust <[email protected]>

With this patch, on top of the existing patches I tested that I no
longer see an oops while mounting with v3 (and yes in my kernel
config I have DISABLE_UDP enabled).

> ---
> fs/nfs/client.c | 2 --
> fs/nfs/fs_context.c | 54 +++++++++++++++++++++++++++++----------------
> 2 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 94d47be1d1f6..2aeb4e52a4f1 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -476,7 +476,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
> to->to_maxval = to->to_initval;
> to->to_exponential = 0;
> break;
> -#ifndef CONFIG_NFS_DISABLE_UDP_SUPPORT
> case XPRT_TRANSPORT_UDP:
> if (retrans == NFS_UNSPEC_RETRANS)
> to->to_retries = NFS_DEF_UDP_RETRANS;
> @@ -487,7 +486,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
> to->to_maxval = NFS_MAX_UDP_TIMEOUT;
> to->to_exponential = 1;
> break;
> -#endif
> default:
> BUG();
> }
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 902db1262d2b..cdf32b9a6c35 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -283,20 +283,40 @@ static int nfs_verify_server_address(struct sockaddr *addr)
> return 0;
> }
>
> +#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
> +static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
> +{
> + return true;
> +}
> +#else
> +static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
> +{
> + if (ctx->version == 4)
> + return true;
> + return false;
> +}
> +#endif
> +
> /*
> * Sanity check the NFS transport protocol.
> - *
> */
> -static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
> +static int nfs_validate_transport_protocol(struct fs_context *fc,
> + struct nfs_fs_context *ctx)
> {
> switch (ctx->nfs_server.protocol) {
> case XPRT_TRANSPORT_UDP:
> + if (nfs_server_transport_udp_invalid(ctx))
> + goto out_invalid_transport_udp;
> + break;
> case XPRT_TRANSPORT_TCP:
> case XPRT_TRANSPORT_RDMA:
> break;
> default:
> ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> }
> + return 0;
> +out_invalid_transport_udp:
> + return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> }
>
> /*
> @@ -305,8 +325,6 @@ static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
> */
> static void nfs_set_mount_transport_protocol(struct nfs_fs_context *ctx)
> {
> - nfs_validate_transport_protocol(ctx);
> -
> if (ctx->mount_server.protocol == XPRT_TRANSPORT_UDP ||
> ctx->mount_server.protocol == XPRT_TRANSPORT_TCP)
> return;
> @@ -929,6 +947,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
> struct nfs_fh *mntfh = ctx->mntfh;
> struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
> int extra_flags = NFS_MOUNT_LEGACY_INTERFACE;
> + int ret;
>
> if (data == NULL)
> goto out_no_data;
> @@ -1054,6 +1073,10 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
> goto generic;
> }
>
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> +
> ctx->skip_reconfig_option_check = true;
> return 0;
>
> @@ -1155,6 +1178,7 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
> {
> struct nfs_fs_context *ctx = nfs_fc2context(fc);
> struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
> + int ret;
> char *c;
>
> if (!data) {
> @@ -1227,9 +1251,9 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
> ctx->acdirmin = data->acdirmin;
> ctx->acdirmax = data->acdirmax;
> ctx->nfs_server.protocol = data->proto;
> - nfs_validate_transport_protocol(ctx);
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> done:
> ctx->skip_reconfig_option_check = true;
> return 0;
> @@ -1240,9 +1264,6 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
>
> out_no_address:
> return nfs_invalf(fc, "NFS4: mount program didn't pass remote address");
> -
> -out_invalid_transport_udp:
> - return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> }
> #endif
>
> @@ -1307,6 +1328,10 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> if (!nfs_verify_server_address(sap))
> goto out_no_address;
>
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> +
> if (ctx->version == 4) {
> if (IS_ENABLED(CONFIG_NFS_V4)) {
> if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
> @@ -1315,9 +1340,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> port = NFS_PORT;
> max_namelen = NFS4_MAXNAMLEN;
> max_pathlen = NFS4_MAXPATHLEN;
> - nfs_validate_transport_protocol(ctx);
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> ctx->flags &= ~(NFS_MOUNT_NONLM | NFS_MOUNT_NOACL |
> NFS_MOUNT_VER3 | NFS_MOUNT_LOCAL_FLOCK |
> NFS_MOUNT_LOCAL_FCNTL);
> @@ -1326,10 +1348,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> }
> } else {
> nfs_set_mount_transport_protocol(ctx);
> -#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> -#endif
> if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
> port = NFS_RDMA_PORT;
> }
> @@ -1363,8 +1381,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> out_v4_not_compiled:
> nfs_errorf(fc, "NFS: NFSv4 is not compiled into kernel");
> return -EPROTONOSUPPORT;
> -out_invalid_transport_udp:
> - return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> out_no_address:
> return nfs_invalf(fc, "NFS: mount program didn't pass remote address");
> out_mountproto_mismatch:
> --
> 2.30.2
>

2021-04-01 03:55:14

by Eryu Guan

[permalink] [raw]
Subject: Re: [PATCH] NFS: Fix up the support for CONFIG_NFS_DISABLE_UDP_SUPPORT

On Tue, Mar 30, 2021 at 09:40:10AM -0400, [email protected] wrote:
> From: Trond Myklebust <[email protected]>
>
> Rather than removing the support in nfs_init_timeout_values(), we should
> just fix up the validation checks in the mount option parsers.
>
> Signed-off-by: Trond Myklebust <[email protected]>

Add a 'Fixes' tag? i.e.

Fixes: b24ee6c64ca7 ("NFS: allow deprecation of NFS UDP protocol")

Thanks,
Eryu

> ---
> fs/nfs/client.c | 2 --
> fs/nfs/fs_context.c | 54 +++++++++++++++++++++++++++++----------------
> 2 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 94d47be1d1f6..2aeb4e52a4f1 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -476,7 +476,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
> to->to_maxval = to->to_initval;
> to->to_exponential = 0;
> break;
> -#ifndef CONFIG_NFS_DISABLE_UDP_SUPPORT
> case XPRT_TRANSPORT_UDP:
> if (retrans == NFS_UNSPEC_RETRANS)
> to->to_retries = NFS_DEF_UDP_RETRANS;
> @@ -487,7 +486,6 @@ void nfs_init_timeout_values(struct rpc_timeout *to, int proto,
> to->to_maxval = NFS_MAX_UDP_TIMEOUT;
> to->to_exponential = 1;
> break;
> -#endif
> default:
> BUG();
> }
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 902db1262d2b..cdf32b9a6c35 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -283,20 +283,40 @@ static int nfs_verify_server_address(struct sockaddr *addr)
> return 0;
> }
>
> +#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
> +static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
> +{
> + return true;
> +}
> +#else
> +static bool nfs_server_transport_udp_invalid(const struct nfs_fs_context *ctx)
> +{
> + if (ctx->version == 4)
> + return true;
> + return false;
> +}
> +#endif
> +
> /*
> * Sanity check the NFS transport protocol.
> - *
> */
> -static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
> +static int nfs_validate_transport_protocol(struct fs_context *fc,
> + struct nfs_fs_context *ctx)
> {
> switch (ctx->nfs_server.protocol) {
> case XPRT_TRANSPORT_UDP:
> + if (nfs_server_transport_udp_invalid(ctx))
> + goto out_invalid_transport_udp;
> + break;
> case XPRT_TRANSPORT_TCP:
> case XPRT_TRANSPORT_RDMA:
> break;
> default:
> ctx->nfs_server.protocol = XPRT_TRANSPORT_TCP;
> }
> + return 0;
> +out_invalid_transport_udp:
> + return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> }
>
> /*
> @@ -305,8 +325,6 @@ static void nfs_validate_transport_protocol(struct nfs_fs_context *ctx)
> */
> static void nfs_set_mount_transport_protocol(struct nfs_fs_context *ctx)
> {
> - nfs_validate_transport_protocol(ctx);
> -
> if (ctx->mount_server.protocol == XPRT_TRANSPORT_UDP ||
> ctx->mount_server.protocol == XPRT_TRANSPORT_TCP)
> return;
> @@ -929,6 +947,7 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
> struct nfs_fh *mntfh = ctx->mntfh;
> struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
> int extra_flags = NFS_MOUNT_LEGACY_INTERFACE;
> + int ret;
>
> if (data == NULL)
> goto out_no_data;
> @@ -1054,6 +1073,10 @@ static int nfs23_parse_monolithic(struct fs_context *fc,
> goto generic;
> }
>
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> +
> ctx->skip_reconfig_option_check = true;
> return 0;
>
> @@ -1155,6 +1178,7 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
> {
> struct nfs_fs_context *ctx = nfs_fc2context(fc);
> struct sockaddr *sap = (struct sockaddr *)&ctx->nfs_server.address;
> + int ret;
> char *c;
>
> if (!data) {
> @@ -1227,9 +1251,9 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
> ctx->acdirmin = data->acdirmin;
> ctx->acdirmax = data->acdirmax;
> ctx->nfs_server.protocol = data->proto;
> - nfs_validate_transport_protocol(ctx);
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> done:
> ctx->skip_reconfig_option_check = true;
> return 0;
> @@ -1240,9 +1264,6 @@ static int nfs4_parse_monolithic(struct fs_context *fc,
>
> out_no_address:
> return nfs_invalf(fc, "NFS4: mount program didn't pass remote address");
> -
> -out_invalid_transport_udp:
> - return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> }
> #endif
>
> @@ -1307,6 +1328,10 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> if (!nfs_verify_server_address(sap))
> goto out_no_address;
>
> + ret = nfs_validate_transport_protocol(fc, ctx);
> + if (ret)
> + return ret;
> +
> if (ctx->version == 4) {
> if (IS_ENABLED(CONFIG_NFS_V4)) {
> if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
> @@ -1315,9 +1340,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> port = NFS_PORT;
> max_namelen = NFS4_MAXNAMLEN;
> max_pathlen = NFS4_MAXPATHLEN;
> - nfs_validate_transport_protocol(ctx);
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> ctx->flags &= ~(NFS_MOUNT_NONLM | NFS_MOUNT_NOACL |
> NFS_MOUNT_VER3 | NFS_MOUNT_LOCAL_FLOCK |
> NFS_MOUNT_LOCAL_FCNTL);
> @@ -1326,10 +1348,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> }
> } else {
> nfs_set_mount_transport_protocol(ctx);
> -#ifdef CONFIG_NFS_DISABLE_UDP_SUPPORT
> - if (ctx->nfs_server.protocol == XPRT_TRANSPORT_UDP)
> - goto out_invalid_transport_udp;
> -#endif
> if (ctx->nfs_server.protocol == XPRT_TRANSPORT_RDMA)
> port = NFS_RDMA_PORT;
> }
> @@ -1363,8 +1381,6 @@ static int nfs_fs_context_validate(struct fs_context *fc)
> out_v4_not_compiled:
> nfs_errorf(fc, "NFS: NFSv4 is not compiled into kernel");
> return -EPROTONOSUPPORT;
> -out_invalid_transport_udp:
> - return nfs_invalf(fc, "NFS: Unsupported transport protocol udp");
> out_no_address:
> return nfs_invalf(fc, "NFS: mount program didn't pass remote address");
> out_mountproto_mismatch:
> --
> 2.30.2