luci: show wifi vlan interface instead of base interface

[tobiaswaldvogel]

• This PR is not from my main or master branch 💩, but a separate branch ✅
• Each commit has a valid ✒️ Signed-off-by: <[email protected]> row (via git commit --signoff)
• Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
• Tested on: aarch, mips, mipsel, openwrt version SNAPSHOT, r28432+19-4b6eb631e1, browser Chrome ✅
• Description: This patch adds a badge to the associated wireless stations with the vlan id and name. It is displayed in the same color as the network, to which it is bridged, so the color corresponds to the color in the network configuration
  page.

[systemcrash]

Q: is this 'instead of' or 'as well as'? And can you show me an example of this in use?

I want to know what to look for to verify that existing behaviour is not affected for those not using dynamic VLANS.

This regards #1324, correct?

Also, please amend your commit message with 79 char line-wrap to include the description, after you've fixed the strange bit: When using wifi the vlans the base wifi interface in the network object is show. Did you mean:

When using wifi with dynamic vlans, the base wifi interface in the network object is shown?

[tobiaswaldvogel]

I adjusted the wording as you suggested. Yes, it is partly related to #1324. The issue was actually that clients connected to wifi interfaces with dynamic vlans don't show up in the status and the wireless page. This is not addressed with this patch.

For detecting the VLANs Luci relies on the output of ubus call network.wireless and the array vlans inside the interface objects. hostapd does not populate them for dynamic vlans hence Luci does not see them. This would require some changes in hostapd. I had a look at this, but as there is a very complex interaction between netifd and hostapd, so this is not easy to address.

An alternative for the dynamic vlans is using wifi-vlan sections instead. They populate VLAN array correctly and and the clients show up.in Luci. However you have to configure the tagged network interfaces, bridges and luci interfaces yourself.
The only catch is, that the base interface is shown instead of the VLAN name. This is what this patch is about.

Here is an example (not using dynamic vlans):

/etc/config/wireless

config wifi-vlan
        option vid '3'
        option network 'lan'
        option name 'lan'

config wifi-vlan
        option vid '12'
        option network 'guest'
        option name 'gst'

config wifi-iface 'wlan2'
        option ifname 'wlan2'
        option device 'radio_2g'
        option mode 'ap'
        option ssid '2'
        option wpa_psk_file '/etc/config/wireless.wpa_psk'
...

This will create the base interface wlan2, which is not connected to any network, wlan2-lan connected to lan and wlan2-gst connect to gst..

Without this patch all clients will be reported with 2 (wlan2).
After this patch clients connected to vlan 3 will show up as 2 (wlan-lan) and clients connected to vlan 12 will show up as 2 (wlan-gst)

Configuration of lan and gst in /etc/config/network:

...
config device
        option type '8021q'
        option name 'eth1-lan'
        option ifname 'eth1'
        option vid '3'

config device
        option type '8021q'
        option name 'eth1-gst'
        option ifname 'eth1'
        option vid '12'
...
config device
        option type 'bridge'
        option name 'br-lan'
        list ports 'eth1-lan'

config device
        option type 'bridge'
        option name 'br-gst'
        list ports 'eth1-gst'
...

[systemcrash]

Can you show me an example of this in use? A screenshot of the changes.

[tobiaswaldvogel]

Before

After:

( I have some more vlans and radios than in the example )

[tobiaswaldvogel]

Description wording improved and added to the commit

[systemcrash]

In this case, why not simply add a VLAN column? Since it's not clear from reading wlan2-iot what iot is, it's not denoted anywhere. VLAN Interface here is a misnomer since it seems that no VLAN is revealed, but the underlying 'network'?

You can pack a couple or triplet in the VLAN column, similar to the Sig/Noise column. e.g. header has VLAN: VID, Name, , and each cell has:
vid
name

[tobiaswaldvogel]

The interface name is actually the name which was chosen in the wifi-vlan section. so if you prefer the vlan id you can change the option name to '3'

config wifi-vlan
        option vid '3'
        option network 'lan'
        option name '3'

That would create a vlan interface with the suffix -3 instead -lan in my previous example and show up e.g. as wlan2-3

Adding another column might be a little bit tricky as that might break the rendering in mobile devices. I can do a test though.
Right this looks like:

[systemcrash]

The old notation for VLANs in at least ifconfig and some config notations is .<vlan>. But this may be confusing in the wireless domain where VLANs are not possible. So if it really must go in the wireless info, it should be: VLAN: <vlan>. But let's see if we can avoid that.

[tobiaswaldvogel]

So adding a column is not an option at all because that require touching the stylesheets of all themes. There are several rules for displays smaller than 800px or mobile devices to automatically hide some columns like the mac address, and they are based on the column number.

	.assoclist tr > *:nth-of-type(2) {
		display: none;
	}

Trying to change the layout of the assoclist will certainly break the rendering on some devices/themes and would required thousands of regression tests.

The text itself in the badge comes also via the stylesheet:

.assoclist td > .ifacebadge[data-ssid][data-ifname] > span {
	display: none;
}

.assoclist td > .ifacebadge[data-ssid][data-ifname]::after {
	content: attr(data-ssid) " (" attr(data-ifname) ")";
}

So it is ssd ( ifname )

Without changing the stylesheets of all themes we can modify only the values for ssid and ifname.
So something like 2 ( wlan2, vlan 3 ) would be possible. But it makes it unnecessarily long and might be easily truncated with longer ssid names.

So from my point of showing the vlan interface is still the best option. As said before you can name them as you like e.g. just the VLAN number, that would result in <interface>-<vlan>

[tobiaswaldvogel]

I did one more experiment using y zone badge element, which seems to render ok in all scenarios.
The name (iot) will only be added if name and vid in the wifi-vlan section is different

The color is the same as the color of the network interface of the option network in the wifi vlan section:

What do you think?

[tobiaswaldvogel]

Seems to render ok also with the default theme (bootstrap)

Bootstrap dark

[systemcrash]

@hnyman @dannil @stokito any thoughts on this?

Would also be good to have some tests with those who get auto-vlan assignment (via RADIUS?)

[tobiaswaldvogel]

Would also be good to have some tests with those who get auto-vlan assignment (via RADIUS?)

That would be great. I understand that in this scenario the vlans would also be defined via wifi-vlan sections and just the assignment is performed by the radius server instead of the wpa_psk file. So I would expect this work as well without any issues.

The only limitation for now is dynamic_vlans, as the code does not update the ubus network,wireless objects with the vlans.

[systemcrash]

Please don't include merges: Merge branch 'master' into luci_wifi_dynamic_vlan but rebase.

[dannil]

Overall looks good except some small things, haven't run-tested it myself tho since I don't use VLAN's in my setup. You've mixed and matched var and let in the implemented code, for example in getVlans, feel free to use ES6-syntax throughout if you'd like.

[systemcrash]

Hi @tobiaswaldvogel are you satisfied with the current state? If you're active in the forum, it would be nice to get this tested by dynamic vlan users. Those kinds of threads appear from time to time.

[tobiaswaldvogel]

Yes, I'm happy with the implementation now. Thanks for all the comments.
Lately I didn't use the forum that much but I'll look for a thread with this topic.
But I also don't mind setting up a test radius server. I also considered that before, in order to avoid synching the wpa_psk_file, but at the end I didn't want to introduce another dependency. But if I don't find anybody I'll prepare a test this weekend.