← Back to team overview

netplan-developers team mailing list archive

[Merge] netplan:cyphermox/stp into netplan:master

 

Mathieu Trudel-Lapierre has proposed merging netplan:cyphermox/stp into netplan:master.

Requested reviews:
  Tiago Stürmer Daitx (tdaitx)

For more details, see:
https://code.launchpad.net/~netplan-developers/netplan/+git/netplan/+merge/318941

Add STP support for bridges. This is part of the typical set of features for parameterizing bridge configurations, but was forgotten while adding the possiblity of setting the other parameters.
-- 
Your team Developers of netplan is subscribed to branch netplan:master.
diff --git a/doc/netplan.md b/doc/netplan.md
index 35378dd..5c25fe2 100644
--- a/doc/netplan.md
+++ b/doc/netplan.md
@@ -276,6 +276,11 @@ Properties for device type ``bridges:``
           a lower cost. This allows a finer control on the network topology
           so that the fastest paths are available whenever possible.
 
+     ``stp`` (bool)
+     :    Define whether the bridge should use Spanning Tree Protocol. The
+          default value is "true", which means that Spanning Tree should be
+          used.
+
 
 Properties for device type ``bonds:``
 =======================================
diff --git a/src/networkd.c b/src/networkd.c
index 20b0621..6a72370 100644
--- a/src/networkd.c
+++ b/src/networkd.c
@@ -61,23 +61,25 @@ write_bridge_params(GString* s, net_definition* def)
 {
     GString *params = NULL;
 
-    params = g_string_sized_new(200);
+    if (def->custom_bridging) {
+        params = g_string_sized_new(200);
+
+        if (def->bridge_params.ageing_time)
+            g_string_append_printf(params, "AgeingTimeSec=%u\n", def->bridge_params.ageing_time);
+        if (def->bridge_params.priority)
+            g_string_append_printf(params, "Priority=%u\n", def->bridge_params.priority);
+        if (def->bridge_params.forward_delay)
+            g_string_append_printf(params, "ForwardDelaySec=%u\n", def->bridge_params.forward_delay);
+        if (def->bridge_params.hello_time)
+            g_string_append_printf(params, "HelloTimeSec=%u\n", def->bridge_params.hello_time);
+        if (def->bridge_params.max_age)
+            g_string_append_printf(params, "MaxAgeSec=%u\n", def->bridge_params.max_age);
+        g_string_append_printf(params, "STP=%s\n", def->bridge_params.stp ? "true" : "false");
 
-    if (def->bridge_params.ageing_time)
-        g_string_append_printf(params, "AgeingTimeSec=%u\n", def->bridge_params.ageing_time);
-    if (def->bridge_params.priority)
-        g_string_append_printf(params, "Priority=%u\n", def->bridge_params.priority);
-    if (def->bridge_params.forward_delay)
-        g_string_append_printf(params, "ForwardDelaySec=%u\n", def->bridge_params.forward_delay);
-    if (def->bridge_params.hello_time)
-        g_string_append_printf(params, "HelloTimeSec=%u\n", def->bridge_params.hello_time);
-    if (def->bridge_params.max_age)
-        g_string_append_printf(params, "MaxAgeSec=%u\n", def->bridge_params.max_age);
-
-    if (params->len > 0)
         g_string_append_printf(s, "\n[Bridge]\n%s", params->str);
 
-    g_string_free(params, TRUE);
+        g_string_free(params, TRUE);
+    }
 }
 
 static void
