Re: [PATCH v4 6/9] tap: change signature of function tap_push_l2h()

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

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

On Tue, Aug 19, 2025 at 11:10:02PM -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>
> ---
>  tap.c | 15 +++++++++------
>  tap.h |  3 ++-
>  tcp.c |  4 ++--
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index a5de8b7..0349835 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -171,17 +171,20 @@ 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 */
> +
>  	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);

This assumes that src_mac is non-NULL...

>  	eh->h_proto = ntohs(proto);
>  	return eh + 1;
>  }
> @@ -261,7 +264,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, NULL, ETH_P_IP);

... but then you pass NULL.

>  	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 +284,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, NULL, ETH_P_IP);
>  	struct icmphdr *icmp4h = tap_push_ip4h(ip4h, src, dst,
>  					       l4len, IPPROTO_ICMP);
>  
> @@ -367,7 +370,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, NULL, 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 +392,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, NULL, 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 936ae93..6ec171d 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 1dd2e24..bdcd477 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1898,7 +1898,7 @@ 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, NULL, ETH_P_IP);
>  		const struct in_addr *rst_src = daddr;
>  		const struct in_addr *rst_dst = saddr;
>  
> @@ -1908,7 +1908,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, NULL, ETH_P_IPV6);
>  		const struct in6_addr *rst_src = daddr;
>  		const struct in6_addr *rst_dst = saddr;
>  

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