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

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

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

On Tue, Aug 19, 2025 at 11:10:01PM -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>

> ---
>  passt.c        |  7 +++----
>  passt.h        |  3 +--
>  pasta.c        |  2 +-
>  tap.c          |  2 +-
>  tcp.c          |  5 ++++-
>  tcp.h          |  2 +-
>  tcp_buf.c      | 29 ++++++++++++-----------------
>  tcp_internal.h |  2 +-
>  tcp_vu.c       |  5 ++---
>  udp.c          |  1 -
>  10 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/passt.c b/passt.c
> index 477a01f..746c9ff 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -149,11 +149,10 @@ static void timer_init(struct ctx *c, const struct timespec *now)
>  /**
>   * proto_update_l2_buf() - Update scatter-gather L2 buffers in protocol handlers
>   * @eth_d:	Ethernet destination address, NULL if unchanged
> - * @eth_s:	Ethernet source address, NULL if unchanged
>   */
> -void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
> +void proto_update_l2_buf(const unsigned char *eth_d)
>  {
> -	tcp_update_l2_buf(eth_d, eth_s);
> +	tcp_update_l2_buf(eth_d);
>  	udp_update_l2_buf(eth_d);
>  }
>  
> @@ -255,7 +254,7 @@ int main(int argc, char **argv)
>  	if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c)))
>  		_exit(EXIT_FAILURE);
>  
> -	proto_update_l2_buf(c.guest_mac, c.our_tap_mac);
> +	proto_update_l2_buf(c.guest_mac);
>  
>  	if (c.ifi4 && !c.no_dhcp)
>  		dhcp_init();
> diff --git a/passt.h b/passt.h
> index 4cfd6eb..2c5b3e1 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -324,7 +324,6 @@ struct ctx {
>  	bool migrate_exit;
>  };
>  
> -void proto_update_l2_buf(const unsigned char *eth_d,
> -			 const unsigned char *eth_s);
> +void proto_update_l2_buf(const unsigned char *eth_d);
>  
>  #endif /* PASST_H */
> diff --git a/pasta.c b/pasta.c
> index c207692..f251331 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -409,7 +409,7 @@ void pasta_ns_conf(struct ctx *c)
>  		}
>  	}
>  
> -	proto_update_l2_buf(c->guest_mac, NULL);
> +	proto_update_l2_buf(c->guest_mac);
>  }
>  
>  /**
> diff --git a/tap.c b/tap.c
> index 6db5d88..a5de8b7 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1085,7 +1085,7 @@ void tap_add_packet(struct ctx *c, ssize_t l2len, char *p,
>  
>  	if (memcmp(c->guest_mac, eh->h_source, ETH_ALEN)) {
>  		memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
> -		proto_update_l2_buf(c->guest_mac, NULL);
> +		proto_update_l2_buf(c->guest_mac);
>  	}
>  
>  	switch (ntohs(eh->h_proto)) {
> diff --git a/tcp.c b/tcp.c
> index 957b498..1dd2e24 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -920,7 +920,7 @@ static void tcp_fill_header(struct tcphdr *th,
>   * @no_tcp_csum:	Do not set TCP checksum
>   */
>  void tcp_fill_headers(const struct tcp_tap_conn *conn,
> -		      struct tap_hdr *taph,
> +		      struct tap_hdr *taph, struct ethhdr *eh,
>  		      struct iphdr *ip4h, struct ipv6hdr *ip6h,
>  		      struct tcphdr *th, struct iov_tail *payload,
>  		      const uint16_t *ip4_check, uint32_t seq, bool no_tcp_csum)
> @@ -952,6 +952,7 @@ void tcp_fill_headers(const struct tcp_tap_conn *conn,
>  			psum = proto_ipv4_header_psum(l4len, IPPROTO_TCP,
>  						      *src4, *dst4);
>  		}
> +		eh->h_proto = htons_constant(ETH_P_IP);
>  	}
>  
>  	if (ip6h) {
> @@ -972,7 +973,9 @@ void tcp_fill_headers(const struct tcp_tap_conn *conn,
>  						      &ip6h->saddr,
>  						      &ip6h->daddr);
>  		}
> +		eh->h_proto = htons_constant(ETH_P_IPV6);
>  	}
> +	eth_update_mac(eh, NULL, conn->f.tap_omac);
>  
>  	tcp_fill_header(th, conn, seq);
>  
> diff --git a/tcp.h b/tcp.h
> index 234a803..c1b8385 100644
> --- a/tcp.h
> +++ b/tcp.h
> @@ -24,7 +24,7 @@ int tcp_init(struct ctx *c);
>  void tcp_timer(struct ctx *c, const struct timespec *now);
>  void tcp_defer_handler(struct ctx *c);
>  
> -void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s);
> +void tcp_update_l2_buf(const unsigned char *eth_d);
>  
>  extern bool peek_offset_cap;
>  
> diff --git a/tcp_buf.c b/tcp_buf.c
> index d1fca67..dac5788 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -40,8 +40,7 @@
>  /* Static buffers */
>  
>  /* Ethernet header for IPv4 and IPv6 frames */
> -static struct ethhdr		tcp4_eth_src;
> -static struct ethhdr		tcp6_eth_src;
> +static struct ethhdr		tcp_eth_hdr[TCP_FRAMES_MEM];
>  
>  static struct tap_hdr		tcp_payload_tap_hdr[TCP_FRAMES_MEM];
>  
> @@ -67,12 +66,13 @@ static struct iovec	tcp_l2_iov[TCP_FRAMES_MEM][TCP_NUM_IOVS];
>  /**
>   * tcp_update_l2_buf() - Update Ethernet header buffers with addresses
>   * @eth_d:	Ethernet destination address, NULL if unchanged
> - * @eth_s:	Ethernet source address, NULL if unchanged
>   */
> -void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s)
> +void tcp_update_l2_buf(const unsigned char *eth_d)
>  {
> -	eth_update_mac(&tcp4_eth_src, eth_d, eth_s);
> -	eth_update_mac(&tcp6_eth_src, eth_d, eth_s);
> +	int i;
> +
> +	for (i = 0; i < TCP_FRAMES_MEM; i++)
> +		eth_update_mac(&tcp_eth_hdr[i], eth_d, NULL);
>  }
>  
>  /**
> @@ -85,9 +85,6 @@ void tcp_sock_iov_init(const struct ctx *c)
>  	struct iphdr iph = L2_BUF_IP4_INIT(IPPROTO_TCP);
>  	int i;
>  
> -	tcp6_eth_src.h_proto = htons_constant(ETH_P_IPV6);
> -	tcp4_eth_src.h_proto = htons_constant(ETH_P_IP);
> -
>  	for (i = 0; i < ARRAY_SIZE(tcp_payload); i++) {
>  		tcp6_payload_ip[i] = ip6;
>  		tcp4_payload_ip[i] = iph;
> @@ -164,6 +161,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
>  	struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
>  	const struct flowside *tapside = TAPFLOW(conn);
>  	const struct in_addr *a4 = inany_v4(&tapside->oaddr);
> +	struct ethhdr *eh = iov[TCP_IOV_ETH].iov_base;
>  	struct ipv6hdr *ip6h = NULL;
>  	struct iphdr *ip4h = NULL;
>  
> @@ -172,7 +170,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
>  	else
>  		ip6h = iov[TCP_IOV_IP].iov_base;
>  
> -	tcp_fill_headers(conn, taph, ip4h, ip6h, th, &tail,
> +	tcp_fill_headers(conn, taph, eh, ip4h, ip6h, th, &tail,
>  			 check, seq, no_tcp_csum);
>  }
>  
> @@ -194,14 +192,12 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  	int ret;
>  
>  	iov = tcp_l2_iov[tcp_payload_used];
> -	if (CONN_V4(conn)) {
> +	if (CONN_V4(conn))
>  		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[tcp_payload_used]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src;
> -	} else {
> +	else
>  		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src;
> -	}
>  
> +	iov[TCP_IOV_ETH] = IOV_OF_LVALUE(tcp_eth_hdr[tcp_payload_used]);
>  	payload = iov[TCP_IOV_PAYLOAD].iov_base;
>  	seq = conn->seq_to_tap;
>  	ret = tcp_prepare_flags(c, conn, flags, &payload->th,
> @@ -259,11 +255,10 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  			check = &iph->check;
>  		}
>  		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp4_payload_ip[tcp_payload_used]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp4_eth_src;
>  	} else if (CONN_V6(conn)) {
>  		iov[TCP_IOV_IP] = IOV_OF_LVALUE(tcp6_payload_ip[tcp_payload_used]);
> -		iov[TCP_IOV_ETH].iov_base = &tcp6_eth_src;
>  	}
> +	iov[TCP_IOV_ETH].iov_base = &tcp_eth_hdr[tcp_payload_used];
>  	payload = iov[TCP_IOV_PAYLOAD].iov_base;
>  	payload->th.th_off = sizeof(struct tcphdr) / 4;
>  	payload->th.th_x2 = 0;
> diff --git a/tcp_internal.h b/tcp_internal.h
> index 36c6533..6c2d1ef 100644
> --- a/tcp_internal.h
> +++ b/tcp_internal.h
> @@ -167,7 +167,7 @@ void tcp_rst_do(const struct ctx *c, struct tcp_tap_conn *conn);
>  struct tcp_info_linux;
>  
>  void tcp_fill_headers(const struct tcp_tap_conn *conn,
> -		      struct tap_hdr *taph,
> +		      struct tap_hdr *taph, struct ethhdr *eh,
>  		      struct iphdr *ip4h, struct ipv6hdr *ip6h,
>  		      struct tcphdr *th, struct iov_tail *payload,
>  		      const uint16_t *ip4_check, uint32_t seq, bool no_tcp_csum);
> diff --git a/tcp_vu.c b/tcp_vu.c
> index cb39bc2..a895d6c 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -135,7 +135,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  	flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
>  	payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>  
> -	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
> +	tcp_fill_headers(conn, NULL, eh, ip4h, ip6h, th, &payload,
>  			 NULL, seq, !*c->pcap);
>  
>  	if (*c->pcap) {
> @@ -315,7 +315,6 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>  	eh = vu_eth(base);
>  
>  	memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest));
> -	memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
>  
>  	/* initialize header */
>  
> @@ -339,7 +338,7 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>  	th->ack = 1;
>  	th->psh = push;
>  
> -	tcp_fill_headers(conn, NULL, ip4h, ip6h, th, &payload,
> +	tcp_fill_headers(conn, NULL, eh, ip4h, ip6h, th, &payload,
>  			 *check, conn->seq_to_tap, no_tcp_csum);
>  	if (ip4h)
>  		*check = &ip4h->check;
> diff --git a/udp.c b/udp.c
> index 35e3603..30937dd 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -207,7 +207,6 @@ void udp_portmap_clear(void)
>  /**
>   * udp_update_l2_buf() - Update L2 buffers with Ethernet and IPv4 addresses
>   * @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)
>  {

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