[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: 98.797% (+0.07%) from 98.728%
when pulling c77078b on issues/195-dsa-switch
into 571af14 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.

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

Rebased on latest master

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 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 added 11 commits January 22, 2024 21:32

[feature] Added support for DSA #195

593ac3b

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

[req-changes] Added default interface for VLANs

4afcddc

[req-changes] Implemented parsing of bridge VLANs

9710831

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

5b5970d

[req-changes] Implemented Primary VID option

983f70d

[docs] Added docs

32bc1d2

[req-changes] Improved docs

40e3bc3

[change] Allow user to set logical name of interface

e4d3fc0

[fix] Fixed network name for bridge vlan filtering

13b8a12

[fix] Fixed network name for bridge vlan filtering

e42b1d4

[tests] Fixed failing test

c77078b

Rebased on latest master

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

nemesifier added this to In progress in OpenWISP Priorities for next releases via automation May 2, 2024

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

pandafy commented Mar 31, 2023 •

edited

pandafy commented Apr 5, 2023

nemesifier left a comment

ValerioBob May 10, 2023

nemesifier May 14, 2023

ValerioBob May 10, 2023 •

edited

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

[feature] Added support for DSA #195 #261

Are you sure you want to change the base?

[feature] Added support for DSA #195 #261

Conversation

pandafy commented Mar 30, 2023

coveralls commented Mar 30, 2023 • edited

pandafy commented Mar 31, 2023 • edited

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

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

coveralls commented Mar 30, 2023 •

edited

pandafy commented Mar 31, 2023 •

edited

ValerioBob May 10, 2023 •

edited