Re: [PATCH] udp: Split activity timeouts for UDP flows

[Stefano Brivio <sbrivio@redhat.com>]

Oops, I missed one point at a first review, and also during a quick
test.

I just tried outbound DNS queries in pasta with single responses, not
inbound traffic or passt in vhost-user mode. Then I realised
that:

On Thu, 12 Feb 2026 16:04:14 +0800
Yumei Huang <yuhuang@redhat.com> wrote:

> [...]
> @@ -954,6 +964,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref,
>  
>  		flow_trace(uflow, "Received data on reply socket");
>  		uflow->ts = now->tv_sec;
> +		udp_flow_activity(uflow, !tosidx.sidei);

...this only covers three of the four paths we need to act upon:

1. inbound datagrams received on the reply socket via
   udp_buf_sock_to_tap(), called from here

2. inbound datagrams received on the reply socket in passt's vhost-user
   mode, that's udp_vu_sock_recv(), also called from here

3. "spliced" sockets (that's not really the case for UDP, we can't call
   splice(), but a pair of recvmmsg() / sendmmsg()), that is, loopback
   UDP traffic, handled by udp_sock_to_sock(), called from here as well

but not:

4. outbound, non-spliced datagrams from container/guest: that's
   udp_tap_handler(), in both vhost-user and non-vhost-user cases, or
   udp_flow_from_tap() in udp_flow.c.

   I guess we want to take care of this directly from udp_flow_from_tap(),
   for consistency, because that's also where we update the timestamp
   value:

	sidx = flow_lookup_sa(c, IPPROTO_UDP, pif, s_in, dst, port);
	if ((uflow = udp_at_sidx(sidx))) {
		uflow->ts = now->tv_sec;

		^^^ here

		return flow_sidx_opposite(sidx);
	}

I haven't really tested this side of it but it should be fairly easy
with socat and a UDP "server" inside pasta or a guest.

Another thing I noticed later:

> [...]
>
> diff --git a/udp_flow.h b/udp_flow.h
> index 14e0f92..158a0f6 100644
> --- a/udp_flow.h
> +++ b/udp_flow.h
> @@ -16,6 +16,7 @@
>   * @flush1:	@s[1] may have datagrams queued for other flows
>   * @ts:		Activity timestamp
>   * @s:		Socket fd (or -1) for each side of the flow
> + * @activity:	Activity for each side of the flow
>   */
>  struct udp_flow {
>  	/* Must be first element */
> @@ -29,8 +30,20 @@ struct udp_flow {
>  
>  	time_t ts;
>  	int s[SIDES];
> +	uint8_t activity[SIDES];
>  };
>  
> +/**
> + * udp_flow_activity() - Track activity of a udp flow
> + * @uflow:	UDP flow
> + * @sidei:	Side index of the flow
> + */
> +static inline void udp_flow_activity(struct udp_flow *uflow, unsigned int sidei)
> +{
> +	if (uflow->activity[sidei] < UINT8_MAX)
> +		uflow->activity[sidei]++;
> +}

This is an inline function in a header file for no good reason. It
could be a normal static function in udp.c. See also:

  https://www.kernel.org/doc/html/latest/process/coding-style.html#the-inline-disease

...and yes, it's two lines of code, but there's really no reason to
decide we want to inline this instead of letting the compiler decide.

-- 
Stefano