Re: [PATCH 1/6] tap: Split out handling of EPOLLIN events

[Stefano Brivio <sbrivio@redhat.com>]

On Tue,  3 Sep 2024 22:02:30 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently, tap_handler_pas{st,ta}() check for EPOLLRDHUP, EPOLLHUP and
> EPOLLERR events, then assume anything left is EPOLLIN.  We have some future
> cases that may want to also handle EPOLLOUT, so in preparation explicitly
> handle EPOLLIN, moving the logic to a subfunction.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  tap.c | 50 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index 852d8376..14c88871 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -982,24 +982,17 @@ static void tap_sock_reset(struct ctx *c)
>  }
>  
>  /**
> - * tap_handler_passt() - Packet handler for AF_UNIX file descriptor
> + * tap_passt_input() - Handler for new data on the socket to qemu
>   * @c:		Execution context
> - * @events:	epoll events
>   * @now:	Current timestamp
>   */
> -void tap_handler_passt(struct ctx *c, uint32_t events,
> -		       const struct timespec *now)
> +static void tap_passt_input(struct ctx *c, const struct timespec *now)
>  {
>  	static const char *partial_frame;
>  	static ssize_t partial_len = 0;
>  	ssize_t n;
>  	char *p;
>  
> -	if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR)) {
> -		tap_sock_reset(c);
> -		return;
> -	}
> -
>  	tap_flush_pools();
>  
>  	if (partial_len) {
> @@ -1052,20 +1045,33 @@ void tap_handler_passt(struct ctx *c, uint32_t events,
>  }
>  
>  /**
> - * tap_handler_pasta() - Packet handler for /dev/net/tun file descriptor
> + * tap_handler_passt() - Event handler for AF_UNIX file descriptor
>   * @c:		Execution context
>   * @events:	epoll events
>   * @now:	Current timestamp
>   */
> -void tap_handler_pasta(struct ctx *c, uint32_t events,
> +void tap_handler_passt(struct ctx *c, uint32_t events,
>  		       const struct timespec *now)
> +{
> +	if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR)) {
> +		tap_sock_reset(c);
> +		return;
> +	}
> +
> +	if (events & EPOLLIN)
> +		tap_passt_input(c, now);
> +}
> +
> +/**
> + * tap_passt_input() - Handler for new data on the socket to qemu

tap_pasta_input(), QEMU (we could just call it hypervisor, given that
libkrun/krun also use this path).

> + * @c:		Execution context
> + * @now:	Current timestamp
> + */
> +static void tap_pasta_input(struct ctx *c, const struct timespec *now)
>  {
>  	ssize_t n, len;
>  	int ret;
>  
> -	if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR))
> -		die("Disconnect event on /dev/net/tun device, exiting");
> -
>  redo:
>  	n = 0;
>  
> @@ -1102,6 +1108,22 @@ restart:
>  	die("Error on tap device, exiting");
>  }
>  
> +/**
> + * tap_handler_pasta() - Packet handler for /dev/net/tun file descriptor
> + * @c:		Execution context
> + * @events:	epoll events
> + * @now:	Current timestamp
> + */
> +void tap_handler_pasta(struct ctx *c, uint32_t events,
> +		       const struct timespec *now)
> +{
> +	if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR))
> +		die("Disconnect event on /dev/net/tun device, exiting");
> +
> +	if (events & EPOLLIN)
> +		tap_pasta_input(c, now);
> +}
> +
>  /**
>   * tap_sock_unix_open() - Create and bind AF_UNIX socket
>   * @sock_path:	Socket path. If empty, set on return (UNIX_SOCK_PATH as prefix)

-- 
Stefano