2024-05-06 21:04:18

by Chuck Lever

[permalink] [raw]
Subject: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous

From: Chuck Lever <[email protected]>

We've discovered that delivering a CB_OFFLOAD operation can be
unreliable in some pretty unremarkable situations, and the Linux
NFS client does not yet support sending an OFFLOAD_STATUS
operation to probe whether an asynchronous COPY operation has
finished. On Linux NFS clients, COPY can hang until manually
interrupted.

I've tried a couple of remedies, but so far the side-effects are
worse than the disease. For now, force COPY operations to be
synchronous so that the use of CB_OFFLOAD is avoided entirely.

I have some patches that add an OFFLOAD_STATUS implementation to the
Linux NFS client, but that is not likely to fix older clients.

Signed-off-by: Chuck Lever <[email protected]>
---
fs/nfsd/nfs4proc.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index ea3cc3e870a7..12722c709cc6 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
__be32 status;
struct nfsd4_copy *async_copy = NULL;

+ /*
+ * Currently, async COPY is not reliable. Force all COPY
+ * requests to be synchronous to avoid client application
+ * hangs waiting for completion.
+ */
+ nfsd4_copy_set_sync(copy, true);
+
copy->cp_clp = cstate->clp;
if (nfsd4_ssc_is_inter(copy)) {
trace_nfsd_copy_inter(copy);

base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
--
2.44.0



2024-05-06 22:30:34

by Rick Macklem

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous

On Mon, May 6, 2024 at 2:04 PM <[email protected]> wrote:
>
> From: Chuck Lever <[email protected]>
>
> We've discovered that delivering a CB_OFFLOAD operation can be
> unreliable in some pretty unremarkable situations, and the Linux
> NFS client does not yet support sending an OFFLOAD_STATUS
> operation to probe whether an asynchronous COPY operation has
> finished. On Linux NFS clients, COPY can hang until manually
> interrupted.
>
> I've tried a couple of remedies, but so far the side-effects are
> worse than the disease. For now, force COPY operations to be
> synchronous so that the use of CB_OFFLOAD is avoided entirely.
Just a yellow warning light from my experience with the FreeBSD server
(which always does synchronous Copy's).
A large synchronous Copy can take a long time, resulting in a slow RPC
response time. A user reported 25sec.
The 25sec case turned out to be a ZFS specific issue that could be avoided
via a ZFS tunable.

I do not have a good generic solution, but I do have a tunable that can
be used to clip the Copy len, which is a rather blunt and ugly workaround.
(The generic copy routine internal to FreeBSD can accept a flag that indicates
"return after 1sec with a partial copy done", but that is not implemented for
file systems like ZFS.)

Although there is no hard limit in the RFCs for RPC response times,
I've typically
assumed a max of 1-2sec is desirable.

rick

>
> I have some patches that add an OFFLOAD_STATUS implementation to the
> Linux NFS client, but that is not likely to fix older clients.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index ea3cc3e870a7..12722c709cc6 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> __be32 status;
> struct nfsd4_copy *async_copy = NULL;
>
> + /*
> + * Currently, async COPY is not reliable. Force all COPY
> + * requests to be synchronous to avoid client application
> + * hangs waiting for completion.
> + */
> + nfsd4_copy_set_sync(copy, true);
> +
> copy->cp_clp = cstate->clp;
> if (nfsd4_ssc_is_inter(copy)) {
> trace_nfsd_copy_inter(copy);
>
> base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
> --
> 2.44.0
>
>

2024-05-06 23:37:31

by Dai Ngo

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous


On 5/6/24 2:04 PM, [email protected] wrote:
> From: Chuck Lever <[email protected]>
>
> We've discovered that delivering a CB_OFFLOAD operation can be
> unreliable in some pretty unremarkable situations,

Since the fore and back channel use the same connection so I assume
this is not a connection related problem.

Sounds like this is a bug that we should find and fix if possible
instead of work around it. Do you know any scenarios where the
CB_OFFLOAD operation is unreliable?

-Dai

> and the Linux
> NFS client does not yet support sending an OFFLOAD_STATUS
> operation to probe whether an asynchronous COPY operation has
> finished. On Linux NFS clients, COPY can hang until manually
> interrupted.
>
> I've tried a couple of remedies, but so far the side-effects are
> worse than the disease. For now, force COPY operations to be
> synchronous so that the use of CB_OFFLOAD is avoided entirely.
>
> I have some patches that add an OFFLOAD_STATUS implementation to the
> Linux NFS client, but that is not likely to fix older clients.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index ea3cc3e870a7..12722c709cc6 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> __be32 status;
> struct nfsd4_copy *async_copy = NULL;
>
> + /*
> + * Currently, async COPY is not reliable. Force all COPY
> + * requests to be synchronous to avoid client application
> + * hangs waiting for completion.
> + */
> + nfsd4_copy_set_sync(copy, true);
> +
> copy->cp_clp = cstate->clp;
> if (nfsd4_ssc_is_inter(copy)) {
> trace_nfsd_copy_inter(copy);
>
> base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2

2024-05-07 00:22:42

by Chuck Lever III

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous

On Mon, May 06, 2024 at 03:30:18PM -0700, Rick Macklem wrote:
> On Mon, May 6, 2024 at 2:04 PM <[email protected]> wrote:
> >
> > From: Chuck Lever <[email protected]>
> >
> > We've discovered that delivering a CB_OFFLOAD operation can be
> > unreliable in some pretty unremarkable situations, and the Linux
> > NFS client does not yet support sending an OFFLOAD_STATUS
> > operation to probe whether an asynchronous COPY operation has
> > finished. On Linux NFS clients, COPY can hang until manually
> > interrupted.
> >
> > I've tried a couple of remedies, but so far the side-effects are
> > worse than the disease. For now, force COPY operations to be
> > synchronous so that the use of CB_OFFLOAD is avoided entirely.
> Just a yellow warning light from my experience with the FreeBSD server
> (which always does synchronous Copy's).
> A large synchronous Copy can take a long time, resulting in a slow RPC
> response time. A user reported 25sec.
> The 25sec case turned out to be a ZFS specific issue that could be avoided
> via a ZFS tunable.
>
> I do not have a good generic solution, but I do have a tunable that can
> be used to clip the Copy len, which is a rather blunt and ugly workaround.
> (The generic copy routine internal to FreeBSD can accept a flag that indicates
> "return after 1sec with a partial copy done", but that is not implemented for
> file systems like ZFS.)
>
> Although there is no hard limit in the RFCs for RPC response times,
> I've typically
> assumed a max of 1-2sec is desirable.

This is not meant to be a permanent change, but rather one that can
be backported to LTS kernels if we see the need. A long-term fix
will re-enable async COPY but deal properly with the loss of a
CB_OFFLOAD.

The server should return NFS4ERR_DELAY if it expects to take a long
time, no?

> rick
>
> >
> > I have some patches that add an OFFLOAD_STATUS implementation to the
> > Linux NFS client, but that is not likely to fix older clients.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > fs/nfsd/nfs4proc.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index ea3cc3e870a7..12722c709cc6 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > __be32 status;
> > struct nfsd4_copy *async_copy = NULL;
> >
> > + /*
> > + * Currently, async COPY is not reliable. Force all COPY
> > + * requests to be synchronous to avoid client application
> > + * hangs waiting for completion.
> > + */
> > + nfsd4_copy_set_sync(copy, true);
> > +
> > copy->cp_clp = cstate->clp;
> > if (nfsd4_ssc_is_inter(copy)) {
> > trace_nfsd_copy_inter(copy);
> >
> > base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
> > --
> > 2.44.0
> >
> >

--
Chuck Lever

2024-05-07 00:31:48

by Rick Macklem

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous

On Mon, May 6, 2024 at 5:14 PM Chuck Lever <[email protected]> wrote:
>
> On Mon, May 06, 2024 at 03:30:18PM -0700, Rick Macklem wrote:
> > On Mon, May 6, 2024 at 2:04 PM <[email protected]> wrote:
> > >
> > > From: Chuck Lever <[email protected]>
> > >
> > > We've discovered that delivering a CB_OFFLOAD operation can be
> > > unreliable in some pretty unremarkable situations, and the Linux
> > > NFS client does not yet support sending an OFFLOAD_STATUS
> > > operation to probe whether an asynchronous COPY operation has
> > > finished. On Linux NFS clients, COPY can hang until manually
> > > interrupted.
> > >
> > > I've tried a couple of remedies, but so far the side-effects are
> > > worse than the disease. For now, force COPY operations to be
> > > synchronous so that the use of CB_OFFLOAD is avoided entirely.
> > Just a yellow warning light from my experience with the FreeBSD server
> > (which always does synchronous Copy's).
> > A large synchronous Copy can take a long time, resulting in a slow RPC
> > response time. A user reported 25sec.
> > The 25sec case turned out to be a ZFS specific issue that could be avoided
> > via a ZFS tunable.
> >
> > I do not have a good generic solution, but I do have a tunable that can
> > be used to clip the Copy len, which is a rather blunt and ugly workaround.
> > (The generic copy routine internal to FreeBSD can accept a flag that indicates
> > "return after 1sec with a partial copy done", but that is not implemented for
> > file systems like ZFS.)
> >
> > Although there is no hard limit in the RFCs for RPC response times,
> > I've typically
> > assumed a max of 1-2sec is desirable.
>
> This is not meant to be a permanent change, but rather one that can
> be backported to LTS kernels if we see the need. A long-term fix
> will re-enable async COPY but deal properly with the loss of a
> CB_OFFLOAD.
>
> The server should return NFS4ERR_DELAY if it expects to take a long
> time, no?
And then the client is going to try the same Copy again after waiting a while,
yes?
I think you need the Copy to make some progress, so that the client doesn't
just try try again. (Also, in FreeBSD it wouldn't be easy to "interrupt" a copy
that is in progress on a server file system so, at least in FreeBSD, I don't
know how the NFS server would know to expect a long delay. As I noted,
there is a "stop copying after 1sec" flag that, if obeyed by all file systems,
does allow some progress without taking an excessive time.)

rick

>
> > rick
> >
> > >
> > > I have some patches that add an OFFLOAD_STATUS implementation to the
> > > Linux NFS client, but that is not likely to fix older clients.
> > >
> > > Signed-off-by: Chuck Lever <[email protected]>
> > > ---
> > > fs/nfsd/nfs4proc.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > index ea3cc3e870a7..12722c709cc6 100644
> > > --- a/fs/nfsd/nfs4proc.c
> > > +++ b/fs/nfsd/nfs4proc.c
> > > @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > __be32 status;
> > > struct nfsd4_copy *async_copy = NULL;
> > >
> > > + /*
> > > + * Currently, async COPY is not reliable. Force all COPY
> > > + * requests to be synchronous to avoid client application
> > > + * hangs waiting for completion.
> > > + */
> > > + nfsd4_copy_set_sync(copy, true);
> > > +
> > > copy->cp_clp = cstate->clp;
> > > if (nfsd4_ssc_is_inter(copy)) {
> > > trace_nfsd_copy_inter(copy);
> > >
> > > base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
> > > --
> > > 2.44.0
> > >
> > >
>
> --
> Chuck Lever

2024-05-07 00:32:07

by Chuck Lever III

[permalink] [raw]
Subject: Re: [RFC PATCH] NFSD: Force all NFSv4.2 COPY requests to be synchronous

On Mon, May 06, 2024 at 04:37:15PM -0700, Dai Ngo wrote:
>
> On 5/6/24 2:04 PM, [email protected] wrote:
> > From: Chuck Lever <[email protected]>
> >
> > We've discovered that delivering a CB_OFFLOAD operation can be
> > unreliable in some pretty unremarkable situations,
>
> Since the fore and back channel use the same connection so I assume
> this is not a connection related problem.

This is totally a connection-related problem. The underlying issue
is that NFSD does not retransmit backchannel requests when the
connection is lost, and the Linux NFS client does not implement
OFFLOAD_STATUS. Neither side right now recovers from connection
loss while a COPY operation is pending.


> Sounds like this is a bug that we should find and fix if possible
> instead of work around it.

I've been looking for a fix for the past several months. The last
fix I put in, you asked me to revert. So, I would prefer to fix
the root cause of this issue, but right now the best we can do is
create a surgical patch that can be backported to LTS kernels, and
keep working on a longer term fix.

It's either temporarily force all COPY operations to become
synchronous, or temporarily drop support for COPY in NFSD. Actually
the latter sounds safer.


> Do you know any scenarios where the CB_OFFLOAD operation is
> unreliable?

Any scenario where the connection is dropped (say, because the
server wants the client to retransmit forechannel requests, or
because of a GSS sequence number window under-run, or because of a
network partition, etc) can potentially result in the loss of a
backchannel operation.

I can reproduce this issue 100% of the time with an NFSv4.2 mount
from a 6.8.7-200.fc39.x86_64 NFS client, using the git regression
suite.


> > and the Linux
> > NFS client does not yet support sending an OFFLOAD_STATUS
> > operation to probe whether an asynchronous COPY operation has
> > finished. On Linux NFS clients, COPY can hang until manually
> > interrupted.
> >
> > I've tried a couple of remedies, but so far the side-effects are
> > worse than the disease. For now, force COPY operations to be
> > synchronous so that the use of CB_OFFLOAD is avoided entirely.
> >
> > I have some patches that add an OFFLOAD_STATUS implementation to the
> > Linux NFS client, but that is not likely to fix older clients.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > fs/nfsd/nfs4proc.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index ea3cc3e870a7..12722c709cc6 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -1807,6 +1807,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > __be32 status;
> > struct nfsd4_copy *async_copy = NULL;
> > + /*
> > + * Currently, async COPY is not reliable. Force all COPY
> > + * requests to be synchronous to avoid client application
> > + * hangs waiting for completion.
> > + */
> > + nfsd4_copy_set_sync(copy, true);
> > +
> > copy->cp_clp = cstate->clp;
> > if (nfsd4_ssc_is_inter(copy)) {
> > trace_nfsd_copy_inter(copy);
> >
> > base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2

--
Chuck Lever