On Tuesday October 11, [email protected] wrote:
> >
> > 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... ;-)
Maybe, but either we make it ready for future needs now, or we only
support current needs and allow a clear upgrade path.
Ignoring fields which might later have a meaning is bad because it
means that user-space code which works now much break later for no
good reason.
My basic position wrt this is that if the fields are there, they
should be checked.
>
> > '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...
Uhmmmm.... that code fragment isn't in the patch you sent. I got:
> @@ -104,14 +152,17 @@ nfsd_svc(unsigned short port, int nrserv
> nfsd_serv = svc_create(&nfsd_program, NFSD_BUFSIZE);
> if (nfsd_serv == NULL)
> goto out;
> - error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
> - if (error < 0)
> - goto failure;
> -
> + if (!nfsd_portbits || NFSCTL_UDPISSET(nfsd_portbits)) {
> + error = svc_makesock(nfsd_serv, IPPROTO_UDP, port);
> + if (error < 0)
> + goto failure;
> + }
> #ifdef CONFIG_NFSD_TCP
> - error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
> - if (error < 0)
> - goto failure;
> + if (!nfsd_portbits || NFSCTL_TCPISSET(nfsd_portbits)) {
> + error = svc_makesock(nfsd_serv, IPPROTO_TCP, port);
> + if (error < 0)
> + goto failure;
> + }
> #endif
> do_gettimeofday(&nfssvc_boot); /* record boot time */
> } else
I guess if you merged these two, you might get something usable..
> >
> > 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?
Ok, I guess I presented by difficulty with this rather badly. The
order does seem arbitrary and non-obvious, but you are right that
other things are no-obvious too...
The point I should have made was that it is non-extensible.
Suppose NFSv4.4 is defined to work over SCTP to even DCCP (which might
be ideal for NFS), how would we specify those?
>
> > 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...
Yes... but I'm not sure of the rest of the rules about defaults...
- should you need to explicitly remove the defaults if you don't want
them?
- should there be an easy way to revert-to-defaults? Maybe when the
last nfsd dies, all setting are forgotten (just like currently all
exports are removed).
>
> >
> > 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?
I don't see why it would break backwards compat. Isn't that exactly
what the nfssvc syscall does? You give it a port number, and it
starts the number of threads with two sockets, a udp bound to ADDR_ANY
and the given port, and a TCP bound the same way? What is what I
meant to say above.
>
> >
> > 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..
>
I think the principle of "do as much as possible in user-space" is a
good one.
socket / bind / pmap_register
can all be done in user-space, so maybe they should be.
You could possible even argue that
listen / accept
the be done in user-space, and so should be, but I'm not sure I want
to push it that far.
The nice thing about just passing down a filedescriptor for a socket
is that it allows for any future additions of protocols.
I don't see how permissions could be a problem, but then I know very
little about selinux, so maybe there is something. Is there something
specific you foresee, or is it just general concern?
NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
