[PATCH] NFSD: fix use of setsockopt

[PATCH] NFSD: fix use of setsockopt

[Olga Kornievskaia]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

The following patch fixes NFS server's use of setsockopt. For this 
function to take an effect it first needs be called after socket 
creation but before sock binding.

This patch also changes the size of the receive sock buffer to be same 
as the send sock buffer. Both buffers are now a multiple of maxpayload 
and number of nfsd threads.

This patch fixes the problem that receive window never opens beyond the 
default TCP receive window size set by the 2nd parameter of the 
net.ipv4.tcp_rmem sysctl.

Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>

[-- Attachment #2: nfsd-fix-sockopt-7.patch --]
[-- Type: text/x-patch, Size: 1242 bytes --]

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index c75bffe..178b397 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1191,7 +1191,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp)
 		 */
 		svc_sock_setbufsize(svsk->sk_sock,
 				    (serv->sv_nrthreads+3) * serv->sv_max_mesg,
-				    3 * serv->sv_max_mesg);
+				    (serv->sv_nrthreads+3) * serv->sv_max_mesg);
 
 	clear_bit(SK_DATA, &svsk->sk_flags);
 
@@ -1372,11 +1372,6 @@ svc_tcp_init(struct svc_sock *svsk)
 		 * receive and respond to one request.
 		 * svc_tcp_recvfrom will re-adjust if necessary
 		 */
-		svc_sock_setbufsize(svsk->sk_sock,
-				    3 * svsk->sk_server->sv_max_mesg,
-				    3 * svsk->sk_server->sv_max_mesg);
-
-		set_bit(SK_CHNGBUF, &svsk->sk_flags);
 		set_bit(SK_DATA, &svsk->sk_flags);
 		if (sk->sk_state != TCP_ESTABLISHED)
 			set_bit(SK_CLOSE, &svsk->sk_flags);
@@ -1761,6 +1756,8 @@ static int svc_create_socket(struct svc_serv *serv, int protocol,
 
 	if (type == SOCK_STREAM)
 		sock->sk->sk_reuse = 1;		/* allow address reuse */
+	svc_sock_setbufsize(sock, (serv->sv_nrthreads+3) * serv->sv_max_mesg,
+				(serv->sv_nrthreads+3) * serv->sv_max_mesg);
 	error = kernel_bind(sock, sin, len);
 	if (error < 0)
 		goto bummer;

Re: [PATCH] NFSD: fix use of setsockopt

[Dean Hildebrand]

Hi Olga,

This makes sense, if NFSD is going to ignore global Linux TCP settings 
and 'go it alone', then it shouldn't be constrained by them.

At least now we can increase the rcv buffer size by increasing the 
number of NFSDs.  I would still like to pursue my sysctl patch for the 
rcv and snd buffer though since we have seen situations where too many 
NFSDs can increase the randomness of requests to the underlying file 
system, reducing the effectiveness readahead/write gathering.

Dean

Olga Kornievskaia wrote:
> The following patch fixes NFS server's use of setsockopt. For this 
> function to take an effect it first needs be called after socket 
> creation but before sock binding.
>
> This patch also changes the size of the receive sock buffer to be same 
> as the send sock buffer. Both buffers are now a multiple of maxpayload 
> and number of nfsd threads.
>
> This patch fixes the problem that receive window never opens beyond 
> the default TCP receive window size set by the 2nd parameter of the 
> net.ipv4.tcp_rmem sysctl.
>
> Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>
Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>

------------------------------------------------------------------------

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index c75bffe..178b397 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1191,7 +1191,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp)
 		 */
 		svc_sock_setbufsize(svsk->sk_sock,
 				    (serv->sv_nrthreads+3) * serv->sv_max_mesg,
-				    3 * serv->sv_max_mesg);
+				    (serv->sv_nrthreads+3) * serv->sv_max_mesg);
 
 	clear_bit(SK_DATA, &svsk->sk_flags);
 
@@ -1372,11 +1372,6 @@ svc_tcp_init(struct svc_sock *svsk)
 		 * receive and respond to one request.
 		 * svc_tcp_recvfrom will re-adjust if necessary
 		 */
-		svc_sock_setbufsize(svsk->sk_sock,
-				    3 * svsk->sk_server->sv_max_mesg,
-				    3 * svsk->sk_server->sv_max_mesg);
-
-		set_bit(SK_CHNGBUF, &svsk->sk_flags);

 		set_bit(SK_DATA, &svsk->sk_flags);
 		if (sk->sk_state != TCP_ESTABLISHED)
 			set_bit(SK_CLOSE, &svsk->sk_flags);
