luci-mod-network: include prefix in interface name check
The interface name check only considered the user input when validating the length, which could result in the actual name exceeding the kernel limit of 15 characters when the bridge/protocol prefix was included as well. For example the interface name abcdefgh1234567 was accepted as valid but with prefixes included (such as br-abcdefgh1234567 or gre-abcdefgh1234567) it exceeded the limit.
The following interface:
Results in the following logs:
Fri Sep 20 21:45:32 2024 daemon.notice netifd: Interface 'abcdefgh1234567' is setting up now
Fri Sep 20 21:45:32 2024 daemon.warn netifd: Cannot set device name: 'gre4-abcdefgh1234567' is longer than max size 15
Fri Sep 20 21:45:32 2024 daemon.warn netifd: Failed to initalize device 'gre4-abcdefgh1234567'
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): Command failed: ubus call network.interface notify_proto { "action": 0, "ifname": "gre4-abcdefgh1234567", "link-up": true, "tunnel": { "mode": "greip", "mtu": 1280, "ipv6": true, "df": true, "multicast": true, "local": "192.168.80.10", "remote": "1.2.3.4", "link": "mng", "data": { } }, "data": { }, "keep": false, "interface": "abcdefgh1234567" } (Invalid argument)
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): Usage: ubus [<options>] <command> [arguments...]
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): Options:
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -s <socket>: Set the unix domain socket to connect to
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -t <timeout>: Set the timeout (in seconds) for a command to complete
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -S: Use simplified output (for scripts)
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -v: More verbose output
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -m <type>: (for monitor): include a specific message type
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): (can be used more than once)
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): -M <r|t> (for monitor): only capture received or transmitted traffic
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363):
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): Commands:
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - list [<path>] List objects
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - call <path> <method> [<message>] Call an object method
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - subscribe <path> [<path>...] Subscribe to object(s) notifications
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - listen [<path>...] Listen for events
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - send <type> [<message>] Send an event
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - wait_for <object> [<object>...] Wait for multiple objects to appear on ubus
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363): - monitor Monitor ubus traffic
Fri Sep 20 21:45:32 2024 daemon.notice netifd: abcdefgh1234567 (53363):
Fri Sep 20 21:45:32 2024 daemon.notice netifd: Interface 'abcdefgh1234567' is now down
After this change:
Effective length of gre-abcdefgh123 (15 characters) is allowed.
Effective length of gre-abcdefgh1234 (16 characters) isn't allowed.
- [X] This PR is not from my main or master branch :poop:, but a separate branch :white_check_mark:
- [X] Each commit has a valid :black_nib:
Signed-off-by: <[email protected]>row (viagit commit --signoff) - [X] Each commit and PR title has a valid :memo:
<package name>: titlefirst line subject for packages - [X] Incremented :up: any
PKG_VERSIONin the Makefile - [X] Tested on: (x86/64, SNAPSHOT r27466-f368e2d5ec, Firefox) :white_check_mark:
- [X] ( Preferred ) Mention: @ the original code author for feedback @jow-
- [X] ( Preferred ) Screenshot or mp4 of changes:
- [ ] ( Optional ) Closes: e.g. openwrt/luci#issue-number
- [ ] ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
- [X] Description: (describe the changes proposed in this PR)
Opening as draft as there's some cases I haven't tested yet if switching the check to prefix + interface name causes any problem.
I also just noticed that gre-abcdefgh1234567 is being brought up by netifd as gre4-abcdefgh1234567, which would mean that even with this change, an interface with name abcdefgh123 (11 characters) with prefix gre would be valid in the interface setup as gre-abcdefgh123 (15 characters), even if it isn't (as seen by the logs below), a bit unsure how to fix that at the moment as it seems we can't rely entirely on Protocol.getProtocol.
config interface 'abcdefgh123'
option proto 'gre'
option force_link '1'
option peeraddr '4.5.6.7'
option tunlink 'internal'
Fri Sep 20 21:59:48 2024 daemon.notice netifd: Interface 'abcdefgh123' is setting up now
Fri Sep 20 21:59:48 2024 daemon.warn netifd: Cannot set device name: 'gre4-abcdefgh123' is longer than max size 15
Fri Sep 20 21:59:48 2024 daemon.warn netifd: Failed to initalize device 'gre4-abcdefgh123'
Just for reference, the issue (protocol name prefix) has been there for ages. Links to a few relevant items, starting from year 2015...
- https://dev.archive.openwrt.org/ticket/20380.html#comment:3
- #2944
- #2740
This proved a bit harder than I first thought, since there doesn't seem to be any consistency for some virtual protocols in how they define their interface names, for example:
-
modemmanagerinvokesnetwork.registerPatternVirtual(/^mobiledata-.+$/)to register every virtual interface undermobiledata-but the interface name in the protocol is defined asmodemmanager-<interface name>andgetProtocol()returnsmodemmanagerdue tonetwork.registerProtocol('modemmanager'). -
batman-advinokesnetwork.registerPatternVirtual(/^bat\d+/)to register every virtual interface underbat<number>but the interface name in the protocol is defined as<interface name>andgetProtocol()returnsbatadvdue tonetwork.registerProtocol('batadv').
Short of redefining how the naming strategy works for bringing up virtual protocols' interfaces, I'm pretty sure this won't be possible to solve in an nice and concise manner.