* [PATCH 0/3] client-side lockd doesn't start UDP listener
@ 2008-10-03 21:15 Chuck Lever
[not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2008-10-03 21:15 UTC (permalink / raw)
To: bfields, neilb; +Cc: linux-nfs
Hi Bruce, Neil-
Here's my initial proposal to address the NFSv2/v3 lock recovery issue
that results from having no UDP lockd listener.
Comments? Did I miss anything?
---
Chuck Lever (3):
NLM: Remove unused argument from svc_addsock() function
NLM: Remove "proto" argument from lockd_up()
NLM: Always start both UDP and TCP listeners
fs/lockd/clntlock.c | 4 ++--
fs/lockd/svc.c | 40 ++++++++++++++++++++--------------------
fs/nfsd/nfsctl.c | 5 ++---
fs/nfsd/nfssvc.c | 19 +++++++------------
include/linux/lockd/bind.h | 2 +-
include/linux/sunrpc/svcsock.h | 5 +----
net/sunrpc/svcsock.c | 4 +---
7 files changed, 34 insertions(+), 45 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 14+ messages in thread[parent not found: <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>]
* [PATCH 1/3] NLM: Always start both UDP and TCP listeners [not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org> @ 2008-10-03 21:15 ` Chuck Lever 2008-10-03 21:15 ` [PATCH 2/3] NLM: Remove "proto" argument from lockd_up() Chuck Lever ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2008-10-03 21:15 UTC (permalink / raw) To: bfields, neilb; +Cc: linux-nfs Commit 24e36663, which first appeared in 2.6.19, changed lockd so that the client side starts a UDP listener only if there is a UDP NFSv2/v3 mount. Its description notes: This... means that lockd will *not* listen on UDP if the only mounts are TCP mount (and nfsd hasn't started). The latter is the only one that concerns me at all - I don't know if this might be a problem with some servers. Unfortunately it is a problem for Linux itself. The rpc.statd daemon on Linux uses UDP for contacting the local lockd, no matter which protocol is used for NFS mounts. Without a local lockd UDP listener, NFSv2/v3 lock recovery from Linux NFS clients always fails. Revert parts of commit 24e36663 so lockd_up() always starts both listeners. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: Neil Brown <neilb@suse.de> --- fs/lockd/svc.c | 37 +++++++++++++++++++------------------ 1 files changed, 19 insertions(+), 18 deletions(-) diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 1553fec..583d30d 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -203,25 +203,28 @@ lockd(void *vrqstp) } /* - * Make any sockets that are needed but not present. - * If nlm_udpport or nlm_tcpport were set as module - * options, make those sockets unconditionally + * Ensure there are active UDP and TCP listeners for lockd. + * + * Even if we have only TCP NFS mounts and/or TCP NFSDs, some + * local services (such as rpc.statd) still require UDP, and + * some NFS servers do not yet support NLM over TCP. + * + * Returns zero if all listeners are available; otherwise a + * negative errno value is returned. */ -static int make_socks(struct svc_serv *serv, int proto) +static int make_socks(struct svc_serv *serv) { static int warned; struct svc_xprt *xprt; int err = 0; - if (proto == IPPROTO_UDP || nlm_udpport) { - xprt = svc_find_xprt(serv, "udp", 0, 0); - if (!xprt) - err = svc_create_xprt(serv, "udp", nlm_udpport, - SVC_SOCK_DEFAULTS); - else - svc_xprt_put(xprt); - } - if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) { + xprt = svc_find_xprt(serv, "udp", 0, 0); + if (!xprt) + err = svc_create_xprt(serv, "udp", nlm_udpport, + SVC_SOCK_DEFAULTS); + else + svc_xprt_put(xprt); + if (err >= 0) { xprt = svc_find_xprt(serv, "tcp", 0, 0); if (!xprt) err = svc_create_xprt(serv, "tcp", nlm_tcpport, @@ -251,11 +254,8 @@ lockd_up(int proto) /* Maybe add a 'family' option when IPv6 is supported ?? */ /* * Check whether we're already up and running. */ - if (nlmsvc_rqst) { - if (proto) - error = make_socks(nlmsvc_rqst->rq_server, proto); + if (nlmsvc_rqst) goto out; - } /* * Sanity check: if there's no pid, @@ -272,7 +272,8 @@ lockd_up(int proto) /* Maybe add a 'family' option when IPv6 is supported ?? */ goto out; } - if ((error = make_socks(serv, proto)) < 0) + error = make_socks(serv); + if (error < 0) goto destroy_and_out; /* ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] NLM: Remove "proto" argument from lockd_up() [not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org> 2008-10-03 21:15 ` [PATCH 1/3] NLM: Always start both UDP and TCP listeners Chuck Lever @ 2008-10-03 21:15 ` Chuck Lever 2008-10-03 21:15 ` [PATCH 3/3] NLM: Remove unused argument from svc_addsock() function Chuck Lever 2008-10-04 21:36 ` [PATCH 0/3] client-side lockd doesn't start UDP listener J. Bruce Fields 3 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2008-10-03 21:15 UTC (permalink / raw) To: bfields, neilb; +Cc: linux-nfs Clean up: Now that lockd_up() starts listeners for both transports, the "proto" argument is no longer needed. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: Neil Brown <neilb@suse.de> --- fs/lockd/clntlock.c | 4 ++-- fs/lockd/svc.c | 3 +-- fs/nfsd/nfsctl.c | 5 ++--- fs/nfsd/nfssvc.c | 19 +++++++------------ include/linux/lockd/bind.h | 2 +- 5 files changed, 13 insertions(+), 20 deletions(-) diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c index 2976bf0..8307dd6 100644 --- a/fs/lockd/clntlock.c +++ b/fs/lockd/clntlock.c @@ -54,7 +54,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init) u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4; int status; - status = lockd_up(nlm_init->protocol); + status = lockd_up(); if (status < 0) return ERR_PTR(status); @@ -215,7 +215,7 @@ reclaimer(void *ptr) /* This one ensures that our parent doesn't terminate while the * reclaim is in progress */ lock_kernel(); - lockd_up(0); /* note: this cannot fail as lockd is already running */ + lockd_up(); /* note: this cannot fail as lockd is already running */ dprintk("lockd: reclaiming locks for host %s\n", host->h_name); diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 583d30d..ea9cfa4 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -244,8 +244,7 @@ static int make_socks(struct svc_serv *serv) /* * Bring up the lockd process if it's not already up. */ -int -lockd_up(int proto) /* Maybe add a 'family' option when IPv6 is supported ?? */ +int lockd_up(void) { struct svc_serv *serv; int error = 0; diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index c53e65f..862dff5 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -614,10 +614,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) return -EINVAL; err = nfsd_create_serv(); if (!err) { - int proto = 0; - err = svc_addsock(nfsd_serv, fd, buf, &proto); + err = svc_addsock(nfsd_serv, fd, buf, NULL); if (err >= 0) { - err = lockd_up(proto); + err = lockd_up(); if (err < 0) svc_sock_names(buf+strlen(buf)+1, nfsd_serv, buf); } diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 4eed938..052325a 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -244,25 +244,20 @@ static int nfsd_init_socks(int port) if (!list_empty(&nfsd_serv->sv_permsocks)) return 0; - error = lockd_up(IPPROTO_UDP); - if (error >= 0) { - error = svc_create_xprt(nfsd_serv, "udp", port, + error = svc_create_xprt(nfsd_serv, "udp", port, SVC_SOCK_DEFAULTS); - if (error < 0) - lockd_down(); - } if (error < 0) return error; - error = lockd_up(IPPROTO_TCP); - if (error >= 0) { - error = svc_create_xprt(nfsd_serv, "tcp", port, + error = svc_create_xprt(nfsd_serv, "tcp", port, SVC_SOCK_DEFAULTS); - if (error < 0) - lockd_down(); - } if (error < 0) return error; + + error = lockd_up(); + if (error < 0) + return error; + return 0; } diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h index 3d25bcd..7fa42fd 100644 --- a/include/linux/lockd/bind.h +++ b/include/linux/lockd/bind.h @@ -53,7 +53,7 @@ extern void nlmclnt_done(struct nlm_host *host); extern int nlmclnt_proc(struct nlm_host *host, int cmd, struct file_lock *fl); -extern int lockd_up(int proto); +extern int lockd_up(void); extern void lockd_down(void); unsigned long get_nfs_grace_period(void); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] NLM: Remove unused argument from svc_addsock() function [not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org> 2008-10-03 21:15 ` [PATCH 1/3] NLM: Always start both UDP and TCP listeners Chuck Lever 2008-10-03 21:15 ` [PATCH 2/3] NLM: Remove "proto" argument from lockd_up() Chuck Lever @ 2008-10-03 21:15 ` Chuck Lever 2008-10-04 21:36 ` [PATCH 0/3] client-side lockd doesn't start UDP listener J. Bruce Fields 3 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2008-10-03 21:15 UTC (permalink / raw) To: bfields, neilb; +Cc: linux-nfs Clean up: The svc_addsock() function no longer uses its "proto" argument, so remove it. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: Neil Brown <neilb@suse.de> --- fs/nfsd/nfsctl.c | 2 +- include/linux/sunrpc/svcsock.h | 5 +---- net/sunrpc/svcsock.c | 4 +--- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 862dff5..97543df 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -614,7 +614,7 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) return -EINVAL; err = nfsd_create_serv(); if (!err) { - err = svc_addsock(nfsd_serv, fd, buf, NULL); + err = svc_addsock(nfsd_serv, fd, buf); if (err >= 0) { err = lockd_up(); if (err < 0) diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h index 8cff696..483e103 100644 --- a/include/linux/sunrpc/svcsock.h +++ b/include/linux/sunrpc/svcsock.h @@ -39,10 +39,7 @@ int svc_send(struct svc_rqst *); void svc_drop(struct svc_rqst *); void svc_sock_update_bufs(struct svc_serv *serv); int svc_sock_names(char *buf, struct svc_serv *serv, char *toclose); -int svc_addsock(struct svc_serv *serv, - int fd, - char *name_return, - int *proto); +int svc_addsock(struct svc_serv *serv, int fd, char *name_return); void svc_init_xprt_sock(void); void svc_cleanup_xprt_sock(void); diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index f91377c..95293f5 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -1167,8 +1167,7 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv, int svc_addsock(struct svc_serv *serv, int fd, - char *name_return, - int *proto) + char *name_return) { int err = 0; struct socket *so = sockfd_lookup(fd, &err); @@ -1203,7 +1202,6 @@ int svc_addsock(struct svc_serv *serv, sockfd_put(so); return err; } - if (proto) *proto = so->sk->sk_protocol; return one_sock_name(name_return, svsk); } EXPORT_SYMBOL_GPL(svc_addsock); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener [not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org> ` (2 preceding siblings ...) 2008-10-03 21:15 ` [PATCH 3/3] NLM: Remove unused argument from svc_addsock() function Chuck Lever @ 2008-10-04 21:36 ` J. Bruce Fields 2008-10-12 23:15 ` Neil Brown 3 siblings, 1 reply; 14+ messages in thread From: J. Bruce Fields @ 2008-10-04 21:36 UTC (permalink / raw) To: Chuck Lever; +Cc: neilb, linux-nfs On Fri, Oct 03, 2008 at 05:15:14PM -0400, Chuck Lever wrote: > Hi Bruce, Neil- > > Here's my initial proposal to address the NFSv2/v3 lock recovery issue > that results from having no UDP lockd listener. > > Comments? Did I miss anything? Looks fine; I can't see any problem. So I've applied to for-2.6.28. (An ack from Neil would be reassuring, though, if he gets a chance.) --b. > > --- > > Chuck Lever (3): > NLM: Remove unused argument from svc_addsock() function > NLM: Remove "proto" argument from lockd_up() > NLM: Always start both UDP and TCP listeners > > fs/lockd/clntlock.c | 4 ++-- > fs/lockd/svc.c | 40 ++++++++++++++++++++-------------------- > fs/nfsd/nfsctl.c | 5 ++--- > fs/nfsd/nfssvc.c | 19 +++++++------------ > include/linux/lockd/bind.h | 2 +- > include/linux/sunrpc/svcsock.h | 5 +---- > net/sunrpc/svcsock.c | 4 +--- > 7 files changed, 34 insertions(+), 45 deletions(-) > > -- > Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-04 21:36 ` [PATCH 0/3] client-side lockd doesn't start UDP listener J. Bruce Fields @ 2008-10-12 23:15 ` Neil Brown [not found] ` <18674.34066.316972.695627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Neil Brown @ 2008-10-12 23:15 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Chuck Lever, linux-nfs On Saturday October 4, bfields@fieldses.org wrote: > On Fri, Oct 03, 2008 at 05:15:14PM -0400, Chuck Lever wrote: > > Hi Bruce, Neil- > > > > Here's my initial proposal to address the NFSv2/v3 lock recovery issue > > that results from having no UDP lockd listener. > > > > Comments? Did I miss anything? > > Looks fine; I can't see any problem. So I've applied to for-2.6.28. > (An ack from Neil would be reassuring, though, if he gets a chance.) Sorry for my tardiness. September was a very hectic month for me. One consequence of this change is that lockd always listens on TCP even if NFSD and NFS are only using UDP. Do we care? I suspect not. An alternative fix to the problem would be to always listen on UDP and only listen on TCP if it was requested. This would probably be a smaller code change, and might in some sense be more flexible. Will we ever want to talk NLM over any other protocol? RDMA? Is UDP6 a different protocol in this context? I suspect not. I guess I have a small leaning towards just always listening on UDP and leaving everything else the same, but it is small, not strong. So unless either of you lean the same way I'm happy to give my Acked-By: NeilBrown <neilb@suse.de> to the current patches. Thanks, NeilBrown ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <18674.34066.316972.695627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>]
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener [not found] ` <18674.34066.316972.695627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2008-10-13 15:42 ` Chuck Lever 2008-10-13 17:46 ` J. Bruce Fields 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2008-10-13 15:42 UTC (permalink / raw) To: Neil Brown; +Cc: J. Bruce Fields, linux-nfs Hi Neil- On Oct 12, 2008, at Oct 12, 2008, 7:15 PM, Neil Brown wrote: > On Saturday October 4, bfields@fieldses.org wrote: >> On Fri, Oct 03, 2008 at 05:15:14PM -0400, Chuck Lever wrote: >>> Hi Bruce, Neil- >>> >>> Here's my initial proposal to address the NFSv2/v3 lock recovery >>> issue >>> that results from having no UDP lockd listener. >>> >>> Comments? Did I miss anything? >> >> Looks fine; I can't see any problem. So I've applied to for-2.6.28. >> (An ack from Neil would be reassuring, though, if he gets a chance.) > > Sorry for my tardiness. September was a very hectic month for me. > > One consequence of this change is that lockd always listens on TCP > even if NFSD and NFS are only using UDP. > Do we care? I suspect not. The server side usually has to start both anyway, so I thought making both sides work the same way was slightly nicer than keeping the "proto" argument to lockd_up(), and the run-time cost on the client is fairly minimal. Plus, the overall trend is away from NFS over UDP, and towards NFS over TCP. UDP is legacy, and TCP is the common case, going forward, so it's likely the TCP listener will nearly always be running anyway. However, do we care about the -T and -U options on rpc.nfsd affecting how server-side lockd works? Maybe that is a valid reason to keep the "proto" argument to lockd_up(). > An alternative fix to the problem would be to always listen on UDP and > only listen on TCP if it was requested. This would probably be a > smaller code change, and might in some sense be more flexible. > > Will we ever want to talk NLM over any other protocol? RDMA? > Is UDP6 a different protocol in this context? I suspect not. > > I guess I have a small leaning towards just always listening on UDP > and leaving everything else the same, but it is small, not strong. So > unless either of you lean the same way I'm happy to give my > > Acked-By: NeilBrown <neilb@suse.de> > > to the current patches. Thanks for the review. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-13 15:42 ` Chuck Lever @ 2008-10-13 17:46 ` J. Bruce Fields 2008-10-13 20:45 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: J. Bruce Fields @ 2008-10-13 17:46 UTC (permalink / raw) To: Chuck Lever; +Cc: Neil Brown, linux-nfs On Mon, Oct 13, 2008 at 11:42:47AM -0400, Chuck Lever wrote: > Hi Neil- > > On Oct 12, 2008, at Oct 12, 2008, 7:15 PM, Neil Brown wrote: >> On Saturday October 4, bfields@fieldses.org wrote: >>> On Fri, Oct 03, 2008 at 05:15:14PM -0400, Chuck Lever wrote: >>>> Hi Bruce, Neil- >>>> >>>> Here's my initial proposal to address the NFSv2/v3 lock recovery >>>> issue >>>> that results from having no UDP lockd listener. >>>> >>>> Comments? Did I miss anything? >>> >>> Looks fine; I can't see any problem. So I've applied to for-2.6.28. >>> (An ack from Neil would be reassuring, though, if he gets a chance.) >> >> Sorry for my tardiness. September was a very hectic month for me. >> >> One consequence of this change is that lockd always listens on TCP >> even if NFSD and NFS are only using UDP. >> Do we care? I suspect not. > > The server side usually has to start both anyway, so I thought making > both sides work the same way was slightly nicer than keeping the "proto" > argument to lockd_up(), and the run-time cost on the client is fairly > minimal. Plus, the overall trend is away from NFS over UDP, and towards > NFS over TCP. UDP is legacy, and TCP is the common case, going forward, > so it's likely the TCP listener will nearly always be running anyway. > > However, do we care about the -T and -U options on rpc.nfsd affecting > how server-side lockd works? Maybe that is a valid reason to keep the > "proto" argument to lockd_up(). We might at least want to fix the man page. I suppose worst case scenarios would be: - a bug is found that affects lockd/tcp and not lockd/udp, and people that used -T are affected when they needn't have been, or - someone uses -T and only bothers to firewall the udp port? Is there any evidence that anyone uses -U or -T? --b. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-13 17:46 ` J. Bruce Fields @ 2008-10-13 20:45 ` Chuck Lever 2008-10-13 20:57 ` J. Bruce Fields 2008-10-14 9:54 ` Neil Brown 0 siblings, 2 replies; 14+ messages in thread From: Chuck Lever @ 2008-10-13 20:45 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, linux-nfs On Oct 13, 2008, at Oct 13, 2008, 1:46 PM, J. Bruce Fields wrote: > On Mon, Oct 13, 2008 at 11:42:47AM -0400, Chuck Lever wrote: >> Hi Neil- >> >> On Oct 12, 2008, at Oct 12, 2008, 7:15 PM, Neil Brown wrote: >>> On Saturday October 4, bfields@fieldses.org wrote: >>>> On Fri, Oct 03, 2008 at 05:15:14PM -0400, Chuck Lever wrote: >>>>> Hi Bruce, Neil- >>>>> >>>>> Here's my initial proposal to address the NFSv2/v3 lock recovery >>>>> issue >>>>> that results from having no UDP lockd listener. >>>>> >>>>> Comments? Did I miss anything? >>>> >>>> Looks fine; I can't see any problem. So I've applied to >>>> for-2.6.28. >>>> (An ack from Neil would be reassuring, though, if he gets a >>>> chance.) >>> >>> Sorry for my tardiness. September was a very hectic month for me. >>> >>> One consequence of this change is that lockd always listens on TCP >>> even if NFSD and NFS are only using UDP. >>> Do we care? I suspect not. >> >> The server side usually has to start both anyway, so I thought making >> both sides work the same way was slightly nicer than keeping the >> "proto" >> argument to lockd_up(), and the run-time cost on the client is fairly >> minimal. Plus, the overall trend is away from NFS over UDP, and >> towards >> NFS over TCP. UDP is legacy, and TCP is the common case, going >> forward, >> so it's likely the TCP listener will nearly always be running anyway. >> >> However, do we care about the -T and -U options on rpc.nfsd affecting >> how server-side lockd works? Maybe that is a valid reason to keep >> the >> "proto" argument to lockd_up(). > > We might at least want to fix the man page. > > I suppose worst case scenarios would be: > > - a bug is found that affects lockd/tcp and not lockd/udp, and > people that used -T are affected when they needn't have been, > or > - someone uses -T and only bothers to firewall the udp port? > > Is there any evidence that anyone uses -U or -T? NFSD start-up code makes two separate and unconditional calls to lockd_up(): one for UDP and one for TCP. So whether or not NFSD actually honors the "-T" and "-U" flags, lockd certainly does not honor them in current (unpatched) code. In other words, the patches you've already reviewed shouldn't change the server's behavior around -T/-U. Only the client side should be affected. It looks like, prior to commit 24e36663, the client side started both lockd listeners no matter what type of mount was requested. Additional review is welcome, but let's just move forward with what you've already got. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-13 20:45 ` Chuck Lever @ 2008-10-13 20:57 ` J. Bruce Fields 2008-10-14 9:54 ` Neil Brown 1 sibling, 0 replies; 14+ messages in thread From: J. Bruce Fields @ 2008-10-13 20:57 UTC (permalink / raw) To: Chuck Lever; +Cc: Neil Brown, linux-nfs On Mon, Oct 13, 2008 at 04:45:47PM -0400, Chuck Lever wrote: > NFSD start-up code makes two separate and unconditional calls to > lockd_up(): one for UDP and one for TCP. So whether or not NFSD > actually honors the "-T" and "-U" flags, lockd certainly does not honor > them in current (unpatched) code. > > In other words, the patches you've already reviewed shouldn't change the > server's behavior around -T/-U. Only the client side should be > affected. > > It looks like, prior to commit 24e36663, the client side started both > lockd listeners no matter what type of mount was requested. > > Additional review is welcome, but let's just move forward with what > you've already got. OK, sounds fine to me. The rpc.nfsd man page could still use revision, though. --b. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-13 20:45 ` Chuck Lever 2008-10-13 20:57 ` J. Bruce Fields @ 2008-10-14 9:54 ` Neil Brown [not found] ` <18676.27741.921124.742371-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 1 sibling, 1 reply; 14+ messages in thread From: Neil Brown @ 2008-10-14 9:54 UTC (permalink / raw) To: Chuck Lever; +Cc: J. Bruce Fields, linux-nfs On Monday October 13, chuck.lever@oracle.com wrote: > > NFSD start-up code makes two separate and unconditional calls to > lockd_up(): one for UDP and one for TCP. So whether or not NFSD > actually honors the "-T" and "-U" flags, lockd certainly does not > honor them in current (unpatched) code. Not exactly correct. The calls are not unconditional. I assume we are looking at nfsd_init_socks in nfssvc.c Note the condition at the top: if (!list_empty(&nfsd_serv->sv_permsocks)) return 0; If a UDP port has previously been created by a write to "portlist", then when the first thread is stared, no new sockets will be created, and nfsd (and lockd) will just listen on UDP. So your patches do make a real functional change which is more than just "always listen on UDP". I agree that the man page should reflect reality. NeilBrown ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <18676.27741.921124.742371-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>]
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener [not found] ` <18676.27741.921124.742371-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2008-10-14 15:52 ` Chuck Lever 2008-10-14 17:43 ` J. Bruce Fields 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2008-10-14 15:52 UTC (permalink / raw) To: Neil Brown; +Cc: J. Bruce Fields, linux-nfs On Oct 14, 2008, at Oct 14, 2008, 5:54 AM, Neil Brown wrote: > On Monday October 13, chuck.lever@oracle.com wrote: >> >> NFSD start-up code makes two separate and unconditional calls to >> lockd_up(): one for UDP and one for TCP. So whether or not NFSD >> actually honors the "-T" and "-U" flags, lockd certainly does not >> honor them in current (unpatched) code. > > Not exactly correct. The calls are not unconditional. > I assume we are looking at nfsd_init_socks in nfssvc.c > Note the condition at the top: > if (!list_empty(&nfsd_serv->sv_permsocks)) > return 0; > > If a UDP port has previously been created by a write to "portlist", > then when the first thread is stared, no new sockets will be created, > and nfsd (and lockd) will just listen on UDP. > > So your patches do make a real functional change which is more than > just "always listen on UDP". OK, I'm already collecting a set of additional fixes and cleanups in this area for 2.6.29. We can add this to the list. Why are there two separate and undocumented kernel interfaces for starting NFSD? > I agree that the man page should reflect reality. Several people have said that, but no-one has suggested what specific parts of the man page are problematic. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-14 15:52 ` Chuck Lever @ 2008-10-14 17:43 ` J. Bruce Fields 2008-10-14 18:14 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: J. Bruce Fields @ 2008-10-14 17:43 UTC (permalink / raw) To: Chuck Lever; +Cc: Neil Brown, linux-nfs On Tue, Oct 14, 2008 at 11:52:52AM -0400, Chuck Lever wrote: > On Oct 14, 2008, at Oct 14, 2008, 5:54 AM, Neil Brown wrote: >> I agree that the man page should reflect reality. > > Several people have said that, but no-one has suggested what specific > parts of the man page are problematic. Look at, e.g., the text for "-T": Disable rpc.nfsd from accepting TCP connections from clients. It should probably say lockd is unaffected. (If that's what we want.) Also the wording is a little imprecise, since rpc.nfsd itself doesn't accept network connections. --b. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] client-side lockd doesn't start UDP listener 2008-10-14 17:43 ` J. Bruce Fields @ 2008-10-14 18:14 ` Chuck Lever 0 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2008-10-14 18:14 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Neil Brown, linux-nfs On Oct 14, 2008, at Oct 14, 2008, 1:43 PM, J. Bruce Fields wrote: > On Tue, Oct 14, 2008 at 11:52:52AM -0400, Chuck Lever wrote: >> On Oct 14, 2008, at Oct 14, 2008, 5:54 AM, Neil Brown wrote: >>> I agree that the man page should reflect reality. >> >> Several people have said that, but no-one has suggested what specific >> parts of the man page are problematic. > > Look at, e.g., the text for "-T": > Disable rpc.nfsd from accepting TCP connections from clients. > > It should probably say lockd is unaffected. (If that's what we want.) That's the problem. I'm not sure how this should behave. I don't know what the implications are of always having both lockd listeners on the server. It seems to me the previous behavior must have been broken -- if -U is specified, rpc.nfsd prevents the creation of both lockd and nfsd listeners on UDP, thus we have the same lock recovery problem we have on the client. So we must always have a lockd UDP listener on the server, at least. I would like to understand why we have -T and -U, and whether users of these options might want lockd to reflect the setting of this option. Is it acceptable (does it make sense) to have both UDP and TCP listeners for lockd, while having just one listener for NFSD? And I don't understand what is being accomplished with the check that prevents nfsd_init_socks() from starting additional listeners. Neil did this recently enough that I'm hoping he remembers what the rationale was and then we can add a comment or two in fs/nfsd/nfsctl.c to help inform future generations. > Also the wording is a little imprecise, since rpc.nfsd itself doesn't > accept network connections. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2008-10-14 18:14 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-03 21:15 [PATCH 0/3] client-side lockd doesn't start UDP listener Chuck Lever
[not found] ` <20081003211305.9870.6835.stgit-07a7zB5ZJzbwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
2008-10-03 21:15 ` [PATCH 1/3] NLM: Always start both UDP and TCP listeners Chuck Lever
2008-10-03 21:15 ` [PATCH 2/3] NLM: Remove "proto" argument from lockd_up() Chuck Lever
2008-10-03 21:15 ` [PATCH 3/3] NLM: Remove unused argument from svc_addsock() function Chuck Lever
2008-10-04 21:36 ` [PATCH 0/3] client-side lockd doesn't start UDP listener J. Bruce Fields
2008-10-12 23:15 ` Neil Brown
[not found] ` <18674.34066.316972.695627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-10-13 15:42 ` Chuck Lever
2008-10-13 17:46 ` J. Bruce Fields
2008-10-13 20:45 ` Chuck Lever
2008-10-13 20:57 ` J. Bruce Fields
2008-10-14 9:54 ` Neil Brown
[not found] ` <18676.27741.921124.742371-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-10-14 15:52 ` Chuck Lever
2008-10-14 17:43 ` J. Bruce Fields
2008-10-14 18:14 ` Chuck Lever
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.