Re: git: fe33e0ab83d1 - main - ifnet/API: Move the IfAPI from if_var.h to if.h

On 12 Jan 2023, at 18:01, Justin Hibbits wrote:

> Hi Gleb,
>
> On Thu, 12 Jan 2023 15:37:43 -0800
> Gleb Smirnoff <glebius@freebsd.org> 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=fe33e0ab83d1fbc3c5cd4a2591ba0036e47b1fec
>> J>
>> J> commit fe33e0ab83d1fbc3c5cd4a2591ba0036e47b1fec
>> J> Author:     Justin Hibbits <jhibbits@FreeBSD.org>
>> J> AuthorDate: 2023-01-11 16:56:39 +0000
>> J> Commit:     Justin Hibbits <jhibbits@FreeBSD.org>
>> 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.html
>>
>
> 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 original
design goal when if_var.h was created (yes, long ago).  I’ll 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 require
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