From nobody Mon Feb 17 21:02:31 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YxZqH161jz5nQRp; Mon, 17 Feb 2025 21:02:35 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YxZqG57Jdz3Dtl; Mon, 17 Feb 2025 21:02:34 +0000 (UTC) (envelope-from kp@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1739826154; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8pmSJw35qFM2oiyrHrKRekvr6w+AwS6KfU+bFu+IC2U=; b=Lpx0/BKpC+962lbg4AFA37ex77CH8iL0+eEdHpcLT9lh4uCbEg5CuM64S2f7S5AYJkGDeE SWJV7hmPallM5c78r6fREoq6bu3q48pN1nDLX1lMx5bt9+Ce7rZIZp9DvdqyzvqG5tS2wv 1Mbvd+OfHUHc/HdOGRvfGdPBUqYuX0WxC6Uie+Hap6O1TrRmLLFFBIJH4s+pbRpgUfzeaQ xHY0CZ95St6gr9TAddD9ZffAu0WkQRt1QCUDcYr6vn3FLksn6/fv/k7odcgiVhCC2NGQ1k g+sujNZRQwC+KK0OvwiUuaLpBCRcSA54lrswlvs/YHJ5N1H0Fvzotse73HYbHw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1739826154; a=rsa-sha256; cv=none; b=NEp5vUxtMjhigH3SBPG+LDc+9DlSVfUfIS6fVEzCcBFyXsscz8Kn3gTsvDoISNso+sIlQM PaoO8C1lzxSdN+ki9oC/R7ukWunbGRp8JpRITbUlTCUrwO6bBFM/NnlaITPY3ZTfQa6TC+ aVwQH7aoVgZbxFg7h8Lr9Czxio1vJjg4UBYveXHSD0nI8KoGXGxJRlSa9WDho90Cexheba ILwgz9FQhDQnvAZxbGw+S7HR5MO9H/s8sTsoPdpQfLzI2LOjs2PP9CWKYQkaiy7See0iw5 A6CeZ41Ug1Km0jEclgV0KD1ytXIHnopTLICqEqQGdjkUl7XrdYOXLk7s0Cm3aA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1739826154; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8pmSJw35qFM2oiyrHrKRekvr6w+AwS6KfU+bFu+IC2U=; b=FOBRTFnzf3HMkM/T9rHOJxP3O6SWlnrpssDXkmIPCp5dhMR5UCd7BTvbu+GyGHl4dC8zDg FyQxDVEozSUszZLt4iHQJ3ygUz17Jgs+M29HXp0lZWvNLfA6odF02yblmuPr4JK7lPrSn/ hbI6m8nb1b5AxlkVEJ43r1hhWdIiWsJzdDPSuz14oCg27uM6dGBkdKGQ6WR+N8NxnqJLi6 IgZ5NwHsHP68Re+nwQFX+Kxsh81fwr+26vz2TLrrYAY+JSg9q1OxazClYEp6pveZynryJl 9+rDlVVYm/cNBi08k0BJruRX5T+gLKq2v/yY+7p00uz1hkNLZh8z9yHsqbEjfg== Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "mx1.codepro.be", Issuer "R10" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 4YxZqG3TBkzBgg; Mon, 17 Feb 2025 21:02:34 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: by venus.codepro.be (Postfix, authenticated sender kp) id 30F051F96E; Mon, 17 Feb 2025 22:02:32 +0100 (CET) From: Kristof Provost To: John Baldwin Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 7e7f88001d7d - main - pf: use time_t for storing time_t values Date: Mon, 17 Feb 2025 22:02:31 +0100 X-Mailer: MailMate (2.0r6222) Message-ID: In-Reply-To: <06849674-d51f-4a20-9ddb-687e29ece68e@FreeBSD.org> References: <202502141750.51EHoOFm061342@gitrepo.freebsd.org> <5c019c51-949b-4255-bc44-926ac973a1af@FreeBSD.org> <1B3E8B07-037B-4DA9-A8D7-81F866078A39@FreeBSD.org> <06849674-d51f-4a20-9ddb-687e29ece68e@FreeBSD.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="=_MailMate_5CA74BE6-CED5-4A7E-8982-9DB900BE9221_=" --=_MailMate_5CA74BE6-CED5-4A7E-8982-9DB900BE9221_= Content-Type: text/plain; charset=UTF-8; format=flowed; markup=markdown Content-Transfer-Encoding: quoted-printable On 17 Feb 2025, at 21:22, John Baldwin wrote: > On 2/17/25 12:08, Kristof Provost wrote: >> On 17 Feb 2025, at 16:24, John Baldwin wrote: >>> On 2/14/25 12:50, Kristof Provost wrote: >>>> The branch main has been updated by kp: >>>> >>>> URL: >>>> https://cgit.FreeBSD.org/src/commit/?id=3D7e7f88001d7dfec83cd7568369= be6a587d4a51ff >>>> >>>> commit 7e7f88001d7dfec83cd7568369be6a587d4a51ff >>>> Author: Kristof Provost >>>> AuthorDate: 2025-02-07 10:29:26 +0000 >>>> Commit: Kristof Provost >>>> CommitDate: 2025-02-14 17:47:52 +0000 >>>> >>>> pf: use time_t for storing time_t values >>>> No change to the underlying type, so no ABI change. >>>> We define __time_t as uint64_t if __LP64__, otherwise >>>> uint32_t, >>>> and only define __LP64__ if long is 64 bits. >>>> In other words: __time_t =3D=3D long. >>>> ok henning@ deraadt@ >>>> Obtained from: OpenBSD, guenther , >>>> 6c1b69a0ff >>>> Sponsored by: Rubicon Communications, LLC ("Netgate") >>>> Differential Revision: https://reviews.freebsd.org/D48963 >>> >>> This is an ABI change on non-i386 32-bit platforms in FreeBSD since >>> they >>> all use a 64-bit type for time_t that is not the same size as long. >>> Not >>> sure if the ABI change matters on FreeBSD though? >>> >> It wasn=E2=80=99t intended to be an ABI change, hence the commit messa= ge. = >> It >> appears that=E2=80=99s only correct for x86 though. > > I assumed the commit message was from OpenBSD as the comments about > defining time_t conditional on __LP64__ are not correct on FreeBSD > (each arch defines a __time_t in , though amd64 > and i386 share x86/include/_types.h which does use an #ifdef that > perhaps is the source of confusion?) > Partially. The =E2=80=9CWe define __time_t as uint64_t if __LP64__, other= wise = uint32_t, and only define __LP64__ if long is 64 bits. In other words: __time_t =3D=3D long.=E2=80=9D bit was me, and that was c= orrect = for x86, but not for other machines. That=E2=80=99s what I got wrong. >> So we=E2=80=99re only talking about armv7 and ppc32, if I=E2=80=99m no= t = >> forgetting >> anything. The former is on the removal list already, and the latter = >> .. >> well, I don=E2=80=99t know how many users there are. Both are likely t= o be >> embedded platforms where the ABI change is going to be even less >> relevant (because it really only matters if the kernel and userspace = >> are >> not updated together, and these are going to be embedded devices that >> are far more likely to have everything updated simultaneously).> So = >> I=E2=80=99m unsure about what to do. I can revert this and we can just= >> carry this (trivial) diff to OpenBSD forever, or we can ignore the = >> ABI >> breakage given the above. I=E2=80=99m not inclined to do anything more= >> involved though. >> >> Do you have any thoughts? > > To be clear, armv7 is planned to be around a bit longer than other = > 32-bit > platforms. That said, 32-bit plaforms are all Tier 2, so an ABI = > breakage > in main is not necessarily the end of the world. Presumably these = > structures > aren't used much in ports but only in base system tools anyway? (That > is what my question about the ABI change mattering was trying to = > allude to) > This affects ioctl calls, so it can and probably does affect ports. = There aren=E2=80=99t many but still a few that use the ioctl interface (t= hings = like pftop and snort). I don=E2=80=99t know offhand if they actually use any of the affected cal= ls = though. I could also revert this now and deal with it when I get around to = converting the relevant ioctl calls to netlink. That=E2=80=99s ongoing an= d = still aspirationally (but getting less likely) to be completed before we = branch 15. That may be a better point to make this change, because once = the netlink conversion is complete the next major release will remove = the entire ioctl interface, and that=E2=80=99s a breaking change anyway. > I agree with Justin that this is not something to MFC. > There=E2=80=99s no plan to MFC this (or any of the other recent pf work, = for = that matter). Best regards, Kristof --=_MailMate_5CA74BE6-CED5-4A7E-8982-9DB900BE9221_= Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On 17 Feb 2025, at 21:22, John Baldwin wrote:

On 2/17/25 12:08, Kristof Provost wrote:

On 17 Feb 2025, at 16:24, John Baldwin wrote:

On 2/14/25 12:50, Kristof Provost wrote:

The branch main has been updated by kp:

URL:
https://cgit.FreeBSD.org/src/commit/?id=3D7e7f88001= d7dfec83cd7568369be6a587d4a51ff

commit 7e7f88001d7dfec83cd7568369be6a587d4a51ff
Author: Kristof Provost kp@FreeBSD.= org
AuthorDate: 2025-02-07 10:29:26 +0000
Commit: Kristof Provost kp@FreeBSD.= org
CommitDate: 2025-02-14 17:47:52 +0000

  =
pf: use time_t for storing time_t values
      No change to the underlying type, so no ABI change.
      We define __time_t as uint64_t if __LP64__, otherwise

uint32_t,
and only define LP64 if long is 64 bits.
In other words: __time_t =3D=3D long.
ok henning@ deraadt@
Obtained from: OpenBSD, guenther guenther@openbsd.org,
6c1b69a0ff
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: ht= tps://reviews.freebsd.org/D48963

This is an ABI change on non-i386 32-bit platforms in Fre= eBSD since
they
all use a 64-bit type for time_t that is not the same size as long.
Not
sure if the ABI change matters on FreeBSD though?

It wasn=E2=80=99t intended to be an ABI change, hence the= commit message. It
appears that=E2=80=99s only correct for x86 though.

I assumed the commit message was from OpenBSD as the comm= ents about
defining time_t conditional on LP64 are not correct on F= reeBSD
(each arch defines a __time_t in <machine/_types.h>, though amd64 and i386 share x86/include/_types.h which does use an #ifdef that
perhaps is the source of confusion?)

Partially. The =E2=80=9CWe define __time_t as uint64_t if= LP64, otherwise uint32_t,
and only define LP64 if long is 64 bits.
In other words: __time_t =3D=3D long.=E2=80=9D bit was me, and that was c= orrect for x86, but not for other machines.

That=E2=80=99s what I got wrong.

So we=E2=80=99re only talking about armv7 and ppc32, if I= =E2=80=99m not forgetting
anything. The former is on the removal list already, and the latter .. well, I don=E2=80=99t know how many users there are. Both are likely to b= e
embedded platforms where the ABI change is going to be even less
relevant (because it really only matters if the kernel and userspace are<= br> not updated together, and these are going to be embedded devices that
= are far more likely to have everything updated simultaneously).> So I=E2= =80=99m unsure about what to do. I can revert this and we can just
carry this (trivial) diff to OpenBSD forever, or we can ignore the ABI breakage given the above. I=E2=80=99m not inclined to do anything more involved though.

Do you have any thoughts?

To be clear, armv7 is planned to be around a bit longer t= han other 32-bit
platforms. That said, 32-bit plaforms are all Tier 2, so an ABI breakage=
in main is not necessarily the end of the world. Presumably these struct= ures
aren't used much in ports but only in base system tools anyway? (That is what my question about the ABI change mattering was trying to allude t= o)

This affects ioctl calls, so it can and probably does aff= ect ports. There aren=E2=80=99t many but still a few that use the ioctl i= nterface (things like pftop and snort).
I don=E2=80=99t know offhand if they actually use any of the affected cal= ls though.

I could also revert this now and deal with it when I get = around to converting the relevant ioctl calls to netlink. That=E2=80=99s = ongoing and still aspirationally (but getting less likely) to be complete= d before we branch 15. That may be a better point to make this change, be= cause once the netlink conversion is complete the next major release will= remove the entire ioctl interface, and that=E2=80=99s a breaking change = anyway.

I agree with Justin that this is not something to MFC.

There=E2=80=99s no plan to MFC this (or any of the other = recent pf work, for that matter).

Best regards,
Kristof

--=_MailMate_5CA74BE6-CED5-4A7E-8982-9DB900BE9221_=--