Re: [PATCH v2 net] bonding: don't use stale speed and duplex information
From: Jay Vosburgh <hidden>
Date: 2016-02-25 13:34:25
zhuyj [off-list ref] wrote: [...]
quoted hunk ↗ jump to hunk
I delved into the source code and Emil's tests. I think that the problem that this patch expects to fix occurs very unusually. Do you agree with me? If so, maybe the following patch can reduce the performance loss. Please comment on it. Thanks a lot.diff --git a/drivers/net/bonding/bond_main.cb/drivers/net/bonding/bond_main.c index b7f1a99..c4c511a 100644--- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c@@ -2129,7 +2129,9 @@ static void bond_miimon_commit(struct bonding *bond) continue; case BOND_LINK_UP: - bond_update_speed_duplex(slave); + if (slave->speed == SPEED_UNKNOWN) + bond_update_speed_duplex(slave); + bond_set_slave_link_state(slave, BOND_LINK_UP,BOND_SLAVE_NOTIFY_NOW); slave->last_link_up = jiffies;
I don't believe the speed is necessarily SPEED_UNKNOWN coming in here. If the race occurs at a time later than the initial enslavement, speed may already be set (and the race manifests if the new speed changes, i.e., the link changes from 1 Gb/sec to 10 Gb/sec), so I don't think this is functionally correct. Also, the call to bond_miimon_commit itself is already gated by bond_miimon_inspect finding a link state change. The performance impact here should be minimal. -J --- -Jay Vosburgh, jay.vosburgh@canonical.com