> On Sep 20, 2024, at 12:34 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Fri, 2024-09-20 at 16:30 +0000, Chuck Lever III wrote:
>>
>>> On Sep 20, 2024, at 12:24 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>>
>>> On Fri, 2024-09-20 at 12:05 -0400, Olga Kornievskaia wrote:
>>>> Current, the server returns that it supports NFS4_SHARE_WANT_DELEG_TIMESTAMPS
>>>> but when the client sends that on the open, knfsd returns back with
>>>> bad_xdr error.
>>>>
>>>> Fixes: d0eab73d48a0 ("nfsd: add support for delegated timestamps")
>>>> Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
>>>> ---
>>>> fs/nfsd/nfs4xdr.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>>>> index c0bad580ab6d..adda8b489175 100644
>>>> --- a/fs/nfsd/nfs4xdr.c
>>>> +++ b/fs/nfsd/nfs4xdr.c
>>>> @@ -1109,6 +1109,7 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
>>>> case NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED:
>>>> case (NFS4_SHARE_SIGNAL_DELEG_WHEN_RESRC_AVAIL |
>>>> NFS4_SHARE_PUSH_DELEG_WHEN_UNCONTENDED):
>>>> + case NFS4_SHARE_WANT_DELEG_TIMESTAMPS:
>>>> return nfs_ok;
>>>> }
>>>> return nfserr_bad_xdr;
>>>
>>> Ouch.
>>>
>>> The problem here is that we had to drop the patch that added
>>> OPEN_XOR_DELEGATION support. That patch reworked the flag handling in
>>> this function in a way that allowed the new delstid flags to be
>>> properly supported.
>>>
>>> I think we probably want to resurrect the parts of this patch that
>>> alter nfsd4_decode_share_access:
>>>
>>> https://lore.kernel.org/linux-nfs/20240905-delstid-v4-8-d3e5fd34d107@xxxxxxxxxx/
>>>
>>> Olga, would you be OK with that approach instead?
>>
>> At this point, I'd like to consider postponing the delstid patches
>> until v6.13. They haven't gotten enough testing in their current
>> form...
>
> It's your call, but that seems like an extreme reaction to a flag
> handling fix, given that we have several weeks of -rc's ahead of us.
It's not just this small problem that worries me.
I pulled in v4 of the delstid patches pretty much at the last
moment for v6.12 development. So far, there's been an 80%
performance regression, and the surgery to work around that
issue introduced another bug that invalidates all NFSv4.2 test
results since that surgery, just a week ago.
The delstid patches impact an already heavily-used code path
(OPEN). You've already found one or two bugs in the client
side as well. This isn't something the can be disabled by
distros until it is ready -- it's baked into a popular code
path as soon as it goes upstream, and NFSv4.2 is the current
default.
So I can't say with confidence that this code is stable and
ready to merge. If we had another week or two before v6.11
final, there wouldn't be a question, but it's already a week
into the v6.12 merge window, and I need to submit a pull
request in the next few days. A 6.11-rc8 would have been
extremely helpful, but we didn't get one.
Exposing this change set to the cauldron that is bake-a-thon
would be valuable, IMO, for both the client and the server.
--
Chuck Lever