@@ -1761,6 +1756,8 @@ static int svc_create_socket(struct svc_serv *serv, int protocol,
 
 	if (type == SOCK_STREAM)
 		sock->sk->sk_reuse = 1;		/* allow address reuse */
+	svc_sock_setbufsize(sock, (serv->sv_nrthreads+3) * serv->sv_max_mesg,
+				(serv->sv_nrthreads+3) * serv->sv_max_mesg);
 	error = kernel_bind(sock, sin, len);
 	if (error < 0)
 		goto bummer;

Re: [PATCH] NFSD: fix use of setsockopt

[J. Bruce Fields]

On Tue, Jun 24, 2008 at 05:50:31PM -0700, Dean Hildebrand wrote:
> Hi Olga,
>
> This makes sense, if NFSD is going to ignore global Linux TCP settings  
> and 'go it alone', then it shouldn't be constrained by them.
>
> At least now we can increase the rcv buffer size by increasing the  
> number of NFSDs.  I would still like to pursue my sysctl patch for the  
> rcv and snd buffer though since we have seen situations where too many  
> NFSDs can increase the randomness of requests to the underlying file  
> system, reducing the effectiveness readahead/write gathering.

Olga says she's also seeing some performance decrease with increasing
numbers of threads in our 10 gigabit testing, and I was wondering if
something like that could explain the change.

Anyone have ideas how we could measure how ordered our IO requests are?
(Or how much seeking the drives in our raid array are doing?)

--b.

>
> Dean
>
> Olga Kornievskaia wrote:
>> The following patch fixes NFS server's use of setsockopt. For this  
>> function to take an effect it first needs be called after socket  
>> creation but before sock binding.
>>
>> This patch also changes the size of the receive sock buffer to be same  
>> as the send sock buffer. Both buffers are now a multiple of maxpayload  
>> and number of nfsd threads.
>>
>> This patch fixes the problem that receive window never opens beyond  
>> the default TCP receive window size set by the 2nd parameter of the  
>> net.ipv4.tcp_rmem sysctl.
>>
>> Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>
> Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>
>
> ------------------------------------------------------------------------
>
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index c75bffe..178b397 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1191,7 +1191,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp)
> 		 */
> 		svc_sock_setbufsize(svsk->sk_sock,
> 				    (serv->sv_nrthreads+3) * serv->sv_max_mesg,
> -				    3 * serv->sv_max_mesg);
> +				    (serv->sv_nrthreads+3) * serv->sv_max_mesg);
>
> 	clear_bit(SK_DATA, &svsk->sk_flags);
>
> @@ -1372,11 +1372,6 @@ svc_tcp_init(struct svc_sock *svsk)
> 		 * receive and respond to one request.
> 		 * svc_tcp_recvfrom will re-adjust if necessary
> 		 */
> -		svc_sock_setbufsize(svsk->sk_sock,
> -				    3 * svsk->sk_server->sv_max_mesg,
> -				    3 * svsk->sk_server->sv_max_mesg);
> -
> -		set_bit(SK_CHNGBUF, &svsk->sk_flags);
>
> 		set_bit(SK_DATA, &svsk->sk_flags);
> 		if (sk->sk_state != TCP_ESTABLISHED)
> 			set_bit(SK_CLOSE, &svsk->sk_flags);
> @@ -1761,6 +1756,8 @@ static int svc_create_socket(struct svc_serv *serv, int protocol,
>
> 	if (type == SOCK_STREAM)
> 		sock->sk->sk_reuse = 1;		/* allow address reuse */
> +	svc_sock_setbufsize(sock, (serv->sv_nrthreads+3) * serv->sv_max_mesg,
> +				(serv->sv_nrthreads+3) * serv->sv_max_mesg);
> 	error = kernel_bind(sock, sin, len);
> 	if (error < 0)
> 		goto bummer;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NFSD: fix use of setsockopt

[J. Bruce Fields]

On Thu, Jun 19, 2008 at 09:33:39AM -0400, Olga Kornievskaia wrote:
> The following patch fixes NFS server's use of setsockopt. For this  
> function to take an effect it first needs be called after socket  
> creation but before sock binding.

The tcp(7) man page actually claims that it's listen() and connect()
that matter (so the setsockopt is effective on (and only on) unconnected
sockets), so probably this could go after the bind and before the
listen?  Not that it matters.

> This patch also changes the size of the receive sock buffer to be same  
> as the send sock buffer. Both buffers are now a multiple of maxpayload  
> and number of nfsd threads.

It would be nice if we could get some review from someone who remembers
what the justification for the smaller receive buffer size was (Neil?).

> This patch fixes the problem that receive window never opens beyond the  
> default TCP receive window size set by the 2nd parameter of the  
> net.ipv4.tcp_rmem sysctl.

Do you know what it does in the udp case?

--b.

>
> Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>

> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index c75bffe..178b397 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1191,7 +1191,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp)
>  		 */
>  		svc_sock_setbufsize(svsk->sk_sock,
>  				    (serv->sv_nrthreads+3) * serv->sv_max_mesg,
> -				    3 * serv->sv_max_mesg);
> +				    (serv->sv_nrthreads+3) * serv->sv_max_mesg);
>  
>  	clear_bit(SK_DATA, &svsk->sk_flags);
>  
> @@ -1372,11 +1372,6 @@ svc_tcp_init(struct svc_sock *svsk)
>  		 * receive and respond to one request.
>  		 * svc_tcp_recvfrom will re-adjust if necessary
>  		 */
> -		svc_sock_setbufsize(svsk->sk_sock,
> -				    3 * svsk->sk_server->sv_max_mesg,
> -				    3 * svsk->sk_server->sv_max_mesg);
> -
> -		set_bit(SK_CHNGBUF, &svsk->sk_flags);
>  		set_bit(SK_DATA, &svsk->sk_flags);
>  		if (sk->sk_state != TCP_ESTABLISHED)
>  			set_bit(SK_CLOSE, &svsk->sk_flags);
> @@ -1761,6 +1756,8 @@ static int svc_create_socket(struct svc_serv *serv, int protocol,
>  
>  	if (type == SOCK_STREAM)
>  		sock->sk->sk_reuse = 1;		/* allow address reuse */
> +	svc_sock_setbufsize(sock, (serv->sv_nrthreads+3) * serv->sv_max_mesg,
> +				(serv->sv_nrthreads+3) * serv->sv_max_mesg);
>  	error = kernel_bind(sock, sin, len);
>  	if (error < 0)
>  		goto bummer;

Re: [PATCH] NFSD: fix use of setsockopt

[Olga Kornievskaia]

J. Bruce Fields wrote:
> On Thu, Jun 19, 2008 at 09:33:39AM -0400, Olga Kornievskaia wrote:
>   
>> The following patch fixes NFS server's use of setsockopt. For this  
>> function to take an effect it first needs be called after socket  
>> creation but before sock binding.
>>     
>
> The tcp(7) man page actually claims that it's listen() and connect()
> that matter (so the setsockopt is effective on (and only on) unconnected
> sockets), so probably this could go after the bind and before the
> listen?  Not that it matters.
>
>   
>> This patch also changes the size of the receive sock buffer to be same  
>> as the send sock buffer. Both buffers are now a multiple of maxpayload  
>> and number of nfsd threads.
>>     
>
> It would be nice if we could get some review from someone who remembers
> what the justification for the smaller receive buffer size was (Neil?).
>
>   
>> This patch fixes the problem that receive window never opens beyond the  
>> default TCP receive window size set by the 2nd parameter of the  
>> net.ipv4.tcp_rmem sysctl.
>>     
>
> Do you know what it does in the udp case?
>   
Looking at the kernel code, when setsockopt() is called on a UDP socket 
to set send/receive buffer for UPD the code will not do anything: 
udp_setsockopt() and udp_lib_setsockopt() will return -ENOPROTOOPT. 
However, we bypass the call to setsockopt() and instead set the buffer 
sizes directly. From what I understand sk_sndbuf/sk_rcvbuf are not used 
by the UDP code. We are setting the fields that are never used.

Then perhaps we can remove calls to svc_sock_setbufsize() from 
svc_udp_init() and svc_udp_recvfrom()?
> --b.
>
>   
>> Signed-off-by: Olga Kornievskaia <aglo@citi.umich.edu>
>>     
>
>   
>> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
>> index c75bffe..178b397 100644
>> --- a/net/sunrpc/svcsock.c
>> +++ b/net/sunrpc/svcsock.c
>> @@ -1191,7 +1191,7 @@ svc_tcp_recvfrom(struct svc_rqst *rqstp)
>>  		 */
>>  		svc_sock_setbufsize(svsk->sk_sock,
>>  				    (serv->sv_nrthreads+3) * serv->sv_max_mesg,
>> -				    3 * serv->sv_max_mesg);
>> +				    (serv->sv_nrthreads+3) * serv->sv_max_mesg);
>>  
>>  	clear_bit(SK_DATA, &svsk->sk_flags);
>>  
>> @@ -1372,11 +1372,6 @@ svc_tcp_init(struct svc_sock *svsk)
>>  		 * receive and respond to one request.
>>  		 * svc_tcp_recvfrom will re-adjust if necessary
>>  		 */
>> -		svc_sock_setbufsize(svsk->sk_sock,
>> -				    3 * svsk->sk_server->sv_max_mesg,
>> -				    3 * svsk->sk_server->sv_max_mesg);
>> -
>> -		set_bit(SK_CHNGBUF, &svsk->sk_flags);
>>  		set_bit(SK_DATA, &svsk->sk_flags);
>>  		if (sk->sk_state != TCP_ESTABLISHED)
>>  			set_bit(SK_CLOSE, &svsk->sk_flags);
>> @@ -1761,6 +1756,8 @@ static int svc_create_socket(struct svc_serv *serv, int protocol,
>>  
>>  	if (type == SOCK_STREAM)
>>  		sock->sk->sk_reuse = 1;		/* allow address reuse */
>> +	svc_sock_setbufsize(sock, (serv->sv_nrthreads+3) * serv->sv_max_mesg,
>> +				(serv->sv_nrthreads+3) * serv->sv_max_mesg);
>>  	error = kernel_bind(sock, sin, len);
>>  	if (error < 0)
>>  		goto bummer;
>>     
>
>
>   

Re: [PATCH] NFSD: fix use of setsockopt

[J. Bruce Fields]

On Wed, Jun 25, 2008 at 04:44:15PM -0400, Olga Kornievskaia wrote:
> Looking at the kernel code, when setsockopt() is called on a UDP socket  
> to set send/receive buffer for UPD the code will not do anything:  
> udp_setsockopt() and udp_lib_setsockopt() will return -ENOPROTOOPT.  
> However, we bypass the call to setsockopt() and instead set the buffer  
> sizes directly. From what I understand sk_sndbuf/sk_rcvbuf are not used  
> by the UDP code. We are setting the fields that are never used.
>
> Then perhaps we can remove calls to svc_sock_setbufsize() from  
> svc_udp_init() and svc_udp_recvfrom()?

Assuming you're correct about udp not using those fields (haven't
checked myself)--yes, that'd be great.

