[PATCH 3/3] bonding: make device count build-time configurable

[PATCH 3/3] bonding: make device count build-time configurable

[Lubomir Rintel]

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(-)

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;
-- 
2.5.0

Re: [PATCH 3/3] bonding: make device count build-time configurable

[Jay Vosburgh]

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

Re: [PATCH 3/3] bonding: make device count build-time configurable

[Lubomir Rintel]

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

Re: [PATCH 3/3] bonding: make device count build-time configurable

[Jay Vosburgh]

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

Re: [PATCH 3/3] bonding: make device count build-time configurable

[David Miller]

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.

Re: [PATCH 3/3] bonding: make device count build-time configurable

[David Miller]

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.

Re: [PATCH 3/3] bonding: make device count build-time configurable

[Bjørn Mork]

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

Re: [PATCH 3/3] bonding: make device count build-time configurable

[Lubomir Rintel]

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