lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 25 Sep 2020 08:13:24 -0400 From: Jarod Wilson <jarod@...hat.com> To: Jay Vosburgh <jay.vosburgh@...onical.com> Cc: LKML <linux-kernel@...r.kernel.org>, Veaceslav Falico <vfalico@...il.com>, Andy Gospodarek <andy@...yhouse.net>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Thomas Davis <tadavis@....gov>, Netdev <netdev@...r.kernel.org> Subject: Re: [PATCH net-next 0/5] bonding: rename bond components On Tue, Sep 22, 2020 at 6:19 PM Jay Vosburgh <jay.vosburgh@...onical.com> wrote: > > Jarod Wilson <jarod@...hat.com> wrote: > > >The bonding driver's use of master and slave, while largely understood > >in technical circles, poses a barrier for inclusion to some potential > >members of the development and user community, due to the historical > >context of masters and slaves, particularly in the United States. This > >is a first full pass at replacing those phrases with more socially > >inclusive ones, opting for aggregator to replace master and link to > >replace slave, as the bonding driver itself is a link aggregation > >driver. > > First, I think there should be some direction from the kernel > development leadership as to whether or not this type of large-scale > search and replace of socially sensitive terms of art or other > terminology is a task that should be undertaken, given the noted issues > it will cause in stable release maintenance going forwards. Admittedly, part of the point of this patch is to help drive such conversations. Having a concrete example of how these changes would look makes it easier to discuss, I think. I understand the burden here, though as you noted later, bonding doesn't really churn that much, so in this specific case, the maintenance load wouldn't be terrible, and I think worth it in this case, from a social standpoint. I know this can start to get political and personal real fast though... > Second, on the merits of the proposed changes (presuming for the > moment that this goes forward), I would prefer different nomenclature > that does not reuse existing names for different purposes, i.e., "link" > and "aggregator." > > Both of those have specific meanings in the current code, and > old kernels will retain that meaning. Changing them to have new > meanings going forward will lead to confusion, in my opinion for no good > reason, as there are other names suited that do not conflict. > > For example, instead of "master" call everything a "bond," which > matches common usage in discussion. Changing "master" to "aggregator," > the replacement results in cumbersome descriptions like "the > aggregator's active aggregator" in the context of LACP. > > A replacement term for "slave" is trickier; my first choice > would be "port," but that may make more churn from a code change point > of view, although struct slave could become struct bond_port, and leave > the existing struct port for its current LACP use. I did briefly have the idea of renaming 'port' in the LACP code to 'lacp_port', which would allow reuse of 'port', and would then be consistent with the team driver (and bridge driver, iirc). I could certainly pursue that option, or try going with "bond_port", but I'd like something so widely used throughout the code to be shorter if possible, I think. It really is LACP that throws a wrench into most sensible naming schemes I could think of. Simply renaming current "master" to "bond" does make a fair bit of sense though, didn't really occur to me. But replacing "slave" is definitely the far more involved and messy one. > >There are a few problems with this change. First up, "link" is used for > >link state already in the bonding driver, so the first step here is to > >rename link to link_state. Second, aggregator is already used in the > >802.3ad code, but I feel the usage is actually consistent with referring > >to the bonding aggregation virtual device as the aggregator. Third, we > >have the issue of not wanting to break any existing userspace, which I > >believe this patchset accomplishes, while also adding alternative > >interfaces using new terminology, and a Kconfig option that will let > >people make the conscious decision to break userspace and no longer > >expose the original master/slave interfaces, once their userspace is > >able to cope with their removal. > > I'm opposed to the Kconfig option because it will lead to > balkanization of the UAPI, which would be worse than a clean break > (which I'm also opposed to). I suspected this might be a point of contention. Easy enough to simply omit that bit from the series, if that's the consensus. > >Lastly, we do still have the issue of ease of backporting fixes to > >-stable trees. I've not had a huge amount of time to spend on it, but > >brief forays into coccinelle didn't really pay off (since it's meant to > >operate on code, not patches), and the best solution I can come up with > >is providing a shell script someone could run over git-format-patch > >output before git-am'ing the result to a -stable tree, though scripting > >these changes in the first place turned out to be not the best thing to > >do anyway, due to subtle cases where use of master or slave can NOT yet > >be replaced, so a large amount of work was done by hand, inspection, > >trial and error, which is why this set is a lot longer in coming than > >I'd originally hoped. I don't expect -stable backports to be horrible to > >figure out one way or another though, and I don't believe that a bit of > >inconvenience on that front is enough to warrant not making these > >changes. > > I'm skeptical that, given the scope of the changes involved, > that it's really feasible to have effective automated massaging of > patches. I think the reality is that a large fraction of the bonding > fixes in the future will have to be backported entirely by hand. The > only saving grace here is that the quantity of such patches is generally > low (~40 in 2020 year to date). Yeah, requiring manual backporting by hand is a very distinct possibility. As noted, such patches are usually pretty low in number, and I'll note that they're also generally fairly small patches too. I would be happy to help with any manual backporting as well, as a consolation if scripting them doesn't really work out. -- Jarod Wilson jarod@...hat.com
Powered by blists - more mailing lists