serve: Refactor some socket utility code into its own module.

We'll be using this in proxy mode later

Rakefile

@@ -130,6 +130,7 @@ file check("client") =>
   build/mbox.o
   build/mirror.o
   build/status.o
+  build/sockutil.o
   build/util.o} do |t|
   gcc_link t.name, t.prerequisites + [LIBCHECK]
 end
@@ -165,6 +166,7 @@ file check("serve") =>
   build/acl.o
   build/mbox.o
   build/ioutil.o
+  build/sockutil.o
   build/util.o} do |t|
   gcc_link t.name, t.prerequisites + [LIBCHECK]
 end
@@ -185,6 +187,7 @@ file check("readwrite") =>
   build/nbdtypes.o
   build/mbox.o
   build/ioutil.o
+  build/sockutil.o
   build/util.o} do |t|
   gcc_link t.name, t.prerequisites + [LIBCHECK]
 end
@@ -194,6 +197,7 @@ file check("flexnbd") =>
 %w{build/tests/check_flexnbd.o
   build/flexnbd.o
   build/ioutil.o
+  build/sockutil.o
   build/util.o
   build/control.o
   build/mbox.o

src/serve.c

@@ -2,6 +2,7 @@
 #include "client.h"
 #include "nbdtypes.h"
 #include "ioutil.h"
+#include "sockutil.h"
 #include "util.h"
 #include "bitset.h"
 #include "control.h"
@@ -20,22 +21,6 @@
 #include <sys/socket.h>
 #include <netinet/tcp.h>
 
-static inline void* sockaddr_address_data(struct sockaddr* sockaddr)
-{
-	NULLCHECK( sockaddr );
-
-	struct sockaddr_in*  in  = (struct sockaddr_in*) sockaddr;
-	struct sockaddr_in6* in6 = (struct sockaddr_in6*) sockaddr;
-
-	if (sockaddr->sa_family == AF_INET) {
-		return &in->sin_addr;
-	}
-	if (sockaddr->sa_family == AF_INET6) {
-		return &in6->sin6_addr;
-	}
-	return NULL;
-}
-
 struct server * server_create (
 	struct flexnbd * flexnbd,
 	char* s_ip_address,
@@ -231,82 +216,15 @@ int server_port( struct server * server )
 }
 
 
-/* Try to bind to our serving socket, retrying until it works or gives a
- * fatal error. */
-void serve_bind( struct server * serve )
-{
-	int bind_result;
-
-	char s_address[64];
-	memset( s_address, 0, 64 );
-	strcpy( s_address, "???" );
-	inet_ntop( serve->bind_to.generic.sa_family,
-			sockaddr_address_data( &serve->bind_to.generic),
-			s_address, 64 );
-
-	do {
-		bind_result = bind(
-				serve->server_fd,
-				&serve->bind_to.generic,
-				sizeof(serve->bind_to));
-
-		if ( 0 == bind_result ) {
-			info( "Bound to %s port %d",
-					s_address,
-					ntohs(serve->bind_to.v4.sin_port));
-			break;
-		}
-		else {
-
-			warn( "Couldn't bind to %s port %d: %s",
-					s_address,
-					ntohs(serve->bind_to.v4.sin_port),
-					strerror( errno ) );
-
-			switch (errno){
-				/* bind() can give us EACCES,
-				 * EADDRINUSE, EADDRNOTAVAIL, EBADF,
-				 * EINVAL or ENOTSOCK.
-				 *
-				 * Any of these other than EACCES,
-				 * EADDRINUSE or EADDRNOTAVAIL signify
-				 * that there's a logic error somewhere.
-				 *
-				 * EADDRINUSE is fatal: if there's
-				 * something already where we want to be
-				 * listening, we have no guarantees that
-				 * any clients will cope with it.
-				 */
-				case EACCES:
-				case EADDRNOTAVAIL:
-					debug("retrying");
-					sleep(1);
-					continue;
-				case EADDRINUSE:
-					fatal( "%s port %d in use, giving up.",
-							s_address,
-							ntohs(serve->bind_to.v4.sin_port));
-				default:
-					fatal( "Giving up" );
-			}
-		}
-	} while ( 1 );
-}
-
-
-
 /** Prepares a listening socket for the NBD server, binding etc. */
 void serve_open_server_socket(struct server* params)
 {
 	NULLCHECK( params );
 
-	int optval=1;
-
 	params->server_fd = socket(params->bind_to.generic.sa_family == AF_INET ?
 	  PF_INET : PF_INET6, SOCK_STREAM, 0);
 
-	FATAL_IF_NEGATIVE(params->server_fd,
+	FATAL_IF_NEGATIVE( params->server_fd, "Couldn't create server socket" );
-	  "Couldn't create server socket");
 
 	/* We need SO_REUSEADDR so that when we switch from listening to
 	 * serving we don't have to change address if we don't want to.
@@ -317,8 +235,7 @@ void serve_open_server_socket(struct server* params)
 	 * we barf.
 	 */
 	FATAL_IF_NEGATIVE(
-		setsockopt(params->server_fd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval)),
+		sock_set_reuseaddr( params->server_fd, 1 ), "Couldn't set SO_REUSEADDR"
-		"Couldn't set SO_REUSEADDR"
 	);
 
 	/* TCP_NODELAY makes everything not be slow.  If we can't set
@@ -326,14 +243,16 @@ void serve_open_server_socket(struct server* params)
 	 * understand.
 	 */
 	FATAL_IF_NEGATIVE(
-		setsockopt(params->server_fd, IPPROTO_TCP, TCP_NODELAY, &optval, sizeof(optval)),
+		sock_set_tcp_nodelay( params->server_fd, 1 ), "Couldn't set TCP_NODELAY"
-		"Couldn't set TCP_NODELAY"
 	);
 
 	/* If we can't bind, presumably that's because someone else is
 	 * squatting on our ip/port combo, or the ip isn't yet
 	 * configured. Ideally we want to retry this. */
-	serve_bind(params);
+	FATAL_UNLESS_ZERO(
+		sock_try_bind( params->server_fd, &params->bind_to.generic ),
+		SHOW_ERRNO( "Failed to bind() socket" )
+	);
 
 	FATAL_IF_NEGATIVE(
 		listen(params->server_fd, params->tcp_backlog),
@@ -354,17 +273,13 @@ int tryjoin_client_thread( struct client_tbl_entry *entry, int (*joinfunc)(pthre
 	int join_errno;
 
 	if (entry->thread != 0) {
-		char s_client_address[64];
+		char s_client_address[128];
 
-		memset(s_client_address, 0, 64);
+		sockaddr_address_string( &entry->address.generic, &s_client_address[0], 128 );
-		strcpy(s_client_address, "???");
-		inet_ntop( entry->address.generic.sa_family,
-				sockaddr_address_data(&entry->address.generic),
-				s_client_address,
-				64 );
 
 		debug( "%s(%p,...)", joinfunc == pthread_join ? "joining" : "tryjoining", entry->thread );
 		join_errno = joinfunc(entry->thread, &status);
+
 		/* join_errno can legitimately be ESRCH if the thread is
 		 * already dead, but the client still needs tidying up. */
 		if (join_errno != 0 && !entry->client->stopped ) {
@@ -483,9 +398,11 @@ int server_should_accept_client(
 	NULLCHECK( client_address );
 	NULLCHECK( s_client_address );
 
-	if (inet_ntop(client_address->generic.sa_family,
+	const char* result = sockaddr_address_string(
-				sockaddr_address_data(&client_address->generic),
+		&client_address->generic, s_client_address, s_client_address_len
-				s_client_address, s_client_address_len ) == NULL) {
+	);
+
+	if ( NULL == result ) {
 		warn( "Rejecting client %s: Bad client_address", s_client_address );
 		return 0;
 	}

src/sockutil.c

@@ -0,0 +1,104 @@
+#include "sockutil.h"
+#include "util.h"
+
+size_t sockaddr_size(const struct sockaddr* sa)
+{
+	size_t ret = 0;
+
+	switch( sa->sa_family ) {
+		case AF_INET:
+			ret = sizeof( struct sockaddr_in );
+			break;
+		case AF_INET6:
+			ret = sizeof( struct sockaddr_in6 );
+			break;
+	}
+
+	return ret;
+}
+
+const char* sockaddr_address_string(const struct sockaddr* sa, char* dest, size_t len)
+{
+	NULLCHECK( sa );
+	NULLCHECK( dest );
+
+	struct sockaddr_in*  in  = ( struct sockaddr_in*  ) sa;
+	struct sockaddr_in6* in6 = ( struct sockaddr_in6* ) sa;
+
+	unsigned short real_port = ntohs( in->sin_port ); // common to in and in6
+	size_t size;
+	const char* ret = NULL;
+
+	memset( dest, 0, len );
+
+	if ( sa->sa_family == AF_INET ) {
+		ret = inet_ntop( AF_INET, &in->sin_addr, dest, len );
+	} else if ( sa->sa_family == AF_INET6 ) {
+		ret = inet_ntop( AF_INET6, &in6->sin6_addr, dest, len );
+	} else {
+		strncpy( dest, "???", len );
+	}
+
+	if ( NULL != ret && real_port > 0 ) {
+		size = strlen( dest );
+		snprintf( dest + size, len - size, " port %d", real_port );
+	}
+
+	return ret;
+}
+
+int sock_set_reuseaddr(int fd, int optval)
+{
+	return setsockopt( fd, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval) );
+}
+
+/* Set the tcp_nodelay option */
+int sock_set_tcp_nodelay(int fd, int optval)
+{
+	return setsockopt( fd, IPPROTO_TCP, TCP_NODELAY, &optval, sizeof(optval) );
+}
+
+int sock_try_bind(int fd, const struct sockaddr* sa)
+{
+	int bind_result;
+	char s_address[256];
+
+	sockaddr_address_string( sa, &s_address[0], 256 );
+
+	do {
+		bind_result = bind( fd, sa, sockaddr_size( sa ) );
+		if ( 0 == bind_result ) {
+			info( "Bound to %s", s_address );
+			break;
+		}
+		else {
+			warn( SHOW_ERRNO( "Couldn't bind to %s", s_address ) );
+
+			switch ( errno ) {
+				/* bind() can give us EACCES, EADDRINUSE, EADDRNOTAVAIL, EBADF,
+				 * EINVAL, ENOTSOCK, EFAULT, ELOOP, ENAMETOOLONG, ENOENT,
+				 * ENOMEM, ENOTDIR, EROFS
+				 *
+				 * Any of these other than  EADDRINUSE & EADDRNOTAVAIL signify
+				 * that there's a logic error somewhere.
+				 *
+				 * EADDRINUSE is fatal: if there's something already where we
+				 * want to be listening, we have no guarantees that any clients
+				 * will cope with it.
+				 */
+				case EADDRNOTAVAIL:
+					debug( "retrying" );
+					sleep( 1 );
+					continue;
+				case EADDRINUSE:
+					warn( "%s in use, giving up.", s_address );
+					break;
+				default:
+					warn( "giving up" );
+			}
+		}
+	} while ( 1 );
+
+	return bind_result;
+}
+

src/sockutil.h

@@ -0,0 +1,32 @@
+#ifndef SOCKUTIL_H
+
+#define SOCKUTIL_H
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <arpa/inet.h>
+#include <netinet/tcp.h>
+
+/* Returns the size of the sockaddr, or 0 on error */
+size_t sockaddr_size(const struct sockaddr* sa);
+
+/* Convert a sockaddr into an address. Like inet_ntop, it returns dest if
+ * successful, NULL otherwise. In the latter case, dest will contain "???"
+ */
+const char* sockaddr_address_string(const struct sockaddr* sa, char* dest, size_t len);
+
+/* Set the SOL_REUSEADDR otion */
+int sock_set_reuseaddr(int fd, int optval);
+
+/* Set the tcp_nodelay option */
+int sock_set_tcp_nodelay(int fd, int optval);
+
+/* TODO: Set the tcp_cork option */
+// int sock_set_cork(int fd, int optval);
+
+/* Attempt to bind the fd to the sockaddr, retrying common transient failures */
+int sock_try_bind(int fd, const struct sockaddr* sa);
+
+#endif
+

src/util.h

@@ -148,7 +148,7 @@ void mylog(int line_level, const char* format, ...);
 
 #define NULLCHECK(value) FATAL_IF_NULL(value, "BUG: " #value " is null")
 
-#define SHOW_ERRNO( msg ) msg ": %s (%i)", ( errno == 0 ? "EOF" : strerror(errno) ), errno
+#define SHOW_ERRNO( msg, ... ) msg ": %s (%i)", ##__VA_ARGS__, ( errno == 0 ? "EOF" : strerror(errno) ), errno
 
 #endif