[feature] Added support for DSA #195 #261

pandafy · 2023-03-30T14:55:48Z

Added VLAN 802.1q and VLAN 802.1ad interfaces

Closes #195

coveralls · 2023-03-30T14:57:28Z

coverage: 99.522% (+0.03%) from 99.497%
when pulling 16a59e7 on issues/195-dsa-switch
into 1ddddcf on master.

pandafy · 2023-03-31T18:14:41Z

In the last review with @nemesisdesign, we discussed the following things:

The code should automatically generate configuration for physical interfaces when creating VLAN 802.1q or VLAN 802.1ad interfaces

NetJSON

{
    "interfaces": [
        {
            "type": "8021q",
            "name": "eth0",
            "mtu": 1500,
            "vid": 1
        }
    ]
}

UCI

package network

config device 'device_eth0_1'
	option ifname 'eth0'
	option mtu '1500'
	option name 'eth0.1'
	option type '8021q'
	option vid '1'

config interface 'vlan1'
	option device 'eth0.1'
	option ifname 'vlan1'
	option proto 'none'

When "VLAN Filtering" is added to a "Bridge Interface", the code should generate phyical interfaces for all VLAN.

NetJSON

{
    "interfaces": [
        {
            "type": "bridge",
            "stp": false,
            "bridge_members": [
                "eth0",
                "eth1"
            ],
            "name": "br-lan",
            "mtu": 1500,
            "disabled": false,
            "network": "",
            "mac": "",
            "vlan_filtering": [
                {
                    "vlan": 1,
                    "ports": [
                        {
                            "ifname": "eth0", 
                            "tagging": "t",
                        },
                        {
                            "ifname": "eth1",
                            "tagging": "u"
                        }
                    ]
                }
            ]
        }
    ]
}

UCI

package network

config device 'device_br_lan'
	option mtu '1500'
	option name 'br-lan'
	list ports 'eth0'
	list ports 'eth1'
	option stp '0'
	option type 'bridge'
	option vlan_filtering '1'

config bridge-vlan 'vlan_br_lan_1'
	option device 'br-lan'
	list ports 'eth0:t'
	list ports 'eth1:u'
	option vlan '1'

config interface 'br_lan'
	option device 'br-lan'
	option enabled '1'
	option proto 'none'

config interface 'vlan1'
	option device 'br-lan.1'
	option proto 'none'

We will add another interface type "Bridge VLAN Interface". This interface type will allow advanced users to overwrite the interfaces automatically generated in the above step. Implementation details: It should be similar to the "Network Interface" with "virtual" type. It should only allow defining L3 options

pandafy · 2023-04-05T19:18:19Z

In the last review with @nemesisdesign, we concluded the following:

