diff mbox series

[iproute2] ip link: add sub-command to view and change DSA master

Message ID 20220904190025.813574-1-vladimir.oltean@nxp.com (mailing list archive)
State RFC
Delegated to: David Ahern
Headers show
Series [iproute2] ip link: add sub-command to view and change DSA master | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Vladimir Oltean Sept. 4, 2022, 7 p.m. UTC
Support the "dsa" kind of rtnl_link_ops exported by the kernel, and
export reads/writes to IFLA_DSA_MASTER.

Examples:

$ ip link set swp0 type dsa master eth1

$ ip -d link show dev swp0
    (...)
    dsa master eth0

$ ip -d -j link show swp0
[
	{
		"link": "eth1",
		"linkinfo": {
			"info_kind": "dsa",
			"info_data": {
				"master": "eth1"
			}
		},
	}
]

Note that by construction and as shown in the example, the IFLA_LINK
reported by a DSA user port is identical to what is reported through
IFLA_DSA_MASTER. However IFLA_LINK is not writable, and overloading its
meaning to make it writable would clash with other users of IFLA_LINK
(vlan etc) for which writing this property does not make sense.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
This is semi-RFC, as in: pls don't apply yet, because the kernel patches
weren't merged yet (according to feedback so far, it's likely that the
proposed UAPI won't change, but still).

 include/uapi/linux/if_link.h | 10 ++++++
 ip/Makefile                  |  2 +-
 ip/iplink_dsa.c              | 67 ++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 ip/iplink_dsa.c

Comments

Stephen Hemminger Sept. 6, 2022, 3:29 p.m. UTC | #1
On Sun,  4 Sep 2022 22:00:25 +0300
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> Support the "dsa" kind of rtnl_link_ops exported by the kernel, and
> export reads/writes to IFLA_DSA_MASTER.
> 
> Examples:
> 
> $ ip link set swp0 type dsa master eth1
> 
> $ ip -d link show dev swp0
>     (...)
>     dsa master eth0
> 
> $ ip -d -j link show swp0
> [
> 	{
> 		"link": "eth1",
> 		"linkinfo": {
> 			"info_kind": "dsa",
> 			"info_data": {
> 				"master": "eth1"
> 			}
> 		},
> 	}
> ]
> 
> Note that by construction and as shown in the example, the IFLA_LINK
> reported by a DSA user port is identical to what is reported through
> IFLA_DSA_MASTER. However IFLA_LINK is not writable, and overloading its
> meaning to make it writable would clash with other users of IFLA_LINK
> (vlan etc) for which writing this property does not make sense.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Using the term master is an unfortunate choice.
Although it is common practice in Linux it is not part of any
current standard and goes against the Linux Foundation non-inclusive
naming policy.
Vladimir Oltean Sept. 6, 2022, 4:41 p.m. UTC | #2
On Tue, Sep 06, 2022 at 08:29:07AM -0700, Stephen Hemminger wrote:
> On Sun,  4 Sep 2022 22:00:25 +0300
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> 
> > Support the "dsa" kind of rtnl_link_ops exported by the kernel, and
> > export reads/writes to IFLA_DSA_MASTER.
> > 
> > Examples:
> > 
> > $ ip link set swp0 type dsa master eth1
> > 
> > $ ip -d link show dev swp0
> >     (...)
> >     dsa master eth0
> > 
> > $ ip -d -j link show swp0
> > [
> > 	{
> > 		"link": "eth1",
> > 		"linkinfo": {
> > 			"info_kind": "dsa",
> > 			"info_data": {
> > 				"master": "eth1"
> > 			}
> > 		},
> > 	}
> > ]
> > 
> > Note that by construction and as shown in the example, the IFLA_LINK
> > reported by a DSA user port is identical to what is reported through
> > IFLA_DSA_MASTER. However IFLA_LINK is not writable, and overloading its
> > meaning to make it writable would clash with other users of IFLA_LINK
> > (vlan etc) for which writing this property does not make sense.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> 
> Using the term master is an unfortunate choice.
> Although it is common practice in Linux it is not part of any
> current standard and goes against the Linux Foundation non-inclusive
> naming policy.

