Re: [nfs-utils] [PATCH 1/3] rpc.mountd: set nonblocking mode if no libtirpc

[NeilBrown <neilb@suse.de>]

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

On 05 Nov 2014 21:22:56 +0100 bstroesser@ts.fujitsu.com wrote:

> From: Bodo Stroesser <bstroesser@ts.fujitsu.com>
> Date: Thu, 09 Oct 2014 13:06:19 +0200
> Subject: [nfs-utils] [PATCH 1/3] rpc.mountd: set nonblocking mode if no libtirpc
> 
> If mountd is built without libtirpc and it is started using "-p XXX" option,
> the tcp listeners and the sockets waiting for UDP messages are not in
> non-blocking mode. Thus if running with multiple threads (-t XX),
> all threads will wake up from select on a connection request or a UDP message,
> but only one thread will succeed. All others will wait on accept() or read()
> for the next event.
> 
> Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com>
> ---

Reviewed-by: NeilBrown <neilb@suse.de>

This is just taking the code that already applies in svc_socket() when no
explicit port number is requested, and applying it also in makesock() which
is used when an explicit port number *is* requested.

Thanks,
NeilBrown


> 
> --- nfs-utils-1.3.1/support/include/nfslib.h	2014-10-09 12:52:30.000000000 +0200
> +++ nfs-utils-1.3.1/support/include/nfslib.h	2014-10-09 12:53:37.000000000 +0200
> @@ -174,6 +174,7 @@ void closeall(int min);
>  
>  int			svctcp_socket (u_long __number, int __reuse);
>  int			svcudp_socket (u_long __number);
> +int			svcsock_nonblock (int __sock);
>  
>  /* Misc shared code prototypes */
>  size_t  strlcat(char *, const char *, size_t);
> --- nfs-utils-1.3.1/support/nfs/svc_socket.c	2014-10-09 12:56:14.000000000 +0200
> +++ nfs-utils-1.3.1/support/nfs/svc_socket.c	2014-10-09 13:10:44.000000000 +0200
> @@ -76,6 +76,39 @@ int getservport(u_long number, const cha
>  	return 0;
>  }
>  
> +int
> +svcsock_nonblock(int sock)
> +{
> +	int flags;
> +
> +	if (sock < 0)
> +		return sock;
> +
> +	/* This socket might be shared among multiple processes
> +	 * if mountd is run multi-threaded.  So it is safest to
> +	 * make it non-blocking, else all threads might wake
> +	 * one will get the data, and the others will block
> +	 * indefinitely.
> +	 * In all cases, transaction on this socket are atomic
> +	 * (accept for TCP, packet-read and packet-write for UDP)
> +	 * so O_NONBLOCK will not confuse unprepared code causing
> +	 * it to corrupt messages.
> +	 * It generally safest to have O_NONBLOCK when doing an accept
> +	 * as if we get a RST after the SYN and before accept runs,
> +	 * we can block despite being told there was an acceptable
> +	 * connection.
> +	 */
> +	if ((flags = fcntl(sock, F_GETFL)) < 0)
> +		perror(_("svc_socket: can't get socket flags"));
> +	else if (fcntl(sock, F_SETFL, flags|O_NONBLOCK) < 0)
> +		perror(_("svc_socket: can't set socket flags"));
> +	else
> +		return sock;
> +
> +	(void) __close(sock);
> +	return -1;
> +}
> +
>  static int
>  svc_socket (u_long number, int type, int protocol, int reuse)
>  {
> @@ -113,38 +146,7 @@ svc_socket (u_long number, int type, int
>        sock = -1;
>      }
>  
> -  if (sock >= 0)
> -    {
> -	    /* This socket might be shared among multiple processes
> -	     * if mountd is run multi-threaded.  So it is safest to
> -	     * make it non-blocking, else all threads might wake
> -	     * one will get the data, and the others will block
> -	     * indefinitely.
> -	     * In all cases, transaction on this socket are atomic
> -	     * (accept for TCP, packet-read and packet-write for UDP)
> -	     * so O_NONBLOCK will not confuse unprepared code causing
> -	     * it to corrupt messages.
> -	     * It generally safest to have O_NONBLOCK when doing an accept
> -	     * as if we get a RST after the SYN and before accept runs,
> -	     * we can block despite being told there was an acceptable
> -	     * connection.
> -	     */
> -	int flags;
> -	if ((flags = fcntl(sock, F_GETFL)) < 0)
> -	  {
> -	      perror (_("svc_socket: can't get socket flags"));
> -	      (void) __close (sock);
> -	      sock = -1;
> -	  }
> -	else if (fcntl(sock, F_SETFL, flags|O_NONBLOCK) < 0)
> -	  {
> -	      perror (_("svc_socket: can't set socket flags"));
> -	      (void) __close (sock);
> -	      sock = -1;
> -	  }
> -    }
> -
> -  return sock;
> +  return svcsock_nonblock(sock);
>  }
>  
>  /*
> --- nfs-utils-1.3.1/support/nfs/rpcmisc.c	2014-10-08 21:22:04.000000000 +0200
> +++ nfs-utils-1.3.1/support/nfs/rpcmisc.c	2014-10-08 21:22:36.000000000 +0200
> @@ -104,7 +104,7 @@ makesock(int port, int proto)
>  		return -1;
>  	}
>  
> -	return sock;
> +	return svcsock_nonblock(sock);
>  }
>  
>  void


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]