Re: [PATCH 1/2] tap: Use a common function to start a new connection

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Dec 09, 2024 at 05:54:49PM +0100, Laurent Vivier wrote:
> Merge code from tap_backend_init(), tap_sock_tun_init() and
> tap_listen_handler() to set epoll_ref entry and to add it
> to epollfd.
> 
> No functionality change
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tap.c | 66 +++++++++++++++++++++++++++--------------------------------
>  1 file changed, 30 insertions(+), 36 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index c4180643bd0a..b2d30456e8dc 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1255,6 +1255,33 @@ static void tap_sock_unix_init(const struct ctx *c)
>  	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap_listen, &ev);
>  }
>  
> +/**
> + * tap_start_connection() - start a new connection
> + * @c:		Execution context
> + */
> +static void tap_start_connection(const struct ctx *c)
> +{
> +	struct epoll_event ev = { 0 };
> +	union epoll_ref ref = { 0 };
> +
> +	ref.fd = c->fd_tap;
> +	switch (c->mode) {
> +	case MODE_PASST:
> +		ref.type = EPOLL_TYPE_TAP_PASST;
> +		break;
> +	case MODE_PASTA:
> +		ref.type = EPOLL_TYPE_TAP_PASTA;
> +		break;
> +	case MODE_VU:
> +		ref.type = EPOLL_TYPE_VHOST_CMD;
> +		break;
> +	}
> +
> +	ev.events = EPOLLIN | EPOLLRDHUP;
> +	ev.data.u64 = ref.u64;
> +	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
> +}
> +
>  /**
>   * tap_listen_handler() - Handle new connection on listening socket
>   * @c:		Execution context
> @@ -1262,8 +1289,6 @@ static void tap_sock_unix_init(const struct ctx *c)
>   */
>  void tap_listen_handler(struct ctx *c, uint32_t events)
>  {
> -	struct epoll_event ev = { 0 };
> -	union epoll_ref ref = { 0 };
>  	int v = INT_MAX / 2;
>  	struct ucred ucred;
>  	socklen_t len;
> @@ -1302,14 +1327,7 @@ void tap_listen_handler(struct ctx *c, uint32_t events)
>  	    setsockopt(c->fd_tap, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v)))
>  		trace("tap: failed to set SO_SNDBUF to %i", v);
>  
> -	ref.fd = c->fd_tap;
> -	if (c->mode == MODE_VU)
> -		ref.type = EPOLL_TYPE_VHOST_CMD;
> -	else
> -		ref.type = EPOLL_TYPE_TAP_PASST;
> -	ev.events = EPOLLIN | EPOLLRDHUP;
> -	ev.data.u64 = ref.u64;
> -	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
> +	tap_start_connection(c);
>  }
>  
>  /**
> @@ -1353,19 +1371,13 @@ static int tap_ns_tun(void *arg)
>   */
>  static void tap_sock_tun_init(struct ctx *c)
>  {
> -	union epoll_ref ref = { .type = EPOLL_TYPE_TAP_PASTA };
> -	struct epoll_event ev = { 0 };
> -
>  	NS_CALL(tap_ns_tun, c);
>  	if (c->fd_tap == -1)
>  		die("Failed to set up tap device in namespace");
>  
>  	pasta_ns_conf(c);
>  
> -	ref.fd = c->fd_tap;
> -	ev.events = EPOLLIN | EPOLLRDHUP;
> -	ev.data.u64 = ref.u64;
> -	epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
> +	tap_start_connection(c);
>  }
>  
>  /**
> @@ -1399,26 +1411,8 @@ void tap_backend_init(struct ctx *c)
>  		tap_sock_update_pool(pkt_buf, sizeof(pkt_buf));
>  
>  	if (c->fd_tap != -1) { /* Passed as --fd */
> -		struct epoll_event ev = { 0 };
> -		union epoll_ref ref;
> -
>  		ASSERT(c->one_off);
> -		ref.fd = c->fd_tap;
> -		switch (c->mode) {
> -		case MODE_PASST:
> -			ref.type = EPOLL_TYPE_TAP_PASST;
> -			break;
> -		case MODE_PASTA:
> -			ref.type = EPOLL_TYPE_TAP_PASTA;
> -			break;
> -		case MODE_VU:
> -			ref.type = EPOLL_TYPE_VHOST_CMD;
> -			break;
> -		}
> -
> -		ev.events = EPOLLIN | EPOLLRDHUP;
> -		ev.data.u64 = ref.u64;
> -		epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev);
> +		tap_start_connection(c);
>  		return;
>  	}
>  

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]