Re: [PATCH net] bonding: set default miimon value for non-arp modes if not set

[Jarod Wilson]

On 2018-07-18 3:51 PM, Mahesh Bandewar (àààà ààààààà) wrote:
> On Wed, Jul 18, 2018 at 11:49 AM, Jarod Wilson <jarod@xxxxxxxxxx> wrote:
> > For some time now, if you load the bonding driver and configure bond
> > parameters via sysfs using minimal config options, such as specifying
> > nothing but the mode, relying on defaults for everything else, modes
> > that cannot use arp monitoring (802.3ad, balance-tlb, balance-alb) all
> > wind up with both arp_interval=0 (as it should be) and miimon=0, which
> > means the miimon monitor thread never actually runs. This is particularly
> > problematic for 802.3ad.
> >
> > For example, from an LNST recipe I've set up:
> >
> > $ modprobe bonding max_bonds=0"
> > $ echo "+t_bond0" > /sys/class/net/bonding_masters"
> > $ ip link set t_bond0 down"
> > $ echo "802.3ad" > /sys/class/net/t_bond0/bonding/mode"
> > $ ip link set ens1f1 down"
> > $ echo "+ens1f1" > /sys/class/net/t_bond0/bonding/slaves"
> > $ ip link set ens1f0 down"
> > $ echo "+ens1f0" > /sys/class/net/t_bond0/bonding/slaves"
> > $ ethtool -i t_bond0"
> > $ ip link set ens1f1 up"
> > $ ip link set ens1f0 up"
> > $ ip link set t_bond0 up"
> > $ ip addr add 192.168.9.1/24 dev t_bond0"
> > $ ip addr add 2002::1/64 dev t_bond0"
> >
> > This bond comes up okay, but things look slightly suspect in
> > /proc/net/bonding/t_bond0 output:
> >
> > $ grep -i mii /proc/net/bonding/t_bond0
> > MII Status: up
> > MII Polling Interval (ms): 0
> > MII Status: up
> > MII Status: up
> This doesn't seem correct since the MII interval set is 0. It should
> set to 100 by default for this mode. This may be the side effect of
> brining up the bond with default more (balance-rr) and then bringing
> bond-down before configuring it. You can probably get away by just not
> bringing down the bond (step 'set ip link bond0 down) in your recipe
> above. But irrespective of that step, this mode  needs miimon and
> should have been set correctly.

I don't think bringing the bond down matters. We run bond_check_params() 
at module load time, and above, it's loaded via 'modprobe bonding 
max_bonds=0', with no mode= set, so bond_check_params() sets bond_mode 
to BOND_MODE_ROUNDROBIN. When we get down to the miimon checks, we check 
for !bond_mode_uses_arp(BOND_MODE_ROUNDROBIN), which ends up false, so 
we never drop into the code block that sets miimon to 100, meaning it's 
still 0. Then we set up a new bond via sysfs as mode 802.3ad, which goes 
through bond_option_mode_set(), which then also does a 
bond_mode_uses_arp() check, but doesn't currently DO anything if you 
also (correctly) have arp_interval=0, so the bit there that would have 
set miimon to 100 also gets skipped. This just makes sure this setup 
path does get a reasonable default value if none was set.

--
Jarod Wilson
jarod@xxxxxxxxxx