[change] OpenWrt: simplified bridged wifi interface #191

Made it easier for users to bridge wifi interfaces without having to know
they have to define the "network" attribute in the WiFi interface.

Closes #191

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>

Author: pandafy

docs/source/backends/openwrt.rst

@@ -745,8 +745,8 @@ UCI output::
 Wireless attached to a different network
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-In some cases you might want to attach a wireless interface to a different network,
-for example, you might want to attach a wireless interface to a bridge:
+In most cases you want to bridge a wireless interface to a different network,
+usually the LAN bridge:
 
 .. code-block:: python
 
@@ -763,8 +763,8 @@ for example, you might want to attach a wireless interface to a bridge:
                     "radio": "radio0",
                     "mode": "access_point",
                     "ssid": "wifi service",
-                    # the wireless interface will be attached to the "lan" network
-                    "network": ["lan"]
+                    # "network": ["lan"]  this proeprty can be omitted
+                    # but may be overridden if needed
                 }
             },
             {
@@ -776,9 +776,7 @@ for example, you might want to attach a wireless interface to a bridge:
                 ],
                 "addresses": [
                     {
-                        "address": "192.168.0.2",
-                        "mask": 24,
-                        "proto": "static",
+                        "proto": "dhcp",
                         "family": "ipv4"
                     }
                 ]
@@ -800,9 +798,7 @@ Will be rendered as follows::
 
     config interface 'lan'
             option ifname 'eth0 wlan0'
-            option ipaddr '192.168.0.2'
-            option netmask '255.255.255.0'
-            option proto 'static'
+            option proto 'dhcp'
             option type 'bridge'
 
     package wireless

netjsonconfig/backends/openwrt/converters/wireless.py

@@ -6,6 +6,10 @@ class Wireless(OpenWrtConverter):
     intermediate_key = 'wireless'
     _uci_types = ['wifi-iface']
 
+    def to_intermediate(self):
+        self._track_bridged_wifi()
+        return super().to_intermediate()
+
     def to_intermediate_loop(self, block, result, index=None):
         wireless = self.__intermediate_wireless(block)
         if wireless:
@@ -62,9 +66,16 @@ def __intermediate_wireless(self, interface):
         # to its defining interface
         # but this behaviour can be overridden
         if not wireless.get('network'):
-            # get network, default to ifname
-            network = interface.get('network', interface['name'])
-            wireless['network'] = [network]
+            # try to automatically determine whether
+            # we should attach this interface to a bridge
+            try:
+                bridges = self._bridged_wifi[interface['name']]
+            except KeyError:
+                # default to the value of "network" or inteface name
+                network = [interface.get('network', interface['name'])]
+            else:
+                network = bridges
+            wireless['network'] = network
         wireless['network'] = (
             ' '.join(wireless['network']).replace('.', '_').replace('-', '_')
         )
@@ -280,3 +291,54 @@ def __get_netjson_interface(self, wifi):
             if interface['name'] == wifi['ifname']:
                 interface['type'] = 'wireless'
                 return interface
+
+    def to_netjson_clean(self, intermediate_data):
+        result = super().to_netjson_clean(intermediate_data)
+        return self.__fix_netjson_network(result)
+
+    def __fix_netjson_network(self, result):
+        """
+        Figures out whether it should remove the network attribute
+        From the netjson wifi interface (because it's redundant)
+        """
+        self._track_bridged_wifi(self.backend._intermediate_copy)
+        for index, interface in enumerate(result):
+            try:
+                bridges = self._bridged_wifi[interface['ifname']]
+            except KeyError:
+                continue
+            else:
+                if bridges == interface.get('network', '').split(' '):
+                    del result[index]['network']
+        return result
+
+    def _track_bridged_wifi(self, intermediate_data=None):
+        """
+        Keeps track of wireless interfaces which are members of
+        bridges in order to automatically determine the "network"
+        attribute value of the UCI or NetJSON configuration.
+        """
+        self._bridged_wifi = {}
+        if not intermediate_data:
+            intermediate_data = self.intermediate_data
+        interfaces = intermediate_data.get('network', [])
+        # Create a mapping of physical interface to bride interface name
+        for interface in interfaces:
+            if interface.get('type', None) != 'bridge':
+                continue
+            # Get list of bridge members
+            try:
+                bridge_members = interface.get('ifname', None).split(' ')
+            except AttributeError:
+                # Bridge interface does not contain bridge members.
+                # Bridge is empty.
+                continue
+            bridge_name = interface['.name']
+            for physical_interface in bridge_members:
+                # A physical interface can be a member of multiple
+                # bridges. Hence, we create a list of bridge interfaces
+                # for every physical interface.
+                if physical_interface not in self._bridged_wifi:
+                    self._bridged_wifi[physical_interface] = [bridge_name]
+                elif bridge_name not in self._bridged_wifi[physical_interface]:
+                    self._bridged_wifi[physical_interface].append(bridge_name)

