Skip to content

Commit

Permalink
Add STP option for bridge interfaces
Browse files Browse the repository at this point in the history
For Rocky Linux 9, Kayobe will now disable STP on a bridge by default,
to preserve compatibility with network scripts, as Network Manager
enables STP on all bridges by default.
Enabling STP can lead to port down event if BPDU guard is enabled
on the switch.

For CentOS Stream 8 and Rocky Linux 8 enabling STP is not supported.

Closes-Bug: #2028775

Change-Id: I35eaa92f4243af00697306aa801e5a733885ce4f
(cherry picked from commit f1fd127)
  • Loading branch information
bbezak committed Aug 24, 2023
1 parent b9b2e71 commit 967c4be
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 1 deletion.
11 changes: 11 additions & 0 deletions doc/source/configuration/reference/network.rst
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,17 @@ The following attributes are supported:
``bridge_ports``
For bridge interfaces, a list of names of network interfaces to add to the
bridge.
``bridge_stp``
.. note::

For CentOS Stream 8 and Rocky Linux 8 enabling STP is not supported.

For Rocky Linux 9, the ``bridge_stp`` attribute is set to false to preserve
backwards compatibility with network scripts. This is because the Network
Manager sets STP to true by default on bridges.

Enable or disable the Spanning Tree Protocol (STP) on this bridge. Should be
set to a boolean value. The default is not set on Ubuntu systems.
``bond_mode``
For bond interfaces, the bond's mode, e.g. 802.3ad.
``bond_ad_select``
Expand Down
3 changes: 3 additions & 0 deletions kayobe/plugins/filter/networkd.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def _bridge_netdev(context, name, inventory_hostname):
"""
device = networks.net_interface(context, name, inventory_hostname)
mtu = networks.net_mtu(context, name, inventory_hostname)
stp = networks.net_bridge_stp(context, name, inventory_hostname)
config = [
{
'NetDev': [
Expand All @@ -126,6 +127,8 @@ def _bridge_netdev(context, name, inventory_hostname):
]
}
]
if stp is not None:
config[0]['Bridge'] = [{'STP': stp}]
return _filter_options(config)


Expand Down
33 changes: 33 additions & 0 deletions kayobe/plugins/filter/networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,36 @@ def net_mtu(context, name, inventory_hostname=None):
return mtu


@jinja2.pass_context
def net_bridge_stp(context, name, inventory_hostname=None):
"""Return the Spanning Tree Protocol (STP) state for a bridge.
On RL9 if STP is not defined, default it to 'false' to preserve
compatibility with network scripts. STP is 'true' in NetworkManager
by default, so we set it to 'false' here.
:param context: Jinja2 Context object.
:param name: The name of the network.
:param inventory_hostname: Ansible inventory hostname.
:returns: A string "true" or "false" representing the STP state.
"""
bridge_stp = net_attr(context, name, 'bridge_stp', inventory_hostname)
os_family = context['ansible_facts']['os_family']
os_version = context['ansible_facts']['distribution_major_version']

if os_family == 'RedHat' and os_version == '8':
return None

if bridge_stp is None:
if os_family == 'RedHat' and os_version == '9':
return 'false'
else:
return None

bridge_stp = str(utils.call_bool_filter(context, bridge_stp)).lower()
return bridge_stp


net_routes = _make_attr_filter('routes')
net_rules = _make_attr_filter('rules')
net_physical_network = _make_attr_filter('physical_network')
Expand Down Expand Up @@ -425,6 +455,7 @@ def net_bridge_obj(context, name, inventory_hostname=None):
defroute = net_defroute(context, name, inventory_hostname)
ethtool_opts = net_ethtool_opts(context, name, inventory_hostname)
zone = net_zone(context, name, inventory_hostname)
stp = net_bridge_stp(context, name, inventory_hostname)
vip_address = net_vip_address(context, name, inventory_hostname)
allowed_addresses = [vip_address] if vip_address else None
_validate_rules(rules)
Expand All @@ -444,6 +475,7 @@ def net_bridge_obj(context, name, inventory_hostname=None):
'zone': zone,
'allowed_addresses': allowed_addresses,
'onboot': 'yes',
'stp': stp,
}
interface = {k: v for k, v in interface.items() if v is not None}
return interface
Expand Down Expand Up @@ -728,6 +760,7 @@ def get_filters():
'net_defroute': net_defroute,
'net_ethtool_opts': net_ethtool_opts,
'net_zone': net_zone,
'net_bridge_stp': net_bridge_stp,
'net_interface_obj': net_interface_obj,
'net_bridge_obj': net_bridge_obj,
'net_bond_obj': net_bond_obj,
Expand Down
11 changes: 10 additions & 1 deletion kayobe/tests/unit/plugins/filter/test_networkd.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ def setUp(self):
# Bandit complains about Jinja2 autoescaping without nosec.
self.env = jinja2.Environment() # nosec
self.env.filters['bool'] = to_bool
self.variables.update({
'ansible_facts': {
'os_family': 'Debian',
'distribution_major_version': '22'
}
})
self.context = self._make_context(self.variables)

def _make_context(self, parent):
Expand Down Expand Up @@ -202,7 +208,7 @@ def test_bridge(self):
self.assertEqual(expected, devs)

def test_bridge_all_options(self):
self._update_context({"net3_mtu": 1400})
self._update_context({"net3_mtu": 1400, "net3_bridge_stp": True})
devs = networkd.networkd_netdevs(self.context, ["net3"])
expected = {
"50-kayobe-br0": [
Expand All @@ -211,6 +217,9 @@ def test_bridge_all_options(self):
{"Name": "br0"},
{"Kind": "bridge"},
{"MTUBytes": 1400},
],
"Bridge": [
{"STP": "true"},
]
},
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ test_net_eth_vlan_zone: test-zone1
test_net_bridge_cidr: 192.168.36.0/24
test_net_bridge_interface: br0
test_net_bridge_bridge_ports: [dummy3, dummy4]
test_net_bridge_bridge_stp: false
test_net_bridge_zone: test-zone2

# br0.43: VLAN subinterface of br0.
Expand All @@ -80,6 +81,7 @@ test_net_bond_vlan_zone: public
test_net_bridge_noip_cidr: 192.168.40.0/24
test_net_bridge_noip_interface: br1
test_net_bridge_noip_bridge_ports: [dummy7]
test_net_bridge_noip_bridge_stp: true
test_net_bridge_noip_no_ip: true

{% if ansible_os_family == "Debian" %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ def test_network_bridge(host):
interface = host.interface('br0')
assert interface.exists
assert '192.168.36.1' in interface.addresses
stp_status = host.file('/sys/class/net/br0/bridge/stp_state').content_string.strip()
assert '0' == stp_status
ports = ['dummy3', 'dummy4']
sys_ports = host.check_output('ls -1 /sys/class/net/br0/brif')
assert sys_ports == "\n".join(ports)
Expand Down Expand Up @@ -102,6 +104,9 @@ def test_network_bridge_no_ip(host):
interface = host.interface('br1')
assert interface.exists
assert not '192.168.40.1' in interface.addresses
if not host.system_info.release.startswith('8'):
stp_status = host.file('/sys/class/net/br1/bridge/stp_state').content_string.strip()
assert '1' == stp_status


@pytest.mark.skipif(not _is_apt(),
Expand Down
13 changes: 13 additions & 0 deletions releasenotes/notes/add-bridge-stp-option-9be6bf35f6425548.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
features:
- |
The Spanning Tree Protocol (STP) can now be configured on bridge interfaces.
Enable or disable STP by setting the ``bridge_stp`` attribute for a network.
Note that STP is not set by default on Ubuntu, but it is disabled on Rocky
Linux 9 for compatibility with network scripts, as NetworkManager enables
STP on all bridges by default.
For CentOS Stream 8 and Rocky Linux 8 enabling STP is not supported.
upgrade:
- |
For Rocky Linux 9, Kayobe now disables STP on a bridge by default. This
action will cause the bridge interface to restart during the host
configuration process.

0 comments on commit 967c4be

Please sign in to comment.