mirror of
https://github.com/CumulusNetworks/ifupdown2.git
synced 2024-05-06 15:54:50 +00:00
Fix mstp settings ordering issues when bridge stp is toggled on and off
(when mstp settings are specified under the port) Ticket: CM-6626 Reviewed By: CCR-3599 Testing Done: Tested the problem case mentioned in the bug (Plan to re-work the fix a bit for 2.5.5) problem: mstp parameters can be specified under the port or under the bridge When they are specified under the bridge, there should be no problem (When you process the bridge, all things on the bridge port are set in order) When they are specified under the port: we check if the bridge is up, if yes, and stp is already configured, we process mstpctl settings This check today only checks running stp_state We should also check the user configured stp state for the bridge Note that the problem exists only if the bridge and ports are already up and user executes ifup again and when we need to re-evaluate the bridge port settings solution: When the bridge port is being checked for mstp settings, not only check running stp state, but also check user specified stp state and correct bridge stp state before proceeding with mstp configuration Few things to note with this patch: - If user changed stp state on bridge to 'on', and did `ifup <bridge_port>'`, though the user has not ifup'ed the bridge yet, the bridge stp state will be applied (very few people will run into this and practically this should not be a problem atall. But from correctness POV it is a voilation. ). - To avoid this, the other solution would have been to let the bridge port code be as is, but handle this during bringing up the bridge, but, this can confuse the user, because when processing the bridge_port, you will still throw warnings even if the port stp config is later reapplied when the bridge comes up - note that the patch only handles the case when stp state is not on and somebody turned it on. For the case where stp state was on and somebody turned it off, this patch wont throw warnings for the mstpctl config. But it will not cause any issues because once stp is off things will be ok everywhere. I want to keep any changes here out of this patch. I will document this in the bug...and see if i can handle this in 2.5.5
This commit is contained in:
@ -359,15 +359,34 @@ class mstpctl(moduleBase):
|
||||
except Exception, e:
|
||||
self.log_warn(str(e))
|
||||
|
||||
def _is_stp_on(self, ifaceobj, bridgename, ifaceobj_getfunc=None):
|
||||
# Check if running stp state is on
|
||||
running_stp_state = (True if self.read_file_oneline(
|
||||
'/sys/class/net/%s/bridge/stp_state'
|
||||
%bridgename) == '2' else False)
|
||||
if not running_stp_state:
|
||||
# If running stp is not on, and bridge has stp configured
|
||||
# set stp on bridge
|
||||
if ifaceobj_getfunc:
|
||||
bridgeifaceobjlist = ifaceobj_getfunc(bridgename)
|
||||
if not bridgeifaceobjlist:
|
||||
return running_stp_state
|
||||
for b in bridgeifaceobjlist:
|
||||
stp_attrval = b.get_attr_value_first('bridge-stp')
|
||||
if stp_attrval and (stp_attrval == 'on' or
|
||||
stp_attrval == 'yes'):
|
||||
# set stp on bridge and return True
|
||||
self.brctlcmd.set_stp(bridgename, stp_attrval)
|
||||
return True
|
||||
return running_stp_state
|
||||
|
||||
def _up(self, ifaceobj, ifaceobj_getfunc=None):
|
||||
# Check if bridge port
|
||||
bridgename = self.ipcmd.bridge_port_get_bridge_name(ifaceobj.name)
|
||||
if bridgename:
|
||||
mstpd_running = (True if self.mstpctlcmd.is_mstpd_running()
|
||||
else False)
|
||||
stp_on = (True if self.read_file_oneline(
|
||||
'/sys/class/net/%s/bridge/stp_state'
|
||||
%bridgename) == '2' else False)
|
||||
stp_on = self._is_stp_on(ifaceobj, bridgename, ifaceobj_getfunc)
|
||||
self._apply_bridge_port_settings(ifaceobj, bridgename, None,
|
||||
stp_on, mstpd_running)
|
||||
ifaceobj.module_flags[self.name] = ifaceobj.module_flags.setdefault(self.name,0) | \
|
||||
|
Reference in New Issue
Block a user