Concretely, what is it that you propose?
Stephen Hemminger Sept. 6, 2022, 4:55 p.m. UTC | #3
On Tue, 6 Sep 2022 16:41:17 +0000
Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> On Tue, Sep 06, 2022 at 08:29:07AM -0700, Stephen Hemminger wrote:
> > On Sun,  4 Sep 2022 22:00:25 +0300
> > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> >   
> > > Support the "dsa" kind of rtnl_link_ops exported by the kernel, and
> > > export reads/writes to IFLA_DSA_MASTER.
> > > 
> > > Examples:
> > > 
> > > $ ip link set swp0 type dsa master eth1
> > > 
> > > $ ip -d link show dev swp0
> > >     (...)
> > >     dsa master eth0
> > > 
> > > $ ip -d -j link show swp0
> > > [
> > > 	{
> > > 		"link": "eth1",
> > > 		"linkinfo": {
> > > 			"info_kind": "dsa",
> > > 			"info_data": {
> > > 				"master": "eth1"
> > > 			}
> > > 		},
> > > 	}
> > > ]
> > > 
> > > Note that by construction and as shown in the example, the IFLA_LINK
> > > reported by a DSA user port is identical to what is reported through
> > > IFLA_DSA_MASTER. However IFLA_LINK is not writable, and overloading its
> > > meaning to make it writable would clash with other users of IFLA_LINK
> > > (vlan etc) for which writing this property does not make sense.
> > > 
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---  
> > 
> > Using the term master is an unfortunate choice.
> > Although it is common practice in Linux it is not part of any
> > current standard and goes against the Linux Foundation non-inclusive
> > naming policy.  
> 
> Concretely, what is it that you propose?

Maybe "switch" instead of "master"?
Vladimir Oltean Sept. 6, 2022, 7:13 p.m. UTC | #4
On Tue, Sep 06, 2022 at 09:55:17AM -0700, Stephen Hemminger wrote:
> > > Using the term master is an unfortunate choice.
> > > Although it is common practice in Linux it is not part of any
> > > current standard and goes against the Linux Foundation non-inclusive
> > > naming policy.  
> > 
> > Concretely, what is it that you propose?
> 
> Maybe "switch" instead of "master"?

"Switch" and "DSA master" are not interchangeable concepts. A DSA master
is a regular Ethernet controller that is connected to a local (onboard
or embedded) switch and handles its management traffic. The use of the
term has existed since the introduction of DSA in 2008 and has reached a
wide audience among users through the papers that popularized DSA later, mainly
https://legacy.netdevconf.info/2.1/papers/distributed-switch-architecture.pdf
Whereas a switch is a multi-port Ethernet device that handles L2
forwarding by MAC DA and VLAN ID, essentially containing a hardware
implementation of the Linux bridge.

[ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]
Andrew Lunn Sept. 6, 2022, 8:05 p.m. UTC | #5
> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]

Unfortunately, it is not gender neutral, which i assume is a
requirement?

Plus the plural is also schnauzer, which would make your current
multiple CPU/schnauzer patches confusing, unless you throw the rule
book out and use English pluralisation.

	 Andrew
Florian Fainelli Sept. 6, 2022, 8:33 p.m. UTC | #6
On 9/6/2022 1:05 PM, Andrew Lunn wrote:
>> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]
> 
> Unfortunately, it is not gender neutral, which i assume is a
> requirement?
> 
> Plus the plural is also schnauzer, which would make your current
> multiple CPU/schnauzer patches confusing, unless you throw the rule
> book out and use English pluralisation.

What a nice digression, I had no idea you two mastered German that well 
:). How about "conduit" or "mgmt_port" or some variant in the same lexicon?
Stephen Hemminger Sept. 6, 2022, 9:17 p.m. UTC | #7
On Tue, 6 Sep 2022 13:33:09 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 9/6/2022 1:05 PM, Andrew Lunn wrote:
> >> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]  
> > 
> > Unfortunately, it is not gender neutral, which i assume is a
> > requirement?
> > 
> > Plus the plural is also schnauzer, which would make your current
> > multiple CPU/schnauzer patches confusing, unless you throw the rule
> > book out and use English pluralisation.  
> 
> What a nice digression, I had no idea you two mastered German that well 
> :). How about "conduit" or "mgmt_port" or some variant in the same lexicon?

