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 52C675A0271 for ; Wed, 28 Feb 2024 00:58:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1709078292; bh=Qc58gLGcxLdI0bcb95EIY6jm+5XPmT1dfyExvyUwKzM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bD6LPNsXgsc2tj2qaMfRgNcn2grVW7h3mAJqc+FIXWSeP9bB/U8WgZkep45ZHKcDe 3a6LJll2yMWbTSR3/AY4U145hGFR0nUA8L4nM4HzfI3nRlFBiUI00ZX6fgvM59fweR GKWPOcwZzDCAoLtyqqu45wdOr+ecrA2zz/BgkCzjWlC4P+tNGgp71f3C9hYjyReIWp Wx9mN0cHXU54yknG9HRaa9LtMmnHDWq584qMQMadGawSPx5gUpwQ5a8hiCc89z2ZBz faa9zlizy36OJqs2HXjE31wbUkznscpVVaHyC6S9uteLBUrHY7PDvGl+wrLbBGtHba UiMYYZtjXQScw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TkvZD2m19z4wc7; Wed, 28 Feb 2024 10:58:12 +1100 (AEDT) Date: Wed, 28 Feb 2024 10:27:03 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/6] util: Add helper to find offset into io vector Message-ID: References: <20240222055602.1872516-1-david@gibson.dropbear.id.au> <20240222055602.1872516-2-david@gibson.dropbear.id.au> <20240227152335.75ce9127@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="r0ur03qXlMOxJD2r" Content-Disposition: inline In-Reply-To: <20240227152335.75ce9127@elisabeth> Message-ID-Hash: EPEF7C5WBONQSPN24RPOFSSZRMAKF2G4 X-Message-ID-Hash: EPEF7C5WBONQSPN24RPOFSSZRMAKF2G4 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: Laurent Vivier , 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: --r0ur03qXlMOxJD2r Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 27, 2024 at 03:23:35PM +0100, Stefano Brivio wrote: > On Thu, 22 Feb 2024 16:55:57 +1100 > David Gibson wrote: >=20 > > tap_send_frames_passt() needs to determine which buffer element a byte > > offset into an IO vector lies in. We have some upcoming uses for simil= ar > > logic, so split this out into a helper function iov_offset(). > >=20 > > Signed-off-by: David Gibson > > --- > > tap.c | 13 +++++-------- > > util.c | 23 +++++++++++++++++++++++ > > util.h | 1 + >=20 > Laurent, I guess this will need to be moved to iov.h by your series, at > the point where you introduce the new header. To avoid that shuffle, I cherry picked Laurent's first patch into my series, and rebased putting this function directly into iov.c. Several of Laurent's functions can also be slightly simplified using this helper, so I've done that. > > 3 files changed, 29 insertions(+), 8 deletions(-) > >=20 > > diff --git a/tap.c b/tap.c > > index 396dee7e..f15eba6e 100644 > > --- a/tap.c > > +++ b/tap.c > > @@ -390,22 +390,19 @@ static size_t tap_send_frames_passt(const struct = ctx *c, > > .msg_iovlen =3D n, > > }; > > unsigned int i; > > + size_t offset; > > ssize_t sent; > > =20 > > sent =3D sendmsg(c->fd_tap, &mh, MSG_NOSIGNAL | MSG_DONTWAIT); > > if (sent < 0) > > return 0; > > =20 > > - /* Check for any partial frames due to short send */ >=20 > Why would you drop this comment, though? I feel it's needed even more > as we don't open-code that any longer. I guess I thought it was redundant with the comment a few lines down? Anyway, I've put it back in. >=20 > > - for (i =3D 0; i < n; i++) { > > - if ((size_t)sent < iov[i].iov_len) > > - break; > > - sent -=3D iov[i].iov_len; > > - } > > + offset =3D (size_t)sent; > > + i =3D iov_offset(iov, n, &offset); >=20 > I think with these names and interface this becomes quite obscure: it > sounds like 'i' should be an offset at this point... and 'offset', I > have no idea (unless I read the comment to iov_offset()). Slightly > different proposal below. Yeah, that's fair. I was think of "iov_offset" as "offset the IOV", but "offset *of* the IOV" is probably a more obvious interpretation. > > =20 > > - if (i < n && sent) { > > + if (i < n && offset) { > > /* A partial frame was sent */ > > - tap_send_remainder(c, &iov[i], sent); > > + tap_send_remainder(c, &iov[i], offset); > > i++; > > } > > =20 > > diff --git a/util.c b/util.c > > index 21b35ff9..fb6a0430 100644 > > --- a/util.c > > +++ b/util.c > > @@ -574,3 +574,26 @@ int do_clone(int (*fn)(void *), char *stack_area, = size_t stack_size, int flags, > > return clone(fn, stack_area + stack_size / 2, flags, arg); > > #endif > > } > > + > > +/* iov_offset() - interpret offset into an IO vector > > + * @iov: IO vector > > + * @n: Number of entries in @iov > > + * @offset: Pointer to offset into IO vector > > + * > > + * Return: index I of iovec which contains the given offset, or @n if > > + * the given offset is >=3D the total # of bytes in the vector. > > + * *@offset is updated to be the byte offset into (@iov + I), > > + * and is guaranteed to be less than @iov[I].iov_len > > + */ > > +size_t iov_offset(const struct iovec *iov, size_t n, size_t *offset) >=20 > ...what do you think of: >=20 > /* iov_entry_index() - Index and optionally entry offset, given global of= fset Hm, I find this name and and one line description confusing in a different way from mine. In my next spin, I've tried to synthesize your suggestion and mine, into a version under the name iov_skip_bytes(). > * @iov: IO vector > * @n: Number of entries in @iov > * @global_offset: Global offset of byte in IO vector we're looking for > * @entry_offset: If not NULL, set on return: entry offset > * > * Return: index of IO vector entry for given byte offset, @n if not found > * > * Note: @entry_offset is guaranteed to be less than @iov[i].iov_len, whe= re i is > * the return value > */ >=20 > and tap_send_frames_passt() could (more) happily do: >=20 > i =3D iov_entry_index(iov, n, sent, &entry_offset); > if (i < n) { > /* A partial frame was sent */ > tap_send_remainder(c, &iov[i], entry_offset); > i++; > } >=20 > > +{ > > + size_t i; > > + > > + for (i =3D 0; i < n; i++) { > > + if (*offset < iov[i].iov_len) >=20 > Indentation. >=20 > > + break; > > + *offset -=3D iov[i].iov_len; > > + } > > + > > + return i; > > +} > > diff --git a/util.h b/util.h > > index d2320f8c..62fad6fe 100644 > > --- a/util.h > > +++ b/util.h > > @@ -229,6 +229,7 @@ void write_pidfile(int fd, pid_t pid); > > int __daemon(int pidfile_fd, int devnull_fd); > > int fls(unsigned long x); > > int write_file(const char *path, const char *buf); > > +size_t iov_offset(const struct iovec *iov, size_t n, size_t *offset); > > =20 > > /** > > * mod_sub() - Modular arithmetic subtraction >=20 --=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 --r0ur03qXlMOxJD2r Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXeb8YACgkQzQJF27ox 2GdSsw/7B24bVsqp7ld3HWJUJWsrg+AIPQ/IktG4mdeKQnmihfbEw+X0+LovNWBT gdI4COdqQdznmBv5e3si1BMz6mDAYi6xIPyGpBYyErBDS3qYNAn19DrrmGzWGWr+ iJ+TyD0vHEq3wGH+H3zZMqyeGayaK88jr2s5JPk6vye97orXP41AbO9kzSbnXcIB 5/iuGUoh7aM3ztXm6gdKXzdyrjzziBxxrdZwFsrMk0a5FUvID/oa/u2Dt2xE/Ny3 35xCAa+QJWKGtBuSkuCJJii7/NHr/Qcma7tyIj3CvxJnXhJkXkSTQH3nehbULVb3 XJI8+O7AtYwpuVjZfzD44eekDrgcyDUcnC+QyIAjDIfSd9O64i+zf0DGWom6K/UT DGc4pXbga3AezoM95KY+CSOWfiHBPcoqJMxAHZ5WbUc7GHEKd+CS+Q424cS7zyV2 ko0iDNp6mVKs4DBLC6RB5CY97EupFOOOezbJGRcjZWCNWoVpozDPzGMo/iDg/7KB sQUhXsof7n96jKBGeOabl7FzPAiMfL5o0lXtxXoCio2IOaOjq1qAH6xArVw2SE27 7qdU81ee8LZwoAXxcJW86liTZK+97zqZnay0G2qe7G4KrBJOpUdrtQeZUK9544J+ wR6/Bu2ljP9Z9Pyy4LwTPxL5UQnFQpmPZMlkzGzajwsGTG+LDIE= =WQJ8 -----END PGP SIGNATURE----- --r0ur03qXlMOxJD2r--