Re: [PATCH] bonding: Don't allow mode change via sysfs with slaves present
Le 15/11/2011 21:47, Andy Gospodarek a écrit :
I took a look at the ifenslave package for debian more closely and it
actually looks like devices are enslaved last, after mode is set. Can
you please take a look at this package and confirm what I'm seeing in
the 'pre-up' script.
It appears to me that setup_master sets the mode and enslave_slaves is
called after and enslaves the devices:
# Option slaves deprecated, replaced by bond-slaves, but still supported
# for backward compatibility.
if [ "$IF_BOND_MASTER" ] ; then
if [ "$IF_BOND_SLAVES" ] ; then
# Exit if nothing to do...
[ -z "$BOND_MASTER$BOND_SLAVES" ]&& exit
I'm really surprise by this extract. In the most up to date version of the ifenslave-2.6 package
(1.1.0-19), the order is:
early_setup_master was created to be able to do things that absolutely need to be done before
enslavement. (See the comment just before this function). The idea was to do every possible setup in
setup_master, after enslavement, except those that need to be done in early_setup_master. So having
enslave_slaves after setup_master instead of before is definitely a mistake. Some of the operations
in setup_master must be done after enslavement, in particular selecting the primary slave.
In version 1.1.0-18 (change log below), I checked all the possible order constraints of the sysfs
interface and totally reworked most of the setup code, putting everything in the right order to
achieve consistent results.
ifenslave-2.6 (1.1.0-18) experimental; urgency=low
* Apply patch from Nicolas de Pesloüan:
- Major change: Check and fix the order in which the configuration is
written into /sys files, to ensure reliable results, according to the
tests done in the kernel (in drivers/net/bonding/bond_sysfs.c).
- Ensure that master is properly brought down when changing a parameter
that require it to be down.
- Ensure the master is brought up at the end of the setup, in the case
where ifup won't bring it up because it is currently configuring a slave.
- Add support for some previously unsupported /sys files: ad_select,
num_grat_arp, num_unsol_na, primary_reselect and queue_id.
- Enhance the documentation (README.Debian), to describe all the currently
supported bond-* options.
- Many other changes in the documentation.
- Reverse the order of the arguments to most sysfs_* internal functions, for
It was a hard work, because there really exist many such constraints. I fail to find enough time to
insert the result of this work into Documentation/networking/bonding.txt, but still plan to do so,
even if the result is documented in the script you looked at.
Of course, it is possible to change the scripts in ifenslave-2.6 package to arrange for one more
constraint. For as far as I understand, this would cause the Debian kernel that introduce the change
we discuss about and all the future Debian kernels to be flagged with 'Breaks: ifenslave-2.6 (<<
1.1.0-20)'. I'm not really comfortable with this and the Debian kernel team need to be involved. I
All that being said, I really advocate for less constraints on the sysfs setup. This is not strictly
related to sysfs setup. If we eventually add a NETLINK interface for all the things we can setup
using sysfs, we will face the exact same problem.
I perfectly understand, as Veaceslav noted in another email that there are a lot of mode-specific
initialization stuff that's present only in bond_enslave(), but I think this is what needs to be
fixed... Those initialization stuff should be moved out of bond_enslave() and called at appropriate
sime, from bond_enslave() and from other locations, in particular when changing mode.