From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 96B0D5A0272 for ; Tue, 6 Feb 2024 03:25:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1707186324; bh=BQy1215uwdHoRXbyeRrmG8sw0R5Rarg5HARMTeIsdwo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=TcfB+ji8KomIWomFTBDCQ7a2jDWs7RGR4GreOv1UltEN1JgpOoYOM+UtpY5OGwIAQ HK4eBjFxc4gdZKiBaiT4CyZzaV21YAiNz0ctKOss17qrjEe6xLeUSejF5Dw6uXSglM 5a5OtWie+JtaT6TjvxO2dUJg8DMJVMqI4q9qw/3BGbEq1dKEOnj/VUWMFmhdYQfrLs N2pPC4LaIIoJyb9r8j6fNHLLnVaqe0+rcyQMOZcDeM6zknyYhzVhToJiMu0VA0DU/b gzxEJg3AowH5Bs4bavxmtTrbG3T677Ax+xTz/yXLW83Sep9gpcSKUhE8/9GIyaoMdV AAnjhvPced3ZQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TTRtD100pz4wct; Tue, 6 Feb 2024 13:25:24 +1100 (AEDT) Date: Tue, 6 Feb 2024 13:25:18 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 16/24] packet: replace struct desc by struct iovec Message-ID: References: <20240202141151.3762941-1-lvivier@redhat.com> <20240202141151.3762941-17-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0LMWR5NQdWloEItX" Content-Disposition: inline In-Reply-To: <20240202141151.3762941-17-lvivier@redhat.com> Message-ID-Hash: E3NDHL2CBX7R7D4Z436YIUHKJHJJ6P3Z X-Message-ID-Hash: E3NDHL2CBX7R7D4Z436YIUHKJHJJ6P3Z 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: --0LMWR5NQdWloEItX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 02, 2024 at 03:11:43PM +0100, Laurent Vivier wrote: Rationale please. It's probably also worth nothing that this does replace struct desc with a larger structure - struct desc is already padded out to 8 bytes, but on 64-bit machines iovec will be larger still. > Signed-off-by: Laurent Vivier > --- > packet.c | 75 +++++++++++++++++++++++++++++++------------------------- > packet.h | 14 ++--------- > 2 files changed, 43 insertions(+), 46 deletions(-) >=20 > diff --git a/packet.c b/packet.c > index ccfc84607709..af2a539a1794 100644 > --- a/packet.c > +++ b/packet.c > @@ -22,6 +22,36 @@ > #include "util.h" > #include "log.h" > =20 > +static int packet_check_range(const struct pool *p, size_t offset, size_= t len, > + const char *start, const char *func, int line) > +{ > + if (start < p->buf) { > + if (func) { > + trace("add packet start %p before buffer start %p, " > + "%s:%i", (void *)start, (void *)p->buf, func, line); > + } > + return -1; I guess not really in scope for this patch, but IIUC the only place we'd hit this is if the caller has done something badly wrong, so possibly it should be an ASSERT(). > + } > + > + if (start + len + offset > p->buf + p->buf_size) { > + if (func) { > + trace("packet offset plus length %lu from size %lu, " > + "%s:%i", start - p->buf + len + offset, > + p->buf_size, func, line); > + } > + return -1; > + } Same with this one. > + > +#if UINTPTR_MAX =3D=3D UINT64_MAX > + if ((uintptr_t)start - (uintptr_t)p->buf > UINT32_MAX) { > + trace("add packet start %p, buffer start %p, %s:%i", > + (void *)start, (void *)p->buf, func, line); > + return -1; > + } > +#endif This one is relevant to this patch though - because you're expanding struct desc's 32-bit offset to full void * from struct iovec, this check is no longer relevant. > + > + return 0; > +} > /** > * packet_add_do() - Add data as packet descriptor to given pool > * @p: Existing pool > @@ -41,34 +71,16 @@ void packet_add_do(struct pool *p, size_t len, const = char *start, > return; > } > =20 > - if (start < p->buf) { > - trace("add packet start %p before buffer start %p, %s:%i", > - (void *)start, (void *)p->buf, func, line); > + if (packet_check_range(p, 0, len, start, func, line)) > return; > - } > - > - if (start + len > p->buf + p->buf_size) { > - trace("add packet start %p, length: %zu, buffer end %p, %s:%i", > - (void *)start, len, (void *)(p->buf + p->buf_size), > - func, line); > - return; > - } > =20 > if (len > UINT16_MAX) { > trace("add packet length %zu, %s:%i", len, func, line); > return; > } > =20 > -#if UINTPTR_MAX =3D=3D UINT64_MAX > - if ((uintptr_t)start - (uintptr_t)p->buf > UINT32_MAX) { > - trace("add packet start %p, buffer start %p, %s:%i", > - (void *)start, (void *)p->buf, func, line); > - return; > - } > -#endif > - > - p->pkt[idx].offset =3D start - p->buf; > - p->pkt[idx].len =3D len; > + p->pkt[idx].iov_base =3D (void *)start; > + p->pkt[idx].iov_len =3D len; > =20 > p->count++; > } > @@ -104,28 +116,23 @@ void *packet_get_do(const struct pool *p, size_t id= x, size_t offset, > return NULL; > } > =20 > - if (p->pkt[idx].offset + len + offset > p->buf_size) { > + if (len + offset > p->pkt[idx].iov_len) { > if (func) { > - trace("packet offset plus length %zu from size %zu, " > - "%s:%i", p->pkt[idx].offset + len + offset, > - p->buf_size, func, line); > + trace("data length %zu, offset %zu from length %zu, " > + "%s:%i", len, offset, p->pkt[idx].iov_len, > + func, line); > } > return NULL; > } > =20 > - if (len + offset > p->pkt[idx].len) { > - if (func) { > - trace("data length %zu, offset %zu from length %u, " > - "%s:%i", len, offset, p->pkt[idx].len, > - func, line); > - } > + if (packet_check_range(p, offset, len, p->pkt[idx].iov_base, > + func, line)) > return NULL; > - } > =20 > if (left) > - *left =3D p->pkt[idx].len - offset - len; > + *left =3D p->pkt[idx].iov_len - offset - len; > =20 > - return p->buf + p->pkt[idx].offset + offset; > + return (char *)p->pkt[idx].iov_base + offset; > } > =20 > /** > diff --git a/packet.h b/packet.h > index a784b07bbed5..8377dcf678bb 100644 > --- a/packet.h > +++ b/packet.h > @@ -6,16 +6,6 @@ > #ifndef PACKET_H > #define PACKET_H > =20 > -/** > - * struct desc - Generic offset-based descriptor within buffer > - * @offset: Offset of descriptor relative to buffer start, 32-bit limit > - * @len: Length of descriptor, host order, 16-bit limit > - */ > -struct desc { > - uint32_t offset; > - uint16_t len; > -}; > - > /** > * struct pool - Generic pool of packets stored in a buffer > * @buf: Buffer storing packet descriptors > @@ -29,7 +19,7 @@ struct pool { > size_t buf_size; > size_t size; > size_t count; > - struct desc pkt[1]; > + struct iovec pkt[1]; > }; > =20 > void packet_add_do(struct pool *p, size_t len, const char *start, > @@ -54,7 +44,7 @@ struct _name ## _t { \ > size_t buf_size; \ > size_t size; \ > size_t count; \ > - struct desc pkt[_size]; \ > + struct iovec pkt[_size]; \ > } > =20 > #define PACKET_POOL_INIT_NOCAST(_size, _buf, _buf_size) \ --=20 David Gibson | 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 --0LMWR5NQdWloEItX Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXBmI0ACgkQzQJF27ox 2Gc8ZQ//RjyrqzWCdXFQBoj+vAlTNlNPsj7GzEOkmc1TvXkwJnjWH9WZd5DQrktE gRSXHJ7epcZiV37QCTk58O815IfaQM+UOTYiMP/9KlnDdlioIM9CYmIWVdu176nJ 3a3KhDiBVVHJ989HTSeLXS2/mqBu1AFZiOfytu8ryf6uaUPs43+jmRqYnDnrgFbI Z46G3L7baYk4f7Np3++892b9Y3U7v5OSP5qzUEDDJBeLPLKVvpR3aZpLlKLHfY5s MvSaCSmPSBSqgcmMnworeYCHISARcu8WGgXH6+oHDtDETwR/NVXyhZNNvx3YQNPe 9d2aE4NiUigOr9ta1pWqjAfzUw+bYlS3v/41HM3t4obEIGXzC8+kvbY+nnIvn3bL MI3aGxsR+kpj8iCKJKvrsl7yJRw3wReGF9NM97w1pXv3M7BnYxtaMa+c/wS/8n54 JAL1oRaOLqQDAu9EFQtboYtXaWZmqtqDGwe00Lky7uoKYTSnLfiXF7d23BI4vyOR vEsaHDlNa5pCX+Ke/9C4D+xzIYaQDiQuvT1suErJYnDW00rHu5UrCbYBvaO99CTm M6TIyjFrNEGMg7CvCyz4+YQQCHFMWWbq3/TlNK4Xuv4itRM+toXflO1JGM55xPSf 1/7CVLE1ZqPK7ahx8FSWAamGahtbkMpgyaeen1IYh33dQB4cSC8= =yv6k -----END PGP SIGNATURE----- --0LMWR5NQdWloEItX--