Re: [PATCH] kNFSD - Allowing rpc.nfsd to setting of the port, transport and version the server will use

[Steve Dickson <SteveD@redhat.com>]

Hey Neil,

Thanks for taking a look at this...

Neil Brown wrote:
> I've taken the 'version' bit, modified is so:
>  2/ You can only change the setting when there are no active threads. 
Good point...
> New version this part patch follows.  It should be completely
> compatible with your patch. from a user-space perspective.
cool... thanks for thinking about this...

> The 'port' bit I had trouble liking.
> You write:
>
>    family proto proto addr port
>
> to the 'ports' file.
> 'family' and 'addr' are completely ignored.
Well at this point they are not needed since we
only support ipv4... and I can only assume when we do
support other families, changing this interface will be
the least of our problems... ;-)

> 'port' is effectively ignored (value is stored in a variable which
> isn't used).
I'm not sure I understand this... nfsd_port is set and used in
nfsd_svc()
@@ -104,11 +152,14 @@ nfsd_svc(unsigned short port, int nrserv
 		nfsd_serv = svc_create(&nfsd_program, NFSD_BUFSIZE);
 		if (nfsd_serv == NULL)
 			goto out;
+		if (NFSCTL_UDPISSET(nfsd_portbits))
+			port = nfsd_port;
 		error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
 		if (error < 0)
 			goto failure;
 #ifdef CONFIG_NFSD_TCP
+		if (NFSCTL_TCPISSET(nfsd_portbits))
+			port = nfsd_port;
 		error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
 		if (error < 0)
 			goto failure;

iff the nfsd_portbits are set... which is exactly the same concept
used with the version bits... so I am a bit confused on what you want...
> That leaves 'proto' and 'proto'.  One should be 'tcp' or 'notcp', the
> other should be 'udp' or 'noudp'.  Which is which?  Udp comes first,
> but it isn't at all obvious from the interface..
Maybe being the author of these interfaces you have a better perspective
than I, but I could say the same thing about all the interfaces under
/proc/fs/nfsd... :-) So I do agree... its not that obvious, but I guess
I didn't think it needed to be.. Who else do you see, other than
rpc.nfsd, using this interface?

> I think you should write:
>  [+-]family proto addr port
>
> and every field must be checked and used.
> So while we only support ipv4, the 'family' must by 'ipv4' or an error
> is returned.
> '+' adds an endpoint.  '-' removes it.
Understood... And I'll assume the default of both TCP and UDP
listening on port 2048 will stay the same when nothing
is specified...

> The old nfssvc syscall should add 'ipv4 udp * %port' and 'ipv4 tcp *
> %port' if they don't already exist.
Won't this break backwards compatibility with all the 2.4 kernels?
The beauty of seq files is that you can change the interface
and have no effect on kABI at all... which is really a good thing
in my world... So why do we even care about the old syscall interface?

> An alternate interface, which would be quite appealing, would be to
> require the user-space program to create and bind a socket and then
> communicate it to the kernel, possibly by writing a file-descriptor
> number to a file in the nfsd filesystem.
> 'nfsd' would check it is an appropriate type of socket, take
> an extra reference, and use it.
> This would probably be best done *after* the nfsd threads were
> started, so there would need to be a way to start threads without
> them automatically opening sockets.  I'm not sure what the best
> interface for that would be... Maybe establishing sockets before the
> thread would be ok.
Maybe I'm missing something here.... but I'm not quite sure how passing
a fd to the kernel would help, other than (possibly) with error 
processing... The kernel will still need to know the port and proto so
it can register them with the  portmapper... plus permissions could
become an problem especially with things like selinux running around..


steved.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/