Re: [PATCH ethtool v1] netlink: add master/slave configuration support

[Michal Kubecek]

On Tue, Jun 09, 2020 at 01:29:42PM -0700, Kees Cook wrote:
> On Tue, Jun 09, 2020 at 01:05:17PM -0700, David Miller wrote:
> > From: Kees Cook <keescook@xxxxxxxxxxxx>
> > Date: Tue, 9 Jun 2020 12:49:48 -0700
> > 
> > > Okay, for now, how about:
> > > 
> > > - If we're dealing with an existing spec, match the language.
> > 
> > Yes.
> > 
> > > - If we're dealing with a new spec, ask the authors to fix their language.
> > 
> > Please be more specific about "new", if it's a passed and ratified standard
> > then to me it is "existing".
> 
> Sure. But many kernel devs are also interacting with specifications as
> they're being developed. We can have an eye out for this when we're in
> such positions.

I fully agree that this is the right place to raise concern like this.
But I have to remind that here we are talking about implementation of an
existing standard.

> > > - If a new version of a spec has updated its language, adjust the kernel's.
> > 
> > Unless you're willing to break UAPI, which I'm not, I don't see how this is
> > tenable.
> 
> We'll get there when we get there. I honestly don't think any old spec is
> actually going to change their language; I look forward to being proven
> wrong. But many times there is no UAPI. If it's some register states
> between driver and hardware, no users sees or cares what the register
> is named.

In my eyes, that would be one less reason to invent different names for
them just to avoid someone being offended.

If you look into include/linux/tcp.h, you can find this comment near the
beginning of struct tcp_sock definition:

/*
 *	RFC793 variables by their proper names. This means you can
 *	read the code and the spec side by side (and laugh ...)
 *	See RFC793 and RFC1122. The RFC writes these in capitals.
 */

It is a bit confusing now as there have been some reorderings but you
can see that even if it's an internal kernel data structure, original
author(s) of the code considered it beneficial to use the same names as
standard for the state variables so that people reading the code don't
need a translation table between state variables in kernel code and in
standards.

The same IMHO holds for your example with register states or names:
I believe it is highly beneficial to make them consistent with technical
documentation. There are even cases where we violate kernel coding style
(e.g. by using camelcase) to match the names from specification.

> > This is why I'm saying, just make sure new specs use language that is
> > less controversial.  Then we just use what the specs use.
> > 
> > Then you don't have to figure out what to do about established UAPIs
> > and such, it's a non-issue.
> 
> Yes, but there are places where people use these terms where they are
> NOT part of specs, and usually there is actually _better_ terminology
> to be used, and we can easily stop adding those. And we can start to
> rename old "independent" cases too.

Surely there are - but it is not the case here.

Michal