From nobody Fri Jan 13 00:39:37 2023 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 4NtMxl36Jcz2sg4W; Fri, 13 Jan 2023 00:39:39 +0000 (UTC) (envelope-from mike@karels.net) Received: from mail.karels.net (mail.karels.net [216.160.39.52]) by mx1.freebsd.org (Postfix) with ESMTP id 4NtMxk68Xjz3Pl2; Fri, 13 Jan 2023 00:39:38 +0000 (UTC) (envelope-from mike@karels.net) Authentication-Results: mx1.freebsd.org; none Received: from mail.karels.net (localhost [127.0.0.1]) by mail.karels.net (8.16.1/8.16.1) with ESMTP id 30D0dcKB076122; Thu, 12 Jan 2023 18:39:38 -0600 (CST) (envelope-from mike@karels.net) Received: from [10.0.2.130] ([10.0.1.1]) by mail.karels.net with ESMTPSA id o8LyBEqowGNYKQEA4+wvSQ (envelope-from ); Thu, 12 Jan 2023 18:39:38 -0600 From: Mike Karels To: Justin Hibbits Cc: Gleb Smirnoff , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: fe33e0ab83d1 - main - ifnet/API: Move the IfAPI from if_var.h to if.h Date: Thu, 12 Jan 2023 18:39:37 -0600 X-Mailer: MailMate (1.14r5933) Message-ID: <5E017ED9-DF61-4B14-83BE-119C3F46C816@karels.net> In-Reply-To: <20230112190114.238a3662@ralga-linux> References: <202301121620.30CGKtpM047283@gitrepo.freebsd.org> <20230112190114.238a3662@ralga-linux> 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4NtMxk68Xjz3Pl2 X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:209, ipnet:216.160.36.0/22, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 12 Jan 2023, at 18:01, Justin Hibbits wrote: > Hi Gleb, > > On Thu, 12 Jan 2023 15:37:43 -0800 > Gleb Smirnoff wrote: > >> Justin, >> >> On Thu, Jan 12, 2023 at 04:20:55PM +0000, Justin Hibbits wrote: >> J> The branch main has been updated by jhibbits: >> J> >> J> URL: >> J> https://cgit.FreeBSD.org/src/commit/?id=3Dfe33e0ab83d1fbc3c5cd4a259= 1ba0036e47b1fec >> J> >> J> commit fe33e0ab83d1fbc3c5cd4a2591ba0036e47b1fec >> J> Author: Justin Hibbits >> J> AuthorDate: 2023-01-11 16:56:39 +0000 >> J> Commit: Justin Hibbits >> J> CommitDate: 2023-01-12 16:25:41 +0000 >> J> >> J> ifnet/API: Move the IfAPI from if_var.h to if.h >> J> >> J> Summary: >> J> The "public" KPI for ifnet belongs in net/if.h, with >> J> net/if_var.h being implementation details for the netstack. This >> J> is the next step in enforcing that separation. >> J> >> J> Reviewed by: melifaro >> J> Sponsored by: Juniper Networks, Inc. >> J> Differential Revision: https://reviews.freebsd.org/D38030 >> >> I'm very sorry for not reviewing the D38030 in time. I didn't have >> electricity for a few days in a raw. >> >> I agree that DrvAPI needs to be isolated from stack visible header >> file, but I can't agree that if.h needs to be polluted with it. First >> of all net/if.h is a SuS/POSIX mandated header[1], with a very >> limited set of features announced and perfectionist-wise we >> definitely shouldn't pollute it with our kernel driver API. Second, >> it of course has lots of stuff beyond POSIX, but it still was 99% >> userland visible (before this change) and ideally it should be a 100% >> userland header. >> >> I think this change as is needs to be reverted. And to properly >> isolate DrvAPI we have two options: >> >> 1) Leave it in if_var.h and start a new header where we will move >> stuff that should stay private from drivers. Drivers keep using >> if_var.h. 2) Create a new header with DrvAPI. Drivers to stop useing >> if_var.h and switch to new header file. >> >> I personally prefer 1), as it follows what we have started to do long >> time ago, see c29e1ad9304, c3322cb91ca, 76039bc84fa, eedc7fd9e87. >> >> [1] >> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/net_if.h.htm= l >> > > I'll revert this, but I think we should discuss further how to approach= > this, since on our call last week I got the impression that the DrvAPI > does belong in if.h, with if_var.h being exclusively used by netstack. I agree that if.h should be primarily user-visible. That was the origina= l design goal when if_var.h was created (yes, long ago). I=E2=80=99ll note= that there are about 500 references to if_var.h in the kernel, with about half= in drivers. I suspect that if the stack-visible stuff stays in if_var.h,= many of the stack files will work as is. The drivers will presumably req= uire modification anyway, so including a new header in files that are already being touched seems reasonable. I think the new interfaces belong in a new header. Mike