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 BAEA15A0280 for ; Fri, 1 Dec 2023 01:00:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1701388803; bh=hglGfgbQM1Qsq7F5vydLi7f4dwKuTLT30DfNXtXX0+s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Xqb2wa7izaJll076cjfw8BHZBxilFEdojX6sj6T8rZ7Pd02PfTkiWHs8b/TplzHhl OANdFGw/eSoZPUgEnHQFBsDUo5eHftoInrWqJaDVZUS9s5GH9u3eSh1aXSixnYQZd0 n+zmtH9AwY5IKh7IxmbXbwFS26luegHBW85MXRl8= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ShCqR4GBqz4xTg; Fri, 1 Dec 2023 11:00:03 +1100 (AEDT) Date: Fri, 1 Dec 2023 10:13:36 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 3/4] tcp, tcp_splice: CONN_IDX subtraction of pointers isn't always long Message-ID: References: <20231129134610.3796809-1-sbrivio@redhat.com> <20231129134610.3796809-4-sbrivio@redhat.com> <20231129145842.5ed82f48@elisabeth> <20231130100744.16f105df@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XXKEzTHEbZvoPjRj" Content-Disposition: inline In-Reply-To: <20231130100744.16f105df@elisabeth> Message-ID-Hash: K6HBSEHVMNKPWLG3JHKKHIH47FTK3OC7 X-Message-ID-Hash: K6HBSEHVMNKPWLG3JHKKHIH47FTK3OC7 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: lemmi@nerd2nerd.org, 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: --XXKEzTHEbZvoPjRj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Nov 30, 2023 at 10:07:44AM +0100, Stefano Brivio wrote: > On Thu, 30 Nov 2023 11:27:21 +1100 > David Gibson wrote: >=20 > > On Wed, Nov 29, 2023 at 02:58:42PM +0100, Stefano Brivio wrote: > > > On Wed, 29 Nov 2023 14:46:09 +0100 > > > Stefano Brivio wrote: > > > =20 > > > > On 32-bit architectures, it's a regular int. C99 introduced ptrdiff= _t > > > > for this case, with a matching length modifier, 't'. > > > >=20 > > > > Signed-off-by: Stefano Brivio > > > > --- > > > > tcp.c | 39 +++++++++++++++++++++------------------ > > > > tcp_splice.c | 14 +++++++------- > > > > 2 files changed, 28 insertions(+), 25 deletions(-) > > > >=20 > > > > diff --git a/tcp.c b/tcp.c > > > > index 44468ca..c32c9cb 100644 > > > > --- a/tcp.c > > > > +++ b/tcp.c > > > > @@ -727,7 +727,7 @@ static void tcp_timer_ctl(const struct ctx *c, = struct tcp_tap_conn *conn) > > > > it.it_value.tv_sec =3D ACT_TIMEOUT; > > > > } > > > > =20 > > > > - debug("TCP: index %li, timer expires in %lu.%03lus", CONN_IDX(con= n), > > > > + debug("TCP: index %ti, timer expires in %lu.%03lus", CONN_IDX(con= n), > > > > > > > > [...] =20 > > >=20 > > > Oops, I just realised this clashes with your "[PATCH v2 03/11] flow, > > > tcp: Consolidate flow pointer<->index helpers". =20 > >=20 > > And then a bunch will be obsoleted by "flow, tcp: Add logging helpers > > for connection related messages". > >=20 > > > There, however, I guess that the new flow_idx() should return ptrdiff= _t, > > > which is signed. =20 > >=20 > > Actually, no, I don't think so. Yes the expression that generates it > > is naturally of type ptrdiff_t. But it's a bug to call flow_idx() on > > something not in the flow table, and places where we want to pass *in* > > a flow table index it makes more sense for it to be unsigned. So I > > think flow indices should be unsigned throughout. >=20 > I also think it should be unsigned (s/which is signed/which happens to > be signed/ in my comment above). At the same time there's no such thing > as an unsigned version of ptrdiff_t, so, given the other choices, I > would still argue that we should use ptrdiff_t. Again, I don't think so. ptrdiff_t is important because it allows for the difference of two *unconstrained* pointers. In our case the pointer is not unconstrained, and we want to return it as whatever type we typically use for an index into the connection table, which is an unsigned int. > > > I can drop this patch if you re-spin it (assuming it makes sense to > > > you), or I can adapt it on top of your patch -- whatever is most > > > convenient for you. =20 > >=20 > > I have a couple of reasons to re-spin anyway. So how about you drop > > this, and I'll double check that I get the format specifiers sane > > after my series? >=20 > Sure, dropping this. >=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 --XXKEzTHEbZvoPjRj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmVpFx8ACgkQzQJF27ox 2GfbOw//Sn580TSvyiiGzHg85xQRaGQ5afU8uAGQtc7brnwhpFGERW/2rK+mN8o6 Aaak3p1o8x7ai34EpFQIx+2u/UJh4aS1WsgFRYm1kaHjSK1sKI9O4hNXIUf/MnHV 4SdG7z7r6y7ZwSYzwy+eXH4+Sdm56HAFNDHmBlbb6RaBklWtv0zrni3RskFMiVRE zo0/sRltgVx+TBi/mNGR2RffVe5EY1Q0vhZftdejQgK08xVPraVgRwKTuzb3xltP LGsmuuHodM/TFAGFqcWTw48NOAk52M7ga8J0bTOFYHt62kc27WQXjxEc/YfeUyHM IR+NZ4fwCLL+yat4z9eVeqifvuM/7VmCDpcKmTCPEj6nMY61O+0E2ymyAUbs5jHi bNjtuWfUiUZWT9o402D98cc0Y1Tg2EXhGi6QkB2N0G/vy9u0boHbfUm1mrSdbUcj XxvvBA7/WUI/U65uWFtS4lwm0pQ/JYAnZgKPKFV+M1Xarvwocx2wjWtaYRaJGNVD jupeT83qm++IEWgnAwEhXbYtmKP+oHItiAcIp1f0rLzIvmPs3uhU8lEoTm/dIjQY Q3+jJoQnXZtt1OrR+KsZ2hPSKD3Jh4cQwKAFnX1RVHQONp4zOL2dH5HdqP9i/IZt u8wbwIC3gLbcrio10S3P/QaLNCM8aa8QJya4+CXZQP8EqFdMk6Q= =VW0+ -----END PGP SIGNATURE----- --XXKEzTHEbZvoPjRj--