--b.

Sm-notify

[Laurenz, Dirk]

Hi,

Does anybody know how sm-notify exactly works on a suse sles 9 system?

Greetings,

Dirk

Re: Sm-notify

[NeilBrown]

On Fri, June 27, 2008 3:59 am, Laurenz, Dirk wrote:
> Hi,
>
> Does anybody know how sm-notify exactly works on a suse sles 9 system?
>

Yes.

Do you have a specific question about it?

NeilBrown

RE: Sm-notify

[Oeltze, Benjamin]

Hi,
maybe I can specify Dirks question.

We are trying to set up a NFS Cluster and want to use sm-notify to info=
rm the Clients if the Server has failed over.
We have found a Howto that says: sm-notify uses files under /var/lib/nf=
s/sm to determine the Clients it has to inform.
Our Problem is, that ther are no files in this place even if we connet =
with a client. Is there anything we have forgotten?

Mit freundlichen Gr=FC=DFen
Benjamin Oeltze
Systems Engineer
DE ISC SOP PS N/O
=46ujitsu Siemens Computers
Hildesheimer Str. 25
30880 Laatzen
Telephone:      0511-8489 1872
Mobile: 0160-96354617
Email:  mailto: benjamin.oeltze-/ixSogHR0HOS/tZ4Wjpou0EOCMrvLtNR@public.gmane.org
Internet:       http://www.fujitsu-siemens.com
=46irmenangaben: http://www.fujitsu-siemens.de/imprint.html