Is there an IEEE or PCI standard for this? What is used there?
Florian Fainelli Sept. 6, 2022, 9:34 p.m. UTC | #8
On 9/6/2022 2:17 PM, Stephen Hemminger wrote:
> On Tue, 6 Sep 2022 13:33:09 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 9/6/2022 1:05 PM, Andrew Lunn wrote:
>>>> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]
>>>
>>> Unfortunately, it is not gender neutral, which i assume is a
>>> requirement?
>>>
>>> Plus the plural is also schnauzer, which would make your current
>>> multiple CPU/schnauzer patches confusing, unless you throw the rule
>>> book out and use English pluralisation.
>>
>> What a nice digression, I had no idea you two mastered German that well
>> :). How about "conduit" or "mgmt_port" or some variant in the same lexicon?
> 
> Is there an IEEE or PCI standard for this? What is used there?

This is not covered by any standard as it is really attaching a managed 
switch to an Ethernet controller and that is product design more than 
anything. Obviously there is a standard for the electrical interfaces, 
but it does not care whether you have a switch, another Ethernet 
controller, an Ethernet PHY or something else entirely.

Vendors refer to "management port" or even "in-band management port" 
though all that the Ethernet controller does is just pass through 
Ethernet frames to manage the switch.
Andrew Lunn Sept. 6, 2022, 9:37 p.m. UTC | #9
On Tue, Sep 06, 2022 at 02:17:19PM -0700, Stephen Hemminger wrote:
> On Tue, 6 Sep 2022 13:33:09 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> > On 9/6/2022 1:05 PM, Andrew Lunn wrote:
> > >> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]  
> > > 
> > > Unfortunately, it is not gender neutral, which i assume is a
> > > requirement?
> > > 
> > > Plus the plural is also schnauzer, which would make your current
> > > multiple CPU/schnauzer patches confusing, unless you throw the rule
> > > book out and use English pluralisation.  
> > 
> > What a nice digression, I had no idea you two mastered German that well 
> > :). How about "conduit" or "mgmt_port" or some variant in the same lexicon?
> 
> Is there an IEEE or PCI standard for this? What is used there?

The whole DSA concept is comes from Marvell.

commit 91da11f870f00a3322b81c73042291d7f0be5a17
Author: Lennert Buytenhek <buytenh@wantstofly.org>
Date:   Tue Oct 7 13:44:02 2008 +0000

    net: Distributed Switch Architecture protocol support
    
    Distributed Switch Architecture is a protocol for managing hardware
    switch chips.  It consists of a set of MII management registers and
    commands to configure the switch, and an ethernet header format to
    signal which of the ports of the switch a packet was received from
    or is intended to be sent to.
    
    The switches that this driver supports are typically embedded in
    access points and routers, and a typical setup with a DSA switch
    looks something like this:
    
            +-----------+       +-----------+
            |           | RGMII |           |
            |           +-------+           +------ 1000baseT MDI ("WAN")
            |           |       |  6-port   +------ 1000baseT MDI ("LAN1")
            |    CPU    |       |  ethernet +------ 1000baseT MDI ("LAN2")
            |           |MIImgmt|  switch   +------ 1000baseT MDI ("LAN3")
            |           +-------+  w/5 PHYs +------ 1000baseT MDI ("LAN4")
            |           |       |           |
            +-----------+       +-----------+
    
    The switch driver presents each port on the switch as a separate
    network interface to Linux, polls the switch to maintain software
    link state of those ports, forwards MII management interface
    accesses to those network interfaces (e.g. as done by ethtool) to
    the switch, and exposes the switch's hardware statistics counters
    via the appropriate Linux kernel interfaces.
    
    This initial patch supports the MII management interface register
    layout of the Marvell 88E6123, 88E6161 and 88E6165 switch chips, and
    supports the "Ethertype DSA" packet tagging format.
    
    (There is no officially registered ethertype for the Ethertype DSA
    packet format, so we just grab a random one.  The ethertype to use
    is programmed into the switch, and the switch driver uses the value
    of ETH_P_EDSA for this, so this define can be changed at any time in
    the future if the one we chose is allocated to another protocol or
    if Ethertype DSA gets its own officially registered ethertype, and
    everything will continue to work.)

