[v3,0/9] Various NFSv4 state error handling fixes
mbox series

Message ID 20190920112348.69496-1-trond.myklebust@hammerspace.com
Headers show
Series
  • Various NFSv4 state error handling fixes
Related show

Message

Trond Myklebust Sept. 20, 2019, 11:23 a.m. UTC
Various NFSv4 fixes to ensure we handle state errors correctly. In
particular, we need to ensure that for COMPOUNDs like CLOSE and
DELEGRETURN, that may have an embedded LAYOUTRETURN, we handle the
layout state errors so that a retry of either the LAYOUTRETURN, or
the later CLOSE/DELEGRETURN does not corrupt the LAYOUTRETURN
reply.

Also ensure that if we get a NFS4ERR_OLD_STATEID, then we do our
best to still try to destroy the state on the server, in order to
avoid causing state leakage.

v2: Fix bug reports from Olga
 - Try to avoid sending old stateids on CLOSE/OPEN_DOWNGRADE when
   doing fully serialised NFSv4.0.
 - Ensure LOCKU initialises the stateid correctly.
v3: Fix locking
 - Ensure the patch "Handle NFS4ERR_OLD_STATEID in LOCKU" locks the
   stateid when copying it in nfs4_alloc_unlockdata().

Trond Myklebust (9):
  pNFS: Ensure we do clear the return-on-close layout stateid on fatal
    errors
  NFSv4: Clean up pNFS return-on-close error handling
  NFSv4: Handle NFS4ERR_DELAY correctly in return-on-close
  NFSv4: Handle RPC level errors in LAYOUTRETURN
  NFSv4: Add a helper to increment stateid seqids
  pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state
    seqid
  NFSv4: Fix OPEN_DOWNGRADE error handling
  NFSv4: Handle NFS4ERR_OLD_STATEID in CLOSE/OPEN_DOWNGRADE
  NFSv4: Handle NFS4ERR_OLD_STATEID in LOCKU

 fs/nfs/nfs4_fs.h   |  11 ++-
 fs/nfs/nfs4proc.c  | 209 +++++++++++++++++++++++++++++++--------------
 fs/nfs/nfs4state.c |  16 ----
 fs/nfs/pnfs.c      |  71 +++++++++++++--
 fs/nfs/pnfs.h      |  17 +++-
 5 files changed, 233 insertions(+), 91 deletions(-)

Comments

Olga Kornievskaia Sept. 20, 2019, 9:01 p.m. UTC | #1
Hi Trond,

This version works for me. I went back to v2 and verified that it
didn't work so whatever you did here fixed it for me.

On Fri, Sep 20, 2019 at 7:26 AM Trond Myklebust <trondmy@gmail.com> wrote:
>
> Various NFSv4 fixes to ensure we handle state errors correctly. In
> particular, we need to ensure that for COMPOUNDs like CLOSE and
> DELEGRETURN, that may have an embedded LAYOUTRETURN, we handle the
> layout state errors so that a retry of either the LAYOUTRETURN, or
> the later CLOSE/DELEGRETURN does not corrupt the LAYOUTRETURN
> reply.
>
> Also ensure that if we get a NFS4ERR_OLD_STATEID, then we do our
> best to still try to destroy the state on the server, in order to
> avoid causing state leakage.
>
> v2: Fix bug reports from Olga
>  - Try to avoid sending old stateids on CLOSE/OPEN_DOWNGRADE when
>    doing fully serialised NFSv4.0.
>  - Ensure LOCKU initialises the stateid correctly.
> v3: Fix locking
>  - Ensure the patch "Handle NFS4ERR_OLD_STATEID in LOCKU" locks the
>    stateid when copying it in nfs4_alloc_unlockdata().
>
> Trond Myklebust (9):
>   pNFS: Ensure we do clear the return-on-close layout stateid on fatal
>     errors
>   NFSv4: Clean up pNFS return-on-close error handling
>   NFSv4: Handle NFS4ERR_DELAY correctly in return-on-close
>   NFSv4: Handle RPC level errors in LAYOUTRETURN
>   NFSv4: Add a helper to increment stateid seqids
>   pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state
>     seqid
>   NFSv4: Fix OPEN_DOWNGRADE error handling
>   NFSv4: Handle NFS4ERR_OLD_STATEID in CLOSE/OPEN_DOWNGRADE
>   NFSv4: Handle NFS4ERR_OLD_STATEID in LOCKU
>
>  fs/nfs/nfs4_fs.h   |  11 ++-
>  fs/nfs/nfs4proc.c  | 209 +++++++++++++++++++++++++++++++--------------
>  fs/nfs/nfs4state.c |  16 ----
>  fs/nfs/pnfs.c      |  71 +++++++++++++--
>  fs/nfs/pnfs.h      |  17 +++-
>  5 files changed, 233 insertions(+), 91 deletions(-)
>
> --
> 2.21.0
>
Trond Myklebust Sept. 21, 2019, 4:11 a.m. UTC | #2
Hi Olga

