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=esZ0PSwy; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id C062C5A0279 for ; Wed, 06 Aug 2025 03:58:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1754445502; bh=/m/VKB0V2bPFt0Ry0YE6REK0PwQNlkbYar4yVbjjph8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=esZ0PSwya0utcNYWQ8zMqs/voK44U9sSykrCvaajDu0WuX74z7yXzo0YovK1P3rSP ILI2mCWDqm0ia2LphPdH+1bxW/vS7TLwBbxZ6OdzsuHLVWGOlQrT+v1s7h4YAgCZGq oHL85R4QA3AuqK7RQ7uj/EE0QyI4IDS5fz2p++1YDKeaSRb12CRrf/q96tD9DOxKCr vGraIex0apx3veVdfNYU2n8/ec6YwV/LLiQRQvM9ukj1CevNtHRN+Q64xBBRIQ/ICd qxEi3CFoLj+vp70dC1mn8MOvEkmJWrCYoasHeEqInsBo4AL0Euffi4pK48ks1jPQaT MOT85h0LJhe6Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bxYNZ6Sqkz4wbR; Wed, 6 Aug 2025 11:58:22 +1000 (AEST) Date: Wed, 6 Aug 2025 11:45:23 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v8 03/30] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() Message-ID: References: <20250805154628.301343-1-lvivier@redhat.com> <20250805154628.301343-4-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="pVtc/d/ReT9d/nVo" Content-Disposition: inline In-Reply-To: <20250805154628.301343-4-lvivier@redhat.com> Message-ID-Hash: ONIM7YFXJKODLHRQW4P75VOVK6UHEUUY X-Message-ID-Hash: ONIM7YFXJKODLHRQW4P75VOVK6UHEUUY 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: --pVtc/d/ReT9d/nVo Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 05, 2025 at 05:46:01PM +0200, Laurent Vivier wrote: > Provide a temporary variable of the wanted type to store > the header if the memory in the iovec array is not contiguous. >=20 > Signed-off-by: Laurent Vivier > --- > iov.c | 55 +++++++++++++++++++++++++++++++++++++++++++++---------- > iov.h | 55 +++++++++++++++++++++++++++++++++++++++++-------------- > tcp_buf.c | 2 +- > 3 files changed, 87 insertions(+), 25 deletions(-) >=20 > diff --git a/iov.c b/iov.c > index 9d282d4af461..d39bb099fa69 100644 > --- a/iov.c > +++ b/iov.c > @@ -109,7 +109,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t i= ov_cnt, > * > * Return: the number of bytes successfully copied. > */ > -/* cppcheck-suppress unusedFunction */ > +/* cppcheck-suppress [staticFunction] */ > size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, > size_t offset, void *buf, size_t bytes) > { > @@ -127,6 +127,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov= _cnt, > /* copying data */ > for (copied =3D 0; copied < bytes && i < iov_cnt; i++) { > size_t len =3D MIN(iov[i].iov_len - offset, bytes - copied); > + /* NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) */ Which parameter was cppcheck complaining about? > memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset, > len); > copied +=3D len; > @@ -208,7 +209,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len) > } > =20 > /** > - * iov_peek_header_() - Get pointer to a header from an IOV tail > + * iov_check_header() - Check if a header can be accessed > * @tail: IOV tail to get header from > * @len: Length of header to get, in bytes > * @align: Required alignment of header, in bytes > @@ -219,8 +220,7 @@ bool iov_tail_drop(struct iov_tail *tail, size_t len) > * overruns the IO vector, is not contiguous or doesn't have the > * requested alignment. > */ > -/* cppcheck-suppress [staticFunction,unmatchedSuppression] */ > -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align) > +static void *iov_check_header(struct iov_tail *tail, size_t len, size_t = align) > { > char *p; > =20 > @@ -240,27 +240,62 @@ void *iov_peek_header_(struct iov_tail *tail, size_= t len, size_t align) > return p; > } > =20 > +/** > + * iov_peek_header_() - Get pointer to a header from an IOV tail > + * @tail: IOV tail to get header from > + * @v: Temporary memory to use if the memory in @tail > + * is discontinuous > + * @len: Length of header to get, in bytes > + * @align: Required alignment of header, in bytes > + * > + * @tail may be pruned, but will represent the same bytes as before. > + * > + * Return: pointer to the first @len logical bytes of the tail, or to > + * a copy if that overruns the IO vector, is not contiguous or > + * doesn't have the requested alignment. NULL if that overruns t= he > + * IO vector. > + */ > +/* cppcheck-suppress [staticFunction,unmatchedSuppression] */ > +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_= t align) > +{ > + char *p =3D iov_check_header(tail, len, align); > + size_t l; > + > + if (p) > + return p; > + > + l =3D iov_to_buf(tail->iov, tail->cnt, tail->off, v, len); > + if (l !=3D len) > + return NULL; > + > + return v; > +} > + > /** > * iov_remove_header_() - Remove a header from an IOV tail > * @tail: IOV tail to remove header from (modified) > + * @v: Temporary memory to use if the memory in @tail > + * is discontinuous > * @len: Length of header to remove, in bytes > * @align: Required alignment of header, in bytes > * > * On success, @tail is updated so that it longer includes the bytes of = the > * returned header. > * > - * Return: pointer to the first @len logical bytes of the tail, NULL if = that > - * overruns the IO vector, is not contiguous or doesn't have the > - * requested alignment. > + * Return: pointer to the first @len logical bytes of the tail, or to > + * a copy if that overruns the IO vector, is not contiguous or > + * doesn't have the requested alignment. NULL if that overruns t= he > + * IO vector. > */ > -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align) > +void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, siz= e_t align) > { > - char *p =3D iov_peek_header_(tail, len, align); > + char *p =3D iov_peek_header_(tail, v, len, align); > =20 > if (!p) > return NULL; > =20 > tail->off =3D tail->off + len; Do you want to use your new iov_tail_drop() here? > + > return p; > } > =20 > @@ -275,7 +310,7 @@ void *iov_remove_header_(struct iov_tail *tail, size_= t len, size_t align) > * > * Return: the number of elements successfully referenced from the desti= nation > * iov array, a negative value if there is not enough room in the > - * destination iov array > + * destination iov array > */ > /* cppcheck-suppress unusedFunction */ > ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, > diff --git a/iov.h b/iov.h > index bf9820ac52ab..ccdb690ef3f1 100644 > --- a/iov.h > +++ b/iov.h > @@ -70,41 +70,68 @@ struct iov_tail { > #define IOV_TAIL(iov_, cnt_, off_) \ > (struct iov_tail){ .iov =3D (iov_), .cnt =3D (cnt_), .off =3D (off_) } > =20 > +/** > + * IOV_TAIL_FROM_BUF() - Create a new IOV tail from a buffer > + * @buf_: Buffer address to use in the iovec > + * @len_: Buffer size > + * @off_: Byte offset in the buffer where the tail begins > + */ > +#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \ > + IOV_TAIL((&(const struct iovec){ .iov_base =3D (buf_), \ > + .iov_len =3D (len_) }), \ > + 1, \ > + (off_)) > + This seems unrelated to the rest of the patch, does it belong in a different one? Also, given that you're constructing the iovec entries yourself, why not set tail->off to zero and update iov_base and len accordingly. > bool iov_tail_prune(struct iov_tail *tail); > size_t iov_tail_size(struct iov_tail *tail); > bool iov_tail_drop(struct iov_tail *tail, size_t len); > -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align); > -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align= ); > +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_= t align); > +void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, siz= e_t align); > ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, > struct iov_tail *tail); > =20 > /** > * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail > * @tail_: IOV tail to get header from > - * @type_: Data type of the header > + * @var_: Temporary buffer of the type of the header to use if > + * the memory in the iovec array is not contiguous. > * > * @tail_ may be pruned, but will represent the same bytes as before. > * > - * Return: pointer of type (@type_ *) located at the start of @tail_, NU= LL if > - * we can't get a contiguous and aligned pointer. > + * Return: pointer of type (@type_ *) located at the start of @tail_ > + * or to @var_ if iovec memory is not contiguous, NULL if > + * that overruns the iovec. The NULL case can't happen if given a variable of the right type. > */ > -#define IOV_PEEK_HEADER(tail_, type_) \ > - ((type_ *)(iov_peek_header_((tail_), \ > - sizeof(type_), __alignof__(type_)))) > + > +#define IOV_PEEK_HEADER(tail_, var_) \ > + ((__typeof__(var_) *)(iov_peek_header_((tail_), &(var_), \ > + sizeof(var_), \ > + __alignof__(var_)))) > =20 > /** > * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail > * @tail_: IOV tail to remove header from (modified) > - * @type_: Data type of the header to remove > + * @var_: Temporary buffer of the type of the header to use if > + * the memory in the iovec array is not contiguous. > * > * On success, @tail_ is updated so that it longer includes the bytes of= the > * returned header. > * > - * Return: pointer of type (@type_ *) located at the old start of @tail_= , NULL > - * if we can't get a contiguous and aligned pointer. > + * Return: pointer of type (@type_ *) located at the start of @tail_ > + * or to @var_ if iovec memory is not contiguous, NULL if > + * that overruns the iovec. Again, NULL case can't happen. > + */ > + > +#define IOV_REMOVE_HEADER(tail_, var_) \ > + ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \ > + sizeof(var_), __alignof__(var_)))) > + > +/** IOV_DROP_HEADER() - Remove a typed header from an IOV tail > + * @tail_: IOV tail to remove header from (modified) > + * @type_: Data type of the header to remove > + * > + * Return: true if the tail still contains any bytes, otherwise false > */ > -#define IOV_REMOVE_HEADER(tail_, type_) \ > - ((type_ *)(iov_remove_header_((tail_), \ > - sizeof(type_), __alignof__(type_)))) > +#define IOV_DROP_HEADER(tail_, type_) iov_tail_drop((tail_), sizeof(typ= e_)) > =20 > #endif /* IOVEC_H */ > diff --git a/tcp_buf.c b/tcp_buf.c > index d1fca676c9a7..bc898de86919 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_= tap_conn *conn, > uint32_t seq, bool no_tcp_csum) > { > struct iov_tail tail =3D IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); > - struct tcphdr *th =3D IOV_REMOVE_HEADER(&tail, struct tcphdr); > + struct tcphdr th_storage, *th =3D IOV_REMOVE_HEADER(&tail, th_storage); > struct tap_hdr *taph =3D iov[TCP_IOV_TAP].iov_base; > const struct flowside *tapside =3D TAPFLOW(conn); > const struct in_addr *a4 =3D inany_v4(&tapside->oaddr); --=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 --pVtc/d/ReT9d/nVo Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmiSs6YACgkQzQJF27ox 2Gc9Qw//USYApMQUWiGqLZZ9xbhN0TECA5usE1yzljc202JVgxdA8h5WASQvrUh8 2raS5sd2xFxlPXszejYv6T3ycp3wzDFPDwWNyz7foWRlShkhsJ3A5P9fY9qPX9Je lGS9/lIpTzupr72wYB6r1G2pc9DEo0KJjWBE9+D6qihLGxwchNz2b+yZDdpVjIEc 74USSDL3gG+0gVW3PKkLoDeuHqR5LfEdAgtULKNI+LeoLClCnOdOdapDfb3y1v9a NBhhxVV++J1wRle+JAq3JBlDWmabJBGTHq5oBJcmGpR6zWnYgcQqhJG2h4WWWGwe yYFZXW7C01RW1YQeqGdjENjX96m47nNBzji2kkIMXyCnPLDLuwHYrz/z/NK2ZlWW OLEw8lIamTBjMeMcuvPOaW7vWSC9rlb9ooWQRQGJgETMNaBlhOdR3RDHJY3jKv6L qaULlrWqHrfZ0H2i4BfOUQj3heE/cH932d+7Qoph4z3v60hUFxQsUN66gOC+3pxu lxXEg8TNpcRx7ZiUUwsHRqNSdfYIWufowVcu1jCFuIEj10npagUZ52+afexxBAEy Nm+EcfOYKxVSMBR2QNHUaiNS0bgULEZ5Gk+UyNZOQmJZDjBjjZhI8lPhu7wSQMSe 7Fb+RAJ9VVtlh1NB2TzeN0WFOx0ylgRvnha0gwzDOo6gDHeUw1k= =vejz -----END PGP SIGNATURE----- --pVtc/d/ReT9d/nVo--