tests/openwrt/test_wireless.py

@@ -200,7 +200,7 @@ def test_parse_multiple_wifi(self):
     option device 'radio0'
     option ifname 'wlan0'
     option mode 'ap'
-    option network 'wlan0'
+    option network 'br_lan'
     option ssid 'open'
 """
 
@@ -211,7 +211,8 @@ def test_render_wifi_bridge(self):
 
     def test_parse_wifi_bridge(self):
         o = OpenWrt(native=self._wifi_bridge_uci)
-        self.assertEqual(o.config, self._wifi_bridge_netjson)
+        wifi_bridge_netjson = self._wifi_bridge_netjson.copy()
+        self.assertEqual(o.config, wifi_bridge_netjson)
 
     _wifi_networks_netjson = {
         "interfaces": [
@@ -663,7 +664,6 @@ def test_maclist_format(self):
                 "wireless": {
                     "mode": "station",
                     "radio": "radio0",
-                    "network": ["wds_bridge"],
                     "ssid": "FreeRomaWifi",
                     "bssid": "C0:4A:00:2D:05:FD",
                     "wds": True,
@@ -676,7 +676,6 @@ def test_maclist_format(self):
                 "wireless": {
                     "mode": "access_point",
                     "radio": "radio1",
-                    "network": ["wds_bridge"],
                     "ssid": "FreeRomaWifi",
                 },
             },
@@ -801,7 +800,6 @@ def test_parse_access_point_80211r(self):
                     "radio": "radio0",
                     "mode": "802.11s",
                     "mesh_id": "ninux",
-                    "network": ["lan"],
                 },
             },
             {
@@ -1042,3 +1040,52 @@ def test_render_wifi_custom_id(self):
     def test_parse_wifi_custom_id(self):
         o = OpenWrt(native=self._custom_id_uci)
         self.assertEqual(o.config, self._custom_id_netjson)
+
+    _wifi_simplified_bridge_netjson = {
+        "interfaces": [
+            {
+                "name": "br-lan",
+                "network": "lan",
+                "type": "bridge",
+                "bridge_members": ["eth0", "wlan0"],
+            },
+            {
+                "name": "wlan0",
+                "type": "wireless",
+                "wireless": {
+                    "radio": "radio0",
+                    "mode": "access_point",
+                    "ssid": "open",
+                },
+            },
+        ]
+    }
+    _wifi_simplified_bridge_uci = """package network
+
+config interface 'lan'
+    option ifname 'eth0 wlan0'
+    option proto 'none'
+    option type 'bridge'
+
+config interface 'wlan0'
+    option ifname 'wlan0'
+    option proto 'none'
+
+package wireless
+
+config wifi-iface 'wifi_wlan0'
+    option device 'radio0'
+    option ifname 'wlan0'
+    option mode 'ap'
+    option network 'lan'
+    option ssid 'open'
+"""
+
+    def test_render_simplified_wifi_bridge(self):
+        o = OpenWrt(self._wifi_simplified_bridge_netjson)
+        expected = self._tabs(self._wifi_simplified_bridge_uci)
+        self.assertEqual(o.render(), expected)
+
+    def test_parse_simplified_wifi_bridge(self):
+        o = OpenWrt(native=self._wifi_simplified_bridge_uci)
+        self.assertEqual(o.config, self._wifi_simplified_bridge_netjson)