On Fri, 2019-09-20 at 17:01 -0400, Olga Kornievskaia wrote:
> Hi Trond,
> 
> This version works for me. I went back to v2 and verified that it
> didn't work so whatever you did here fixed it for me.
> 

That has me a little confused because I can't really see how the lack
of spinlocks could cause incorrect behaviour that consistently, however
I'm fine with the result. ☺

Thanks for the help with testing!

Trond

> On Fri, Sep 20, 2019 at 7:26 AM Trond Myklebust <trondmy@gmail.com>
> wrote:
> > Various NFSv4 fixes to ensure we handle state errors correctly. In
> > particular, we need to ensure that for COMPOUNDs like CLOSE and
> > DELEGRETURN, that may have an embedded LAYOUTRETURN, we handle the
> > layout state errors so that a retry of either the LAYOUTRETURN, or
> > the later CLOSE/DELEGRETURN does not corrupt the LAYOUTRETURN
> > reply.
> > 
> > Also ensure that if we get a NFS4ERR_OLD_STATEID, then we do our
> > best to still try to destroy the state on the server, in order to
> > avoid causing state leakage.
> > 
> > v2: Fix bug reports from Olga
> >  - Try to avoid sending old stateids on CLOSE/OPEN_DOWNGRADE when
> >    doing fully serialised NFSv4.0.
> >  - Ensure LOCKU initialises the stateid correctly.
> > v3: Fix locking
> >  - Ensure the patch "Handle NFS4ERR_OLD_STATEID in LOCKU" locks the
> >    stateid when copying it in nfs4_alloc_unlockdata().
> > 
> > Trond Myklebust (9):
> >   pNFS: Ensure we do clear the return-on-close layout stateid on
> > fatal
> >     errors
> >   NFSv4: Clean up pNFS return-on-close error handling
> >   NFSv4: Handle NFS4ERR_DELAY correctly in return-on-close
> >   NFSv4: Handle RPC level errors in LAYOUTRETURN
> >   NFSv4: Add a helper to increment stateid seqids
> >   pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the
> > state
> >     seqid
> >   NFSv4: Fix OPEN_DOWNGRADE error handling
> >   NFSv4: Handle NFS4ERR_OLD_STATEID in CLOSE/OPEN_DOWNGRADE
> >   NFSv4: Handle NFS4ERR_OLD_STATEID in LOCKU
> > 
> >  fs/nfs/nfs4_fs.h   |  11 ++-
> >  fs/nfs/nfs4proc.c  | 209 +++++++++++++++++++++++++++++++--------
> > ------
> >  fs/nfs/nfs4state.c |  16 ----
> >  fs/nfs/pnfs.c      |  71 +++++++++++++--
> >  fs/nfs/pnfs.h      |  17 +++-
> >  5 files changed, 233 insertions(+), 91 deletions(-)
> > 
> > --
> > 2.21.0
> >