We will not add a special interface type "Bridge VLAN Interface" in the schema because this locks down the granularity offered by OpenWrt. Instead, we will direct the users to create interfaces using the existing interface types (Network Interface, Dialup Interface. etc.). If the interface names contains a period (.), e.g.br-lan.1, the code will check if a bridge named br-lanis defined in the configuration with a VLAN1. If it is present, then the interface configuration will be updated to add the device` option.

nemesifier

Next step: docs.

docs/source/backends/openwrt.rst

ValerioBob · 2023-05-10T08:16:18Z

netjsonconfig/backends/openwrt/converters/interfaces.py

@@ -193,6 +225,17 @@ def _intermediate_vxlan(self, interface):
        interface['vid'] = interface.pop('vni')
        return interface

+    def _intermediate_8021_vlan(self, interface):
+        interface['name'] = '{}.{}'.format(interface['ifname'], interface['vid'])
+        interface['.name'] = '{}_{}'.format(interface['.name'], interface['vid'])


First of all I want to thank you for these features! Really appreciated.
I just have a stupid question: why formatting the name in this way?
For example if I use as logical interface name "CORPORATE_WAN" with vid 201 I'll get VLAN_CORPORATE_WAN_201.
I think that is better to let the user choose the full name.

Suggested change

interface['.name'] = '{}_{}'.format(interface['.name'], interface['vid'])

In general we shouldn't require the user to mess with these internal details, however it may be good to allow overriding this if needed in some corner case.

ValerioBob · 2023-05-10T08:18:49Z

netjsonconfig/backends/openwrt/converters/interfaces.py

+                    'egress_qos_mapping': interface.pop('egress_qos_mapping', []),
+                }
+            )
+            interface['.name'] = 'vlan_{}'.format(interface['.name'])


Suggested change

interface['.name'] = 'vlan_{}'.format(interface['.name'])

And also that.
Hope this can be useful.
Thank you!

codingHahn · 2023-05-12T18:01:51Z

While testing I found an error while configuring vlans in bridges. If I create a template where vlan-interface lan.1234 is inside a bridge called br-guest, it works. If I then clone that template where I change lan.1234 to br-lan.1234 and remove the first template and apply the second, the network hangs and the configuration fails to apply.
Here are both advanced jsons:

Config with lan.1234 inside bridge

{
    "interfaces": [
        {
            "type": "8021q",
            "name": "lan",
            "mtu": 1500,
            "disabled": false,
            "network": "",
            "mac": "",
            "autostart": true,
            "addresses": [],
            "vid": 1234,
            "ingress_qos_mapping": [],
            "egress_qos_mapping": []
        },
        {
            "type": "bridge",
            "stp": false,
            "bridge_members": [
                "lan.1234"
            ],
            "name": "br-guest",
            "mtu": 1500,
            "disabled": false,
            "network": "",
            "mac": "",
            "autostart": true,
            "addresses": [],
            "igmp_snooping": false,
            "multicast_querier": false,
            "query_interval": 12500,
            "query_response_interval": 1000,
            "last_member_interval": 100,
            "hash_max": 512,
            "robustness": 2,
            "forward_delay": 4,
            "hello_time": 2,
            "priority": 32767,
            "ageing_time": 300,
            "max_age": 20,
            "vlan_filtering": []
        }
    ]
}

Config with br-lan.1234 in bridge:

{
    "interfaces": [
        {
            "type": "8021q",
            "name": "br-lan",
            "mtu": 1500,
            "disabled": false,
            "network": "",
            "mac": "",
            "autostart": true,
            "addresses": [],
            "vid": 1234,
            "ingress_qos_mapping": [],
            "egress_qos_mapping": []
        },
        {
            "type": "bridge",
            "stp": false,
            "bridge_members": [
                "br-lan.1234"
            ],
            "name": "br-guest",
            "mtu": 1500,
            "disabled": false,
            "network": "",
            "mac": "",
            "autostart": true,
            "addresses": [],
            "igmp_snooping": false,
            "multicast_querier": false,
            "query_interval": 12500,
            "query_response_interval": 1000,
            "last_member_interval": 100,
            "hash_max": 512,
            "robustness": 2,
            "forward_delay": 4,
            "hello_time": 2,
            "priority": 32767,
            "ageing_time": 300,
            "max_age": 20,
            "vlan_filtering": []
        }
    ]
}

Used antenna: Ubiquiti UniFi 6 Lite

Steps to reproduce:

Apply first config
Wait for successful apply
Deselct first config and select second config, then hit save
Watch as config fails to apply

After deselecting config 2 in openwisp and hitting save, the config 1 is still present on the antenna in uci show network as well as /etc/config/network. Even a restart of the openwisp_config service does not reset the config to the state wanted by the server.

I suspect as both lan and br-lan reference the same physical interface, it fails at registering the vid for both interfaces.

The logread during that time is here: logread_vlan_bridge_bug.txt

pandafy · 2023-05-17T12:21:52Z

@codingHahn I tried to replicate the locally of the configuration that you provided in your example were applied successfully on my router. My router has OpenWrt 22.03.3.

I have shared the output of logread from applying both configuration on hastebin.

My router does not have any lan device, therefore no bridge was shown in brctl show command after applying the first configuration. But, after applying the second config, the br-guest bridge came up.

pandafy · 2023-05-18T16:23:46Z

tests/openwrt/test_interfaces_dsa.py

+        expected['interfaces'][0]['vlan_filtering'][0]['ports'][1][
+            'primary_vid'
+        ] = False
+        expected['interfaces'][0]['name'] = 'br-home_vlan'


After allowing user to set the logical name for VLAN interface, the parsing of the bridge VLAN breaks.

codingHahn · 2023-05-23T08:57:17Z

@pandafy Does it also work for you if you use a device with only one physical ethernet port?

pandafy · 2023-05-23T16:35:02Z

@codingHahn are you able to get the desired results from LuCI? If so, can you share the UCI configuration generated by LuCI?

zagi-tng · 2023-05-26T06:51:15Z

@pandafy Does it also work for you if you use a device with only one physical ethernet port?

In my case i have eth0 as br-lan member (untagged vlan) and other lans (iot, guest, ...) as separate bridges with eth0.X vlans as members.

codingHahn · 2023-08-01T15:09:49Z

@pandafy applying the both configs manually works (both configs work standalone, but when switching configs using OpenWISP, the device's config is left in a corrupted state). It breaks when following the steps outlined in my comment.

nemesifier · 2024-08-01T21:28:56Z

I've been using this successfully on a couple of deployments. @codingHahn @zagi-tng thanks for your comments, @pandafy asked if you had an example of successful configuration applied via LuCi (the OpenWrt web interface) or manually so we could compare that with the result we get from our code and check if there's any difference.

- Added VLAN 802.1q and VLAN 802.1ad interfaces Closes #195

Rebased on latest master

nemesifier

Can you please merge (not rebase) the latest master, resolve conflicts and double check the generated output still matches tests and docs? There's been changes on master which may affect tests and docs.

The rest looks good.

pandafy force-pushed the issues/195-dsa-switch branch from f0d3c13 to 08fa5df Compare April 6, 2023 13:58

nemesifier mentioned this pull request Apr 11, 2023

[change] Allow installing development version of OpenWISP (23 alpha) openwisp/ansible-openwisp2#433

Closed

nemesifier requested changes Apr 13, 2023

View reviewed changes

nemesifier requested changes May 4, 2023

View reviewed changes

docs/source/backends/openwrt.rst Outdated Show resolved Hide resolved

pandafy force-pushed the issues/195-dsa-switch branch from a2ec357 to 2e16343 Compare May 5, 2023 18:59

nemesifier mentioned this pull request May 6, 2023

Initial VLAN implementation #269

Closed

ValerioBob reviewed May 10, 2023

View reviewed changes

pandafy commented May 18, 2023

View reviewed changes

pandafy force-pushed the issues/195-dsa-switch branch from 740e470 to c77078b Compare January 22, 2024 16:35

pandafy added 9 commits August 2, 2024 11:58

[feature] Added support for DSA #195

1ff1658

- Added VLAN 802.1q and VLAN 802.1ad interfaces Closes #195

[req-changes] Added default interface for VLANs

18792aa

[req-changes] Implemented parsing of bridge VLANs

a6a7b34

[req-changes] Allowed network interface to use software VLAN switch

3f7ac98

[req-changes] Implemented Primary VID option

350d298

[docs] Added docs

a055103

[req-changes] Improved docs

104e056

[change] Allow user to set logical name of interface

9c20348

[fix] Fixed network name for bridge vlan filtering

81fe11a

pandafy added 3 commits August 2, 2024 12:08

[fix] Fixed network name for bridge vlan filtering

83a7b48

[tests] Fixed failing test

295a0bf

Rebased on latest master

[qa] Formatted code

acc4192

pandafy force-pushed the issues/195-dsa-switch branch from c77078b to acc4192 Compare August 2, 2024 06:58

Merge branch 'master' into issues/195-dsa-switch

553af7b

nemesifier requested changes Nov 2, 2024

View reviewed changes

pandafy added 2 commits November 4, 2024 18:42

Merge branch 'master' into issues/195-dsa-switch

bfb054c

Merge branch 'master' into issues/195-dsa-switch

16a59e7

nemesifier approved these changes Nov 4, 2024

View reviewed changes

nemesifier merged commit f5115c6 into master Nov 4, 2024
8 checks passed

nemesifier deleted the issues/195-dsa-switch branch November 4, 2024 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Added support for DSA #195 #261

[feature] Added support for DSA #195 #261

pandafy commented Mar 30, 2023

coveralls commented Mar 30, 2023 •

edited

Loading

pandafy commented Mar 31, 2023 •

edited

Loading

pandafy commented Apr 5, 2023

nemesifier left a comment

ValerioBob May 10, 2023

nemesifier May 14, 2023

ValerioBob May 10, 2023 •

edited

Loading

codingHahn commented May 12, 2023

pandafy commented May 17, 2023

pandafy May 18, 2023

codingHahn commented May 23, 2023

pandafy commented May 23, 2023

zagi-tng commented May 26, 2023

codingHahn commented Aug 1, 2023

nemesifier commented Aug 1, 2024

nemesifier left a comment

[feature] Added support for DSA #195 #261

[feature] Added support for DSA #195 #261

Conversation

pandafy commented Mar 30, 2023

coveralls commented Mar 30, 2023 • edited Loading

pandafy commented Mar 31, 2023 • edited Loading

pandafy commented Apr 5, 2023

nemesifier left a comment

Choose a reason for hiding this comment

ValerioBob May 10, 2023

Choose a reason for hiding this comment

nemesifier May 14, 2023

Choose a reason for hiding this comment

ValerioBob May 10, 2023 • edited Loading

Choose a reason for hiding this comment

codingHahn commented May 12, 2023

pandafy commented May 17, 2023

pandafy May 18, 2023

Choose a reason for hiding this comment

codingHahn commented May 23, 2023

pandafy commented May 23, 2023

zagi-tng commented May 26, 2023

codingHahn commented Aug 1, 2023

nemesifier commented Aug 1, 2024

nemesifier left a comment

Choose a reason for hiding this comment

coveralls commented Mar 30, 2023 •

edited

Loading

pandafy commented Mar 31, 2023 •

edited

Loading

ValerioBob May 10, 2023 •

edited

Loading