statd and some servers always need lockd to listen on UDP, so always
start both lockd listener sockets.
This probably leaks an svc_serv if the TCP listener can't be created,
but it will do for now.
Signed-off-by: Chuck Lever <[email protected]>
---
I currently have this workaround lurking in my git repo. Is there some kind
of fix for this issue planned for 2.6.28?
This is only a reminder, not a suggested patch.
fs/lockd/svc.c | 31 +++++++++++++------------------
1 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index a7b604c..36b6d03 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -213,25 +213,23 @@ 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
+ * Create UDP and TCP listeners for lockd. Even if we only
+ * have TCP NFS mounts and TCP NFSDs, some services (such
+ * as rpc.statd) still require UDP.
*/
-static int make_socks(struct svc_serv *serv, const unsigned short 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,
@@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
/*
* 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,
@@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
goto out;
}
- if ((error = make_socks(serv, proto)) < 0)
+ if ((error = make_socks(serv)) < 0)
goto destroy_and_out;
/*
At 02:54 PM 9/24/2008, Trond Myklebust wrote:
>On Wed, 2008-09-24 at 14:50 -0400, Chuck Lever wrote:
>> statd and some servers always need lockd to listen on UDP, so always
>> start both lockd listener sockets.
>>
>> This probably leaks an svc_serv if the TCP listener can't be created,
>> but it will do for now.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>
>ACK. We need this also in order to deal with servers that only send NLM
>callbacks on UDP (yeah, I know that's lame...).
Lame it may be, but performing a single callback over TCP is expensive,
and if it's frequent enough, can lead to socket exhaustion from TIME_WAIT.
Thumbs-up for this patch. The worst thing about the unpatched issue is that
if the server tries UDP callbacks for a TCP mount, then the client never knows
the lock was available. It enters a polling loop and many races can occur.
I posted some patches to that a while back (when we used sourceforge),
they're not a prerequisite to this change, but perhaps relevant for hardening
purposes:
<http://sourceforge.net/mailarchive/forum.php?thread_name=EXNANE01OSQeSOzNCWb00000734%40exnane01.hq.netapp.com&forum_name=nfs>
Tom.
>
>Trond
>
>> ---
>>
>> I currently have this workaround lurking in my git repo. Is there some kind
>> of fix for this issue planned for 2.6.28?
>>
>> This is only a reminder, not a suggested patch.
>>
>> fs/lockd/svc.c | 31 +++++++++++++------------------
>> 1 files changed, 13 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index a7b604c..36b6d03 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -213,25 +213,23 @@ 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
>> + * Create UDP and TCP listeners for lockd. Even if we only
>> + * have TCP NFS mounts and TCP NFSDs, some services (such
>> + * as rpc.statd) still require UDP.
>> */
>> -static int make_socks(struct svc_serv *serv, const unsigned short 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,
>> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
>> /*
>> * 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,
>> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
>> goto out;
>> }
>>
>> - if ((error = make_socks(serv, proto)) < 0)
>> + if ((error = make_socks(serv)) < 0)
>> goto destroy_and_out;
>>
>> /*
>>
>--
>Trond Myklebust
>Linux NFS client maintainer
>
>NetApp
>[email protected]
>http://www.netapp.com
>--
>To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2008-09-24 at 14:50 -0400, Chuck Lever wrote:
> statd and some servers always need lockd to listen on UDP, so always
> start both lockd listener sockets.
>
> This probably leaks an svc_serv if the TCP listener can't be created,
> but it will do for now.
>
> Signed-off-by: Chuck Lever <[email protected]>
ACK. We need this also in order to deal with servers that only send NLM
callbacks on UDP (yeah, I know that's lame...).
Trond
> ---
>
> I currently have this workaround lurking in my git repo. Is there some kind
> of fix for this issue planned for 2.6.28?
>
> This is only a reminder, not a suggested patch.
>
> fs/lockd/svc.c | 31 +++++++++++++------------------
> 1 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index a7b604c..36b6d03 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -213,25 +213,23 @@ 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
> + * Create UDP and TCP listeners for lockd. Even if we only
> + * have TCP NFS mounts and TCP NFSDs, some services (such
> + * as rpc.statd) still require UDP.
> */
> -static int make_socks(struct svc_serv *serv, const unsigned short 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,
> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
> /*
> * 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,
> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
> goto out;
> }
>
> - if ((error = make_socks(serv, proto)) < 0)
> + if ((error = make_socks(serv)) < 0)
> goto destroy_and_out;
>
> /*
>
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com