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
Now, pull a cable on one of the ports in the bond, then reconnect it, and you'll see:
Slave Interface: ens1f0 MII Status: down Speed: 1000 Mbps Duplex: full
I believe this became a major issue as of commit 4d2c0cda0744, which for 802.3ad bonds, sets slave->link = BOND_LINK_DOWN, with a comment about relying on link monitoring via miimon to set it correctly, but since the miimon work queue never runs, the link just stays marked down.
If we simply tweak bond_option_mode_set() slightly, we can check for the non-arp modes having no miimon value set, and insert BOND_DEFAULT_MIIMON, which gets things back in full working order. This problem exists as far back as 4.14, and might be worth fixing in all stable trees since, though the work-around is to simply specify an miimon value yourself.
Reported-by: Bob Ball ball@umich.edu CC: Jay Vosburgh j.vosburgh@gmail.com CC: Veaceslav Falico vfalico@gmail.com CC: Andy Gospodarek andy@greyhouse.net CC: Mahesh Bandewar maheshb@google.com CC: David S. Miller davem@davemloft.net CC: netdev@vger.kernel.org CC: stable@vger.kernel.org Signed-off-by: Jarod Wilson jarod@redhat.com --- drivers/net/bonding/bond_options.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 98663c50ded0..4d5d01cb8141 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -743,15 +743,20 @@ const struct bond_option *bond_opt_get(unsigned int option) static int bond_option_mode_set(struct bonding *bond, const struct bond_opt_value *newval) { - if (!bond_mode_uses_arp(newval->value) && bond->params.arp_interval) { - netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n", - newval->string); - /* disable arp monitoring */ - bond->params.arp_interval = 0; - /* set miimon to default value */ - bond->params.miimon = BOND_DEFAULT_MIIMON; - netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n", - bond->params.miimon); + if (!bond_mode_uses_arp(newval->value)) { + if (bond->params.arp_interval) { + netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n", + newval->string); + /* disable arp monitoring */ + bond->params.arp_interval = 0; + } + + if (!bond->params.miimon) { + /* set miimon to default value */ + bond->params.miimon = BOND_DEFAULT_MIIMON; + netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n", + bond->params.miimon); + } }
if (newval->value == BOND_MODE_ALB)
On Wed, Jul 18, 2018 at 11:49 AM, Jarod Wilson jarod@redhat.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.
Now, pull a cable on one of the ports in the bond, then reconnect it, and you'll see:
Slave Interface: ens1f0 MII Status: down Speed: 1000 Mbps Duplex: full
I believe this became a major issue as of commit 4d2c0cda0744, which for 802.3ad bonds, sets slave->link = BOND_LINK_DOWN, with a comment about relying on link monitoring via miimon to set it correctly, but since the miimon work queue never runs, the link just stays marked down.
If we simply tweak bond_option_mode_set() slightly, we can check for the non-arp modes having no miimon value set, and insert BOND_DEFAULT_MIIMON, which gets things back in full working order. This problem exists as far back as 4.14, and might be worth fixing in all stable trees since, though the work-around is to simply specify an miimon value yourself.
Reported-by: Bob Ball ball@umich.edu CC: Jay Vosburgh j.vosburgh@gmail.com CC: Veaceslav Falico vfalico@gmail.com CC: Andy Gospodarek andy@greyhouse.net CC: Mahesh Bandewar maheshb@google.com CC: David S. Miller davem@davemloft.net CC: netdev@vger.kernel.org CC: stable@vger.kernel.org Signed-off-by: Jarod Wilson jarod@redhat.com
Acked-by: Mahesh Bandewar maheshb@google.com
drivers/net/bonding/bond_options.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 98663c50ded0..4d5d01cb8141 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c @@ -743,15 +743,20 @@ const struct bond_option *bond_opt_get(unsigned int option) static int bond_option_mode_set(struct bonding *bond, const struct bond_opt_value *newval) {
if (!bond_mode_uses_arp(newval->value) && bond->params.arp_interval) {netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n",newval->string);/* disable arp monitoring */bond->params.arp_interval = 0;/* set miimon to default value */bond->params.miimon = BOND_DEFAULT_MIIMON;netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",bond->params.miimon);
if (!bond_mode_uses_arp(newval->value)) {if (bond->params.arp_interval) {netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n",newval->string);/* disable arp monitoring */bond->params.arp_interval = 0;}if (!bond->params.miimon) {/* set miimon to default value */bond->params.miimon = BOND_DEFAULT_MIIMON;netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",bond->params.miimon);} } if (newval->value == BOND_MODE_ALB)-- 2.16.1
On 2018-07-18 3:51 PM, Mahesh Bandewar (महेश बंडेवार) wrote:
On Wed, Jul 18, 2018 at 11:49 AM, Jarod Wilson jarod@redhat.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.
From: Jarod Wilson jarod@redhat.com Date: Wed, 18 Jul 2018 14:49:36 -0400
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.
...
I believe this became a major issue as of commit 4d2c0cda0744, which for 802.3ad bonds, sets slave->link = BOND_LINK_DOWN, with a comment about relying on link monitoring via miimon to set it correctly, but since the miimon work queue never runs, the link just stays marked down.
If we simply tweak bond_option_mode_set() slightly, we can check for the non-arp modes having no miimon value set, and insert BOND_DEFAULT_MIIMON, which gets things back in full working order. This problem exists as far back as 4.14, and might be worth fixing in all stable trees since, though the work-around is to simply specify an miimon value yourself.
Reported-by: Bob Ball ball@umich.edu
...
Signed-off-by: Jarod Wilson jarod@redhat.com
Applied and queued up for -stable, thank you.
linux-stable-mirror@lists.linaro.org
-
David Miller -
Jarod Wilson -
Mahesh Bandewar (महेश बंडेवार)