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=IcD7g5fg; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C87E55A0652 for ; Thu, 03 Apr 2025 07:38:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743658684; bh=a7cU9QA05IOgayQ31mkwHiye033io3gtlKWBw8OH3Ak=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IcD7g5fg8SjuGqTuM4YuH76evqt09dcK+jM7/XAUzoYW0PX8aoK+ks4GgfTIhWtb0 ShUu5OlUavYaCOP4dZZuLlaXZZnQWbZCoJTzWhUNvsoQNyilZZcc57tBJ7vKLGvrTZ gK0hjzywBdS2epeWCcGH/WCBwzcuqRUbcqaFF9qY1a0D9jmzVyy++gBHvwu2TbuyJu dbS95jA+uRQhVftvx6pTXd5SUAMAXKAj4zP02+wiQZ3HWXDAXMASuL+zMpcnBsqIMF qbUonTovtPsNYb6QNNOjal9lUstNB+tsTyecTLCKCc2JJEZgS5GOCSOc8YdmhYi8b0 ssRbxO5xuw3qg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZSr9m5WNNz4xN1; Thu, 3 Apr 2025 16:38:04 +1100 (AEDT) Date: Thu, 3 Apr 2025 16:11:11 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 09/18] udp: Convert to iov_tail Message-ID: References: <20250402172343.858187-1-lvivier@redhat.com> <20250402172343.858187-10-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="TUEKZm5IAsKeK12j" Content-Disposition: inline In-Reply-To: <20250402172343.858187-10-lvivier@redhat.com> Message-ID-Hash: L54KFNDVL2T4Q6TYMII73JAQL72ISQJP X-Message-ID-Hash: L54KFNDVL2T4Q6TYMII73JAQL72ISQJP 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: --TUEKZm5IAsKeK12j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 02, 2025 at 07:23:34PM +0200, Laurent Vivier wrote: > Use packet_base() and extract headers using IOV_REMOVE_HEADER() > and IOV_PEEK_HEADER() rather than packet_get(). >=20 > Signed-off-by: Laurent Vivier > --- > iov.c | 1 - > udp.c | 37 ++++++++++++++++++++++++++----------- > 2 files changed, 26 insertions(+), 12 deletions(-) >=20 > diff --git a/iov.c b/iov.c > index 508fb6da91fb..0c69379316aa 100644 > --- a/iov.c > +++ b/iov.c > @@ -173,7 +173,6 @@ size_t iov_size(const struct iovec *iov, size_t iov_c= nt) > * Returns: The number of elements successfully copied to the desti= nation > * iov array. > */ > -/* cppcheck-suppress unusedFunction */ > unsigned iov_copy(struct iovec *dst_iov, size_t dst_iov_cnt, > const struct iovec *iov, size_t iov_cnt, > size_t offset, size_t bytes) > diff --git a/udp.c b/udp.c > index 80520cbdf188..03906d72e75c 100644 > --- a/udp.c > +++ b/udp.c > @@ -858,15 +858,20 @@ int udp_tap_handler(const struct ctx *c, uint8_t pi= f, > struct iovec m[UIO_MAXIOV]; > const struct udphdr *uh; > struct udp_flow *uflow; > - int i, s, count =3D 0; > + int i, j, s, count =3D 0; > + struct iov_tail data; > flow_sidx_t tosidx; > in_port_t src, dst; > + struct udphdr uhc; > uint8_t topif; > socklen_t sl; > =20 > ASSERT(!c->no_udp); > =20 > - uh =3D packet_get(p, idx, 0, sizeof(*uh), NULL); > + if (!packet_base(p, idx, &data)) > + return 1; > + > + uh =3D IOV_PEEK_HEADER(&data, uhc); > if (!uh) > return 1; > =20 > @@ -903,23 +908,33 @@ int udp_tap_handler(const struct ctx *c, uint8_t pi= f, > =20 > pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport); > =20 > - for (i =3D 0; i < (int)p->count - idx; i++) { > - struct udphdr *uh_send; > - size_t len; > + for (i =3D 0, j =3D 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) { > + const struct udphdr *uh_send; > + > + if (!packet_base(p, idx + i, &data)) > + return p->count - idx; > =20 > - uh_send =3D packet_get(p, idx + i, 0, sizeof(*uh), &len); > + uh_send =3D IOV_REMOVE_HEADER(&data, uhc); > if (!uh_send) > return p->count - idx; > =20 > + iov_tail_prune(&data); Maybe we should make remove_header always prune, rather than having to do it manually. > + > + if (data.cnt + j >=3D UIO_MAXIOV) Obviously it's equivalent, but this would make more intuitive sense to me as (j + data.cnt): the amount we've already used in our array, plus the extra we're going to need. > + return p->count - idx; > + > mm[i].msg_hdr.msg_name =3D &to_sa; > mm[i].msg_hdr.msg_namelen =3D sl; > =20 > - if (len) { > - m[i].iov_base =3D (char *)(uh_send + 1); > - m[i].iov_len =3D len; > + if (data.cnt ) { Stray space. > + unsigned int len; > + > + len =3D iov_copy(&m[j], UIO_MAXIOV - j, > + &data.iov[0], data.cnt, data.off, SIZE_MAX); So, as I mentioned on iov_copy(), it doesn't obviously report the case where it copies less than expected not because the source is missing data, but because the destination doesn't have enough iovecs. You've avoided this case with a test above, but that's a bit non-obvious, it would be nice if we could just call this and check for an error. > - mm[i].msg_hdr.msg_iov =3D m + i; > - mm[i].msg_hdr.msg_iovlen =3D 1; > + mm[i].msg_hdr.msg_iov =3D &m[j]; > + mm[i].msg_hdr.msg_iovlen =3D len; > + j +=3D len; > } else { > mm[i].msg_hdr.msg_iov =3D NULL; > mm[i].msg_hdr.msg_iovlen =3D 0; --=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 --TUEKZm5IAsKeK12j Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfuGG4ACgkQzQJF27ox 2Gdf0w//Wk513q/EJtRhqI9JWI8UfpkcRoROM+6IhfadBwHAkbHhyjKff5j1OeZT jXbxCld2ga8RDwZi/ZOy77YAayk9oGj7m4uDTQ+W/dBnA49XEgV1IaPmbEByWDcZ /EmV3YvZQqQiRe38Kl2tcIUT0hYxOMB72JJs//J+cQn4EzU0ZXfe0cyiGe8HsJ0/ CGjl5qBzLHRjH83E8c/LG1m82uyFllvb4KFb2n34AIutK/GKk218WUxFV+jIWPWn h01N/U9W9a8P9sHFMQH2pJvRHoHGtgzEE57T0BBcGUswpsJdYJKjJjDZYi5vtk2/ DclxL64kg87FsdVQ5q19UmqaatJyrWcBR+g97Pr1lDSDD7bQnyrZHppIHe7ubYDC sNbVBECcqX+F9N1RzlVSBKbwJZ2F16ncgBUOBuDinjhav+9yny45ztmKL3itGBqq FBTwanrb7z1ywKKpSr45mfbezfixFQxDj8KUFf7xtvK/meO4SJItNLgee3voXkwf hm2rK9jDxvvuvK0nZZ5NM+fQTtd9tqkB3R3NgtZJPpk+pa6kDnZYmeAM4boT3T7M TKZxuhEVTsNc4quW7iB/zOa1XL1WUMKPW1NWfgqCl6tBE5u7Qbrat8OWmZGmQ8ib PA5nMZHAXmQa59y///UesyXKJEA+VU0LJX6BeLUutmcbcMFEX4o= =xR3t -----END PGP SIGNATURE----- --TUEKZm5IAsKeK12j--