Bug 248149 - [RFE][PATCH] adding a mechanism to allow multihomed NFS clients to use a specific address
Summary: [RFE][PATCH] adding a mechanism to allow multihomed NFS clients to use a spec...
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Steve Dickson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-07-13 13:59 UTC by Jeff Layton
Modified: 2019-02-13 22:29 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)
patch -- bind nfsv4 sockets to clientaddr (1.39 KB, patch)
2007-07-13 14:12 UTC, Jeff Layton
no flags Details | Diff
patch -- add new mount option "srcaddr=" (7.30 KB, patch)
2007-07-16 19:30 UTC, Jeff Layton
no flags Details | Diff
patch -- add new srcaddr option (6.97 KB, patch)
2007-07-18 12:59 UTC, Jeff Layton
no flags Details | Diff

Comment 1 Jeff Layton 2007-07-13 14:08:37 UTC
As foundation, we'll need some patches that have just hit Trond's git tree:

commit c98451bdb2f3e6d6cc1e03adad641e9497512b49
Author: Frank van Maarseveen <frankvm>
Date:   Mon Jul 9 22:25:29 2007 +0200

    NLM: fix source address of callback to client
    
    Use the destination address of the original NLM request as the
    source address in callbacks to the client.
    
    Signed-off-by: Frank van Maarseveen <frankvm>
    Signed-off-by: Trond Myklebust <Trond.Myklebust>

commit d3bc9a1deb8964d774af8535814cb91bf8f6def0
Author: Frank van Maarseveen <frankvm>
Date:   Mon Jul 9 22:23:35 2007 +0200

    SUNRPC client: add interface for binding to a local address
    
    In addition to binding to a local privileged port the NFS client should
    allow binding to a specific local address. This is used by the server
    for callbacks. The patch adds the necessary interface.
    
    Signed-off-by: Frank van Maarseveen <frankvm>
    Signed-off-by: Trond Myklebust <Trond.Myklebust>

commit a97476926ec061f90b77da478620ea6dc71a3237
Author: Frank van Maarseveen <frankvm>
Date:   Mon Jul 9 22:21:39 2007 +0200

    SUNRPC server: record the destination address of a request
    
    Save the destination address of an incoming request over TCP like is
    done already for UDP. It is necessary later for callbacks by the server.
    
    Signed-off-by: Frank van Maarseveen <frankvm>
    Signed-off-by: Trond Myklebust <Trond.Myklebust>

commit 96802a095171f5b35cf0e1e0d4be943e6696a253
Author: Frank van Maarseveen <frankvm>
Date:   Sun Jul 8 13:08:54 2007 +0200

    SUNRPC: cleanup transport creation argument passing
    
    Cleanup argument passing to functions for creating an RPC transport.
    
    Signed-off-by: Frank van Maarseveen <frankvm>
    Signed-off-by: Trond Myklebust <Trond.Myklebust>

...I'm thinking these will probably hit 2.6.23. Technically, I think we only
really need:

96802a095171f5b35cf0e1e0d4be943e6696a253
d3bc9a1deb8964d774af8535814cb91bf8f6def0

...but these were proposed as a set. We'll also need a small patch on top of
these that takes advantage of the new xs_bind interface.

Comment 2 Jeff Layton 2007-07-13 14:12:53 UTC
Created attachment 159189 [details]
patch -- bind nfsv4 sockets to clientaddr

This patch makes it so that NFSv4 sockets are automatically bound to the
clientaddr callback address. It makes sense to me that the socket should use
the same src address as the callback, though perhaps there are reasons not to
do so. This patch relies on the 4 patches in the previous comment. Cursory
testing indicates that it works.

I'll write this up and propose upstream soon.

Comment 3 Jeff Layton 2007-07-13 15:09:00 UTC
Similar patch posted to nfs and nfsv4:

Subject: [PATCH] have NFSv4 nfs sockets bind to the callback address

Patch I posted is slightly different -- it does extra initialization of the
sin_client struct. I'm not sure that's needed, but it shouldn't hurt. Awaiting
feedback...

Comment 4 Jeff Layton 2007-07-16 12:06:25 UTC
Trond pointed out that the callback address need not necessarily be on any
interface on the machine (consider a NAT'ed setup). So this scheme won't work at
all.

If this is to be done, it's going to require a new mount option (srcaddr or
something maybe). I need to have a closer look at Chuck's string option patches
and see how best to do this.


Comment 5 Jeff Layton 2007-07-16 19:30:40 UTC
Created attachment 159363 [details]
patch -- add new mount option "srcaddr="

This is a first (untested) pass at a kernel patch for this. It adds a new
srcaddr= mount option. Currently, it doesn't do any validation of the address
at all, so that will likely need to be added in before it goes upstream.

Comment 7 Jeff Layton 2007-07-17 20:25:32 UTC
I've been doing some work on this today. I've gotten something written (but
untested as of yet). I've been doing it against current upstream now, which
means I'm integrating it in with Chuck's string-based option patches. This is
most likely a 2.6.24-ish patch at best anyway.

I don't see this going in before f8 most likely anyway...




Comment 8 Jeff Layton 2007-07-18 12:59:07 UTC
Created attachment 159516 [details]
patch -- add new srcaddr option

Newest patch that I'm looking at. This should add the srcaddr option, and make
it so that the local socket it bound to it. This is still very experimental.

Some nits:
1) I don't like all of the copying of the address from struct to struct, but I
don't see a good way to avoid it.

2) I don't like messing with includes. nfs4_mount.h doesn't know anything about
sockaddr structs, so I had to include linux/socket.h from it. There may be a
better way to do it that doesn't require this.

Comment 9 Jeff Layton 2007-07-18 13:00:16 UTC
Note that there are also some v6 patches that are a WIP upstream too and this
may well affect them. So it's quite possible we'll need to respin this to be
more transport agnostic.


Comment 10 Jeff Layton 2007-07-18 14:23:19 UTC
Patch seems to work. I tested it using a simple C program that passed hardcoded
strings into a mount() call, and looking at the socket created in netstat -an.

I'm going to hold off on proposing this upstream just yet, however. The patches
for string-based mount options seem to be in Linus' tree now, but we still don't
have mount.nfs[4] programs that pass strings as mount options. This patch adds
fields to nfs_mount_data/nfs4_mount_data and so the versions of those structs
need to be revved. That's always a painful experience, so it's prob best to wait
until all of the string-based stuff is actually upstream.


Comment 11 Frank Hirtz 2007-08-03 18:17:38 UTC
Jeff,

It appears to be so from a quick, inexpert look at the patch, but to confirm
this would be useful for both NFSv3 and NFSv4?

Comment 12 Jeff Layton 2007-08-03 18:35:07 UTC
Yes, the new option should work regardless of NFS version.


Comment 13 Jeff Layton 2007-08-21 18:03:18 UTC
Upstream has merged a good portion of Chuck Lever's IPv6 patches. The patches in
this BZ will probably no longer apply and will need to be respun. I'll likely
also have to make the patch accept an IPv6 source address as well in order to
gain upstream acceptance.


Note You need to log in before you can comment on or make changes to this bug.