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

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

[Oleksij Rempel]

This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
auto-negotiation support, we needed to be able to configure the
MASTER-SLAVE role of the port manually or from an application in user
space.

The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
force MASTER or SLAVE role. See IEEE 802.3-2018:
22.2.4.3.7 MASTER-SLAVE control register (Register 9)
22.2.4.3.8 MASTER-SLAVE status register (Register 10)
40.5.2 MASTER-SLAVE configuration resolution
45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)

The MASTER-SLAVE role affects the clock configuration:

-------------------------------------------------------------------------------
When the  PHY is configured as MASTER, the PMA Transmit function shall
source TX_TCLK from a local clock source. When configured as SLAVE, the
PMA Transmit function shall source TX_TCLK from the clock recovered from
data stream provided by MASTER.

iMX6Q                     KSZ9031                XXX
------\                /-----------\        /------------\
      |                |           |        |            |
 MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
      |<--- 125 MHz ---+-<------/  |        | \          |
------/                \-----------/        \------------/
                                               ^
                                                \-TX_TCLK

-------------------------------------------------------------------------------

Since some clock or link related issues are only reproducible in a
specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial
to provide generic (not 100BASE-T1 specific) interface to the user space
for configuration flexibility and trouble shooting.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 netlink/desc-ethtool.c       |  2 ++
 netlink/settings.c           | 50 ++++++++++++++++++++++++++++++++++++
 uapi/linux/ethtool.h         | 11 ++++++++
 uapi/linux/ethtool_netlink.h |  2 ++
 4 files changed, 65 insertions(+)

diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index 76c6f13..b0a793c 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -85,6 +85,8 @@ static const struct pretty_nla_desc __linkmodes_desc[] = {
 	NLATTR_DESC_NESTED(ETHTOOL_A_LINKMODES_PEER, bitset),
 	NLATTR_DESC_U32(ETHTOOL_A_LINKMODES_SPEED),
 	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_DUPLEX),
+	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG),
+	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE),
 };
 
 static const struct pretty_nla_desc __linkstate_desc[] = {
diff --git a/netlink/settings.c b/netlink/settings.c
index 8be5a22..76ca862 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -37,6 +37,21 @@ static const char *const names_duplex[] = {
 	[DUPLEX_FULL]		= "Full",
 };
 
+static const char *const names_master_slave_state[] = {
+	[PORT_MODE_STATE_UNKNOWN]	= "Unknown",
+	[PORT_MODE_STATE_MASTER]	= "Master",
+	[PORT_MODE_STATE_SLAVE]		= "Slave",
+	[PORT_MODE_STATE_ERR]		= "Resolution error",
+};
+
+static const char *const names_master_slave_cfg[] = {
+	[PORT_MODE_CFG_UNKNOWN]			= "Unknown",
+	[PORT_MODE_CFG_MASTER_PREFERRED]	= "preferred Master",
+	[PORT_MODE_CFG_SLAVE_PREFERRED]		= "preferred Slave",
+	[PORT_MODE_CFG_MASTER_FORCE]		= "forced Master",
+	[PORT_MODE_CFG_SLAVE_FORCE]		= "forced Slave",
+};
+
 static const char *const names_port[] = {
 	[PORT_TP]		= "Twisted Pair",
 	[PORT_AUI]		= "AUI",
@@ -520,6 +535,25 @@ int linkmodes_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 		printf("\tAuto-negotiation: %s\n",
 		       (autoneg == AUTONEG_DISABLE) ? "off" : "on");
 	}
+	if (tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]) {
+		uint8_t val;
+
+		val = mnl_attr_get_u8(tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG]);
+
+		print_banner(nlctx);
+		print_enum(names_master_slave_cfg,
+			   ARRAY_SIZE(names_master_slave_cfg), val,
+			   "Port mode cfg");
+	}
+	if (tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE]) {
+		uint8_t val;
+
+		val = mnl_attr_get_u8(tb[ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE]);
+		print_banner(nlctx);
+		print_enum(names_master_slave_state,
+			   ARRAY_SIZE(names_master_slave_state), val,
+			   "Port mode status");
+	}
 
 	return MNL_CB_OK;
 err:
@@ -827,6 +861,14 @@ static const struct lookup_entry_u32 duplex_values[] = {
 	{}
 };
 
