Re: [PATCH 09/12] udp_flow: Take pif and port as explicit parameters to udp_flow_from_sock()

[Stefano Brivio <sbrivio@redhat.com>]

On Fri,  4 Apr 2025 21:15:39 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Currently udp_flow_from_sock() is only used when receiving a datagram
> from a "listening" socket.  It takes the listening socket's epoll
> reference to get the interface and port on which the datagram arrived.
> 
> We have some upcoming cases where we want to use this in different
> contexts, so make it take the pif and port as direct parameters instead.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  udp.c      |  4 +++-
>  udp_flow.c | 15 +++++++--------
>  udp_flow.h |  2 +-
>  3 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/udp.c b/udp.c
> index f6de2924..dbb33f2a 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -728,7 +728,9 @@ static void udp_listen_sock_data(const struct ctx *c, union epoll_ref ref,
>  	union sockaddr_inany src;
>  
>  	while (udp_peek_addr(ref.fd, &src) == 0) {
> -		flow_sidx_t tosidx = udp_flow_from_sock(c, ref, &src, now);
> +		flow_sidx_t tosidx = udp_flow_from_sock(c, ref.udp.pif,
> +							ref.udp.port, &src,
> +							now);
>  		uint8_t topif = pif_at_sidx(tosidx);
>  
>  		if (pif_is_socket(topif)) {
> diff --git a/udp_flow.c b/udp_flow.c
> index d50bddb2..b95c3176 100644
> --- a/udp_flow.c
> +++ b/udp_flow.c
> @@ -160,8 +160,10 @@ cancel:
>  }
>  
>  /**
> - * udp_flow_from_sock() - Find or create UDP flow for "listening" socket
> + * udp_flow_from_sock() - Find or create UDP flow for incoming datagram
>   * @c:		Execution context
> + * @pif:	Interface the datagram is arriving from
> + * @port:	Our (local) port number to which the datagram is arriving
>   * @ref:	epoll reference of the receiving socket

@ref went away. Fixed on merge.

>   * @s_in:	Source socket address, filled in by recvmmsg()
>   * @now:	Timestamp
> @@ -171,7 +173,7 @@ cancel:
>   * Return: sidx for the destination side of the flow for this packet, or
>   *         FLOW_SIDX_NONE if we couldn't find or create a flow.
>   */
> -flow_sidx_t udp_flow_from_sock(const struct ctx *c, union epoll_ref ref,
> +flow_sidx_t udp_flow_from_sock(const struct ctx *c, uint8_t pif, in_port_t port,
>  			       const union sockaddr_inany *s_in,
>  			       const struct timespec *now)
>  {
> @@ -180,9 +182,7 @@ flow_sidx_t udp_flow_from_sock(const struct ctx *c, union epoll_ref ref,
>  	union flow *flow;
>  	flow_sidx_t sidx;
>  
> -	ASSERT(ref.type == EPOLL_TYPE_UDP_LISTEN);
> -
> -	sidx = flow_lookup_sa(c, IPPROTO_UDP, ref.udp.pif, s_in, ref.udp.port);
> +	sidx = flow_lookup_sa(c, IPPROTO_UDP, pif, s_in, port);
>  	if ((uflow = udp_at_sidx(sidx))) {
>  		uflow->ts = now->tv_sec;
>  		return flow_sidx_opposite(sidx);
> @@ -192,12 +192,11 @@ flow_sidx_t udp_flow_from_sock(const struct ctx *c, union epoll_ref ref,
>  		char sastr[SOCKADDR_STRLEN];
>  
>  		debug("Couldn't allocate flow for UDP datagram from %s %s",
> -		      pif_name(ref.udp.pif),
> -		      sockaddr_ntop(s_in, sastr, sizeof(sastr)));
> +		      pif_name(pif), sockaddr_ntop(s_in, sastr, sizeof(sastr)));
>  		return FLOW_SIDX_NONE;
>  	}
>  
> -	ini = flow_initiate_sa(flow, ref.udp.pif, s_in, ref.udp.port);
> +	ini = flow_initiate_sa(flow, pif, s_in, port);
>  
>  	if (!inany_is_unicast(&ini->eaddr) ||
>  	    ini->eport == 0 || ini->oport == 0) {
> diff --git a/udp_flow.h b/udp_flow.h
> index 9a1b059c..d4e4c8b9 100644
> --- a/udp_flow.h
> +++ b/udp_flow.h
> @@ -24,7 +24,7 @@ struct udp_flow {
>  };
>  
>  struct udp_flow *udp_at_sidx(flow_sidx_t sidx);
> -flow_sidx_t udp_flow_from_sock(const struct ctx *c, union epoll_ref ref,
> +flow_sidx_t udp_flow_from_sock(const struct ctx *c, uint8_t pif, in_port_t port,
>  			       const union sockaddr_inany *s_in,
>  			       const struct timespec *now);
>  flow_sidx_t udp_flow_from_tap(const struct ctx *c,

-- 
Stefano