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=yNKUo1l8; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id DDF565A027A for ; Wed, 06 Aug 2025 04:23:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1754447034; bh=0t3KsswZTzXmtENe+Cqwrbkmno7FplUfethuOjg0868=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=yNKUo1l8uWF7wcV6KvCbU2/pMYXCinE0UAEKSzGz181YJsFfHgTDwZvB8Gryab2PJ YzqH9pUVYsDLjvZX835zNbOFe+lJPbaxYoYXAoc3dSmoktdebbt6r+V742TrPXHiZF DojtbrLiYW1GPMXNUHAcgdrpp5iwLK+MsKciHKUK4Xi4gdTCQU8aaa0ZDIFl3Jphf1 zqCNQ8swy/zRlo/fM3F5e/FNqRklpv2YPLlmgDqzofn29bwL0LSJHexfaHScwurcNE lMGz3PeK5XBXge7QATUvgE4vJek+SQa+vssxjjXOmqmV1vnlqoAM7viMVnvEfOmzp0 DHOW/4ytSHb3Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bxYy23fNyz4xcN; Wed, 6 Aug 2025 12:23:54 +1000 (AEST) Date: Wed, 6 Aug 2025 12:17:56 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v8 07/30] arp: Convert to iov_tail Message-ID: References: <20250805154628.301343-1-lvivier@redhat.com> <20250805154628.301343-8-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="pPrqlxud6iijxWQR" Content-Disposition: inline In-Reply-To: <20250805154628.301343-8-lvivier@redhat.com> Message-ID-Hash: YIX7HYNW77BNNLQQYYYLTQL232L4YHHS X-Message-ID-Hash: YIX7HYNW77BNNLQQYYYLTQL232L4YHHS 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: --pPrqlxud6iijxWQR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 05, 2025 at 05:46:05PM +0200, Laurent Vivier wrote: > Use packet_data() and extract headers using IOV_REMOVE_HEADER() > rather than packet_get(). >=20 > Signed-off-by: Laurent Vivier > Reviewed-by: David Gibson Still R-b, but making an observation below that's perhaps more relevant to the previous patch. > --- > arp.c | 12 +++++++++--- > packet.c | 1 - > 2 files changed, 9 insertions(+), 4 deletions(-) >=20 > diff --git a/arp.c b/arp.c > index 9f1fedeafec0..b3ac42082841 100644 > --- a/arp.c > +++ b/arp.c > @@ -74,14 +74,20 @@ int arp(const struct ctx *c, const struct pool *p) > struct arphdr ah; > struct arpmsg am; > } __attribute__((__packed__)) resp; > + struct arphdr ah_storage; > + struct ethhdr eh_storage; > + struct arpmsg am_storage; > const struct ethhdr *eh; > const struct arphdr *ah; > const struct arpmsg *am; > + struct iov_tail data; > =20 > - 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 (!packet_data(p, 0, &data)) > + return -1; The only case where packet_data() will return false is if you give it a bad packet index. That should never happen, by construction. So I'm wondering if that should be an ASSSERT() in packet_data() rather than a return value. > + eh =3D IOV_REMOVE_HEADER(&data, eh_storage); > + ah =3D IOV_REMOVE_HEADER(&data, ah_storage); > + am =3D IOV_REMOVE_HEADER(&data, am_storage); > if (!eh || !ah || !am) > return -1; > =20 > diff --git a/packet.c b/packet.c > index 82adc9fd1a39..34b1722b9a03 100644 > --- a/packet.c > +++ b/packet.c > @@ -201,7 +201,6 @@ void *packet_get_do(const struct pool *p, const size_= t idx, > * Return: false if packet index is invalid, true otherwise. > * If something wrong with @data, don't return at all (assert). > */ > -/* cppcheck-suppress unusedFunction */ > bool packet_data_do(const struct pool *p, size_t idx, > struct iov_tail *data, > const char *func, int line) --=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 --pPrqlxud6iijxWQR Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmiSu0wACgkQzQJF27ox 2GfLjg//fR35aYAd60aV3HwgMbilMDM23iosZaruS+TufkoUtFYgcfH4MT8S7qcZ eJbN5dKMxePHC99oXdvq06xF6flcyP7QJRVca3HTF1JXl/hDgG7TBIOofVozuVKD thskd2E5zQch+/+bScUq9dltnZQMj1LmsPmRQQortCzuHQOqLQF1YoyN7a1WBI2S aK5CDrkoFJ7LbhtWG05qUmttQe8Wwf87+O6tjv1EOkDe7KqMfS5pP6HcLl9NjJul zPKZo1Hz6JFjQGDC4UoGPYa2wUVbHmOhTSNs5ujJItbCuVmgVR6Cs6sAJot/3U04 sCB7+nt4EcVOitPPqmQxblEGrf0umyoSJzc58TPgSncFXDGr1n2DqWPAVAZjfA/0 Ft1dFupMVAcE86FH1KcxVgrix29ebSwejKiWJqjEvpBm2OMr8FKpWjoPNs+4fJaQ 26NtLk3glF4qw2+ea63L6rl0eqYeqmRQiykiCaekgAN6DOOvug0uIdgk1LO8z7MF QC8HCn+Gj5EZCko8LYuC0///CJv46FSHfTReUPorp2ZPLVv9rDZr9gRUm2udpGYq xD1gktzPVXFxT/vkoaq8VPFLdIFCCZZ8ITMidWr+C8HU8c+iG1IaPOJESga1X7/m bm3bHHAgnO1l8HT4UdQuAbcDlqESl5nnb34r/y7o6eBt46jqSZU= =633U -----END PGP SIGNATURE----- --pPrqlxud6iijxWQR--