Re: [PATCH v3 4/5] nfsd: add a header describing upcall to nfsdcld

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 21 Dec 2011 16:37:30 -0500
Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:

> 
> On Dec 21, 2011, at 4:33 PM, Jeff Layton wrote:
> 
> > On Wed, 21 Dec 2011 15:45:01 -0500
> > Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> > 
> >> 
> >> On Dec 21, 2011, at 3:34 PM, Jeff Layton wrote:
> >> 
> >>> The daemon takes a versioned binary struct. Hopefully this should allow
> >>> us to revise the struct later if it becomes necessary.
> >> 
> >> This breaks the pattern of using text-based up- and downcalls in NFSD.  I assume this is binary because nfs_client_id4 is a string of opaque bytes?
> >> 
> > 
> > That's the main reason. We could, of course encode that string in hex or
> > something, and decode it on the other end. No one has presented a
> > strong argument for doing it that way as of yet though.
> > 
> > If anyone feels strongly about that, then it would be helpful to have
> > them pipe up now and state why they do...
> 
> <pipe>Because binary data structures are difficult to work with over time, which is why other NFSD user space interfaces are text-based.</pipe>
> 
> ;-)
> 
> 

Now that I've started looking at actually implementing this, I'm
leaning toward keeping this as a packed binary struct. The upcalls to
which you refer mostly use the cache_* interfaces in the kernel, and
the actual upcall "pipes" are in /proc. That code is already set up to
use primarily text based interfaces so it makes sense there.

I looked at using that here and determined that the cache_* interface
was not well suited to this task. Those are geared toward interfaces
(like exportfs or mountd) where the information is primarily stored and
manipulated in userspace and the kernel upcalls to fetch that info and
populate its cache.

In this case, the info is mostly coming from kernel space originally
and I believe that rpc_pipefs was better suited for this task. The
upcalls that use rpc_pipefs almost universally use a binary struct to
pass data between userspace and the kernel.

Certainly this could use text or XDR, but I don't see that as a clear
benefit here. It's going to add a lot of extra overhead and complexity
to both the kernel and userspace code and we'll still have similar
concerns with extending the interface later.

The union that the code currently uses for passing call-specific args
is 1k currently. If we're concerned about that expandablity then we can
simply pad that out to a larger size.

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux