From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202508 header.b=AcxWKu96; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 9FB075A0271 for ; Mon, 08 Sep 2025 05:21:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1757301693; bh=uqJ2vRhx/C7Wotebrqp74sMnvnq39PtRJXxmX81ZJo8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AcxWKu96HFrNMrIGKzWzSx+8l76j0JxsV7nw+pIrsJLbDxczAW0KAfJx2tcWcQPsw T2iMrOUMezpBA/ItA3x2oN/9LwopmyRXVqwqwiWAPMMXAYR1gfmd9PUkC36gdzTFmn UF1fmbjDkbPbC2CXD91qu1zWl2aSNMknyhCHpIrpdxHnt6TkYUDeQI/oq4MYtdEKak 2h5CkU2iR/LyiFbCs+DtFa5me7ZjcON6pS8GFOwwGD+B6L30cOWlY26mNKgNSjhCx0 VUUnAszCwOo2DixDdtmpbhVLcc4EajtrTMSOvyAvJHS7O8fWWjIs+/+8XS6vPAvwDF /vs6dq8Nif6Zg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cKsgK4dXZz4w2S; Mon, 8 Sep 2025 13:21:33 +1000 (AEST) Date: Mon, 8 Sep 2025 13:21:28 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v5 08/10] tap: change signature of function tap_push_l2h() Message-ID: References: <20250906021154.2760611-1-jmaloy@redhat.com> <20250906021154.2760611-9-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="0VIMy5D3bfY/S7pt" Content-Disposition: inline In-Reply-To: <20250906021154.2760611-9-jmaloy@redhat.com> Message-ID-Hash: KHI7YFU3FNE5RCRTHQMO5VWCVJRHZS6T X-Message-ID-Hash: KHI7YFU3FNE5RCRTHQMO5VWCVJRHZS6T X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: sbrivio@redhat.com, dgibson@redhat.com, passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --0VIMy5D3bfY/S7pt Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 05, 2025 at 10:11:52PM -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. >=20 > Signed-off-by: Jon Maloy LGTM. > --- > 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. > --- > tap.c | 15 +++++++++------ > tap.h | 3 ++- > tcp.c | 5 +++-- > 3 files changed, 14 insertions(+), 9 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index 74557e1..05429aa 100644 > --- a/tap.c > +++ b/tap.c > @@ -171,17 +171,20 @@ const struct in6_addr *tap_ip6_daddr(const struct c= tx *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 =3D (struct ethhdr *)buf; > =20 > /* TODO: ARP table lookup */ Kind of pre-existing, but the new context of this series makes this comment a bit misleading. We now *have* an ARP table of sorts, but it wouldn't help here - our new ARP table is host side, but this is guest side. What this comment is talking about is not so much an ARP _table_ lookup as an actual ARP lookup on the tap interface (which would involve a caching table, but also sending actual packets). > + Spurious whitespace change. > 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 =3D ntohs(proto); > return eh + 1; > } > @@ -261,7 +264,7 @@ void tap_udp4_send(const struct ctx *c, struct in_add= r src, in_port_t sport, > { > size_t l4len =3D dlen + sizeof(struct udphdr); > char buf[USHRT_MAX]; > - struct iphdr *ip4h =3D tap_push_l2h(c, buf, ETH_P_IP); > + struct iphdr *ip4h =3D tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP); > struct udphdr *uh =3D tap_push_ip4h(ip4h, src, dst, l4len, IPPROTO_UDP); > char *data =3D tap_push_uh4(uh, src, sport, dst, dport, in, dlen); > =20 > @@ -281,7 +284,7 @@ void tap_icmp4_send(const struct ctx *c, struct in_ad= dr src, struct in_addr dst, > const void *in, size_t l4len) > { > char buf[USHRT_MAX]; > - struct iphdr *ip4h =3D tap_push_l2h(c, buf, ETH_P_IP); > + struct iphdr *ip4h =3D tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP); > struct icmphdr *icmp4h =3D tap_push_ip4h(ip4h, src, dst, > l4len, IPPROTO_ICMP); > =20 > @@ -367,7 +370,7 @@ void tap_udp6_send(const struct ctx *c, > { > size_t l4len =3D dlen + sizeof(struct udphdr); > char buf[USHRT_MAX]; > - struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, ETH_P_IPV6); > + struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV= 6); > struct udphdr *uh =3D tap_push_ip6h(ip6h, src, dst, > l4len, IPPROTO_UDP, flow); > char *data =3D 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 =3D tap_push_l2h(c, buf, ETH_P_IPV6); > + struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IPV= 6); > struct icmp6hdr *icmp6h =3D tap_push_ip6h(ip6h, src, dst, l4len, > IPPROTO_ICMPV6, 0); > =20 > 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) > } > =20 > 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 spor= t, > diff --git a/tcp.c b/tcp.c > index 23b45b8..383654c 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1922,7 +1922,8 @@ static void tcp_rst_no_conn(const struct ctx *c, in= t af, > return; > =20 > if (af =3D=3D AF_INET) { > - struct iphdr *ip4h =3D tap_push_l2h(c, buf, ETH_P_IP); > + struct iphdr *ip4h =3D tap_push_l2h(c, buf, c->our_tap_mac, > + ETH_P_IP); > const struct in_addr *rst_src =3D daddr; > const struct in_addr *rst_dst =3D saddr; > =20 > @@ -1932,7 +1933,7 @@ static void tcp_rst_no_conn(const struct ctx *c, in= t af, > *rst_src, *rst_dst); > =20 > } else { > - struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, ETH_P_IPV6); > + struct ipv6hdr *ip6h =3D tap_push_l2h(c, buf, c->our_tap_mac, ETH_P_IP= V6); > const struct in6_addr *rst_src =3D daddr; > const struct in6_addr *rst_dst =3D saddr; > =20 > --=20 > 2.50.1 >=20 --=20 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 --0VIMy5D3bfY/S7pt Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmi+S7cACgkQzQJF27ox 2GfgRBAAm687zhWpxcg6h3H3xLtNnG4GC3Ct7GDfIdFPntf7vHWvph61eRE17oP3 2HY8opx/jYEvB5+2hzQOTciaOuCkq3hI+GuHj/KRag5H3m9KbW5oAbELun39Th9z mmyJzgIPEE7ZXoT8sGeagXfKR7OBNOf5kTd/o31PUqf+7f4r0EoN5Koo41j1rTkN kO7SBvghrSlPnKse+2l3mNeG3Am+k4H3Aivog020K0lMfUJwP8tVCZlWjtxH+ARS oKiymVTSQk7/lPbZ49IR5JHfK2HC7PaevzM6ZCfOExpvjsCtjEsaQN+s7JKXY2Ac ZY35a/LxlEOi1GhtWaebaTQy0ywT1jOpqkcZ+guBFma2cwTiWVmuB4uR3qKCrr3e IT8zcXcmrHPk/I+AybofrswyA4MHqPmMBO1Q9BuO+uE5byh/G7nj1N9JwCV0EGQj HfuVUM7HqRZxLRfFEJxyehgzWgpdI6js98nj8fNKj28btWKAhe7rU9tatWBErXVY CZyqUZShQqxbt+Np3Tw347Pz5k1swU/4iPJ6+OZue9h3HVHRN4IYIM4rXebkLieB 9fJkMPCNRrD4KNp+fOSI4GHHdU0UYVsdu295JPMDpfmoF+Q3Gy/utdQm4NDO44jS NU7vF6EbBhPVaF3ATKLnYhQhYGDaFVO74ecyTN3BoKP0L76jWms= =Yi/r -----END PGP SIGNATURE----- --0VIMy5D3bfY/S7pt--