Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gluon-core: lua: wireless: add support for PHYs named in board.json #3435

Closed
wants to merge 1 commit into from

Conversation

neocturne
Copy link
Member

@neocturne neocturne commented Jan 26, 2025

On mediatek-mt7622, board.json contains a PHY -> path mapping (which confusingly also uses the phy UCI option). Look up the mapping and pass the path query into iwinfo, like OpenWrt's mac80211.sh does.


Supersedes #3430

Untested on mt7622. Tested on ipq40xx to check that it doesn't break other platforms.

@neocturne neocturne added the 5. needs: testing Testing of the changes is necessary label Jan 26, 2025
@github-actions github-actions bot added the 3. topic: package Topic: Gluon Packages label Jan 27, 2025
On mediatek-mt7622, board.json contains a PHY -> path mapping (which
confusingly also uses the `phy` UCI option). Look up the mapping and pass
the path query into iwinfo, like OpenWrt's mac80211.sh does.
@neocturne
Copy link
Member Author

I have pushed another small update to avoid reading board.json on platforms where it's not needed.

@rotanid
Copy link
Member

rotanid commented Jan 27, 2025

@maurerle

@Djfe

This comment was marked as outdated.

@Djfe

This comment was marked as resolved.

@Djfe
Copy link
Contributor

Djfe commented Jan 28, 2025

test of f85b20d (sysupgrade without keeping the config)

I've been debugging for a while:
It comes down to this: running it in the shell works

root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=platform/18000000.wmac'
phy0
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=1a143000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0'
phy1

Those are the old phy-names pre renaming.
Your code get's the paths correctly.
But iwinfo.nl80211.phyname('path=' .. path) still returns nil instead of the old phy names like it should. No matter what I do.
Even if I set it to iwinfo.nl80211.phyname("path=" .. path) to fix string concatenation.
Does /usr/lib/lua/iwinfo.so not support the path syntax unlike the actual /usr/bin/iwinfo?

errors in config mode:
WLAN page:

500 Interner Serverfehler

Entschuldigung, auf dem Server ist ein unerwarteter Fehler aufgetreten.

Failed to execute dispatcher target for entry '/admin/wifi-config'.
The called action terminated with an exception:
/lib/gluon/config-mode/model/admin/wifi-config.lua:135: bad argument #1 to 'htmodelist' (string expected, got nil)

Privates WLAN page:

500 Interner Serverfehler

Entschuldigung, auf dem Server ist ein unerwarteter Fehler aufgetreten.

Failed to execute dispatcher target for entry '/admin/privatewifi'.
The called action terminated with an exception:
/usr/lib/lua/gluon/wireless.lua:76: attempt to concatenate local 'e' (a nil value)

gluon-reconfigure
root@ffac-80cc9cebd3c7:~# gluon-reconfigure 
Configuring: 001-reset-uci
Cannot parse config file '/etc/fw_env.config': No such file or directory
Failed to find NVMEM device
cfg030f15
cfg01e48a
Configuring: 002-migrate-system
Configuring: 005-set-domain
Configuring: 005-wireless-migration
Configuring: 010-primary-mac
Configuring: 020-interfaces
Configuring: 021-interface-roles
Configuring: 030-system
Configuring: 035-cronloglevel
Configuring: 036-cronloglevel
Configuring: 100-lock-password
Configuring: 110-network
Configuring: 115-swconfig
Configuring: 120-ntp-servers
Configuring: 150-poe-passthrough
Configuring: 180-outdoors
Configuring: 190-preserve-wireless-channels
Configuring: 200-wireless
/usr/bin/lua: ./200-wireless:53: bad argument #1 to 'hwmodelist' (string expected, got nil)
stack traceback:
  [C]: in function 'hwmodelist'
  ./200-wireless:53: in function 'r'
  ./200-wireless:154: in function 'o'
  /usr/lib/lua/gluon/wireless.lua:57: in function 'foreach_radio'
  ./200-wireless:143: in main chunk
  [C]: ?
