Re: [PATCH v4 4/9] udp: forward external source MAC address through tap interface

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

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

On Tue, Aug 19, 2025 at 11:10:00PM -0400, Jon Maloy wrote:
> We forward the incoming MAC address through the tap interface when
> receiving incoming packets from network local hosts. Packets from
> the own host are excepted from this rule, and are still forwarded
> with the default PASST/PASTA MAC address as source.
> 
> This is a part of the solution to bug
> https://bugs.passt.top/show_bug.cgi?id=120
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Excepting nits below.

> ---
>  passt.c |  2 +-
>  udp.c   | 39 ++++++++++++++++++++-------------------
>  udp.h   |  2 +-
>  3 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/passt.c b/passt.c
> index 388d10f..477a01f 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -154,7 +154,7 @@ static void timer_init(struct ctx *c, const struct timespec *now)
>  void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
>  {
>  	tcp_update_l2_buf(eth_d, eth_s);
> -	udp_update_l2_buf(eth_d, eth_s);
> +	udp_update_l2_buf(eth_d);
>  }
>  
>  /**
> diff --git a/udp.c b/udp.c
> index 75edc20..35e3603 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -133,11 +133,8 @@ static int udp_splice_init[IP_VERSIONS][NUM_PORTS];
>  /* UDP header and data for inbound messages */
>  static struct udp_payload_t udp_payload[UDP_MAX_FRAMES];
>  
> -/* Ethernet header for IPv4 frames */
> -static struct ethhdr udp4_eth_hdr;
> -
> -/* Ethernet header for IPv6 frames */
> -static struct ethhdr udp6_eth_hdr;
> +/* Ethernet headers for IPv4 and IPv6 frames */
> +static struct ethhdr udp_eth_hdr[UDP_MAX_FRAMES];
>  
>  /**
>   * struct udp_meta_t - Pre-cooked headers for UDP packets
> @@ -212,10 +209,12 @@ void udp_portmap_clear(void)
>   * @eth_d:	Ethernet destination address, NULL if unchanged
>   * @eth_s:	Ethernet source address, NULL if unchanged
>   */
> -void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
> +void udp_update_l2_buf(const unsigned char *eth_d)
>  {
> -	eth_update_mac(&udp4_eth_hdr, eth_d, eth_s);
> -	eth_update_mac(&udp6_eth_hdr, eth_d, eth_s);
> +	int i;
> +
> +	for (i = 0; i < UDP_MAX_FRAMES; i++)
> +		eth_update_mac(&udp_eth_hdr[i], eth_d, NULL);
>  }
>  
>  /**
> @@ -238,6 +237,7 @@ static void udp_iov_init_one(const struct ctx *c, size_t i)
>  
>  	*siov = IOV_OF_LVALUE(payload->data);
>  
> +	tiov[UDP_IOV_ETH] = IOV_OF_LVALUE(udp_eth_hdr[i]);
>  	tiov[UDP_IOV_TAP] = tap_hdr_iov(c, &meta->taph);
>  	tiov[UDP_IOV_PAYLOAD].iov_base = payload;
>  
> @@ -253,9 +253,6 @@ static void udp_iov_init(const struct ctx *c)
>  {
>  	size_t i;
>  
> -	udp4_eth_hdr.h_proto = htons_constant(ETH_P_IP);
> -	udp6_eth_hdr.h_proto = htons_constant(ETH_P_IPV6);
> -
>  	for (i = 0; i < UDP_MAX_FRAMES; i++)
>  		udp_iov_init_one(c, i);
>  }
> @@ -352,31 +349,34 @@ size_t udp_update_hdr6(struct ipv6hdr *ip6h, struct udp_payload_t *bp,
>   * udp_tap_prepare() - Convert one datagram into a tap frame
>   * @mmh:	Receiving mmsghdr array
>   * @idx:	Index of the datagram to prepare
> + * @tap_omac:	MAC address of remote endpoint as seen from the guest
>   * @toside:	Flowside for destination side
>   * @no_udp_csum: Do not set UDP checksum
>   */
>  static void udp_tap_prepare(const struct mmsghdr *mmh,
> -			    unsigned idx, const struct flowside *toside,
> +			    unsigned int idx,
> +			    const unsigned char *tap_omac,
> +			    const struct flowside *toside,
>  			    bool no_udp_csum)
>  {
>  	struct iovec (*tap_iov)[UDP_NUM_IOVS] = &udp_l2_iov[idx];
>  	struct udp_payload_t *bp = &udp_payload[idx];
>  	struct udp_meta_t *bm = &udp_meta[idx];
> +	struct ethhdr *eh = (*tap_iov)[UDP_IOV_ETH].iov_base;
>  	size_t l4len;
>  
> +	eth_update_mac(eh, 0, tap_omac);

s/0/NULL/ please.

>  	if (!inany_v4(&toside->eaddr) || !inany_v4(&toside->oaddr)) {
>  		l4len = udp_update_hdr6(&bm->ip6h, bp, toside,
>  					mmh[idx].msg_len, no_udp_csum);
> -		tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip6h) +
> -			       sizeof(udp6_eth_hdr));
> -		(*tap_iov)[UDP_IOV_ETH] = IOV_OF_LVALUE(udp6_eth_hdr);
> +		tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip6h) + ETH_HLEN);

s/ETH_HLEN/sizeof(*eh)/

> +		eh->h_proto = htons_constant(ETH_P_IPV6);
>  		(*tap_iov)[UDP_IOV_IP] = IOV_OF_LVALUE(bm->ip6h);
>  	} else {
>  		l4len = udp_update_hdr4(&bm->ip4h, bp, toside,
>  					mmh[idx].msg_len, no_udp_csum);
> -		tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip4h) +
> -			       sizeof(udp4_eth_hdr));
> -		(*tap_iov)[UDP_IOV_ETH] = IOV_OF_LVALUE(udp4_eth_hdr);
> +		tap_hdr_update(&bm->taph, l4len + sizeof(bm->ip4h) + ETH_HLEN);

Ditto.

> +		eh->h_proto = htons_constant(ETH_P_IP);
>  		(*tap_iov)[UDP_IOV_IP] = IOV_OF_LVALUE(bm->ip4h);
>  	}
>  	(*tap_iov)[UDP_IOV_PAYLOAD].iov_len = l4len;
> @@ -801,13 +801,14 @@ static void udp_buf_sock_to_tap(const struct ctx *c, int s, int n,
>  				flow_sidx_t tosidx)
>  {
>  	const struct flowside *toside = flowside_at_sidx(tosidx);
> +	const struct udp_flow *uflow = udp_at_sidx(tosidx);
>  	int i;
>  
>  	if ((n = udp_sock_recv(c, s, udp_mh_recv, n)) <= 0)
>  		return;
>  
>  	for (i = 0; i < n; i++)
> -		udp_tap_prepare(udp_mh_recv, i, toside, false);
> +		udp_tap_prepare(udp_mh_recv, i, uflow->f.tap_omac, toside, false);
>  
>  	tap_send_frames(c, &udp_l2_iov[0][0], UDP_NUM_IOVS, n);
>  }
> diff --git a/udp.h b/udp.h
> index 8f8531a..dd6e5ad 100644
> --- a/udp.h
> +++ b/udp.h
> @@ -21,7 +21,7 @@ int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr,
>  		  const char *ifname, in_port_t port);
>  int udp_init(struct ctx *c);
>  void udp_timer(struct ctx *c, const struct timespec *now);
> -void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
> +void udp_update_l2_buf(const unsigned char *eth_d);
>  
>  /**
>   * union udp_listen_epoll_ref - epoll reference for "listening" UDP sockets

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