That first patch from 2008 uses the name master.

The Marvell datasheets tend to just refer to the management CPU, and
sending to / receiving from frames via one of the switches
ports. There is no reference to the network interface the management
CPU must have in order to receive/send such frames.

	Andrew
Vladimir Oltean Sept. 8, 2022, 12:51 p.m. UTC | #10
On Tue, Sep 06, 2022 at 01:33:09PM -0700, Florian Fainelli wrote:
> On 9/6/2022 1:05 PM, Andrew Lunn wrote:
> > > [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]
> > 
> > Unfortunately, it is not gender neutral, which i assume is a
> > requirement?
> > 
> > Plus the plural is also schnauzer, which would make your current
> > multiple CPU/schnauzer patches confusing, unless you throw the rule
> > book out and use English pluralisation.
> 
> What a nice digression, I had no idea you two mastered German that well :).
> How about "conduit" or "mgmt_port" or some variant in the same lexicon?

Proposing any alternative naming raises the question how far you want to
go with the alternative name. No user of DSA knows the "conduit interface"
or "management port" or whatnot by any other name except "DSA master".
What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
which clearly and consistently uses the 'master' name everywhere?
Do we replace 'master' with something else and act as if it was never
named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
UAPI and explain in the documentation "oh yeah, that's how you change
the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
then?" "Well...."

Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
also change that to reflect the new terminology, or do we just have
documentation stating one thing and the code another?

At this stage, I'm much more likely to circumvent all of this, and avoid
triggering anyone by making a writable IFLA_LINK be the mechanism through
which we change the DSA master.
David Ahern Sept. 8, 2022, 2:08 p.m. UTC | #11
On 9/8/22 6:51 AM, Vladimir Oltean wrote:
> On Tue, Sep 06, 2022 at 01:33:09PM -0700, Florian Fainelli wrote:
>> On 9/6/2022 1:05 PM, Andrew Lunn wrote:
>>>> [ Alternative answer: how about "schnauzer"? I always liked how that word sounds. ]
>>>
>>> Unfortunately, it is not gender neutral, which i assume is a
>>> requirement?
>>>
>>> Plus the plural is also schnauzer, which would make your current
>>> multiple CPU/schnauzer patches confusing, unless you throw the rule
>>> book out and use English pluralisation.
>>
>> What a nice digression, I had no idea you two mastered German that well :).
>> How about "conduit" or "mgmt_port" or some variant in the same lexicon?
> 
> Proposing any alternative naming raises the question how far you want to
> go with the alternative name. No user of DSA knows the "conduit interface"
> or "management port" or whatnot by any other name except "DSA master".
> What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> which clearly and consistently uses the 'master' name everywhere?
> Do we replace 'master' with something else and act as if it was never
> named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> UAPI and explain in the documentation "oh yeah, that's how you change
> the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> then?" "Well...."
> 
> Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> also change that to reflect the new terminology, or do we just have
> documentation stating one thing and the code another?
> 
> At this stage, I'm much more likely to circumvent all of this, and avoid
> triggering anyone by making a writable IFLA_LINK be the mechanism through
> which we change the DSA master.

IMHO, 'master' should be an allowed option giving the precedence of
existing code and existing terminology. An alternative keyword can be
used for those that want to avoid use of 'master' in scripts. vrf is an
example of this -- you can specify 'vrf <device>' as a keyword instead
of 'master <vrf>'
Stephen Hemminger Sept. 8, 2022, 2:25 p.m. UTC | #12
On Thu, 8 Sep 2022 08:08:23 -0600
David Ahern <dsahern@kernel.org> wrote:

