Re: [PATCH net-next] net: ieee802154: Fix compilation error when CONFIG_IEEE802154_NL802154_EXPERIMENTAL is disabled

[Leon Romanovsky <leon@kernel.org>]

On Thu, Sep 01, 2022 at 08:50:16AM -0400, Alexander Aring wrote:
> Hi,
> 
> On Thu, Sep 1, 2022 at 2:38 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Wed, Aug 31, 2022 at 02:09:47PM -0700, Jakub Kicinski wrote:
> > > On Wed, 31 Aug 2022 22:59:14 +0200 Stefan Schmidt wrote:
> > > > I was swamped today and I am only now finding time to go through mail.
> > > >
> > > > Given the problem these ifdef are raising I am ok with having these
> > > > commands exposed without them.
> > > >
> > > > Our main reason for having this feature marked as experimental is that
> > > > it does not have much exposure and we fear that some of it needs rewrites.
> > > >
> > > > If that really is going to happen we will simply treat the current
> > > > commands as reserved/burned and come up with other ones if needed. While
> > > > I hope this will not be needed it is a fair plan for mitigating this.
> > >
> > > Thanks for the replies. I keep going back and forth in my head on
> > > what's better - un-hiding or just using NL802154_CMD_SET_WPAN_PHY_NETNS + 1
> > > as the start of validation, since it's okay to break experimental commands.
> > >
> > > Any preference?
> >
> > Jakub,
> >
> > There is no such thing like experimental UAPI. Once you put something
> > in UAPI headers and/or allowed users to issue calls from userspace
> > to kernel, they can use it. We don't control how users compile their
> > kernels.
> >
> > So it is not break "experimental commands", but break commands that
> > maybe shouldn't exist in first place.
> >
> > nl802154 code suffers from two basic mistakes:
> > 1. User visible defines are not part of UAPI headers. For example,
> > include/net/nl802154.h should be in include/uapi/net/....
> 
> yes, but no because then this will end in breaking UAPI because it
> will be exported to uapi headers if we change them?
> For now we say everybody needs to copy the header on their own into
> their user space application if they want to use the API which means
> it fits for the kernel that they copied from.

It is not how UAPI works. Once you allowed me to send ID number XXX to
the kernel without any header file. I can use it directly, so "hiding"
files from users make their development harder, but not impossible.

Basically, this is how vendoring and fuzzing works.

> 
> > 2. Used Kconfig option for pseudo-UAPI header.
> >
> > In this specific case, I checked that Fedora didn't enable this
> > CONFIG_IEEE802154_NL802154_EXPERIMENTAL knob, but someone needs
> > to check debian and other distros too.
> >
> 
> I would remove the CONFIG_IEEE802154_NL802154_EXPERIMENTAL config
> option but don't move the header to "include/uapi/..." which means
> that the whole nl802154 UAPI (and some others UAPIs) are still
> experimental because it's not part of UAPI "directory".
> btw: the whole subsystem is still experimental because f4671a90c418
> ("net/ieee802154: remove depends on CONFIG_EXPERIMENTAL") was never
> acked by any maintainer... but indeed has other reasons why it got
> removed.

I don't know anything about NL802154, just trying to explain that UAPI
rules are both relevant to binary and compilation compatibility.

In your case, concept of CONFIG_IEEE802154_NL802154_EXPERIMENTAL breaks
binary compatibility.

Thanks

> 
> - Alex
>