[PATCH 1/1] evio: allow not only SOCK_STREAM servers

[PATCH 1/1] evio: allow not only SOCK_STREAM servers

[Vladislav Shpilevoy]

evio is a useful tool for creating servers. Evio sets
all necessary flags, retries bind, parses URI. But it
does not work with SOCK_DGRAM sockets - SOCK_STREAM is
hardcoded. SWIM is going to use SOCK_DGRAM, and this
patch makes it possible to bind datagram sockets.

Needed for #3234
---
https://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-second-preparation
https://github.com/tarantool/tarantool/issues/3234

 src/box/iproto.cc |  4 ++--
 src/coio.cc       |  4 ++--
 src/evio.c        | 48 +++++++++++++++++++++++++++++++++++------------
 src/evio.h        | 19 ++++++++++++++++---
 4 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 223469f22..157789384 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1873,8 +1873,8 @@ net_cord_f(va_list /* ap */)
 	mempool_create(&iproto_connection_pool, &cord()->slabc,
 		       sizeof(struct iproto_connection));
 
-	evio_service_init(loop(), &binary, "binary",
-			  iproto_on_accept, NULL);
+	evio_service_init_tcp(loop(), &binary, "binary", iproto_on_accept,
+			      NULL);
 
 
 	/* Init statistics counter */
diff --git a/src/coio.cc b/src/coio.cc
index 6b94cf2a6..30d18ab3c 100644
--- a/src/coio.cc
+++ b/src/coio.cc
@@ -632,8 +632,8 @@ void
 coio_service_init(struct coio_service *service, const char *name,
 		  fiber_func handler, void *handler_param)
 {
-	evio_service_init(loop(), &service->evio_service, name,
-			  coio_service_on_accept, service);
+	evio_service_init_tcp(loop(), &service->evio_service, name,
+			      coio_service_on_accept, service);
 	service->handler = handler;
 	service->handler_param = handler_param;
 }
diff --git a/src/evio.c b/src/evio.c
index 9ca14c45c..c16fc99db 100644
--- a/src/evio.c
+++ b/src/evio.c
@@ -209,7 +209,8 @@ evio_service_accept_cb(ev_loop *loop, ev_io *watcher, int events)
 static int
 evio_service_reuse_addr(struct evio_service *service, int fd)
 {
-	if ((service->addr.sa_family != AF_UNIX) || (errno != EADDRINUSE)) {
+	if (service->addr.sa_family != AF_UNIX || errno != EADDRINUSE ||
+	    service->sock_type != SOCK_STREAM) {
 		diag_set(SocketError, sio_socketname(fd),
 			 "evio_service_reuse_addr");
 		return -1;
@@ -248,13 +249,13 @@ evio_service_bind_addr(struct evio_service *service)
 	say_debug("%s: binding to %s...", evio_service_name(service),
 		  sio_strfaddr(&service->addr, service->addr_len));
 	/* Create a socket. */
-	int fd = sio_socket(service->addr.sa_family,
-			    SOCK_STREAM, IPPROTO_TCP);
+	int fd = sio_socket(service->addr.sa_family, service->sock_type,
+			    service->sock_protocol);
 	if (fd < 0)
 		return -1;
 
 	if (evio_setsockopt_server(fd, service->addr.sa_family,
-				   SOCK_STREAM) != 0)
+				   service->sock_type) != 0)
 		goto error;
 
 	if (sio_bind(fd, &service->addr, service->addr_len)) {
@@ -282,6 +283,12 @@ error:
 	return -1;
 }
 
+void
+evio_service_start(struct evio_service *service)
+{
+	ev_io_start(service->loop, &service->ev);
+}
+
 /**
  * Listen on bounded port.
  *
@@ -306,30 +313,47 @@ evio_service_listen(struct evio_service *service)
 		}
 		return -1;
 	}
-	ev_io_start(service->loop, &service->ev);
+	evio_service_start(service);
 	return 0;
 }
 
-void
+static void
 evio_service_init(ev_loop *loop, struct evio_service *service, const char *name,
-		  evio_accept_f on_accept, void *on_accept_param)
+		  int sock_type, int sock_protocol)
 {
 	memset(service, 0, sizeof(struct evio_service));
 	snprintf(service->name, sizeof(service->name), "%s", name);
 
 	service->loop = loop;
-
-	service->on_accept = on_accept;
-	service->on_accept_param = on_accept_param;
+	service->sock_type = sock_type;
+	service->sock_protocol = sock_protocol;
 	/*
 	 * Initialize libev objects to be able to detect if they
 	 * are active or not in evio_service_stop().
 	 */
-	ev_init(&service->ev, evio_service_accept_cb);
 	ev_io_set(&service->ev, -1, 0);
 	service->ev.data = service;
 }
 
+void
+evio_service_init_udp(ev_loop *loop, struct evio_service *service,
+		      const char *name, evio_input_f on_input)
+{
+	evio_service_init(loop, service, name, SOCK_DGRAM, IPPROTO_UDP);
+	ev_init(&service->ev, on_input);
+}
+
+void
+evio_service_init_tcp(ev_loop *loop, struct evio_service *service,
+		      const char *name, evio_accept_f on_accept,
+		      void *on_accept_param)
+{
+	evio_service_init(loop, service, name, SOCK_STREAM, IPPROTO_TCP);
+	service->on_accept = on_accept;
+	service->on_accept_param = on_accept_param;
+	ev_init(&service->ev, evio_service_accept_cb);
+}
+
 /**
  * Try to bind.
  */
