Re: [PATCH v4 7/7] passt: Move main event loop processing into passt_worker()

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

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

On Fri, Oct 17, 2025 at 12:31:29PM +0200, Laurent Vivier wrote:
> Extract the epoll event processing logic from main() into a separate
> passt_worker() function. This refactoring prepares the code for future
> threading support where passt_worker() will be called as a worker thread
> callback.
> 
> The new function handles:
> - Processing epoll events and dispatching to protocol handlers
> - Event statistics tracking and printing
> - Post-handler periodic tasks (timers, deferred work)
> - Migration handling
> 
> No functional changes, purely a code restructuring.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Looks good as far as it goes, and I've though often in the past that
it would make more sense for the "engine" to go in its own function.

Wondering if it would make more sense to include the epoll_wait()
itself and the loop in this function, rather than leaving that
outside.

> ---
>  passt.c | 160 +++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 88 insertions(+), 72 deletions(-)
> 
> diff --git a/passt.c b/passt.c
> index 37f2c897be84..5bfa4c6353d9 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -229,6 +229,92 @@ static void print_stats(const struct ctx *c, const struct passt_stats *stats,
>  	lines_printed++;
>  }
>  
> +/**
> + * passt_worker() - Process epoll events and handle protocol operations
> + * @opaque:	Pointer to execution context (struct ctx)
> + * @nfds:	Number of file descriptors ready (epoll_wait return value)
> + * @events:	epoll_event array of ready file descriptors
> + */
> +static void passt_worker(void *opaque, int nfds,  struct epoll_event *events)
> +{
> +	static struct passt_stats stats = { 0 };
> +	struct ctx *c = opaque;
> +	struct timespec now;
> +	int i;
> +
> +	if (clock_gettime(CLOCK_MONOTONIC, &now))
> +		err_perror("Failed to get CLOCK_MONOTONIC time");
> +
> +	for (i = 0; i < nfds; i++) {
> +		union epoll_ref ref = *((union epoll_ref *)&events[i].data.u64);
> +		uint32_t eventmask = events[i].events;
> +
> +		trace("%s: epoll event on %s %i (events: 0x%08x)",
> +		      c->mode == MODE_PASTA ? "pasta" : "passt",
> +		      EPOLL_TYPE_STR(ref.type), ref.fd, eventmask);
> +
> +		switch (ref.type) {
> +		case EPOLL_TYPE_TAP_PASTA:
> +			tap_handler_pasta(c, eventmask, &now);
> +			break;
> +		case EPOLL_TYPE_TAP_PASST:
> +			tap_handler_passt(c, eventmask, &now);
> +			break;
> +		case EPOLL_TYPE_TAP_LISTEN:
> +			tap_listen_handler(c, eventmask);
> +			break;
> +		case EPOLL_TYPE_NSQUIT_INOTIFY:
> +			pasta_netns_quit_inotify_handler(c, ref.fd);
> +			break;
> +		case EPOLL_TYPE_NSQUIT_TIMER:
> +			pasta_netns_quit_timer_handler(c, ref);
> +			break;
> +		case EPOLL_TYPE_TCP:
> +			tcp_sock_handler(c, ref, eventmask);
> +			break;
> +		case EPOLL_TYPE_TCP_SPLICE:
> +			tcp_splice_sock_handler(c, ref, eventmask);
> +			break;
> +		case EPOLL_TYPE_TCP_LISTEN:
> +			tcp_listen_handler(c, ref, &now);
> +			break;
> +		case EPOLL_TYPE_TCP_TIMER:
> +			tcp_timer_handler(c, ref);
> +			break;
> +		case EPOLL_TYPE_UDP_LISTEN:
> +			udp_listen_sock_handler(c, ref, eventmask, &now);
> +			break;
> +		case EPOLL_TYPE_UDP:
> +			udp_sock_handler(c, ref, eventmask, &now);
> +			break;
> +		case EPOLL_TYPE_PING:
> +			icmp_sock_handler(c, ref);
> +			break;
> +		case EPOLL_TYPE_VHOST_CMD:
> +			vu_control_handler(c->vdev, c->fd_tap, eventmask);
> +			break;
> +		case EPOLL_TYPE_VHOST_KICK:
> +			vu_kick_cb(c->vdev, ref, &now);
> +			break;
> +		case EPOLL_TYPE_REPAIR_LISTEN:
> +			repair_listen_handler(c, eventmask);
> +			break;
> +		case EPOLL_TYPE_REPAIR:
> +			repair_handler(c, eventmask);
> +			break;
> +		default:
> +			/* Can't happen */
> +			ASSERT(0);
> +		}
> +		stats.events[ref.type]++;
> +		print_stats(c, &stats, &now);
> +	}
> +
> +	post_handler(c, &now);
> +
> +	migrate_handler(c);
> +}
> +
>  /**
>   * main() - Entry point and main loop
>   * @argc:	Argument count
> @@ -246,8 +332,7 @@ static void print_stats(const struct ctx *c, const struct passt_stats *stats,
>  int main(int argc, char **argv)
>  {
>  	struct epoll_event events[NUM_EPOLL_EVENTS];
> -	struct passt_stats stats = { 0 };
> -	int nfds, i, devnull_fd = -1;
> +	int nfds, devnull_fd = -1;
>  	struct ctx c = { 0 };
>  	struct rlimit limit;
>  	struct timespec now;
> @@ -355,77 +440,8 @@ loop:
>  	if (nfds == -1 && errno != EINTR)
>  		die_perror("epoll_wait() failed in main loop");
>  
> -	if (clock_gettime(CLOCK_MONOTONIC, &now))
> -		err_perror("Failed to get CLOCK_MONOTONIC time");
> -
> -	for (i = 0; i < nfds; i++) {
> -		union epoll_ref ref = *((union epoll_ref *)&events[i].data.u64);
> -		uint32_t eventmask = events[i].events;
> -
> -		trace("%s: epoll event on %s %i (events: 0x%08x)",
> -		      c.mode == MODE_PASTA ? "pasta" : "passt",
> -		      EPOLL_TYPE_STR(ref.type), ref.fd, eventmask);
> -
> -		switch (ref.type) {
> -		case EPOLL_TYPE_TAP_PASTA:
> -			tap_handler_pasta(&c, eventmask, &now);
> -			break;
> -		case EPOLL_TYPE_TAP_PASST:
> -			tap_handler_passt(&c, eventmask, &now);
> -			break;
> -		case EPOLL_TYPE_TAP_LISTEN:
> -			tap_listen_handler(&c, eventmask);
> -			break;
> -		case EPOLL_TYPE_NSQUIT_INOTIFY:
> -			pasta_netns_quit_inotify_handler(&c, ref.fd);
> -			break;
> -		case EPOLL_TYPE_NSQUIT_TIMER:
> -			pasta_netns_quit_timer_handler(&c, ref);
> -			break;
> -		case EPOLL_TYPE_TCP:
> -			tcp_sock_handler(&c, ref, eventmask);
> -			break;
> -		case EPOLL_TYPE_TCP_SPLICE:
> -			tcp_splice_sock_handler(&c, ref, eventmask);
> -			break;
> -		case EPOLL_TYPE_TCP_LISTEN:
> -			tcp_listen_handler(&c, ref, &now);
> -			break;
> -		case EPOLL_TYPE_TCP_TIMER:
> -			tcp_timer_handler(&c, ref);
> -			break;
> -		case EPOLL_TYPE_UDP_LISTEN:
> -			udp_listen_sock_handler(&c, ref, eventmask, &now);
> -			break;
> -		case EPOLL_TYPE_UDP:
> -			udp_sock_handler(&c, ref, eventmask, &now);
> -			break;
> -		case EPOLL_TYPE_PING:
> -			icmp_sock_handler(&c, ref);
> -			break;
> -		case EPOLL_TYPE_VHOST_CMD:
> -			vu_control_handler(c.vdev, c.fd_tap, eventmask);
> -			break;
> -		case EPOLL_TYPE_VHOST_KICK:
> -			vu_kick_cb(c.vdev, ref, &now);
> -			break;
> -		case EPOLL_TYPE_REPAIR_LISTEN:
> -			repair_listen_handler(&c, eventmask);
> -			break;
> -		case EPOLL_TYPE_REPAIR:
> -			repair_handler(&c, eventmask);
> -			break;
> -		default:
> -			/* Can't happen */
> -			ASSERT(0);
> -		}
> -		stats.events[ref.type]++;
> -		print_stats(&c, &stats, &now);
> -	}
> -
> -	post_handler(&c, &now);
>  
> -	migrate_handler(&c);
> +	passt_worker(&c, nfds, events);
>  
>  	goto loop;
>  }
> -- 
> 2.51.0
> 

-- 
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 --]