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=202502 header.b=eNjom6BP; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id E786D5A0272 for ; Thu, 03 Apr 2025 04:36:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743647773; bh=19ZmFA2Ok3QD+otEL+Iywd8wZumdckALtJq3PR8r/AE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=eNjom6BPNBqAd4evkdHTFJa82bGVzAKZHnymWkjqUm/XUJoBSCRvRr89CSQ/EnuH9 8scihuP3jJCAvSedmsKWkBNFb5CbJAH6QE8KD3bg3r1XAec4J7XSnwOdolj8f3gY3H hiiszBlPmAaG3UqPRQMDLTe4xpu0vYM3NR70Lf7v6oT+uou6P/eHYxninzJUzmgvPB lOxTVWgB3CYvyzYf/82q7uhjWQcfTmglg3aforGnb2bPdbfBk5uSjAJRDs9NOJKFS0 xAC9R0LRR+3kqIEjvf4IAEA81cxkEf7JHHT9L4bJIJnyrY7aaboZqklp80kMShoK/p +dn7NmBeBC06g== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZSm7x2VRLz4x04; Thu, 3 Apr 2025 13:36:13 +1100 (AEDT) Date: Thu, 3 Apr 2025 12:57:20 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 01/18] arp: Don't mix incoming and outgoing buffers Message-ID: References: <20250402172343.858187-1-lvivier@redhat.com> <20250402172343.858187-2-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0BPU6/13yG1Bh3mb" Content-Disposition: inline In-Reply-To: <20250402172343.858187-2-lvivier@redhat.com> Message-ID-Hash: JZK7IXVXUPPOZ6CZZID3QK2ZIRBHBXDI X-Message-ID-Hash: JZK7IXVXUPPOZ6CZZID3QK2ZIRBHBXDI 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: 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: --0BPU6/13yG1Bh3mb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 02, 2025 at 07:23:26PM +0200, Laurent Vivier wrote: > Don't use the memory of the incoming packet to build the outgoing buffer > as it can be memory of the TX queue in the case of vhost-user. >=20 > Moreover with vhost-user, the packet can be splitted accross several > iovec and it's easier to rebuild it in a buffer than updating an > existing iovec array. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > arp.c | 84 ++++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 55 insertions(+), 29 deletions(-) >=20 > diff --git a/arp.c b/arp.c > index fc482bbd9938..9d68d7c3b602 100644 > --- a/arp.c > +++ b/arp.c > @@ -31,56 +31,82 @@ > #include "tap.h" > =20 > /** > - * arp() - Check if this is a supported ARP message, reply as needed > + * ignore_arp() - Check if this is a supported ARP message > * @c: Execution context > - * @p: Packet pool, single packet with Ethernet buffer > + * @ah: ARP header > + * @am: ARP message > * > - * Return: 1 if handled, -1 on failure > + * Return: true if the message is supported, false otherwise. > */ > -int arp(const struct ctx *c, const struct pool *p) > +static bool ignore_arp(const struct ctx *c, > + const struct arphdr *ah, const struct arpmsg *am) > { > - unsigned char swap[4]; > - struct ethhdr *eh; > - struct arphdr *ah; > - struct arpmsg *am; > - size_t l2len; > - > - eh =3D packet_get(p, 0, 0, sizeof(*eh), NULL); > - ah =3D packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); > - am =3D packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); > - > - if (!eh || !ah || !am) > - return -1; > - > if (ah->ar_hrd !=3D htons(ARPHRD_ETHER) || > ah->ar_pro !=3D htons(ETH_P_IP) || > ah->ar_hln !=3D ETH_ALEN || > ah->ar_pln !=3D 4 || > ah->ar_op !=3D htons(ARPOP_REQUEST)) > - return 1; > + return true; > =20 > /* Discard announcements, but not 0.0.0.0 "probes" */ > if (memcmp(am->sip, &in4addr_any, sizeof(am->sip)) && > !memcmp(am->sip, am->tip, sizeof(am->sip))) > - return 1; > + return true; > =20 > /* Don't resolve the guest's assigned address, either. */ > if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) > + return true; > + > + return false; > +} > + > +/** > + * arp() - Check if this is a supported ARP message, reply as needed > + * @c: Execution context > + * @p: Packet pool, single packet with Ethernet buffer > + * > + * Return: 1 if handled, -1 on failure > + */ > +int arp(const struct ctx *c, const struct pool *p) > +{ > + struct { > + struct ethhdr eh; > + struct arphdr ah; > + struct arpmsg am; > + } __attribute__((__packed__)) resp; > + const struct ethhdr *eh; > + const struct arphdr *ah; > + const struct arpmsg *am; > + > + eh =3D packet_get(p, 0, 0, sizeof(*eh), NULL); > + ah =3D packet_get(p, 0, sizeof(*eh), sizeof(*ah), NULL); > + am =3D packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL); > + > + if (!eh || !ah || !am) > + return -1; > + > + if (ignore_arp(c, ah, am)) > return 1; > =20 > - ah->ar_op =3D htons(ARPOP_REPLY); > - memcpy(am->tha, am->sha, sizeof(am->tha)); > - memcpy(am->sha, c->our_tap_mac, sizeof(am->sha)); > + /* ethernet header */ > + resp.eh.h_proto =3D htons(ETH_P_ARP); > + memcpy(resp.eh.h_dest, eh->h_source, sizeof(resp.eh.h_dest)); > + memcpy(resp.eh.h_source, c->our_tap_mac, sizeof(resp.eh.h_source)); > =20 > - memcpy(swap, am->tip, sizeof(am->tip)); > - memcpy(am->tip, am->sip, sizeof(am->tip)); > - memcpy(am->sip, swap, sizeof(am->sip)); > + /* ARP header */ > + resp.ah.ar_op =3D htons(ARPOP_REPLY); > + resp.ah.ar_hrd =3D ah->ar_hrd; > + resp.ah.ar_pro =3D ah->ar_pro; > + resp.ah.ar_hln =3D ah->ar_hln; > + resp.ah.ar_pln =3D ah->ar_pln; > =20 > - l2len =3D sizeof(*eh) + sizeof(*ah) + sizeof(*am); > - memcpy(eh->h_dest, eh->h_source, sizeof(eh->h_dest)); > - memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source)); > + /* ARP message */ > + memcpy(resp.am.sha, c->our_tap_mac, sizeof(resp.am.sha)); > + memcpy(resp.am.sip, am->tip, sizeof(resp.am.sip)); > + memcpy(resp.am.tha, am->sha, sizeof(resp.am.tha)); > + memcpy(resp.am.tip, am->sip, sizeof(resp.am.tip)); > =20 > - tap_send_single(c, eh, l2len); > + tap_send_single(c, &resp, sizeof(resp)); > =20 > return 1; > } --=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 --0BPU6/13yG1Bh3mb Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmft6v8ACgkQzQJF27ox 2GdfiQ/+LZNUBGDDICxy1c97glDh364CLCr27a6Ro/YW6xGQ162T0kHMPHLjL8RL 42xGeM7yFQOodHdtIEM9zMg2aa/oRxUZSMKmIF8/SKXO8FdqCZfrM+dnAa9aboIY jQ5E3jM+8YkvBYps7O5w78in7SmDxAB34knEwJR1E1jAn/2Ds4ruWty+5NhpIGMT GHL5GLo/QXr7JdUf+T8gN9eSN2JOu+K5TkpOXcViMggBPnFgVyNAbZoQHruDg4F9 8QI1uAcGr+DmkPNi8VmIARe7zWJrfxVJpZzeBPAauzNZnxtotIhMplvndiWu1BGR /SsCs1WE20ZVxVUQaiuedHcwY6c+kH5FReVuRd7ehKiKOaNr5p5NtpGW7vAb0TrA te2FUV8lGGPCOGBqrsiMOKrxlIE2CSjLBVeSrCAWaLF0oBTn4PdZuJgf56FkVSeF tIg7U7KKX06CHfbn7jeEmvrrvf2KL15auV80i+OrjsA3a1U/JnXKdhvFF5hHOObp J8Di4TRPto+Y9Cs3gHNciawfOdY4jvYK9KY+l3yFv+K9VGmuzKl/9QPQD3PyCbg9 Xvsi+4ArnOMG9iu4RUBtZneQ5EJ1MhQQ2mDWyjbcrhwk+H50v1KMbAATtA3VHA8Z z/W/TcBPZrnQvL3Dwy9sYIKlqHiWFzZANPGlhm+Ve9RgY/06Pvo= =xK2w -----END PGP SIGNATURE----- --0BPU6/13yG1Bh3mb--