@@ -366,7 +390,7 @@ evio_service_bind(struct evio_service *service, const char *uri)
 	struct addrinfo hints, *res;
 	memset(&hints, 0, sizeof(hints));
 	hints.ai_family = AF_UNSPEC;
-	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_socktype = service->sock_type;
 	hints.ai_flags = AI_PASSIVE|AI_ADDRCONFIG;
 
 	/* make no difference between empty string and NULL for host */
diff --git a/src/evio.h b/src/evio.h
index 69d641a60..826b6bb31 100644
--- a/src/evio.h
+++ b/src/evio.h
@@ -72,6 +72,8 @@ struct evio_service;
 typedef int (*evio_accept_f)(struct evio_service *, int, struct sockaddr *,
 			      socklen_t);
 
+typedef void (*evio_input_f)(struct ev_loop *, struct ev_io *, int);
+
 struct evio_service
 {
 	/** Service name. E.g. 'primary', 'secondary', etc. */
@@ -86,7 +88,9 @@ struct evio_service
 		struct sockaddr_storage addrstorage;
 	};
 	socklen_t addr_len;
-
+	/** Socket type (STREAM/DGRAM) and protocol (TCP/UDP). */
+	int sock_type;
+	int sock_protocol;
 	/**
 	 * A callback invoked on every accepted client socket.
 	 * If a callback returned != 0, the accepted socket is
@@ -102,8 +106,13 @@ struct evio_service
 
 /** Initialize the service. Don't bind to the port yet. */
 void
-evio_service_init(ev_loop *loop, struct evio_service *service, const char *name,
-		  evio_accept_f on_accept, void *on_accept_param);
+evio_service_init_tcp(ev_loop *loop, struct evio_service *service,
+		      const char *name, evio_accept_f on_accept,
+		      void *on_accept_param);
+
+void
+evio_service_init_udp(ev_loop *loop, struct evio_service *service,
+		      const char *name, evio_input_f on_input);
 
 /** Bind service to specified uri */
 int
@@ -117,6 +126,10 @@ evio_service_bind(struct evio_service *service, const char *uri);
 int
 evio_service_listen(struct evio_service *service);
 
+/** Run IO event loop. */
+void
+evio_service_start(struct evio_service *service);
+
 /** If started, stop event flow and close the acceptor socket. */
 void
 evio_service_stop(struct evio_service *service);
-- 
2.17.2 (Apple Git-113)

Re: [PATCH 1/1] evio: allow not only SOCK_STREAM servers

[Vladimir Davydov]

On Mon, Dec 17, 2018 at 02:55:44PM +0300, Vladislav Shpilevoy wrote:
> evio is a useful tool for creating servers. Evio sets
> all necessary flags, retries bind, parses URI. But it
> does not work with SOCK_DGRAM sockets - SOCK_STREAM is
> hardcoded. SWIM is going to use SOCK_DGRAM, and this
> patch makes it possible to bind datagram sockets.
> 
> Needed for #3234
> ---
> https://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-second-preparation
> https://github.com/tarantool/tarantool/issues/3234
> 
>  src/box/iproto.cc |  4 ++--
>  src/coio.cc       |  4 ++--
>  src/evio.c        | 48 +++++++++++++++++++++++++++++++++++------------
>  src/evio.h        | 19 ++++++++++++++++---
>  4 files changed, 56 insertions(+), 19 deletions(-)

I don't think it's a good idea to reuse evio_service_bind() for SWIM
configuration as it's blocking (it uses getaddrinfo). Anyway, do we
really need this extra level of abstraction? Wouldn't it be more
straightforward to use sio/ev/coio directly instead?

Re: [tarantool-patches] Re: [PATCH 1/1] evio: allow not only SOCK_STREAM servers

[Vladislav Shpilevoy]

On 18/12/2018 13:08, Vladimir Davydov wrote:
> On Mon, Dec 17, 2018 at 02:55:44PM +0300, Vladislav Shpilevoy wrote:
>> evio is a useful tool for creating servers. Evio sets
>> all necessary flags, retries bind, parses URI. But it
>> does not work with SOCK_DGRAM sockets - SOCK_STREAM is
>> hardcoded. SWIM is going to use SOCK_DGRAM, and this
>> patch makes it possible to bind datagram sockets.
>>
>> Needed for #3234
>> ---
>> https://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-second-preparation
>> https://github.com/tarantool/tarantool/issues/3234
>>
>>   src/box/iproto.cc |  4 ++--
>>   src/coio.cc       |  4 ++--
>>   src/evio.c        | 48 +++++++++++++++++++++++++++++++++++------------
>>   src/evio.h        | 19 ++++++++++++++++---
>>   4 files changed, 56 insertions(+), 19 deletions(-)
> 
> I don't think it's a good idea to reuse evio_service_bind() for SWIM
> configuration as it's blocking (it uses getaddrinfo). Anyway, do we
> really need this extra level of abstraction? Wouldn't it be more
> straightforward to use sio/ev/coio directly instead?
> 

Evio will allow UNIX sockets in future. But as you wish.

[tarantool-patches] Re: [PATCH 1/1] evio: allow not only SOCK_STREAM servers

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/12/18 13:09]:
> On Mon, Dec 17, 2018 at 02:55:44PM +0300, Vladislav Shpilevoy wrote:
> > evio is a useful tool for creating servers. Evio sets
> > all necessary flags, retries bind, parses URI. But it
> > does not work with SOCK_DGRAM sockets - SOCK_STREAM is
> > hardcoded. SWIM is going to use SOCK_DGRAM, and this
> > patch makes it possible to bind datagram sockets.
> > 
> > Needed for #3234

It's high time we integrated c-ares dns resolver. We haven't had
time before. 


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov