[nfsv4] Re: question around CB_GETATTR handling

[Trond Myklebust <[e-mail]>]

On Wed, 2024-09-04 at 15:33 -0400, Jeff Layton wrote:
> On Wed, 2024-09-04 at 19:21 +0000, Trond Myklebust wrote:
> > On Wed, 2024-09-04 at 12:23 -0400, Jeff Layton wrote:
> > > RFC 8881, section 10.4.3 has this pseudo code to describe how to
> > > handle
> > > the size and change attr:
> > > 
> > > ------------------8<-------------------
> > >     if (!modified) {
> > >         do CB_GETATTR for change and size;
> > > 
> > >         if (cc != sc)
> > >             modified = TRUE;
> > >     } else {
> > >         do CB_GETATTR for size;
> > >     }
> > > 
> > >     if (modified) {
> > >         sc = sc + 1;
> > >         time_modify = time_metadata = current_time;
> > >         update sc, time_modify, time_metadata into file's
> > > metadata;
> > >     }
> > > ------------------8<-------------------
> > > 
> > > 
> > > The line that shows sc = sc + 1 seems to indicate that we need to
> > > increment "sc" on every CB_GETATTR, even if nothing changed since
> > > the
> > > last time.
> > > 
> > > Is that correct or should we only increment it if it wasn't
> > > incremented
> > > before? The latter would make more sense, given that it doesn't
> > > query
> > > for change if the file is already considered to be modified.
> > 
> > I believe RFC5661 says
> > 
> >    For simplicity of implementation, the client MAY for each
> > CB_GETATTR
> >    return the same value d.  This is true even if, between
> > successive
> >    CB_GETATTR operations, the client again modifies the file's data
> > or
> >    metadata in its cache.  The client can return the same value
> > because
> >    the only requirement is that the client be able to indicate to
> > the
> >    server that the client holds modified data.  Therefore, the
> > value of
> >    d may always be c + 1.
> > 
> > Which is how the Linux client tries to implement it.
> > 
> 
> Right, 8881 is similar.

Yeah, but nobody has coded to that.

> 
> In this case though, I'm asking about the server-side implementation.
> That has its own algorithm (illustrated above in the pseudocode), and
> it seems to indicate that once the server detects that the client has
> made a single modification to the file, that it needs to increment
> "sc"
> on every subsequent CB_GETATTR.
> 
> That makes no sense on its face, so I suspect this is a mistake in
> the
> pseudocode above. Or am I wrong and this is actually necessary?
> -- 

Oh... No, as far as the server is concerned, I think it is indeed
supposed to increment the change attribute and timestamps on every
reply. That is reinforced here:

   As discussed earlier in this section, the client MAY return the same
   cc value on subsequent CB_GETATTR calls, even if the file was
   modified in the client's cache yet again between successive
   CB_GETATTR calls.  Therefore, the server must assume that the file
   has been modified yet again, and MUST take care to ensure that the
   new nsc it constructs and returns is greater than the previous nsc it
   returned.

The spec also says that "committing the constructed attribute values to
stable storage is OPTIONAL". That seems misleading or possibly just
incorrect. The fact that the client declares that it is caching data
doesn't necessarily mean that it will actually manage to write that
data back.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[e-mail]