Re: [PATCH v2 07/13] epoll: Fold sock_handler into general switch on epoll event fd

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 10 Aug 2023 12:33:09 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> When we process events from epoll_wait(), we check for a number of special
> cases before calling sock_handler() which then dispatches based on the
> protocol type of the socket in the event.
> 
> Fold these cases together into a single switch on the fd type recorded in
> the epoll data field.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  passt.c | 54 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/passt.c b/passt.c
> index 45e9fbf..665262b 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -64,29 +64,6 @@ char *epoll_type_str[EPOLL_TYPE_MAX + 1] = {
>  	[EPOLL_TYPE_TAP]	= "tap device",
>  };
>  
> -/**
> - * sock_handler() - Event handler for L4 sockets
> - * @c:		Execution context
> - * @ref:	epoll reference
> - * @events:	epoll events
> - * @now:	Current timestamp
> - */
> -static void sock_handler(struct ctx *c, union epoll_ref ref,
> -			 uint32_t events, const struct timespec *now)
> -{
> -	trace("%s: packet from %s %i (events: 0x%08x)",
> -	      c->mode == MODE_PASST ? "passt" : "pasta",
> -	      EPOLL_TYPE_STR(ref.type), ref.fd, events);
> -
> -	if (!c->no_tcp && ref.type == EPOLL_TYPE_TCP)
> -		tcp_sock_handler(c, ref, events, now);
> -	else if (!c->no_udp && ref.type == EPOLL_TYPE_UDP)
> -		udp_sock_handler(c, ref, events, now);
> -	else if (!c->no_icmp &&
> -		 (ref.type == EPOLL_TYPE_ICMP || ref.type == EPOLL_TYPE_ICMPV6))
> -		icmp_sock_handler(c, ref, events, now);
> -}
> -
>  /**
>   * post_handler() - Run periodic and deferred tasks for L4 protocol handlers
>   * @c:		Execution context
> @@ -330,13 +307,36 @@ loop:
>  
>  	for (i = 0; i < nfds; i++) {
>  		union epoll_ref ref = *((union epoll_ref *)&events[i].data.u64);
> +		uint32_t eventmask = events[i].events;
>  
> -		if (ref.type == EPOLL_TYPE_TAP)
> +		trace("%s: epoll event on %s %i (events: 0x%08x)",
> +		      c.mode == MODE_PASST ? "passt" : "pasta",
> +		      EPOLL_TYPE_STR(ref.type), ref.fd, events);

Gah, sorry I missed this earlier, but covscan reported it -- you
probably wanted to pass 'eventmask' to trace().

Actually, a long time ago, I was pondering about a macro that would
print constants' names ("EPOLLIN", etc.) instead, but then I thought
that once you remember the values from <sys/epoll.h>, hex values might
be a bit easier on eyes when you're digging through thousands of
them... or maybe not. I don't know actually.

-- 
Stefano