-----Original Message-----
=46rom: NeilBrown [mailto:neilb@suse.de]
Sent: Friday, June 27, 2008 4:23 AM
To: Laurenz, Dirk
Cc: linux-nfs@vger.kernel.org; Oeltze, Benjamin
Subject: Re: Sm-notify

On Fri, June 27, 2008 3:59 am, Laurenz, Dirk wrote:
> Hi,
>
> Does anybody know how sm-notify exactly works on a suse sles 9 system=
?
>

Yes.

Do you have a specific question about it?

NeilBrown

RE: Sm-notify

[Neil Brown]

On Friday June 27, Benjamin.Oeltze-/ixSogHR0HOS/tZ4Wjpou0EOCMrvLtNR@public.gmane.org wrote:
> Hi,
> maybe I can specify Dirks question.

Thanks.

> 
> We are trying to set up a NFS Cluster and want to use sm-notify to inform the Clients if the Server has failed over.
> We have found a Howto that says: sm-notify uses files under /var/lib/nfs/sm to determine the Clients it has to inform.
> Our Problem is, that ther are no files in this place even if we connet with a client. Is there anything we have forgotten?

Files do not get created in /var/lib/nfs/sm until the client tried to
get a lock on some file (or part of a file).  Then the file is
created.
Some minutes after the last lock is dropped, the file should be
removed (though I recall there were some issues related to that, it
might depend on exactly which kernel you are running).

