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

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <83c0327d-8e93-6036-c34b-f8d59223b974@redhat.com>
Date:   Wed, 18 Jul 2018 17:07:07 -0400
From:   Jarod Wilson <jarod@...hat.com>
To:     Mahesh Bandewar (महेश बंडेवार) <maheshb@...gle.com>
Cc:     linux-kernel@...r.kernel.org, Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        "David S . Miller" <davem@...emloft.net>,
        linux-netdev <netdev@...r.kernel.org>, stable@...r.kernel.org
Subject: Re: [PATCH net] bonding: set default miimon value for non-arp modes
 if not set

On 2018-07-18 3:51 PM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Wed, Jul 18, 2018 at 11:49 AM, Jarod Wilson <jarod@...hat.com> 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@...hat.com