> > 
> > Proposing any alternative naming raises the question how far you want to
> > go with the alternative name. No user of DSA knows the "conduit interface"
> > or "management port" or whatnot by any other name except "DSA master".
> > What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> > which clearly and consistently uses the 'master' name everywhere?
> > Do we replace 'master' with something else and act as if it was never
> > named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> > UAPI and explain in the documentation "oh yeah, that's how you change
> > the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> > then?" "Well...."
> > 
> > Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> > also change that to reflect the new terminology, or do we just have
> > documentation stating one thing and the code another?
> > 
> > At this stage, I'm much more likely to circumvent all of this, and avoid
> > triggering anyone by making a writable IFLA_LINK be the mechanism through
> > which we change the DSA master.  
> 
> IMHO, 'master' should be an allowed option giving the precedence of
> existing code and existing terminology. An alternative keyword can be
> used for those that want to avoid use of 'master' in scripts. vrf is an
> example of this -- you can specify 'vrf <device>' as a keyword instead
> of 'master <vrf>'

Agreed, just wanted to start discussion of alternative wording.
Vladimir Oltean Sept. 8, 2022, 4:11 p.m. UTC | #13
On Thu, Sep 08, 2022 at 07:25:19AM -0700, Stephen Hemminger wrote:
> On Thu, 8 Sep 2022 08:08:23 -0600 David Ahern <dsahern@kernel.org> wrote:
> > > Proposing any alternative naming raises the question how far you want to
> > > go with the alternative name. No user of DSA knows the "conduit interface"
> > > or "management port" or whatnot by any other name except "DSA master".
> > > What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> > > which clearly and consistently uses the 'master' name everywhere?
> > > Do we replace 'master' with something else and act as if it was never
> > > named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> > > UAPI and explain in the documentation "oh yeah, that's how you change
> > > the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> > > then?" "Well...."
> > > 
> > > Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> > > also change that to reflect the new terminology, or do we just have
> > > documentation stating one thing and the code another?
> > > 
> > > At this stage, I'm much more likely to circumvent all of this, and avoid
> > > triggering anyone by making a writable IFLA_LINK be the mechanism through
> > > which we change the DSA master.  
> > 
> > IMHO, 'master' should be an allowed option giving the precedence of
> > existing code and existing terminology. An alternative keyword can be
> > used for those that want to avoid use of 'master' in scripts. vrf is an
> > example of this -- you can specify 'vrf <device>' as a keyword instead
> > of 'master <vrf>'
> 
> Agreed, just wanted to start discussion of alternative wording.

So are we or are we not in the clear with IFLA_DSA_MASTER and
"ip link set ... type dsa master ..."? What does being in the clear even
mean technically, and where can I find more details about the policy
which you just mentioned? Like is it optional or mandatory, was there
any public debate surrounding the motivation for flagging some words,
how is it enforced, are there official exceptions, etc?

In a normal code review environment I'd be receiving feedback and a
concrete suggestion for a change from an actual person who has an actual
issue (theoretical or practical, but an issue that he/she can express
and stand for) with the code in its current form. I would not be expected
to act based on something whose fundamental substantiation is hearsay.

In other words, you can't "just" start a discussion of alternative
wording, without actually going through any of the specifics. Or rather,
you can, but you're likely to be ignored, just like you would have been,
were the comment related to a technical aspect.
Florian Fainelli Sept. 8, 2022, 4:35 p.m. UTC | #14
On 9/8/2022 9:11 AM, Vladimir Oltean wrote:
> On Thu, Sep 08, 2022 at 07:25:19AM -0700, Stephen Hemminger wrote:
>> On Thu, 8 Sep 2022 08:08:23 -0600 David Ahern <dsahern@kernel.org> wrote:
>>>> Proposing any alternative naming raises the question how far you want to
>>>> go with the alternative name. No user of DSA knows the "conduit interface"
>>>> or "management port" or whatnot by any other name except "DSA master".
>>>> What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
>>>> which clearly and consistently uses the 'master' name everywhere?
>>>> Do we replace 'master' with something else and act as if it was never
>>>> named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
>>>> UAPI and explain in the documentation "oh yeah, that's how you change
>>>> the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
>>>> then?" "Well...."
>>>>
>>>> Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
>>>> also change that to reflect the new terminology, or do we just have
>>>> documentation stating one thing and the code another?
>>>>
>>>> At this stage, I'm much more likely to circumvent all of this, and avoid
>>>> triggering anyone by making a writable IFLA_LINK be the mechanism through
>>>> which we change the DSA master.
>>>
>>> IMHO, 'master' should be an allowed option giving the precedence of
>>> existing code and existing terminology. An alternative keyword can be
>>> used for those that want to avoid use of 'master' in scripts. vrf is an
>>> example of this -- you can specify 'vrf <device>' as a keyword instead
>>> of 'master <vrf>'
>>
>> Agreed, just wanted to start discussion of alternative wording.
> 
> So are we or are we not in the clear with IFLA_DSA_MASTER and
> "ip link set ... type dsa master ..."? What does being in the clear even
> mean technically, and where can I find more details about the policy
> which you just mentioned? Like is it optional or mandatory, was there
> any public debate surrounding the motivation for flagging some words,
> how is it enforced, are there official exceptions, etc?

