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

[Stefano Brivio <sbrivio@redhat.com>]

On Sat, 27 Sep 2025 15:25:20 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> We forward the incoming mac address through the tap interface when
> receiving incoming packets from network local hosts.
> 
> 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>
> 
> ---
> v3: - Adapted to the signature change in nl_mac_get() function, so that
>       we now consider only the template interface when checking the
>       ARP/NDP table.
> v4: - Adapted to previous name changes in this series
> v5: - Added lookup in ARP/NDP cache and/or table on incoming messages
>       in  case flow->tap_omac wasn't initialized at flow creation,
>       i.e., the flow was initiated from the guest.
> v6: - Using MAC_IS_ZERO() instead of mac_undefined()
>     - I ignored David's suggestion to move the test for
>       undefined flow->tap_omac to tcp_{buf,vu}_data_from_sock(), at
>       least for now. It looks like a sub-optimization to first have to
>       look up the flow instance in these calls, and then do the
>       test for MAC_IS_ZERO(), even though we might in theory end up
>       with fewer such calls (how often?).
> ---
>  passt.c        |  7 +++----
>  passt.h        |  3 +--
>  pasta.c        |  2 +-
>  tap.c          |  2 +-
>  tcp.c          | 13 +++++++++++--
>  tcp.h          |  2 +-
>  tcp_buf.c      | 37 +++++++++++++++++--------------------
>  tcp_internal.h |  4 ++--
>  tcp_vu.c       |  5 ++---
>  9 files changed, 39 insertions(+), 36 deletions(-)
> 
> diff --git a/passt.c b/passt.c
> index fdd275a..a2ce2d9 100644
> --- a/passt.c
> +++ b/passt.c
> @@ -159,11 +159,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);
>  }
>  
> @@ -314,7 +313,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 0075eb4..ff0236c 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -326,7 +326,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 687406b..a42cfd8 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -411,7 +411,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 399eeaa..250a0f6 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1101,7 +1101,7 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data,
>  		memcpy(c->guest_mac, eh->h_source, ETH_ALEN);
>  		debug("New guest MAC address observed: %s",
>  		      eth_ntop(c->guest_mac, bufmac, sizeof(bufmac)));
> -		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 48b1ef2..af05d35 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -919,6 +919,7 @@ static void tcp_fill_header(struct tcphdr *th,
>  
>  /**
>   * tcp_fill_headers() - Fill 802.3, IP, TCP headers
> + * @c:			Execution context
>   * @conn:		Connection pointer
>   * @taph:		tap backend specific header
>   * @ip4h:		Pointer to IPv4 header, or NULL
> @@ -929,14 +930,15 @@ static void tcp_fill_header(struct tcphdr *th,
>   * @seq:		Sequence number for this segment
>   * @no_tcp_csum:	Do not set TCP checksum
>   */
> -void tcp_fill_headers(const struct tcp_tap_conn *conn,
> -		      struct tap_hdr *taph,
> +void tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn,
> +		      struct tap_hdr *taph, struct ethhdr *eh,

You now pass a 802.3 header here, but the function documentation isn't
updated accordingly.

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

-- 
Stefano