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=bl0bDIp4; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 4ED855A027A for ; Wed, 06 Aug 2025 08:35:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1754462131; bh=DvOqrvtdFoIcs890C5SxHjVlxvfuzZjisTifHfmgriM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bl0bDIp4ibI50uo4QIK81PtvfCJHDg+DXgxPDfDSiZjBTuj9VyRAjD8RHBo9iK+Hq i6jCiFMd1W2E9iToIAOLW5sILDuicq6ZfOJHPa0F2+hrOyGTY08ysLp8PPFc1UY/fm fc3mnsScgrO0DOWy+TVsM30FCU46ustMomaU/se9unhWxVdsUinaxw7ZfeHGxxzMDJ Y1d6cwG/x4DYO3BJTNpqpJRfANF+wNNYcasmu9Powl2ZQJPUlu6P+Aq42lfjQtO6/b G0hJCbIJNeHKWjz4pNRlZa+dy5JYUiIPm3t7wWV1fQ04rz0ZvT1jRepv9BneOG63Ir FE1uWK70GZ6Zg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bxgXM0CLrz4xPf; Wed, 6 Aug 2025 16:35:31 +1000 (AEST) Date: Wed, 6 Aug 2025 15:12:03 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v8 18/30] ip: Use iov_tail in ipv6_l4hdr() Message-ID: References: <20250805154628.301343-1-lvivier@redhat.com> <20250805154628.301343-19-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="rdTr/juNaGDfwRZD" Content-Disposition: inline In-Reply-To: <20250805154628.301343-19-lvivier@redhat.com> Message-ID-Hash: 3M6LDMFAFU7IUU4AXEGDGXPDKVFE57HR X-Message-ID-Hash: 3M6LDMFAFU7IUU4AXEGDGXPDKVFE57HR 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: --rdTr/juNaGDfwRZD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 05, 2025 at 05:46:16PM +0200, Laurent Vivier wrote: > Use packet_data() and extract headers using IOV_REMOVE_HEADER() > and IOV_PEEK_HEADER() rather than packet_get(). >=20 > Signed-off-by: Laurent Vivier > --- > ip.c | 32 +++++++++++++++----------------- > ip.h | 3 +-- > packet.c | 1 + > tap.c | 4 +++- > 4 files changed, 20 insertions(+), 20 deletions(-) >=20 > diff --git a/ip.c b/ip.c > index 2cc7f6548aff..50bd69a70596 100644 > --- a/ip.c > +++ b/ip.c > @@ -23,50 +23,48 @@ > =20 > /** > * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract p= rotocol > - * @p: Packet pool, packet number @idx has IPv6 header at @offset > - * @idx: Index of packet in pool > - * @offset: Pre-calculated IPv6 header offset > + * @data: IPv6 packet > * @proto: Filled with L4 protocol number > * @dlen: Data length (payload excluding header extensions), set on retu= rn > * > - * Return: pointer to L4 header, NULL if not found > + * Return: true if the L4 header is found and @data, @proto, @dlen are s= et, > + * false on error. Outputs are indeterminate on failure. > */ > -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *= proto, > - size_t *dlen) > +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) > { > + struct ipv6_opt_hdr o_storage; > const struct ipv6_opt_hdr *o; > + struct ipv6hdr ip6h_storage; > const struct ipv6hdr *ip6h; > - char *base; > int hdrlen; > uint8_t nh; > =20 > - base =3D packet_get(p, idx, 0, 0, NULL); > - ip6h =3D packet_get(p, idx, offset, sizeof(*ip6h), dlen); > + ip6h =3D IOV_REMOVE_HEADER(data, ip6h_storage); > if (!ip6h) > - return NULL; > - > - offset +=3D sizeof(*ip6h); > + return false; > + *dlen =3D iov_tail_size(data); > =20 > nh =3D ip6h->nexthdr; > if (!IPV6_NH_OPT(nh)) > goto found; > =20 > - while ((o =3D packet_get_try(p, idx, offset, sizeof(*o), dlen))) { > + while ((o =3D IOV_PEEK_HEADER(data, o_storage))) { > + *dlen =3D iov_tail_size(data) - sizeof(*o); I don't think this is quite right. This removes the option header =66rom the total, but not the option body (if any). It will be corrected on the next loop, via iova_tail_size() - at the cost of rescanning the IOV. However, I think that means the body of the last option will be incorrectly included in dlen. You could update *dlen incrementally, but AFAICT you don't need it internally, so you could just compute from iov_tail_size() after the found label. > nh =3D o->nexthdr; > hdrlen =3D (o->hdrlen + 1) * 8; > =20 > if (IPV6_NH_OPT(nh)) > - offset +=3D hdrlen; > + iov_tail_drop(data, hdrlen); > else > goto found; > } > =20 > - return NULL; > + return false; > =20 > found: > if (nh =3D=3D 59) Pre-existing: it'd be nice to have a name for that bare 59. > - return NULL; > + return false; > =20 > *proto =3D nh; > - return base + offset; > + return true; > } > diff --git a/ip.h b/ip.h > index 24509d9c11cd..5830b92302e2 100644 > --- a/ip.h > +++ b/ip.h > @@ -115,8 +115,7 @@ static inline uint32_t ip6_get_flow_lbl(const struct = ipv6hdr *ip6h) > ip6h->flow_lbl[2]; > } > =20 > -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *= proto, > - size_t *dlen); > +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen); > =20 > /* IPv6 link-local all-nodes multicast address, ff02::1 */ > static const struct in6_addr in6addr_ll_all_nodes =3D { > diff --git a/packet.c b/packet.c > index 34b1722b9a03..014b353cdf8b 100644 > --- a/packet.c > +++ b/packet.c > @@ -133,6 +133,7 @@ void packet_add_do(struct pool *p, struct iov_tail *d= ata, > * > * Return: pointer to start of data range, NULL on invalid range or desc= riptor > */ > +/* cppcheck-suppress [staticFunction] */ > void *packet_get_try_do(const struct pool *p, size_t idx, size_t offset, > size_t len, size_t *left, const char *func, int line) > { > diff --git a/tap.c b/tap.c > index 8d2b118152f1..d7852fad6069 100644 > --- a/tap.c > +++ b/tap.c > @@ -911,8 +911,10 @@ resume: > if (plen !=3D check) > continue; > =20 > - if (!(l4h =3D ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4len))) > + data =3D IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0); > + if (!ipv6_l4hdr(&data, &proto, &l4len)) > continue; > + l4h =3D (char *)data.iov[0].iov_base + data.off; > =20 > if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) { > char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN]; --=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 --rdTr/juNaGDfwRZD Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmiS5BUACgkQzQJF27ox 2GcaNg/+L6iIq5TCqQXw94ewOPXlMhPswZ1dwhfscUdiGhMKJODdnrP8UAJ21tz/ 2qcTBDHTTc71We8ABv9fJXCoTnOMkWJxALQAFVeHfatg6pf40i7/95TmYCLG3RD/ Z3RATdHtiiZN16Zkms1PKGOpugBbkLnrpPyLkJmnZTT4rtNOQLNWINmfQCvJYm1I 3YqUWhodibWJHI+YW524pc+kk944Cr6gLnSNe/hLOnuECK6CYkyo/6+y9wdz3kZd 96e6w5+gdtYPZCbfKC/5lifrVu2vFmF5p5CLfqt0kYiA4QuazkdLPcdL3IUiKxBv PTQ40nQsPkJTVrWEUxgtuNLUv5fvkGlngoPxiHKCmA1FAbZ4WF2I/P4cy+DmKEOe YYShJMIasDCd/uOuTOzxhGPLUnG3CTI9kMPhDb4QiFLIineCNf5E6sxA2lfqIF7a PNQzo394pWZ24PkLOMyLJK1VFVFMlkajPBqCMb6qtnKbQk6FEIwsdtOdC+8RsHXM 3C3CJmwQaI5ShywyhSd/ibMivZ0DDb1HWo0YJlR7eMjZxB4XPf52MWK4uoDFMXIr Od6AEsaek9VNh8Gvhjsrk8RczBy1jSYrnL7cKwjXmdrRyz/sYWxsA1Ha20jNyvTD IDHJiT5CYyc0mycxqmBO9XTQzhfbyP2i/SjraS5b+4kpMuhjp8E= =m4bS -----END PGP SIGNATURE----- --rdTr/juNaGDfwRZD--