Configuring: 210-interface-mesh
Configuring: 250-cellular
Configuring: 300-firewall-rules
Configuring: 300-gluon-client-bridge-network
Configuring: 300-setup-mode
Configuring: 310-gluon-client-bridge-local-node
Configuring: 310-gluon-mesh-batman-adv-mesh
Configuring: 320-gluon-client-bridge-wireless
/usr/bin/lua: /usr/lib/lua/gluon/wireless.lua:76: attempt to concatenate local 'e' (a nil value)
stack traceback:
  [C]: in function '?'
  /usr/lib/lua/simple-uci.lua:22: in function 'foreach'
  /usr/lib/lua/gluon/wireless.lua:74: in function 'device_supports_mfp'
  ./320-gluon-client-bridge-wireless:36: in function 'h'
  ./320-gluon-client-bridge-wireless:77: in function 'o'
  /usr/lib/lua/gluon/wireless.lua:57: in function 'foreach_radio'
  ./320-gluon-client-bridge-wireless:74: in main chunk
  [C]: ?
Configuring: 320-gluon-mesh-batman-adv-client-bridge
Configuring: 320-setup-ifname
Configuring: 325-gluon-private-wifi
Configuring: 330-gluon-mesh-batman-adv-mac-addresses
Configuring: 400-mesh-vpn-wireguard
Configuring: 400-neighbour-info-firewall
Configuring: 400-respondd-firewall
Configuring: 400-wg-registration
Configuring: 500-autoupdater
Configuring: 500-mesh-vpn
Configuring: 500-node-info-system
Configuring: 500-opkg
Configuring: 500-ssid-changer
Configuring: 500-status-page
Configuring: 510-autoupdater-wifi-fallback
Configuring: 510-node-info-role
Configuring: 520-custom-banner
Configuring: 820-dns-config
Configuring: 997-migrate-preserved
Configuring: 998-commit
Configuring: 999-version
One or more upgrade scripts failed. Please review the above error messages.

@Djfe
Copy link
Contributor

Djfe commented Jan 28, 2025

What could work instead: calling this in a shell via lua (os.execute but with variables for the phy names?)
iw phy0 set name wl0

after running that for all wifi-devices once, this iwinfo command works just fine on this plattform:
iwinfo.nl80211.phyname(config['.name']) (for the names "radio0" as well as "radio1")

But it only works if paths have been renamed like this:

spoiler
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=platform/18000000.wmac'
phy0
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=1a143000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0'
phy1
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# ls -l
drwxr-xr-x    6 root     root             0 Jan 22 19:59 phy0
drwxr-xr-x    5 root     root             0 Jan 22 19:59 phy1
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# . /lib/netifd/wireless/mac80211.sh 
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# phy=wl0
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# find_phy
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# phy=wl1
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# find_phy
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# ls -l
drwxr-xr-x    6 root     root             0 Jan 22 19:59 wl0
drwxr-xr-x    5 root     root             0 Jan 22 19:59 wl1
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=platform/18000000.wmac'
wl0
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'path=1a143000.pcie/pci0000:00/0000:00:00.0/0000:01:00.0'
wl1
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'radio0'
wl0
root@ffac-80cc9cebd3c7:/sys/kernel/debug/ieee80211# iwinfo nl80211 phyname 'radio1'
wl1

But I'm not sure whether we even need to rename the phys.
The only thing I'm sure about is:
The proposed way to get the old phy name via path doesn't work in lua for some reason.
Sure we could just look at which folders currently reside inside
/sys/kernel/debug/ieee80211/ instead but I'm not sure whether that's a good way to proceed.

@neocturne

This comment was marked as outdated.

@neocturne
Copy link
Member Author

Ah, the issue is indeed that the Lua library behaves differently from the iwinfo CLI.

Okay, new idea: I'll fix this in OpenWrt. The whole issue is that the renaming happens when netifd starts the radio, which is much too late and can lead to race conditions in addition to our issues. The renaming should probably be done by a hotplug.d hook instead.

@Djfe
Copy link
Contributor

Djfe commented Jan 28, 2025

Are you sure hotplug.d isn't too early? (=> is board.json even fully populated then?)
maybe ask nbd about it.

edit:
should we incorporate board.json further in the future? (use board.json instead of asking iwinfo etc. for band info and frequency info)

@neocturne
Copy link
Member Author

Are you sure hotplug.d isn't too early? (=> is board.json even fully populated then?)

Ah, I can see that being an issue. So it would have to use the same trick already used for /sbin/wifi config: Run it both from hotplug.d and after board.json generation.

@Djfe
Copy link
Contributor

Djfe commented Jan 28, 2025

took me a while to find the commit again, that you are referring to
openwrt/openwrt@b993a00

edit:
this delayed renaming in a way that caused gluon to stop recognizing the phy early during boot.

@neocturne
Copy link
Member Author

Superseded by openwrt/openwrt#17821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. topic: package Topic: Gluon Packages 5. needs: testing Testing of the changes is necessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants