| Message ID | 1452599916-27511-1-git-send-email-lkundrak@v3.sk |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Lubomir Rintel <lkundrak@v3.sk> wrote: >The devices can be created at run-time for quite some time already and the >load-time device creation collides with attempts to create the device of >the same name: > > # rmmod bonding > # ip link add bond0 type bond > RTNETLINK answers: File exists > >This is pretty much the same situation as was with the block loop devices >which was solved by adding a build-time configuration that the >distributions could use as they deem fit while keeping the default for >compatibility. I agree this is annoying, but I would expect distros to leave this set to 1 (for backwards compatibility with scripts that "modprobe bonding" then assume bond0 exists). This leaves the problem in place for the vast majority of users. Is there a reasonable way to resolve this that would actually fix things for regular distro kernel users? -J --- -Jay Vosburgh, jay.vosburgh@canonical.com
On Tue, 2016-01-12 at 08:34 -0800, Jay Vosburgh wrote: > Lubomir Rintel <lkundrak@v3.sk> wrote: > > > The devices can be created at run-time for quite some time already > > and the > > load-time device creation collides with attempts to create the > > device of > > the same name: > > > > # rmmod bonding > > # ip link add bond0 type bond > > RTNETLINK answers: File exists > > > > This is pretty much the same situation as was with the block loop > > devices > > which was solved by adding a build-time configuration that the > > distributions could use as they deem fit while keeping the default > > for > > compatibility. > > I agree this is annoying, but I would expect distros to leave > this set to 1 (for backwards compatibility with scripts that > "modprobe > bonding" then assume bond0 exists). This leaves the problem in place > for the vast majority of users. It's still an improvement to let the distributions decide if they're keeping "ip link add" broken or possibly affecting the scripts. Given the "modprobe bonding" didn't guarantee the bond0 bevice will be around at least since 2007 it think it's very reasonable for the distros to turn this off. The network management tooling shipped with Fedora (both the legacy network service and NetworkManager) always did the right thing, be it writing to /sys/class/net/bonding_masters or adding the link via rtnetlink. Moreover, NetworkManager already specifically calls "modprobe bonding maxbonds=0" to avoid the creation of an extra "bond0" device (which coincidentally also breaks the naively written scripts if they are executed after NM creates a bond). There's also a good prior art to this; as Daniel Borkmann pointed out in [1], Fedora ships a kernel with CONFIG_BLK_DEV_LOOP_MIN_COUNT=0 happily for 4 releases already. [1] http://marc.info/?l=linux-netdev&m=145261483331891&w=2 > Is there a reasonable way to resolve this that would actually > fix things for regular distro kernel users? Depends on the definition of reasonable. Not being very familiar with the rtnetlink code, it would perhaps be possible to create some half- finished "bond0" device before doing a request_module(), so that the subsequently loaded module wouldn't take it over. It doesn't sound like a good idea to me as it would still cause an extra "bond0" device in case the user chooses a different name and the workarounds such as the one NetworkManager uses would still be necessary. > > -J > > --- > -Jay Vosburgh, jay.vosburgh@canonical.com Lubo
Lubomir Rintel <lkundrak@v3.sk> wrote: >On Tue, 2016-01-12 at 08:34 -0800, Jay Vosburgh wrote: >> Lubomir Rintel <lkundrak@v3.sk> wrote: >> >> > The devices can be created at run-time for quite some time already >> > and the >> > load-time device creation collides with attempts to create the >> > device of >> > the same name: >> > >> > # rmmod bonding >> > # ip link add bond0 type bond >> > RTNETLINK answers: File exists >> > >> > This is pretty much the same situation as was with the block loop >> > devices >> > which was solved by adding a build-time configuration that the >> > distributions could use as they deem fit while keeping the default >> > for >> > compatibility. >> >> I agree this is annoying, but I would expect distros to leave >> this set to 1 (for backwards compatibility with scripts that >> "modprobe >> bonding" then assume bond0 exists). This leaves the problem in place >> for the vast majority of users. > >It's still an improvement to let the distributions decide if they're >keeping "ip link add" broken or possibly affecting the scripts. Given >the "modprobe bonding" didn't guarantee the bond0 bevice will be around >at least since 2007 it think it's very reasonable for the distros to >turn this off. This looks to me like one of those false "let the distros decide" choices, as they'll likely all end up with the same settings, so most users are going to see the same behaviors. If different distros set the various Kconfig options from this patch series to a mish-mash of different settings, then it's not really much of an improvement, either. Anything cross-distro would still have to work all ways, even if it keeps track of the kernel version. >The network management tooling shipped with Fedora (both the legacy >network service and NetworkManager) always did the right thing, be it >writing to /sys/class/net/bonding_masters or adding the link via >rtnetlink. And this is true for the Debian / Ubuntu scripts as well. >Moreover, NetworkManager already specifically calls "modprobe bonding >maxbonds=0" to avoid the creation of an extra "bond0" device (which >coincidentally also breaks the naively written scripts if they are >executed after NM creates a bond). The scripts I'm thinking of are not system provided, but administrator or vendor created scripts that follow some of the now very old instructions in the bonding.txt documentation, e.g., [... from bonding.txt ...] For example, if you wanted to make a simple bond of two e100 devices (presumed to be eth0 and eth1), and have it persist across reboots, edit the appropriate file (/etc/init.d/boot.local or /etc/rc.d/rc.local), and add the following: modprobe bonding mode=balance-alb miimon=100 modprobe e100 ifconfig bond0 192.168.1.1 netmask 255.255.255.0 up ip link set eth0 master bond0 ip link set eth1 master bond0 They're probably (hopefully) old, but there are likely still some lurking out there somewhere. Whether these would ever be upgraded to current kernels is a separate question; I've not seen scripts like this in production use for a few years, but they were common at one time. >There's also a good prior art to this; as Daniel Borkmann pointed out >in [1], Fedora ships a kernel with CONFIG_BLK_DEV_LOOP_MIN_COUNT=0 >happily for 4 releases already. > >[1] http://marc.info/?l=linux-netdev&m=145261483331891&w=2 Ok, if it's been done before and apparently went just fine, then the real intent here is to not auto-create bond0 (and the other devices in the other patches of the series) in distro kernels, correct? I.e., have the "count" values all set to zero, so that the module load doesn't ever auto-create any of these devices. So, why not simply make the change (to not auto-create) unilaterally, without the complexity of a set of Kconfig options that are meant to always be overridden? I'll even help you here by shooting at my own argument: if the old-style bonding scripts from the days of yore would already break if brought forward from, say, RHEL 3 or 4 (about the last place I recall seeing them) to something current, then is there any reason not to simply make the bonding max_bonds=0 by default instead of 1? Even then, the administrator for a system could add "max_bonds=1" to a /etc/modprobe.d/bonding.conf to restore the original behavior. >> Is there a reasonable way to resolve this that would actually >> fix things for regular distro kernel users? > >Depends on the definition of reasonable. Not being very familiar with >the rtnetlink code, it would perhaps be possible to create some half- >finished "bond0" device before doing a request_module(), so that the >subsequently loaded module wouldn't take it over. > >It doesn't sound like a good idea to me as it would still cause an >extra "bond0" device in case the user chooses a different name and the >workarounds such as the one NetworkManager uses would still be >necessary. Yah, any sort of hack in iproute is going to be ugly; I hadn't really thought it through earlier. -J --- -Jay Vosburgh, jay.vosburgh@canonical.com
From: Jay Vosburgh <jay.vosburgh@canonical.com> Date: Tue, 12 Jan 2016 08:34:12 -0800 > Lubomir Rintel <lkundrak@v3.sk> wrote: > >>The devices can be created at run-time for quite some time already and the >>load-time device creation collides with attempts to create the device of >>the same name: >> >> # rmmod bonding >> # ip link add bond0 type bond >> RTNETLINK answers: File exists >> >>This is pretty much the same situation as was with the block loop devices >>which was solved by adding a build-time configuration that the >>distributions could use as they deem fit while keeping the default for >>compatibility. > > I agree this is annoying, but I would expect distros to leave > this set to 1 (for backwards compatibility with scripts that "modprobe > bonding" then assume bond0 exists). This leaves the problem in place > for the vast majority of users. > > Is there a reasonable way to resolve this that would actually > fix things for regular distro kernel users? Yeah I personally don't like this at all. This behavior has 2 decades of precedence, it's therefore hard coded into deep dark nooks and cranies of people's scripts. Just be content with how things are, and document it better if need be.
From: Lubomir Rintel <lkundrak@v3.sk> Date: Tue, 12 Jan 2016 18:19:49 +0100 > It's still an improvement to let the distributions decide if they're > keeping "ip link add" broken or possibly affecting the scripts. That it is "broken" is your opinion. Document the behavior. It is not broken if the user is told to be mindful of what devices are created by default. There is way too much downside to changing this.
David Miller <davem@davemloft.net> writes: > From: Lubomir Rintel <lkundrak@v3.sk> > Date: Tue, 12 Jan 2016 18:19:49 +0100 > >> It's still an improvement to let the distributions decide if they're >> keeping "ip link add" broken or possibly affecting the scripts. > > That it is "broken" is your opinion. > > Document the behavior. It is not broken if the user is told to be > mindful of what devices are created by default. > > There is way too much downside to changing this. Besides, distributions or admins can already change that behaviour if they consider it "broken", using the existing module parameter: # echo "options bonding max_bonds=0" >/etc/modprobe.d/bonding.conf # rmmod bonding # ip link add bond0 type bond (no error here) This method should be well known and understood by most users, contrary to some odd CONFIG_ build time setting. Bjørn
Hi Bjørn, On Tue, 2016-01-12 at 22:40 +0100, Bjørn Mork wrote: > David Miller <davem@davemloft.net> writes: > > From: Lubomir Rintel <lkundrak@v3.sk> > > Date: Tue, 12 Jan 2016 18:19:49 +0100 > > > > > It's still an improvement to let the distributions decide if > > > they're > > > keeping "ip link add" broken or possibly affecting the scripts. > > > > That it is "broken" is your opinion. > > > > Document the behavior. It is not broken if the user is told to be > > mindful of what devices are created by default. > > > > There is way too much downside to changing this. > > Besides, distributions or admins can already change that behaviour if > they consider it "broken", using the existing module parameter: > > # echo "options bonding max_bonds=0" >/etc/modprobe.d/bonding.conf > # rmmod bonding > # ip link add bond0 type bond > (no error here) > > This method should be well known and understood by most users, > contrary > to some odd CONFIG_ build time setting. Yes, that's an alternative solution. We may end up shipping such configuration file, though it's not really clear what package should ship it (probably systemd?). I'd still prefer a kernel build-time option. It's more likely for distributions to do the decision they prefer when running make oldconfig. I'm assuming most distros would like to drop the legacy behavior; at this point noone probably relies on it anyway, given NetworkManager works around this by manually loading the module with the maxbonds=0 manually. Also, there's prior art to addressing this in kernel; the block loopback. > Bjørn Regards, Lubo
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index f94af69..51de664 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -55,6 +55,21 @@ config BONDING To compile this driver as a module, choose M here: the module will be called bonding. +config BONDING_COUNT + int "Number of bonding devices to pre-create at init time" + depends on BONDING + default 1 + help + Static number of bonding devices to be unconditionally pre-created + at init time. + + This default value can be overwritten on the kernel command + line or with module-parameter bonding.max_bonds. + + The historic default is 1. If a mid-2007 version of iproute2 + is used (v2.6.23 or later), it can be set to 0, since needed + bonding devices can be dynamically allocated via rtnetlink. + 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 9e0f8a7..f013cfa 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -88,7 +88,7 @@ /* monitor all links that often (in milliseconds). <=0 disables monitoring */ -static int max_bonds = BOND_DEFAULT_MAX_BONDS; +static int max_bonds = CONFIG_BONDING_COUNT; static int tx_queues = BOND_DEFAULT_TX_QUEUES; static int num_peer_notif = 1; static int miimon;
The devices can be created at run-time for quite some time already and the load-time device creation collides with attempts to create the device of the same name: # rmmod bonding # ip link add bond0 type bond RTNETLINK answers: File exists This is pretty much the same situation as was with the block loop devices which was solved by adding a build-time configuration that the distributions could use as they deem fit while keeping the default for compatibility. Let's do that here as well. Signed-off-by: Lubomir Rintel <lkundrak@v3.sk> --- drivers/net/Kconfig | 15 +++++++++++++++ drivers/net/bonding/bond_main.c | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-)