The "bonding" driver topic has some good context:

https://lkml.iu.edu/hypermail/linux/kernel/2010.0/02186.html
Stephen Hemminger Sept. 8, 2022, 4:39 p.m. UTC | #15
On Thu, 8 Sep 2022 09:35:03 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 9/8/2022 9:11 AM, Vladimir Oltean wrote:
> > On Thu, Sep 08, 2022 at 07:25:19AM -0700, Stephen Hemminger wrote:  
> >> On Thu, 8 Sep 2022 08:08:23 -0600 David Ahern <dsahern@kernel.org> wrote:  
> >>>> Proposing any alternative naming raises the question how far you want to
> >>>> go with the alternative name. No user of DSA knows the "conduit interface"
> >>>> or "management port" or whatnot by any other name except "DSA master".
> >>>> What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> >>>> which clearly and consistently uses the 'master' name everywhere?
> >>>> Do we replace 'master' with something else and act as if it was never
> >>>> named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> >>>> UAPI and explain in the documentation "oh yeah, that's how you change
> >>>> the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> >>>> then?" "Well...."
> >>>>
> >>>> Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> >>>> also change that to reflect the new terminology, or do we just have
> >>>> documentation stating one thing and the code another?
> >>>>
> >>>> At this stage, I'm much more likely to circumvent all of this, and avoid
> >>>> triggering anyone by making a writable IFLA_LINK be the mechanism through
> >>>> which we change the DSA master.  
> >>>
> >>> IMHO, 'master' should be an allowed option giving the precedence of
> >>> existing code and existing terminology. An alternative keyword can be
> >>> used for those that want to avoid use of 'master' in scripts. vrf is an
> >>> example of this -- you can specify 'vrf <device>' as a keyword instead
> >>> of 'master <vrf>'  
> >>
> >> Agreed, just wanted to start discussion of alternative wording.  
> > 
> > So are we or are we not in the clear with IFLA_DSA_MASTER and
> > "ip link set ... type dsa master ..."? What does being in the clear even
> > mean technically, and where can I find more details about the policy
> > which you just mentioned? Like is it optional or mandatory, was there
> > any public debate surrounding the motivation for flagging some words,
> > how is it enforced, are there official exceptions, etc?  
> 
> The "bonding" driver topic has some good context:
> 
> https://lkml.iu.edu/hypermail/linux/kernel/2010.0/02186.html

