Re: [PATCH v6 7/8] tap: change signature of function tap_push_l2h()

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

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

On Tue, Sep 09, 2025 at 09:59:18PM -0400, Jon Maloy wrote:
> In the following commits it must be possible for the callers of
> function tap_push_l2h() to specify which source MAC address should
> be added to the ethernet header sent over the tap interface. As
> a preparation, we now add a new argument to that function, still
> without actually using it.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

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

> 
> ---
> v3: -Improved comment for src_mac argument, as suggested by Stefano.
> v4: -Re-added comment in tap.c and remove redundant argument check
>      in tap_push_l2()
> v5: -Ensured that MAC argument to tap_push_l2h() never is NULL.
> v6: -Clarification of comment about ARP lookup
> ---
>  tap.c | 16 +++++++++-------
>  tap.h |  3 ++-
>  tcp.c |  5 +++--
>  3 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index 74557e1..a390be8 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -171,17 +171,19 @@ const struct in6_addr *tap_ip6_daddr(const struct ctx *c,
>   * tap_push_l2h() - Build an L2 header for an inbound packet
>   * @c:		Execution context
>   * @buf:	Buffer address at which to generate header
> + * @src_mac:	MAC address to be used as source for message.
>   * @proto:	Ethernet protocol number for L3
>   *
>   * Return: pointer at which to write the packet's payload
>   */
> -void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto)
> +void *tap_push_l2h(const struct ctx *c, void *buf,
> +		   const void *src_mac, uint16_t proto)
>  {
>  	struct ethhdr *eh = (struct ethhdr *)buf;
>  
> -	/* TODO: ARP table lookup */
> +	/* TODO: ARP lookup on tap side */
>  	memcpy(eh->h_dest, c->guest_mac, ETH_ALEN);
> -	memcpy(eh->h_source, c->our_tap_mac, ETH_ALEN);
> +	memcpy(eh->h_source, src_mac, ETH_ALEN);
>  	eh->h_proto = ntohs(proto);
>  	return eh + 1;
>  }
> @@ -261,7 +263,7 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport,
>  {
>  	size_t l4len = dlen + sizeof(struct udphdr);
>  	char buf[USHRT_MAX];
> -	struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP);
> +	struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP);
>  	struct udphdr *uh = tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_UDP);
>  	char *data = tap_push_uh4(uh, src, sport, dst, dport, in, dlen);
>  
> @@ -281,7 +283,7 @@ void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst,
>  		    const void *in, size_t l4len)
>  {
>  	char buf[USHRT_MAX];
> -	struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP);
> +	struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP);
>  	struct icmphdr *icmp4h = tap_push_ip4h(ip4h, src, dst,
>  					       l4len, IPPROTO_ICMP);
>  
> @@ -367,7 +369,7 @@ void tap_udp6_send(const struct ctx *c,
>  {
>  	size_t l4len = dlen + sizeof(struct udphdr);
>  	char buf[USHRT_MAX];
> -	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
> +	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6);
>  	struct udphdr *uh = tap_push_ip6h(ip6h, src, dst,
>  					  l4len, IPPROTO_UDP, flow);
>  	char *data = tap_push_uh6(uh, src, sport, dst, dport, in, dlen);
> @@ -389,7 +391,7 @@ void tap_icmp6_send(const struct ctx *c,
>  		    const void *in, size_t l4len)
>  {
>  	char buf[USHRT_MAX];
> -	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
> +	struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6);
>  	struct icmp6hdr *icmp6h = tap_push_ip6h(ip6h, src, dst, l4len,
>  						IPPROTO_ICMPV6, 0);
>  
> diff --git a/tap.h b/tap.h
> index 21db4d2..02f7761 100644
> --- a/tap.h
> +++ b/tap.h
> @@ -70,7 +70,8 @@ static inline void tap_hdr_update(struct tap_hdr *thdr, size_t l2len)
>  }
>  
>  unsigned long tap_l2_max_len(const struct ctx *c);
> -void *tap_push_l2h(const struct ctx *c, void *buf, uint16_t proto);
> +void *tap_push_l2h(const struct ctx *c, void *buf,
> +		   const void *src_mac, uint16_t proto);
>  void *tap_push_ip4h(struct iphdr *ip4h, struct in_addr src,
>  		     struct in_addr dst, size_t l4len, uint8_t proto);
>  void *tap_push_uh4(struct udphdr *uh, struct in_addr src, in_port_t sport,
> diff --git a/tcp.c b/tcp.c
> index 76fc0e4..b44fa62 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1922,7 +1922,8 @@ static void tcp_rst_no_conn(const struct ctx *c, int af,
>  		return;
>  
>  	if (af == AF_INET) {
> -		struct iphdr *ip4h = tap_push_l2h(c, buf, ETH_P_IP);
> +		struct iphdr *ip4h = tap_push_l2h(c, buf, c->our_tap_mac,
> +						  ETH_P_IP);
>  		const struct in_addr *rst_src = daddr;
>  		const struct in_addr *rst_dst = saddr;
>  
> @@ -1932,7 +1933,7 @@ static void tcp_rst_no_conn(const struct ctx *c, int af,
>  					      *rst_src, *rst_dst);
>  
>  	} else {
> -		struct ipv6hdr *ip6h = tap_push_l2h(c, buf, ETH_P_IPV6);
> +		struct ipv6hdr *ip6h = tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV6);
>  		const struct in6_addr *rst_src = daddr;
>  		const struct in6_addr *rst_dst = saddr;
>  
> -- 
> 2.50.1
> 

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