If no file locking is involved, there is no need to inform the client
of failover.  They will transparently start accessing the new server
through the old IP address, possible after a short timeout.

NeilBrown

RE: Sm-notify

[Laurenz, Dirk]

i, the time out is from node a to b seconds, but from node b to a 15 minutes.
what I saw was, that the handle changed in /var/lib/nfs/rmtab from 0x000002 to 0x000003 although
the fsid is the same on both nodes.
how can the 15minute timeout occur?

Mit freundlichem Gruss,


Dirk Laurenz
Systems Engineer
Fujitsu Siemens Computers
R DE ISC SOP PS N/O
Hildesheimer Strasse 25
30880 Laatzen
Germany
Telephone:              +49 511 84 89 18 08
Telefax:                +49 511 84 89 25 18 08
Mobile:                 +49 170 22 10 781
Email:                  mailto:dirk.laurenz-/ixSogHR0HOS/tZ4Wjpou0EOCMrvLtNR@public.gmane.org
Internet:               http://www.fujitsu-siemens.com
Firmenangaben:  http://www.fujitsu-siemens.de/imprint.html

> -----Original Message-----
> From: Neil Brown [mailto:neilb@suse.de]
> Sent: Saturday, June 28, 2008 1:44 AM
> To: Oeltze, Benjamin
> Cc: Laurenz, Dirk; linux-nfs@vger.kernel.org
> Subject: RE: Sm-notify
>
> On Friday June 27, Benjamin.Oeltze-/ixSogHR0HOS/tZ4Wjpou0EOCMrvLtNR@public.gmane.org wrote:
> > Hi,
> > maybe I can specify Dirks question.
>
> Thanks.
>
> >
> > We are trying to set up a NFS Cluster and want to use sm-notify to inform the
> Clients if the Server has failed over.
> > We have found a Howto that says: sm-notify uses files under /var/lib/nfs/sm to
> determine the Clients it has to inform.
> > Our Problem is, that ther are no files in this place even if we connet with a client.
> Is there anything we have forgotten?
>
> Files do not get created in /var/lib/nfs/sm until the client tried to
> get a lock on some file (or part of a file).  Then the file is
> created.
> Some minutes after the last lock is dropped, the file should be
> removed (though I recall there were some issues related to that, it
> might depend on exactly which kernel you are running).
>
> If no file locking is involved, there is no need to inform the client
> of failover.  They will transparently start accessing the new server
> through the old IP address, possible after a short timeout.
>
> NeilBrown

RE: Sm-notify

[Neil Brown]

On Saturday June 28, Dirk.Laurenz-/ixSogHR0HOS/tZ4Wjpou0EOCMrvLtNR@public.gmane.org wrote:
> i, the time out is from node a to b seconds, but from node b to a 15 minutes.
> what I saw was, that the handle changed in /var/lib/nfs/rmtab from 0x000002 to 0x000003 although
> the fsid is the same on both nodes.
> how can the 15minute timeout occur?

The number is rmtab (e.g. 0x000002) is like a reference count.  The
fact that you sometimes see '2' and sometimes '3' is of no real
interest.

I am correct in interpreting what you say as:

  When you fail over from 'a' to 'b', it does so quite quickly, but
  when you then fail back from 'b' to 'a', you get a 15 minute
  timeout?

If not, please explain in more detail.
If so:
  Why exactly are you doing this.  It doesn't seem to make sense.
  Did 'b' get rebooted in the interim?
  It could be that you are hitting the same issue discussed here:

    http://linux-nfs.org/pipermail/nfsv4/2008-June/008673.html

  and in the surrounding thread.

However this has nothing to do with sm-notify.

NeilBrown