On another mail thread, discussed naming with the IEEE 802 committee.
And they said master/slave is not used in either the current version
of the bridging or bonding standards.
Benjamin Poirier Sept. 9, 2022, 6:09 a.m. UTC | #16
On 2022-09-08 16:11 +0000, Vladimir Oltean wrote:
> On Thu, Sep 08, 2022 at 07:25:19AM -0700, Stephen Hemminger wrote:
> > On Thu, 8 Sep 2022 08:08:23 -0600 David Ahern <dsahern@kernel.org> wrote:
> > > > Proposing any alternative naming raises the question how far you want to
> > > > go with the alternative name. No user of DSA knows the "conduit interface"
> > > > or "management port" or whatnot by any other name except "DSA master".
> > > > What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> > > > which clearly and consistently uses the 'master' name everywhere?
> > > > Do we replace 'master' with something else and act as if it was never
> > > > named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> > > > UAPI and explain in the documentation "oh yeah, that's how you change
> > > > the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> > > > then?" "Well...."
> > > > 
> > > > Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> > > > also change that to reflect the new terminology, or do we just have
> > > > documentation stating one thing and the code another?
> > > > 
> > > > At this stage, I'm much more likely to circumvent all of this, and avoid
> > > > triggering anyone by making a writable IFLA_LINK be the mechanism through
> > > > which we change the DSA master.  
> > > 
> > > IMHO, 'master' should be an allowed option giving the precedence of
> > > existing code and existing terminology. An alternative keyword can be
> > > used for those that want to avoid use of 'master' in scripts. vrf is an
> > > example of this -- you can specify 'vrf <device>' as a keyword instead
> > > of 'master <vrf>'
> > 
> > Agreed, just wanted to start discussion of alternative wording.
> 
> So are we or are we not in the clear with IFLA_DSA_MASTER and
> "ip link set ... type dsa master ..."? What does being in the clear even
> mean technically, and where can I find more details about the policy
> which you just mentioned? Like is it optional or mandatory, was there
> any public debate surrounding the motivation for flagging some words,
> how is it enforced, are there official exceptions, etc?

There are more details in
Documentation/process/coding-style.rst, end of §4.
Vladimir Oltean Sept. 9, 2022, 11:23 a.m. UTC | #17
On Fri, Sep 09, 2022 at 03:09:50PM +0900, Benjamin Poirier wrote:
> > So are we or are we not in the clear with IFLA_DSA_MASTER and
> > "ip link set ... type dsa master ..."? What does being in the clear even
> > mean technically, and where can I find more details about the policy
> > which you just mentioned? Like is it optional or mandatory, was there
> > any public debate surrounding the motivation for flagging some words,
> > how is it enforced, are there official exceptions, etc?
> 
> There are more details in
> Documentation/process/coding-style.rst, end of ยง4.

Thanks for the pointer. So it says that if DSA was introduced in 2020 or
later, a master should have probably been named a host controller or
something of that kind. Which is probably reasonable in this context.
But I don't have the time and energy at my disposal to transition DSA to
an inclusive naming convention, at least not in a way that wouldn't then
be detrimential/confusing in the short term to the user base. So I'll
keep using IFLA_DSA_MASTER, my reading of it is that it's ok.
Stephen Hemminger Sept. 9, 2022, 3:03 p.m. UTC | #18
On Fri, 9 Sep 2022 15:09:50 +0900
Benjamin Poirier <benjamin.poirier@gmail.com> wrote:

> On 2022-09-08 16:11 +0000, Vladimir Oltean wrote:
> > On Thu, Sep 08, 2022 at 07:25:19AM -0700, Stephen Hemminger wrote:  
> > > On Thu, 8 Sep 2022 08:08:23 -0600 David Ahern <dsahern@kernel.org> wrote:  
> > > > > Proposing any alternative naming raises the question how far you want to
> > > > > go with the alternative name. No user of DSA knows the "conduit interface"
> > > > > or "management port" or whatnot by any other name except "DSA master".
> > > > > What do we do about the user-visible Documentation/networking/dsa/configuration.rst,
> > > > > which clearly and consistently uses the 'master' name everywhere?
> > > > > Do we replace 'master' with something else and act as if it was never
> > > > > named 'master' in the first place? Do we introduce IFLA_DSA_MGMT_PORT as
> > > > > UAPI and explain in the documentation "oh yeah, that's how you change
> > > > > the DSA master"? "Ahh ok, why didn't you just call it IFLA_DSA_MASTER
> > > > > then?" "Well...."
> > > > > 
> > > > > Also, what about the code in net/dsa/*.c and drivers/net/dsa/, do we
> > > > > also change that to reflect the new terminology, or do we just have
> > > > > documentation stating one thing and the code another?
> > > > > 
> > > > > At this stage, I'm much more likely to circumvent all of this, and avoid
> > > > > triggering anyone by making a writable IFLA_LINK be the mechanism through
> > > > > which we change the DSA master.    
> > > > 
> > > > IMHO, 'master' should be an allowed option giving the precedence of
> > > > existing code and existing terminology. An alternative keyword can be
> > > > used for those that want to avoid use of 'master' in scripts. vrf is an
> > > > example of this -- you can specify 'vrf <device>' as a keyword instead
> > > > of 'master <vrf>'  
> > > 
> > > Agreed, just wanted to start discussion of alternative wording.  
> > 
> > So are we or are we not in the clear with IFLA_DSA_MASTER and
> > "ip link set ... type dsa master ..."? What does being in the clear even
> > mean technically, and where can I find more details about the policy
> > which you just mentioned? Like is it optional or mandatory, was there
> > any public debate surrounding the motivation for flagging some words,
> > how is it enforced, are there official exceptions, etc?  
> 
> There are more details in
> Documentation/process/coding-style.rst, end of §4.

See Also:

https://inclusivenaming.org/
https://docs.linuxfoundation.org/lfx/project-control-center-pre-release/tools/security/manage-non-inclusive-naming
diff mbox series

Patch

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index e0fbbfeeb3a1..6f4d7b1ff9fb 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -1372,4 +1372,14 @@  enum {
 
 #define IFLA_MCTP_MAX (__IFLA_MCTP_MAX - 1)
 
+/* DSA section */
+
+enum {
+	IFLA_DSA_UNSPEC,
+	IFLA_DSA_MASTER,
+	__IFLA_DSA_MAX,
+};
+
+#define IFLA_DSA_MAX	(__IFLA_DSA_MAX - 1)
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/Makefile b/ip/Makefile
index 6c2e072049a2..8fd9e295f344 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -8,7 +8,7 @@  IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
     iplink_macvlan.o ipl2tp.o link_vti.o link_vti6.o link_xfrm.o \
     iplink_vxlan.o tcp_metrics.o iplink_ipoib.o ipnetconf.o link_ip6tnl.o \
     link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.o \
-    iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
+    iplink_bridge.o iplink_bridge_slave.o iplink_dsa.o ipfou.o iplink_ipvlan.o \
     iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
     ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
     ipnexthop.o ipmptcp.o iplink_bareudp.o iplink_wwan.o ipioam6.o \
diff --git a/ip/iplink_dsa.c b/ip/iplink_dsa.c
new file mode 100644
index 000000000000..d984e8b3b534
--- /dev/null
+++ b/ip/iplink_dsa.c
@@ -0,0 +1,67 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * iplink_dsa.c		DSA switch support
+ */
+
+#include "utils.h"
+#include "ip_common.h"
+
+static void print_usage(FILE *f)
+{
+	fprintf(f, "Usage: ... dsa [ master DEVICE ]\n");
+}
+
+static int dsa_parse_opt(struct link_util *lu, int argc, char **argv,
+			 struct nlmsghdr *n)
+{
+	while (argc > 0) {
+		if (strcmp(*argv, "master") == 0) {
+			__u32 ifindex;
+
+			NEXT_ARG();
+			ifindex = ll_name_to_index(*argv);
+			if (!ifindex)
+				invarg("Device does not exist\n", *argv);
+			addattr_l(n, 1024, IFLA_DSA_MASTER, &ifindex, 4);
+		} else if (strcmp(*argv, "help") == 0) {
+			print_usage(stderr);
+			return -1;
+		} else {
+			fprintf(stderr, "dsa: unknown command \"%s\"?\n", *argv);
+			print_usage(stderr);
+			return -1;
+		}
+		argc--;
+		argv++;
+	}
+
+	return 0;
+}
+
+static void dsa_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+	if (!tb)
+		return;
+
+	if (tb[IFLA_DSA_MASTER]) {
+		__u32 master = rta_getattr_u32(tb[IFLA_DSA_MASTER]);
+
+		print_string(PRINT_ANY,
+			     "master", "master %s ",
+			     ll_index_to_name(master));
+	}
+}
+
+static void dsa_print_help(struct link_util *lu, int argc, char **argv,
+			   FILE *f)
+{
+	print_usage(f);
+}
+
+struct link_util dsa_link_util = {
+	.id		= "dsa",
+	.maxattr	= IFLA_DSA_MAX,
+	.parse_opt	= dsa_parse_opt,
+	.print_opt	= dsa_print_opt,
+	.print_help     = dsa_print_help,
+};