Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
Jarod Wilson wrote: >On Fri, Oct 2, 2020 at 6:57 PM David Miller wrote: >> >> From: Jarod Wilson >> Date: Fri, 2 Oct 2020 16:23:46 -0400 >> >> > I'd had a bit of feedback that people would rather see both, and be >> > able to toggle off the old ones, rather than only having one or the >> > other, depending on the toggle, so I thought I'd give this a try. I >> > kind of liked the one or the other route, but I see the problems with >> > that too. >> >> Please keep everything for the entire deprecation period, unconditionally. > >Okay, so 100% drop the Kconfig flag patch, but duplicate sysfs >interface names are acceptable, correct? Then what about the procfs >file having duplicate Slave and Port lines? Just leave them all as >Slave? My preference is to not alter the existing sysfs / proc behavior at all, and instead create a netlink / iproute UAPI that becomes the "preferred" UAPI going forward. Any new functionality would only be added to netlink as incentive to switch. I don't see the value in adding duplicate fields, as userspace code that parses them will not be portable if it only checks for the new field name. Then, down the road, deleting the old names will break the userspace code that was never updated (which will likely be most of it). I would rather see a single clean break from proc and sysfs to netlink in one step than a piecemeal addition and removal from the existing UAPI. That makes for a much clearer flag day event for end users. By this I mean leave proc / sysfs as-is today, and then after a suitable deprecation period, remove it wholesale (rather than a compile time option). -J --- -Jay Vosburgh, jay.vosbu...@canonical.com
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
On Fri, Oct 2, 2020 at 6:42 PM Stephen Hemminger wrote: > > On Fri, 2 Oct 2020 16:23:46 -0400 > Jarod Wilson wrote: > > > On Fri, Oct 2, 2020 at 3:13 PM Stephen Hemminger > > wrote: > > > > > > On Fri, 2 Oct 2020 13:40:01 -0400 > > > Jarod Wilson wrote: > > > > > > > By default, enable retaining all user-facing API that includes the use > > > > of > > > > master and slave, but add a Kconfig knob that allows those that wish to > > > > remove it entirely do so in one shot. ... > > > This is problematic. You are printing both old and new values. > > > Also every distribution will have to enable it. > > > > > > This looks like too much of change to users. > > > > I'd had a bit of feedback that people would rather see both, and be > > able to toggle off the old ones, rather than only having one or the > > other, depending on the toggle, so I thought I'd give this a try. I > > kind of liked the one or the other route, but I see the problems with > > that too. > > > > For simplicity, I'm kind of liking the idea of just not updating the > > proc and sysfs interfaces, have a toggle entirely disable them, and > > work on enhancing userspace to only use netlink, but ... it's going to > > be a while before any such work makes its way to any already shipping > > distros. I don't have a satisfying answer here. > > > > I like the idea of having bonding proc and sysf apis optional. I do too, but I'd see it more as something only userspace developers would care about for a while, as an easy way to make absolutely certain their code/distro is no longer reliant on them and only uses netlink, not as something any normal user really has any reason to do. -- Jarod Wilson ja...@redhat.com
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
On Fri, Oct 2, 2020 at 6:57 PM David Miller wrote: > > From: Jarod Wilson > Date: Fri, 2 Oct 2020 16:23:46 -0400 > > > I'd had a bit of feedback that people would rather see both, and be > > able to toggle off the old ones, rather than only having one or the > > other, depending on the toggle, so I thought I'd give this a try. I > > kind of liked the one or the other route, but I see the problems with > > that too. > > Please keep everything for the entire deprecation period, unconditionally. Okay, so 100% drop the Kconfig flag patch, but duplicate sysfs interface names are acceptable, correct? Then what about the procfs file having duplicate Slave and Port lines? Just leave them all as Slave? -- Jarod Wilson ja...@redhat.com
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
From: Jarod Wilson Date: Fri, 2 Oct 2020 16:23:46 -0400 > I'd had a bit of feedback that people would rather see both, and be > able to toggle off the old ones, rather than only having one or the > other, depending on the toggle, so I thought I'd give this a try. I > kind of liked the one or the other route, but I see the problems with > that too. Please keep everything for the entire deprecation period, unconditionally. Thank you.
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
From: Stephen Hemminger Date: Fri, 2 Oct 2020 12:13:17 -0700 > On Fri, 2 Oct 2020 13:40:01 -0400 > Jarod Wilson wrote: > >> By default, enable retaining all user-facing API that includes the use of >> master and slave, but add a Kconfig knob that allows those that wish to >> remove it entirely do so in one shot. > > This is problematic. You are printing both old and new values. > Also every distribution will have to enable it. > > This looks like too much of change to users. Agreed, no Kconfig knob. Keep everything during the deprecation period, then you can kill off the old names at the end of the deprecation period. I don't want to create a situation where distribution X lists as a "feature" that they are able to enable this Kconfig value even though it breaks UAPI for legacy external code out there. That's the wrong way to go about this and creates the wrong incentive system. I also agree that you can't change the docs to stop mentioning the old names. It's almost like we are pretending we aren't doing the transition and that only the new names matter. The old names matter and must therefore be acknowledged, exist, and be referencable in the documentation until the end of the deprecation period. You can mark them "(DEPRECATED)" or similar, but they can't be removed just yet. Thank you.
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
On Fri, 2 Oct 2020 16:23:46 -0400 Jarod Wilson wrote: > On Fri, Oct 2, 2020 at 3:13 PM Stephen Hemminger > wrote: > > > > On Fri, 2 Oct 2020 13:40:01 -0400 > > Jarod Wilson wrote: > > > > > By default, enable retaining all user-facing API that includes the use of > > > master and slave, but add a Kconfig knob that allows those that wish to > > > remove it entirely do so in one shot. > > > > > > Cc: Jay Vosburgh > > > Cc: Veaceslav Falico > > > Cc: Andy Gospodarek > > > Cc: "David S. Miller" > > > Cc: Jakub Kicinski > > > Cc: Thomas Davis > > > Cc: net...@vger.kernel.org > > > Signed-off-by: Jarod Wilson > > > --- > > > drivers/net/Kconfig | 12 > > > drivers/net/bonding/bond_main.c | 4 ++-- > > > drivers/net/bonding/bond_options.c| 4 ++-- > > > drivers/net/bonding/bond_procfs.c | 8 > > > drivers/net/bonding/bond_sysfs.c | 14 ++ > > > drivers/net/bonding/bond_sysfs_port.c | 6 -- > > > 6 files changed, 38 insertions(+), 10 deletions(-) > > > > > > > This is problematic. You are printing both old and new values. > > Also every distribution will have to enable it. > > > > This looks like too much of change to users. > > I'd had a bit of feedback that people would rather see both, and be > able to toggle off the old ones, rather than only having one or the > other, depending on the toggle, so I thought I'd give this a try. I > kind of liked the one or the other route, but I see the problems with > that too. > > For simplicity, I'm kind of liking the idea of just not updating the > proc and sysfs interfaces, have a toggle entirely disable them, and > work on enhancing userspace to only use netlink, but ... it's going to > be a while before any such work makes its way to any already shipping > distros. I don't have a satisfying answer here. > I like the idea of having bonding proc and sysf apis optional.
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
On Fri, Oct 2, 2020 at 3:13 PM Stephen Hemminger wrote: > > On Fri, 2 Oct 2020 13:40:01 -0400 > Jarod Wilson wrote: > > > By default, enable retaining all user-facing API that includes the use of > > master and slave, but add a Kconfig knob that allows those that wish to > > remove it entirely do so in one shot. > > > > Cc: Jay Vosburgh > > Cc: Veaceslav Falico > > Cc: Andy Gospodarek > > Cc: "David S. Miller" > > Cc: Jakub Kicinski > > Cc: Thomas Davis > > Cc: net...@vger.kernel.org > > Signed-off-by: Jarod Wilson > > --- > > drivers/net/Kconfig | 12 > > drivers/net/bonding/bond_main.c | 4 ++-- > > drivers/net/bonding/bond_options.c| 4 ++-- > > drivers/net/bonding/bond_procfs.c | 8 > > drivers/net/bonding/bond_sysfs.c | 14 ++ > > drivers/net/bonding/bond_sysfs_port.c | 6 -- > > 6 files changed, 38 insertions(+), 10 deletions(-) > > > > This is problematic. You are printing both old and new values. > Also every distribution will have to enable it. > > This looks like too much of change to users. I'd had a bit of feedback that people would rather see both, and be able to toggle off the old ones, rather than only having one or the other, depending on the toggle, so I thought I'd give this a try. I kind of liked the one or the other route, but I see the problems with that too. For simplicity, I'm kind of liking the idea of just not updating the proc and sysfs interfaces, have a toggle entirely disable them, and work on enhancing userspace to only use netlink, but ... it's going to be a while before any such work makes its way to any already shipping distros. I don't have a satisfying answer here. -- Jarod Wilson ja...@redhat.com
Re: [PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
On Fri, 2 Oct 2020 13:40:01 -0400 Jarod Wilson wrote: > By default, enable retaining all user-facing API that includes the use of > master and slave, but add a Kconfig knob that allows those that wish to > remove it entirely do so in one shot. > > Cc: Jay Vosburgh > Cc: Veaceslav Falico > Cc: Andy Gospodarek > Cc: "David S. Miller" > Cc: Jakub Kicinski > Cc: Thomas Davis > Cc: net...@vger.kernel.org > Signed-off-by: Jarod Wilson > --- > drivers/net/Kconfig | 12 > drivers/net/bonding/bond_main.c | 4 ++-- > drivers/net/bonding/bond_options.c| 4 ++-- > drivers/net/bonding/bond_procfs.c | 8 > drivers/net/bonding/bond_sysfs.c | 14 ++ > drivers/net/bonding/bond_sysfs_port.c | 6 -- > 6 files changed, 38 insertions(+), 10 deletions(-) > This is problematic. You are printing both old and new values. Also every distribution will have to enable it. This looks like too much of change to users.
[PATCH net-next v2 6/6] bonding: make Kconfig toggle to disable legacy interfaces
By default, enable retaining all user-facing API that includes the use of
master and slave, but add a Kconfig knob that allows those that wish to
remove it entirely do so in one shot.
Cc: Jay Vosburgh
Cc: Veaceslav Falico
Cc: Andy Gospodarek
Cc: "David S. Miller"
Cc: Jakub Kicinski
Cc: Thomas Davis
Cc: net...@vger.kernel.org
Signed-off-by: Jarod Wilson
---
drivers/net/Kconfig | 12
drivers/net/bonding/bond_main.c | 4 ++--
drivers/net/bonding/bond_options.c| 4 ++--
drivers/net/bonding/bond_procfs.c | 8
drivers/net/bonding/bond_sysfs.c | 14 ++
drivers/net/bonding/bond_sysfs_port.c | 6 --
6 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c3dbe64e628e..1a13894820cb 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -56,6 +56,18 @@ config BONDING
To compile this driver as a module, choose M here: the module
will be called bonding.
+config BONDING_LEGACY_INTERFACES
+ default y
+ bool "Maintain legacy bonding interface names"
+ help
+ The bonding driver historically made use of the terms "master" and
+ "slave" to describe it's component members. This has since been
+ changed to "bond" and "port" as part of a broader effort to remove
+ the use of socially problematic language from the kernel. However,
+ removing all such cases requires breaking long-standing user-facing
+ interfaces in /proc and /sys, which will not be done, unless you
+ opt out of them here, by selecting 'N'.
+
config DUMMY
tristate "Dummy net driver support"
help
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b8a351d85da4..226d5fb76221 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -194,7 +194,7 @@ module_param(lp_interval, uint, 0);
MODULE_PARM_DESC(lp_interval, "The number of seconds between instances where "
"the bonding driver sends learning packets to "
"each port's peer switch. The default is 1.");
-/* legacy compatability module parameters */
+#ifdef CONFIG_BONDING_LEGACY_INTERFACES
module_param_named(all_slaves_active, apa, int, 0644);
MODULE_PARM_DESC(all_slaves_active, "Keep all frames received on an interface "
"by setting active flag for all slaves; "
@@ -205,7 +205,7 @@ MODULE_PARM_DESC(packets_per_slave, "Packets to send per
slave in balance-rr "
"mode; 0 for a random slave, 1 packet per "
"slave (default), >1 packets per slave. "
"(Legacy compat synonym for
packets_per_port).");
-/* end legacy compatability module parameters */
+#endif
/*- Global variables */
diff --git a/drivers/net/bonding/bond_options.c
b/drivers/net/bonding/bond_options.c
index 8e4050c2b08e..630079ba5452 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -434,7 +434,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
.values = bond_intmax_tbl,
.set = bond_option_peer_notif_delay_set
},
-/* legacy sysfs interfaces */
+#ifdef CONFIG_BONDING_LEGACY_INTERFACES
[BOND_OPT_PACKETS_PER_SLAVE] = {
.id = BOND_OPT_PACKETS_PER_SLAVE,
.name = "packets_per_slave",
@@ -467,7 +467,7 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
.flags = BOND_OPTFLAG_RAWVAL,
.set = bond_option_ports_set
},
-/* end legacy sysfs interfaces */
+#endif
};
/* Searches for an option by name */
diff --git a/drivers/net/bonding/bond_procfs.c
b/drivers/net/bonding/bond_procfs.c
index 2e65472e3c58..8e4a03d86329 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -86,8 +86,10 @@ static void bond_info_show_bond_dev(struct seq_file *seq)
primary = rcu_dereference(bond->primary_port);
seq_printf(seq, "Primary Port: %s",
primary ? primary->dev->name : "None");
+#ifdef CONFIG_BONDING_LEGACY_INTERFACES
seq_printf(seq, "Primary Slave: %s",
primary ? primary->dev->name : "None");
+#endif
if (primary) {
optval = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT,
bond->params.primary_reselect);
@@ -97,8 +99,10 @@ static void bond_info_show_bond_dev(struct seq_file *seq)
seq_printf(seq, "\nCurrently Active Port: %s\n",
(curr) ? curr->dev->name : "None");
+#ifdef CONFIG_BONDING_LEGACY_INTERFACES
seq_printf(seq, "Currently