diff --git a/src/nm.c b/src/nm.c
index 9f81df5..d4fee28 100644
--- a/src/nm.c
+++ b/src/nm.c
@@ -201,23 +201,25 @@ write_bridge_params(const net_definition* def, GString *s)
 {
     GString* params = NULL;
 
-    params = g_string_sized_new(200);
-
-    if (def->bridge_params.ageing_time)
-        g_string_append_printf(params, "ageing-time=%u\n", def->bridge_params.ageing_time);
-    if (def->bridge_params.priority)
-        g_string_append_printf(params, "priority=%u\n", def->bridge_params.priority);
-    if (def->bridge_params.forward_delay)
-        g_string_append_printf(params, "forward-delay=%u\n", def->bridge_params.forward_delay);
-    if (def->bridge_params.hello_time)
-        g_string_append_printf(params, "hello-time=%u\n", def->bridge_params.hello_time);
-    if (def->bridge_params.max_age)
-        g_string_append_printf(params, "max-age=%u\n", def->bridge_params.max_age);
+    if (def->custom_bridging) {
+        params = g_string_sized_new(200);
+
+        if (def->bridge_params.ageing_time)
+            g_string_append_printf(params, "ageing-time=%u\n", def->bridge_params.ageing_time);
+        if (def->bridge_params.priority)
+            g_string_append_printf(params, "priority=%u\n", def->bridge_params.priority);
+        if (def->bridge_params.forward_delay)
+            g_string_append_printf(params, "forward-delay=%u\n", def->bridge_params.forward_delay);
+        if (def->bridge_params.hello_time)
+            g_string_append_printf(params, "hello-time=%u\n", def->bridge_params.hello_time);
+        if (def->bridge_params.max_age)
+            g_string_append_printf(params, "max-age=%u\n", def->bridge_params.max_age);
+        g_string_append_printf(params, "stp=%s\n", def->bridge_params.stp ? "true" : "false");
 
-    if (params->len > 0)
         g_string_append_printf(s, "\n[bridge]\n%s", params->str);
 
-    g_string_free(params, TRUE);
+        g_string_free(params, TRUE);
+    }
 }
 
 /**
diff --git a/src/parse.c b/src/parse.c
index 345c8ad..445f755 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -747,9 +747,18 @@ const mapping_entry_handler bridge_params_handlers[] = {
     {"hello-time", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(bridge_params.hello_time)},
     {"max-age", YAML_SCALAR_NODE, handle_netdef_guint, NULL, netdef_offset(bridge_params.max_age)},
     {"path-cost", YAML_MAPPING_NODE, handle_bridge_path_cost, NULL, netdef_offset(bridge_params.path_cost)},
+    {"stp", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(bridge_params.stp)},
     {NULL}
 };
 
+static gboolean
+handle_bridge(yaml_document_t* doc, yaml_node_t* node, const void* _, GError** error)
+{
+    cur_netdef->custom_bridging = TRUE;
+    cur_netdef->bridge_params.stp = TRUE;
+    return process_mapping(doc, node, bridge_params_handlers, error);
+}
+
 /****************************************************
  * Grammar and handlers for network config "routes" entry
  ****************************************************/
@@ -904,7 +913,7 @@ const mapping_entry_handler bridge_def_handlers[] = {
     {"gateway6", YAML_SCALAR_NODE, handle_gateway6},
     {"nameservers", YAML_MAPPING_NODE, NULL, nameservers_handlers},
     {"interfaces", YAML_SEQUENCE_NODE, handle_interfaces, NULL, netdef_offset(bridge)},
-    {"parameters", YAML_MAPPING_NODE, NULL, bridge_params_handlers},
+    {"parameters", YAML_MAPPING_NODE, handle_bridge},
     {"routes", YAML_SEQUENCE_NODE, handle_routes},
     {NULL}
 };
diff --git a/src/parse.h b/src/parse.h
index 75d6531..ca08783 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -118,7 +118,10 @@ typedef struct net_definition {
         guint hello_time;
         guint max_age;
         guint path_cost;
+        gboolean stp;
     } bridge_params;
+    gboolean custom_bridging;
+
 } net_definition;
 
 typedef enum {
diff --git a/tests/generate.py b/tests/generate.py
index e168283..b995436 100755
--- a/tests/generate.py
+++ b/tests/generate.py
@@ -814,6 +814,7 @@ unmanaged-devices+=interface-name:br0,''')
         forward-delay: 12
         hello-time: 6
         max-age: 24
+        stp: true
         path-cost:
           eno1: 70
       dhcp4: true''')
@@ -823,7 +824,8 @@ unmanaged-devices+=interface-name:br0,''')
                                             'Priority=1000\n'
                                             'ForwardDelaySec=12\n'
                                             'HelloTimeSec=6\n'
-                                            'MaxAgeSec=24\n',
+                                            'MaxAgeSec=24\n'
+                                            'STP=true\n',
                               'br0.network': ND_DHCP4 % 'br0',
                               'eno1.network': '[Match]\nName=eno1\n\n'
                                               '[Network]\nBridge=br0\nLinkLocalAddressing=no\nIPv6AcceptRA=no\n\n'
@@ -1907,6 +1909,7 @@ method=auto
         max-age: 24
         path-cost:
           eno1: 70
+        stp: true
       dhcp4: true''')
 
         self.assert_nm({'eno1': '''[connection]
@@ -1949,6 +1952,7 @@ priority=1000
 forward-delay=12
 hello-time=6
 max-age=24
+stp=true
 
 [ipv4]
 method=auto
diff --git a/tests/integration.py b/tests/integration.py
index b1bfbcc..bdd459f 100755
--- a/tests/integration.py
+++ b/tests/integration.py
@@ -406,6 +406,7 @@ class _CommonTests:
       parameters:
         path-cost:
           ethbr: 50
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -437,6 +438,7 @@ class _CommonTests:
       interfaces: [ethbr]
       parameters:
         priority: 8192
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -468,6 +470,7 @@ class _CommonTests:
       interfaces: [ethbr]
       parameters:
         ageing-time: 21
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -499,6 +502,7 @@ class _CommonTests:
       interfaces: [ethbr]
       parameters:
         max-age: 12
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -530,6 +534,7 @@ class _CommonTests:
       interfaces: [ethbr]
       parameters:
         hello-time: 1
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -561,6 +566,7 @@ class _CommonTests:
       interfaces: [ethbr]
       parameters:
         forward-delay: 10
+        stp: false
       dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
         self.generate_and_settle()
         self.assert_iface_up(self.dev_e_client,
@@ -578,6 +584,39 @@ class _CommonTests:
         with open('/sys/class/net/mybr/bridge/forward_delay') as f:
             self.assertEqual(f.read().strip(), '1000')
 
+    def test_bridge_stp_false(self):
+        self.setup_eth(None)
+        self.start_dnsmasq(None, self.dev_e2_ap)
+        with open(self.config, 'w') as f:
+            f.write('''network:
+  renderer: %(r)s
+  ethernets:
+    ethbr:
+      match: {name: %(e2c)s}
+  bridges:
+    mybr:
+      interfaces: [ethbr]
+      parameters:
+        hello-time: 100000
+        max-age: 100000
+        stp: false
+      dhcp4: yes''' % {'r': self.backend, 'ec': self.dev_e_client, 'e2c': self.dev_e2_client})
+        self.generate_and_settle()
+        self.assert_iface_up(self.dev_e_client,
+                             ['inet 192.168.5.[0-9]+/24'],
+                             ['master'])
+        self.assert_iface_up(self.dev_e2_client,
+                             ['master mybr'],
+                             ['inet '])
+        self.assert_iface_up('mybr',
+                             ['inet 192.168.6.[0-9]+/24'])
+        lines = subprocess.check_output(['bridge', 'link', 'show', 'mybr'],
+                                        universal_newlines=True).splitlines()
+        self.assertEqual(len(lines), 1, lines)
+        self.assertIn(self.dev_e2_client, lines[0])
+        with open('/sys/class/net/mybr/bridge/stp_state') as f:
+            self.assertEqual(f.read().strip(), '0')
+
     def test_bond_base(self):
         self.setup_eth(None)
         self.addCleanup(subprocess.call, ['ip', 'link', 'delete', 'mybond'], stderr=subprocess.DEVNULL)

Follow ups