+static const struct lookup_entry_u32 master_slave_values[] = {
+	{ .arg = "master-preferred",	.val = PORT_MODE_CFG_MASTER_PREFERRED },
+	{ .arg = "slave-preferred",	.val = PORT_MODE_CFG_SLAVE_PREFERRED },
+	{ .arg = "master-force",	.val = PORT_MODE_CFG_MASTER_FORCE },
+	{ .arg = "slave-force",		.val = PORT_MODE_CFG_SLAVE_FORCE },
+	{}
+};
+
 char wol_bit_chars[WOL_MODE_COUNT] = {
 	[WAKE_PHY_BIT]		= 'p',
 	[WAKE_UCAST_BIT]	= 'u',
@@ -917,6 +959,14 @@ static const struct param_parser sset_params[] = {
 		.handler_data	= duplex_values,
 		.min_argc	= 1,
 	},
+	{
+		.arg		= "master-slave",
+		.group		= ETHTOOL_MSG_LINKMODES_SET,
+		.type		= ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,
+		.handler	= nl_parse_lookup_u8,
+		.handler_data	= master_slave_values,
+		.min_argc	= 1,
+	},
 	{
 		.arg		= "wol",
 		.group		= ETHTOOL_MSG_WOL_SET,
diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
index d3dcb45..d2872f9 100644
--- a/uapi/linux/ethtool.h
+++ b/uapi/linux/ethtool.h
@@ -1659,6 +1659,17 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 	return 0;
 }
 
+/* Port mode */
+#define PORT_MODE_CFG_UNKNOWN		1
+#define PORT_MODE_CFG_MASTER_PREFERRED	2
+#define PORT_MODE_CFG_SLAVE_PREFERRED	3
+#define PORT_MODE_CFG_MASTER_FORCE	4
+#define PORT_MODE_CFG_SLAVE_FORCE	5
+#define PORT_MODE_STATE_UNKNOWN		1
+#define PORT_MODE_STATE_MASTER		2
+#define PORT_MODE_STATE_SLAVE		3
+#define PORT_MODE_STATE_ERR		4
+
 /* Which connector port. */
 #define PORT_TP			0x00
 #define PORT_AUI		0x01
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index ad6d3a0..73a720c 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -185,6 +185,8 @@ enum {
 	ETHTOOL_A_LINKMODES_PEER,		/* bitset */
 	ETHTOOL_A_LINKMODES_SPEED,		/* u32 */
 	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
+	ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,	/* u8 */
+	ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE,	/* u8 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_LINKMODES_CNT,
-- 
2.26.2

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

[Michal Kubecek]

On Tue, May 26, 2020 at 11:10:25AM +0200, Oleksij Rempel wrote:
> This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
> auto-negotiation support, we needed to be able to configure the
> MASTER-SLAVE role of the port manually or from an application in user
> space.
> 
> The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
> force MASTER or SLAVE role. See IEEE 802.3-2018:
> 22.2.4.3.7 MASTER-SLAVE control register (Register 9)
> 22.2.4.3.8 MASTER-SLAVE status register (Register 10)
> 40.5.2 MASTER-SLAVE configuration resolution
> 45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
> 45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)
> 
> The MASTER-SLAVE role affects the clock configuration:
> 
> -------------------------------------------------------------------------------
> When the  PHY is configured as MASTER, the PMA Transmit function shall
> source TX_TCLK from a local clock source. When configured as SLAVE, the
> PMA Transmit function shall source TX_TCLK from the clock recovered from
> data stream provided by MASTER.
> 
> iMX6Q                     KSZ9031                XXX
> ------\                /-----------\        /------------\
>       |                |           |        |            |
>  MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
>       |<--- 125 MHz ---+-<------/  |        | \          |
> ------/                \-----------/        \------------/
>                                                ^
>                                                 \-TX_TCLK
> 
> -------------------------------------------------------------------------------
> 
> Since some clock or link related issues are only reproducible in a
> specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial
> to provide generic (not 100BASE-T1 specific) interface to the user space
> for configuration flexibility and trouble shooting.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---

Please document the new command line argument in both "ethtool --help"
output and manual page.

I would also prefer updating the UAPI header copies in a separate commit
which would update all of them to a state of a specific kernel commit
(either 4.8-rc1 or current net-next); cherry picking specific changes
may lead to missing some parts. An easy way would be

  # switch to kernel repository and check out what you want to copy from
  make ... INSTALL_HDR_PATH=$somewhere headers_install
  # switch back to ethtool repository
  cd uapi
  find . -type f -exec cp -v ${somewhere}/include/{} {} \;

Also, as the kernel counterpart is only in net-next at the moment, this
should probably wait until after ethtool 5.7 release (perhaps it would
be helpful to have a "next" branch like iproute2). I'll submit my queued
patches for 5.7 later this week; should have done so long ago but
I hoped to have the netlink friendly test framework finished before I do
(test-features.c is tied to ioctl interface too tightly).

[...]
> @@ -827,6 +861,14 @@ static const struct lookup_entry_u32 duplex_values[] = {
>  	{}
>  };
>  
> +static const struct lookup_entry_u32 master_slave_values[] = {

This should be struct lookup_entry_u8, you are using it with
nl_parse_lookup_u8() to generate an NLA_U8 attribute.

Michal

> +	{ .arg = "master-preferred",	.val = PORT_MODE_CFG_MASTER_PREFERRED },
> +	{ .arg = "slave-preferred",	.val = PORT_MODE_CFG_SLAVE_PREFERRED },
> +	{ .arg = "master-force",	.val = PORT_MODE_CFG_MASTER_FORCE },
> +	{ .arg = "slave-force",		.val = PORT_MODE_CFG_SLAVE_FORCE },
> +	{}
> +};
> +
>  char wol_bit_chars[WOL_MODE_COUNT] = {
>  	[WAKE_PHY_BIT]		= 'p',
>  	[WAKE_UCAST_BIT]	= 'u',
> @@ -917,6 +959,14 @@ static const struct param_parser sset_params[] = {
>  		.handler_data	= duplex_values,
>  		.min_argc	= 1,
>  	},
> +	{
> +		.arg		= "master-slave",
> +		.group		= ETHTOOL_MSG_LINKMODES_SET,
> +		.type		= ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,
> +		.handler	= nl_parse_lookup_u8,
> +		.handler_data	= master_slave_values,
> +		.min_argc	= 1,
> +	},
>  	{
>  		.arg		= "wol",
>  		.group		= ETHTOOL_MSG_WOL_SET,

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

[Oleksij Rempel]

[-- Attachment #1: Type: text/plain, Size: 3648 bytes --]

On Tue, May 26, 2020 at 02:41:39PM +0200, Michal Kubecek wrote:
> On Tue, May 26, 2020 at 11:10:25AM +0200, Oleksij Rempel wrote:
> > This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
> > auto-negotiation support, we needed to be able to configure the
> > MASTER-SLAVE role of the port manually or from an application in user
> > space.
> > 
> > The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
> > force MASTER or SLAVE role. See IEEE 802.3-2018:
> > 22.2.4.3.7 MASTER-SLAVE control register (Register 9)
> > 22.2.4.3.8 MASTER-SLAVE status register (Register 10)
> > 40.5.2 MASTER-SLAVE configuration resolution
> > 45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
> > 45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)
> > 
> > The MASTER-SLAVE role affects the clock configuration:
> > 
> > -------------------------------------------------------------------------------
> > When the  PHY is configured as MASTER, the PMA Transmit function shall
> > source TX_TCLK from a local clock source. When configured as SLAVE, the
> > PMA Transmit function shall source TX_TCLK from the clock recovered from
> > data stream provided by MASTER.
> > 
> > iMX6Q                     KSZ9031                XXX
> > ------\                /-----------\        /------------\
> >       |                |           |        |            |
> >  MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
> >       |<--- 125 MHz ---+-<------/  |        | \          |
> > ------/                \-----------/        \------------/
> >                                                ^
> >                                                 \-TX_TCLK
> > 
> > -------------------------------------------------------------------------------
> > 
> > Since some clock or link related issues are only reproducible in a
> > specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial
> > to provide generic (not 100BASE-T1 specific) interface to the user space
> > for configuration flexibility and trouble shooting.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> 
> Please document the new command line argument in both "ethtool --help"
> output and manual page.
> 
> I would also prefer updating the UAPI header copies in a separate commit
> which would update all of them to a state of a specific kernel commit
> (either 4.8-rc1 or current net-next); cherry picking specific changes
> may lead to missing some parts. An easy way would be
> 
>   # switch to kernel repository and check out what you want to copy from
>   make ... INSTALL_HDR_PATH=$somewhere headers_install
>   # switch back to ethtool repository
>   cd uapi
>   find . -type f -exec cp -v ${somewhere}/include/{} {} \;
> 
> Also, as the kernel counterpart is only in net-next at the moment, this
> should probably wait until after ethtool 5.7 release (perhaps it would
> be helpful to have a "next" branch like iproute2). I'll submit my queued
> patches for 5.7 later this week; should have done so long ago but
> I hoped to have the netlink friendly test framework finished before I do
> (test-features.c is tied to ioctl interface too tightly).

OK, should I resend fixed patch now, or wait until kernel 5.8-rc1? 

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Stephen Hemminger]

On Tue, 26 May 2020 11:10:25 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
> auto-negotiation support, we needed to be able to configure the
> MASTER-SLAVE role of the port manually or from an application in user
> space.
> 
> The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
> force MASTER or SLAVE role. See IEEE 802.3-2018:
> 22.2.4.3.7 MASTER-SLAVE control register (Register 9)
> 22.2.4.3.8 MASTER-SLAVE status register (Register 10)
> 40.5.2 MASTER-SLAVE configuration resolution
> 45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
> 45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)
> 
> The MASTER-SLAVE role affects the clock configuration:
> 
> -------------------------------------------------------------------------------
> When the  PHY is configured as MASTER, the PMA Transmit function shall
> source TX_TCLK from a local clock source. When configured as SLAVE, the
> PMA Transmit function shall source TX_TCLK from the clock recovered from
> data stream provided by MASTER.
> 
> iMX6Q                     KSZ9031                XXX
> ------\                /-----------\        /------------\
>       |                |           |        |            |
>  MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
>       |<--- 125 MHz ---+-<------/  |        | \          |
> ------/                \-----------/        \------------/
>                                                ^
>                                                 \-TX_TCLK
> 
> -------------------------------------------------------------------------------
> 
> Since some clock or link related issues are only reproducible in a
> specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial
> to provide generic (not 100BASE-T1 specific) interface to the user space
> for configuration flexibility and trouble shooting.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

NAK
Open source projects have been working hard to remove the terms master and slave
in API's and documentation. Apparently, Linux hasn't gotten the message.
It would make sense not to introduce new instances.

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

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 7 Jun 2020 15:30:19 -0700

> Open source projects have been working hard to remove the terms master and slave
> in API's and documentation. Apparently, Linux hasn't gotten the message.
> It would make sense not to introduce new instances.

Would you also be against, for example, the use of the terminology
expressing the "death" of allocated registers in a compiler backend,
for example?

How far do you plan take this resistence of terminology when it
clearly has a well defined usage and meaning in a specific technical
realm which is entirely disconnected to what the terms might imply,
meaning wise, in other realms?

And if you are going to say not to use this terminology, you must
suggest a reasonable (and I do mean _reasonable_) well understood
and _specific_ replacement.

Thank you.

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

[Stephen Hemminger]

On Sun, 07 Jun 2020 16:45:32 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Sun, 7 Jun 2020 15:30:19 -0700
> 
> > Open source projects have been working hard to remove the terms master and slave
> > in API's and documentation. Apparently, Linux hasn't gotten the message.
> > It would make sense not to introduce new instances.  
> 
> Would you also be against, for example, the use of the terminology
> expressing the "death" of allocated registers in a compiler backend,
> for example?
> 
> How far do you plan take this resistence of terminology when it
> clearly has a well defined usage and meaning in a specific technical
> realm which is entirely disconnected to what the terms might imply,
> meaning wise, in other realms?
> 
> And if you are going to say not to use this terminology, you must
> suggest a reasonable (and I do mean _reasonable_) well understood
> and _specific_ replacement.
> 
> Thank you.

How many times have you or Linus argued about variable naming.
Yes, words do matter and convey a lot of implied connotation and meaning.

Most projects and standards bodies are taking a stance on fixing the
language. The IETF is has proposed making changes as well.

There are a very specific set of trigger words and terms that
should be fixed. Most of these terms do have better alternatives.

A common example is that master/slave is unclear and would be clearer
as primary/secondary or active/backup or controller/worker.

Most of networking is based on standards. When the standards wording changes
(and it will happen soon); then Linux should also change the wording in the
source, api and documentation.


See:


[0] - <https://www.cs.cmu.edu/~mjw/Language/NonSexist/vuw.non-sexist-language-guidelines.txt>, <https://twitter.com/justkelly_ok/status/933011085594066944>
[1] - <https://github.com/django/django/pull/2692>
[2] - <https://bugs.python.org/issue34605>
[3] - <https://github.com/rust-lang-deprecated/rust-buildbot/issues/2>, <https://github.com/rust-community/foss-events-planner/issues/58>
[4] - <https://twitter.com/ISCdotORG/status/942815837299253248>
[5] - <https://gitlab.gnome.org/GNOME/geary/issues/324>
[6] - https://mail.gnome.org/archives/desktop-devel-list/2019-April/msg00049.html
[7] - https://www.ietf.org/archive/id/draft-knodel-terminology-01.txt

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

[Andrew Lunn]

Hi Stephen

> A common example is that master/slave is unclear and would be clearer
> as primary/secondary or active/backup or controller/worker.

802.3, cause 32.1.2, 2015 version:

   A 100BASE-T2 PHY can be configured either as a master PHY or as a
   slave PHY. The master-slave relationship between two stations
   sharing a link segment is established during Auto-Negotiation (see
   Clause 28, 32.5, Annex 28C, and 32.5.2). The master PHY uses an
   external clock to determine the timing of transmitter and receiver
   operations. The slave PHY recovers the clock from the received
   signal and uses it to determine the timing of transmitter
   operations, i.e., it performs loop timing, as illustrated in Figure
   32–2.

In this case, i would say master/slave is very clearly defined.

Given these definitions, would you like to propose alternatives?

Do you have any insights has to how the IEEE 802.3 standard will be
changed?

      Andrew

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

[David Miller]

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 9 Jun 2020 10:19:35 -0700

> Yes, words do matter and convey a lot of implied connotation and
> meaning.

What is your long term plan?  Will you change all of the UAPI for
bonding for example?

Or will we have a partial solution to the problem?

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

[Kees Cook]

On Tue, Jun 09, 2020 at 11:36:33AM -0700, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Tue, 9 Jun 2020 10:19:35 -0700
> 
> > Yes, words do matter and convey a lot of implied connotation and
> > meaning.
> 
> What is your long term plan?  Will you change all of the UAPI for
> bonding for example?
> 
> Or will we have a partial solution to the problem?

As a first step, let's stop _adding_ this language.

Given what I've seen from other communities and what I know of the kernel
community, I don't think we're going to get consensus on some massive
global search/replace any time soon. However, I think we can get started
on making this change with just stopping further introductions. (I view
this like any other treewide change: stop new badness from getting
added, and chip away as old ones as we can until it's all gone.)

-- 
Kees Cook

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

[David Miller]

From: Kees Cook <keescook@chromium.org>
Date: Tue, 9 Jun 2020 12:29:54 -0700

> Given what I've seen from other communities and what I know of the kernel
> community, I don't think we're going to get consensus on some massive
> global search/replace any time soon. However, I think we can get started
> on making this change with just stopping further introductions. (I view
> this like any other treewide change: stop new badness from getting
> added, and chip away as old ones as we can until it's all gone.)

The terminology being suggested by these changes matches what is used
in the standards and literature.

Inventing something creates confusion for those who are familiar with
these pieces of technology already, and those who are not who are
reading about it elsewhere.

Both groups will be terminally confused if we use different words.

For such pain, there should be agood reason.  I don't accept Stephen's
quoted standards bodies "efforts" as a legitimate reason, or evidence
of such, as it has a lot of holes in it as Edward pointed out.  I
found the Orwell references to be quite ironic actually.

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

[Kees Cook]

On Tue, Jun 09, 2020 at 12:34:37PM -0700, David Miller wrote:
> From: Kees Cook <keescook@chromium.org>
> Date: Tue, 9 Jun 2020 12:29:54 -0700
> 
> > Given what I've seen from other communities and what I know of the kernel
> > community, I don't think we're going to get consensus on some massive
> > global search/replace any time soon. However, I think we can get started
> > on making this change with just stopping further introductions. (I view
> > this like any other treewide change: stop new badness from getting
> > added, and chip away as old ones as we can until it's all gone.)
> 
> The terminology being suggested by these changes matches what is used
> in the standards and literature.
> 
> Inventing something creates confusion for those who are familiar with
> these pieces of technology already, and those who are not who are
> reading about it elsewhere.
> 
> Both groups will be terminally confused if we use different words.
> 
> For such pain, there should be agood reason.  I don't accept Stephen's
> quoted standards bodies "efforts" as a legitimate reason, or evidence
> of such, as it has a lot of holes in it as Edward pointed out.  I
> found the Orwell references to be quite ironic actually.

Okay, for now, how about:

- If we're dealing with an existing spec, match the language.
- If we're dealing with a new spec, ask the authors to fix their language.
- If a new version of a spec has updated its language, adjust the kernel's.
- If we're doing with something "internal" to the kernel (including UAPI),
  stop adding new instances.

-- 
Kees Cook

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

[David Miller]

From: Kees Cook <keescook@chromium.org>
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".

> - 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.

> - If we're doing with something "internal" to the kernel (including UAPI),
>   stop adding new instances.

Even if it is part of supporting a technology where the standard uses
those terms?  So we'll use inconsitent terms internally?

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.

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

[Kees Cook]

On Tue, Jun 09, 2020 at 01:05:17PM -0700, David Miller wrote:
> From: Kees Cook <keescook@chromium.org>
> 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.

> > - 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.

> > - If we're doing with something "internal" to the kernel (including UAPI),
> >   stop adding new instances.
> 
> Even if it is part of supporting a technology where the standard uses
> those terms?  So we'll use inconsitent terms internally?

What? I mean "internal" in the sense of "does not have an external
dependency". Maybe I should say "independent"?

> 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.

For example, if MS_SLAVE were being added now, we would rename it.

-- 
Kees Cook

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@chromium.org>
> > 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

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

[Kees Cook]

On Tue, Jun 09, 2020 at 10:53:03PM +0200, Michal Kubecek wrote:
> 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.

Yup, when I saw the original patch it wasn't clear this was matching a
spec. I haven't been arguing for the $subject patch since Dave pointed
that out, and am now trying to shape what the general guidance should
be.

-- 
Kees Cook

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

[Williams, Dan J]

On Tue, 2020-06-09 at 11:36 -0700, David Miller wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Tue, 9 Jun 2020 10:19:35 -0700
> 
> > Yes, words do matter and convey a lot of implied connotation and
> > meaning.
> 
> What is your long term plan?  Will you change all of the UAPI for
> bonding for example?

The long term plan in my view includes talking with standards bodies to
move new content to, for example, master/subordinate. In other words,
practical forward steps, not retroactively changing interfaces.

> Or will we have a partial solution to the problem?

Partial steps toward better inclusion are meaningful.

The root problem is of course much larger than a few changes to
technical terminology could ever solve, but solving *that* problem is
not the goal. The goal is to make the kernel community as productive as
possible and if the antropomorphic association of "slave" can be
replaced with a term that maintains technical meaning it makes kernel-
work that much more approachable.

I recall one of Ingo's explanations of how broken whitespace can make
patches that much harder to read and take him out of the "zone" of
reviewing code. "Slave" is already having that speed bump affect on the
community. If we can replace it without losing meaning and improving
the effectiveness of contributors I think the Linux kernel project is
better for it.

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

[David Miller]

From: "Williams, Dan J" <dan.j.williams@intel.com>
Date: Tue, 9 Jun 2020 19:30:50 +0000

> On Tue, 2020-06-09 at 11:36 -0700, David Miller wrote:
>> From: Stephen Hemminger <stephen@networkplumber.org>
>> Date: Tue, 9 Jun 2020 10:19:35 -0700
>> 
>> > Yes, words do matter and convey a lot of implied connotation and
>> > meaning.
>> 
>> What is your long term plan?  Will you change all of the UAPI for
>> bonding for example?
> 
> The long term plan in my view includes talking with standards bodies to
> move new content to, for example, master/subordinate. In other words,
> practical forward steps, not retroactively changing interfaces.

When that knowledge is established legitimately in standards and
transferred into common knowledge of these technologies, yes then
please come present your proposals.

But right now using different words will create confusion.

I also find master/subordinate an interesting proposal, what if master
is a triggering term?  Why only slave?

I know people feel something needs to change, but do that moving
forward for the technologies themselves.  Not how we implement support
for a technology which is established already.

Plant the seed, don't chop the tree down.

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

[Dan Williams]

On Tue, Jun 9, 2020 at 12:57 PM David Miller <davem@davemloft.net> wrote:
>
> From: "Williams, Dan J" <dan.j.williams@intel.com>
> Date: Tue, 9 Jun 2020 19:30:50 +0000
>
> > On Tue, 2020-06-09 at 11:36 -0700, David Miller wrote:
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Date: Tue, 9 Jun 2020 10:19:35 -0700
> >>
> >> > Yes, words do matter and convey a lot of implied connotation and
> >> > meaning.
> >>
> >> What is your long term plan?  Will you change all of the UAPI for
> >> bonding for example?
> >
> > The long term plan in my view includes talking with standards bodies to
> > move new content to, for example, master/subordinate. In other words,
> > practical forward steps, not retroactively changing interfaces.
>
> When that knowledge is established legitimately in standards and
> transferred into common knowledge of these technologies, yes then
> please come present your proposals.

Our hands are not completely tied by the specifications, as a
community we have a non-zero level of influence over standards bodies,
even direct participation in some. So we could do something stronger
than passively wait for standards to catch up. For example, deprecate
our consideration of future specifications that include this language
and set a cut off date.

I understand the confusion that arises from using terminology
differently from the specification, but at the same time when
specification language actively hurts kernel code maintainability we
change it. For example, when I did the first iteration of what
eventually became libnvdimm Ingo rightly reacted to the naming being
too ACPI specification centric and wanting more kernel-centric naming.

If the common kernel name for what was formerly called a "slave"
device is a "subordinate" device then the confusion is lessened, only
one common kernel translation to consider.

> But right now using different words will create confusion.
>
> I also find master/subordinate an interesting proposal, what if master
> is a triggering term?  Why only slave?

"slave" has a direct connection to human suffering deployed at global scale.

One way I think about this is consider we have our first ever Linux
Plumbers on the African continent, and you had a choice about giving a
talk about how the git master branch operates, or a talk about slave
devices which one would you feel more immediately comfortable leading?
Any hesitation you would feel freely using the word slave with a
predominantly black audience is a similar speed bump a contributor
might feel needing to consume that term to get their job done.

Explaining "no, not that connotation of slave" does not scale as much
as transitioning to another term.

> I know people feel something needs to change, but do that moving
> forward for the technologies themselves.

This is the start of a process that the kernel community can take an
active role to lead, we have it within our control to not wait for the
lowest common denominator to catch up.

> Not how we implement support
> for a technology which is established already.
>
> Plant the seed, don't chop the tree down.

I appreciate the engagement.

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

[Oleksij Rempel]

[-- Attachment #1: Type: text/plain, Size: 5517 bytes --]

On Tue, Jun 09, 2020 at 02:48:51PM -0700, Dan Williams wrote:
> On Tue, Jun 9, 2020 at 12:57 PM David Miller <davem@davemloft.net> wrote:
> >
> > From: "Williams, Dan J" <dan.j.williams@intel.com>
> > Date: Tue, 9 Jun 2020 19:30:50 +0000
> >
> > > On Tue, 2020-06-09 at 11:36 -0700, David Miller wrote:
> > >> From: Stephen Hemminger <stephen@networkplumber.org>
> > >> Date: Tue, 9 Jun 2020 10:19:35 -0700
> > >>
> > >> > Yes, words do matter and convey a lot of implied connotation and
> > >> > meaning.
> > >>
> > >> What is your long term plan?  Will you change all of the UAPI for
> > >> bonding for example?
> > >
> > > The long term plan in my view includes talking with standards bodies to
> > > move new content to, for example, master/subordinate. In other words,
> > > practical forward steps, not retroactively changing interfaces.
> >
> > When that knowledge is established legitimately in standards and
> > transferred into common knowledge of these technologies, yes then
> > please come present your proposals.
> 
> Our hands are not completely tied by the specifications, as a
> community we have a non-zero level of influence over standards bodies,
> even direct participation in some. So we could do something stronger
> than passively wait for standards to catch up. For example, deprecate
> our consideration of future specifications that include this language
> and set a cut off date.
> 
> I understand the confusion that arises from using terminology
> differently from the specification, but at the same time when
> specification language actively hurts kernel code maintainability we
> change it. For example, when I did the first iteration of what
> eventually became libnvdimm Ingo rightly reacted to the naming being
> too ACPI specification centric and wanting more kernel-centric naming.
> 
> If the common kernel name for what was formerly called a "slave"
> device is a "subordinate" device then the confusion is lessened, only
> one common kernel translation to consider.
> 
> > But right now using different words will create confusion.
> >
> > I also find master/subordinate an interesting proposal, what if master
> > is a triggering term?  Why only slave?
> 
> "slave" has a direct connection to human suffering deployed at global scale.
> 
> One way I think about this is consider we have our first ever Linux
> Plumbers on the African continent, and you had a choice about giving a
> talk about how the git master branch operates, or a talk about slave
> devices which one would you feel more immediately comfortable leading?
> Any hesitation you would feel freely using the word slave with a
> predominantly black audience is a similar speed bump a contributor
> might feel needing to consume that term to get their job done.
> 
> Explaining "no, not that connotation of slave" does not scale as much
> as transitioning to another term.
> 
> > I know people feel something needs to change, but do that moving
> > forward for the technologies themselves.
> 
> This is the start of a process that the kernel community can take an
> active role to lead, we have it within our control to not wait for the
> lowest common denominator to catch up.
> 
> > Not how we implement support
> > for a technology which is established already.
> >
> > Plant the seed, don't chop the tree down.
> 
> I appreciate the engagement.

It is interesting to see technical mind arguing about humanitarian
topics. Usually I need to translate IT to theologies, historic teachers
or linguists. So, let me try to do it other way around:

- a language is not a snapshot. The meaning of words is changing
  continually. For example, an ARM is buying Intel networking division and all
  i200 controller should be renamed to arm100..
  I'm not familiar with your local history, so i give some examples to
  German readers:
  - The word "Führer" was abused by some historical person and event.
  - The word "geil" is changed within one generation from sexual
    erection to a positive emotion reusable in almost every context.
  There is many language changes just within last century, please refer
  to DUDEN to learn more about it:
  https://www.duden.de/ueber_duden/auflagengeschichte
  Or for more generic examples:
  https://en.wikipedia.org/wiki/Language_change

- from your argumentation I would assume, you are trying to define a
  language. A language not historically misused, so it will be
  acceptable by every one. Or at least by the technical community.
  Suddenly, I should disappoint you. The number of programming
  languages defined by technical community is is still growing and none of it
  makes happy every one. How do you thing the reality looks for spoken language?
  Welcome to my reality:
  https://en.wikipedia.org/wiki/Universal_language
  In case the historical argumentations are too boring, let me give you
  a modern, less boring example:
  https://youtu.be/7C-aB09i30E

- Are there any other attempts to change historical and linguistic
  reality by removing and replacing something? Sure:
  https://en.wikipedia.org/wiki/Book_burning


Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Edward Cree]

Disclaimer: *definitely* not speaking for my employer.

On 09/06/2020 18:19, Stephen Hemminger wrote:
> How many times have you or Linus argued about variable naming.
> Yes, words do matter and convey a lot of implied connotation and meaning.
Connotation, unlike denotation, is widely variable.  I would aver
 that for most people who are triggered or offended by the technical
 use of master/slave or similar terms, this is a *learned behaviour*
 that occurs because they have been taught that they should be
 offended by these words.  Are these people also incapable of using
 those words to describe actual historical slavery?
There is a difference between stating "X relates to Y as a slave to
 a master" and "You relate to me (or ought to do so) as a slave to a
 master".  The former is a merely factual claim which is often true
 of technical entities (and in the past or in certain parts of the
 world today is true of some humans, however morally wrong); the
 latter is an assertion of power.  It is only the latter which is,
 or at any rate should be, offensive; the attempt to ban the former
 rests on an equivocation between positive and normative uses.
Anyone who can't put connotation aside and stick to denotation
 (a) needs to grow up
 (b) is ill-suited to working with computers.

> Most projects and standards bodies are taking a stance on fixing the
> language. The IETF is has proposed making changes as well.
An expired Internet-Draft authored by a human-rights charity and a
 media studies postgrad student does not constitute "the IETF has
 proposed".  (Besides, it's historically inaccurate; it claims that
 the word "robot" means slave, when in fact it means the labour owed
 by a _serf_ ("robotnik").  And it cites Orwell with apparently *no*
 sense of irony whatsoever... he was arguing _against_ Newspeak, not
 for it!)

> A common example is that master/slave is unclear and would be clearer
> as primary/secondary or active/backup or controller/worker.
So why isn't controller/worker just as offensive, given all those
 labourers throughout history who have suffered under abusive
 managers?  Or does a word need a tenuous connection to race before
 it can be ritually purified from the language?
And is there really an EE anywhere who finds the terminology of
 master and slave clocks unclear?  I suspect very few would gain a
 better understanding from any of your suggested alternatives.

> Most of networking is based on standards. When the standards wording changes
> (and it will happen soon); then Linux should also change the wording in the
> source, api and documentation.
Rather, it seems that this is an attempt to change Linux in order
 to _de facto_ change the standard, thereby creating pressure on
 standards bodies to change it _de jure_ to match.  Yet, in the
 real world, engineers use and understand the current terminology;
 the push for language purification bears but little reference to
 anything outside of itself.

In conclusion, I'd like to quote from Henry Spencer's Ten
 Commandments for C Programmers (Annotated Edition) [1]:
> As a lamentable side issue, there has been some unrest from the
> fanatics of the Pronoun Gestapo over the use of the word "man"
> in [the eighth] Commandment, for they believe that great efforts
> and loud shouting devoted to the ritual purification of the
> language will somehow redound to the benefit of the downtrodden
> (whose real and grievous woes tendeth to get lost amidst all that
> thunder and fury).

Grumpily yours,
-ed

[1] https://www.lysator.liu.se/c/ten-commandments.html

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

[Michal Kubecek]

On Tue, Jun 09, 2020 at 10:19:35AM -0700, Stephen Hemminger wrote:
> On Sun, 07 Jun 2020 16:45:32 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Date: Sun, 7 Jun 2020 15:30:19 -0700
> > 
> > > Open source projects have been working hard to remove the terms master and slave
> > > in API's and documentation. Apparently, Linux hasn't gotten the message.
> > > It would make sense not to introduce new instances.  
> > 
> > Would you also be against, for example, the use of the terminology
> > expressing the "death" of allocated registers in a compiler backend,
> > for example?
> > 
> > How far do you plan take this resistence of terminology when it
> > clearly has a well defined usage and meaning in a specific technical
> > realm which is entirely disconnected to what the terms might imply,
> > meaning wise, in other realms?
> > 
> > And if you are going to say not to use this terminology, you must
> > suggest a reasonable (and I do mean _reasonable_) well understood
> > and _specific_ replacement.
> > 
> > Thank you.
> 
> How many times have you or Linus argued about variable naming.
> Yes, words do matter and convey a lot of implied connotation and meaning.
> 
> Most projects and standards bodies are taking a stance on fixing the
> language. The IETF is has proposed making changes as well.
> 
> There are a very specific set of trigger words and terms that
> should be fixed. Most of these terms do have better alternatives.

Where can this list be found and who is the authority to determine what
should be on this list? I could think of a long list of technical terms
which could be seen as offensive in certain context. Some would feel
just as obvious as master/slave, some would be borderline absurd, most
somewhere between. Who has the authority to define what is acceptable
and what not?

Words can have very different meaning and raise different emotions,
depending on context. Even an innocuous word like "black" can be
offensive in certain context; is it a reason to stop talking about this
color or to invent a new name for it? I don't think so - and for obvious
reasons, it wouldn't help anyway. Should we rename rbtrees because of
that? I don't think so either.

The primary purpose of technical terms is to allow people to communicate
and to express themselves in a way that will be easy to understand for
others working in the field. Inventing our own terms which would differ
both from existing relevant standards and from what people in the
industry have been using for decades would not help anything; it would
only make life of ethtool users harder.

> A common example is that master/slave is unclear and would be clearer
> as primary/secondary or active/backup or controller/worker.
> 
> Most of networking is based on standards. When the standards wording changes
> (and it will happen soon); then Linux should also change the wording in the
> source, api and documentation.

Even if you are right about the upcoming change of IEEE standards (and
I can't say I'm convinced), it would make little sense to invent some
replacement terms now and risk that IEEE chooses something else. Waiting
for the standard change (which might take years - or might not even
happen at all) and not providing support for the feature doesn't seem
like a good solution either.

So what exactly would you like us to do?

Michal