Re: NFS4 crack

[Neil Brown <neilb@suse.de>]

On Sunday September 18, hch@lst.de wrote:
> I've recently turned on NFS4 server support accidentally, just to get
> error messages like:
> 
> "NFSD: recovery directory /var/lib/nfs/v4recovery doesn't exist"
> 
> To my horror I found out that this comes from kernel code, which messes
> with a hardcoded directory, completelyu ingoring any namespace or other
> uses issues.  The fs handling in fs/nfs/nfs4recovery.c is rather broken
> in addition.
> 
> All this comes from "[PATCH] knfsd: nfsd4: initialize recovery directory",
> commit ID 190e4fbf96037e5e526ba3210f2bcc2a3b6fe964.

I confess that I am having trouble finding a convincing basis for your
position, which is why I allowed the patch through in the first place
(despite not particularly liking it).
My problem is: where do you draw the line?

It should be noted first that nfsd is unlike most (all?) other kernel
code.  It is an application that is running in-kernel.  It is a
consumer of kernel services, and provides no (significant) services to
user-space, or to other parts of the kernel. 

Now, this in-kernel-application needs to store stable
application-specific data somewhere.  May it:

 1/ open a directory and create files in it and write to them
 2/ open a directory and create files provided that the name of the
    directory is given by userspace
 3/ create files in a directory that was created by userspace and
    given to the knfsd application as a filedescriptor
 4/ write data to files which were created and opened by used-space
    based on filenames provided by knfsd (hostnames or equivalents in
    this case). 
 5/ pass the data to userspace and let it worry completely.
 6/ sorry, you cannot have application-specific state.

I'm sure you will see a progression here. I ask again: "where do you
draw the line?"  You seem to rule out 1, and probably 2, and possibly
3 based on other comments in the thread.   It cannot set a rational
place to draw the line other than before-1 or after-4.  i.e. if you
allow 4, you may as well allow 1 too.  
If you have give a clear argument for some particular place to draw
the line, I'd love to hear it, together with your justification.

While considering it, you might also like to consider:

 - is it ok for knfsd to bind to port 2049 ?
 - is it ok if userspace tells it the number '2049' ?
 - does user-space have to create/bind the socket and pass it to
   knfsd?
 - does user-space have to receive the packets and pass them to knfsd? 
   (ok, that one is really silly).

and "why?"

The reality is that NFS service is an application.  Currently parts of
it are in-kernel (nfsd, lockd) and parts are in user-space (portmap,
statd(*), mountd).
There are two positions on what-goes-where that make sense to me:
1- pragmatism:  put code where it works best.  I believe that the
     current code fits pragmatism quite well (modulo bugs).
2- "rightness":  If you want to argue from a what-belongs-where
     perspective, you have to say that knfsd doesn't belong in the
     kernel at all.  The kernel should just supply the core services
     (e.g. file-handle <-> fd mapping) and let userspace do the rest.

Were I starting to write knfsd today, I would pick 2.  Given where we
actually are today, I pick 1.


> we really need someone sane review NFS patches I thinkg.

yes please..  pretty please :-)

> 
> (not cc'ed to the nfs list because of its stupid subsribers only
> policy)

Sad, isn't it.  Both nfs@lists.sourceforge.net and nfsv4@linux-nfs.org
are like that, and nfs-devel@linux.kernel.org died long ago. :-(

NeilBrown

(*) There are patches in existence which move statd implementation
into the kernel.  The final conclusion here may well affect those
patches, so I hope Olaf has been listening in....