2020-10-15 13:37:21

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/1] NFSv4.2: support EXCHGID4_FLAG_SUPP_FENCE_OPS 4.2 EXCHANGE_ID flag

From: Olga Kornievskaia <[email protected]>

RFC 7862 introduced a new flag that either client or server is
allowed to set: EXCHGID4_FLAG_SUPP_FENCE_OPS.

Client needs to update its bitmask to allow for this flag value.

Signed-off-by: Olga Kornievskaia <[email protected]>
CC: <[email protected]>
---
fs/nfs/nfs4proc.c | 9 ++++++---
include/uapi/linux/nfs4.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6e95c85fe395..20f2e0f5c5ba 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8039,9 +8039,11 @@ int nfs4_proc_secinfo(struct inode *dir, const struct qstr *name,
* both PNFS and NON_PNFS flags set, and not having one of NON_PNFS, PNFS, or
* DS flags set.
*/
-static int nfs4_check_cl_exchange_flags(u32 flags)
+static int nfs4_check_cl_exchange_flags(u32 flags, int version)
{
- if (flags & ~EXCHGID4_FLAG_MASK_R)
+ if (version >= 2 && (flags & ~EXCHGID4_2_FLAG_MASK_R))
+ goto out_inval;
+ else if (version < 2 && (flags & ~EXCHGID4_FLAG_MASK_R))
goto out_inval;
if ((flags & EXCHGID4_FLAG_USE_PNFS_MDS) &&
(flags & EXCHGID4_FLAG_USE_NON_PNFS))
@@ -8454,7 +8456,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre
if (status != 0)
goto out;

- status = nfs4_check_cl_exchange_flags(resp->flags);
+ status = nfs4_check_cl_exchange_flags(resp->flags,
+ clp->cl_mvops->minor_version);
if (status != 0)
goto out;

diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
index bf197e99b98f..3faa94867fec 100644
--- a/include/uapi/linux/nfs4.h
+++ b/include/uapi/linux/nfs4.h
@@ -146,6 +146,7 @@
*/
#define EXCHGID4_FLAG_MASK_A 0x40070103
#define EXCHGID4_FLAG_MASK_R 0x80070103
+#define EXCHGID4_2_FLAG_MASK_R 0x80070104

#define SEQ4_STATUS_CB_PATH_DOWN 0x00000001
#define SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRING 0x00000002
--
2.18.2


2020-10-15 14:09:33

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/1] NFSv4.2: support EXCHGID4_FLAG_SUPP_FENCE_OPS 4.2 EXCHANGE_ID flag

Hi Olga,

On Thu, 2020-10-15 at 09:35 -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> RFC 7862 introduced a new flag that either client or server is
> allowed to set: EXCHGID4_FLAG_SUPP_FENCE_OPS.
>
> Client needs to update its bitmask to allow for this flag value.
>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> CC: <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 9 ++++++---
> include/uapi/linux/nfs4.h | 1 +
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6e95c85fe395..20f2e0f5c5ba 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8039,9 +8039,11 @@ int nfs4_proc_secinfo(struct inode *dir, const
> struct qstr *name,
> * both PNFS and NON_PNFS flags set, and not having one of NON_PNFS,
> PNFS, or
> * DS flags set.
> */
> -static int nfs4_check_cl_exchange_flags(u32 flags)
> +static int nfs4_check_cl_exchange_flags(u32 flags, int version)
> {
> - if (flags & ~EXCHGID4_FLAG_MASK_R)
> + if (version >= 2 && (flags & ~EXCHGID4_2_FLAG_MASK_R))
> + goto out_inval;
> + else if (version < 2 && (flags & ~EXCHGID4_FLAG_MASK_R))
> goto out_inval;
> if ((flags & EXCHGID4_FLAG_USE_PNFS_MDS) &&
> (flags & EXCHGID4_FLAG_USE_NON_PNFS))
> @@ -8454,7 +8456,8 @@ static int _nfs4_proc_exchange_id(struct
> nfs_client *clp, const struct cred *cre
> if (status != 0)
> goto out;
>
> - status = nfs4_check_cl_exchange_flags(resp->flags);
> + status = nfs4_check_cl_exchange_flags(resp->flags,
> + clp->cl_mvops->minor_version);
> if (status != 0)
> goto out;
>
> diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
> index bf197e99b98f..3faa94867fec 100644
> --- a/include/uapi/linux/nfs4.h
> +++ b/include/uapi/linux/nfs4.h
> @@ -146,6 +146,7 @@
> */
> #define EXCHGID4_FLAG_MASK_A 0x40070103
> #define EXCHGID4_FLAG_MASK_R 0x80070103
> +#define EXCHGID4_2_FLAG_MASK_R 0x80070104
>
> #define SEQ4_STATUS_CB_PATH_DOWN 0x00000001
> #define SEQ4_STATUS_CB_GSS_CONTEXTS_EXPIRING 0x00000002

Thanks! I find it very annoying that the NFSv4.2 spec allows the server
to return a new EXCHANGE_ID flag that is not backward compatible with
NFSv4.1, despite the client not changing its arguments and asking for
that behaviour.

Can you please add in a definition for EXCHGID4_FLAG_SUPP_FENCE_OPS
while we're at it, so that we can document why there is a change in the
mask?

Also please note that EXCHGID4_2_FLAG_MASK_R needs to take a value of
(EXCHGID4_FLAG_MASK_R | EXCHGID4_FLAG_SUPP_FENCE_OPS), since all the
existing flags in EXCHGID4_FLAG_MASK_R are still valid. So the above
define needs to read as

#define EXCHGID4_2_FLAG_MASK_R 0x80070107


--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]