Re: [PATCH 08/31] nfsd41: sanity check client drc maxreqs

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Thu, 2009-04-30 at 20:34 +0300, Benny Halevy wrote:
> On Apr. 28, 2009, 19:59 +0300, andros@netapp.com wrote:
> > From: Andy Adamson <andros@netapp.com>
> > 
> > Ensure the client requested maximum requests are between 1 and
> > NFSD_MAX_SLOTS_PER_SESSION
> > 
> > Signed-off-by: Andy Adamson <andros@netapp.com>
> > ---
> >  fs/nfsd/nfs4state.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index e216169..59b601b 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -427,6 +427,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
> >  {
> >  	int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
> >  
> > +	if (fchan->maxreqs < 1)
> > +		return nfserr_inval;
> 
> Is 0 prohibited by the protocol?
> The server can set it to whatever value it wants,
> or if we can live with it, it actually provides a
> nice way to test the server end-cases.
> 
> Benny

The draft spec doesn't appear to explicitly exclude the value 0 for
ca_maxrequests.

I suppose a client might be able to request a session with a back
channel, but no fore channel if it wants to. The problem is that it
wouldn't be able to send SEQUENCE or BACKCHANNEL_CTL requests, and so
managing that back channel would be tough. There would be no way for the
server to tell the client of a back channel fault, or of the GSS context
expiring, and no way for the client to change that context.

In practice, therefore, it probably isn't a bad idea to return
NFS4ERR_INVAL...

Cheers
  Trond