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

APuP support for LibreMesh #1134

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

G10h4ck
Copy link
Member

@G10h4ck G10h4ck commented Oct 20, 2024

Initial support for APuP in libremesh

APuP WiFi is now usable and testeable in LiMe
Some part of the code have workaround to OpenWrt ubus and netifd current
bugs/limitations and will be modified once upstream limitations are solved

To use APuP just set 'apup' as LiMe wifi mode
as an example using the followings commands

uci del lime-community.wifi.modes
uci add_list lime-community.wifi.modes=apup
uci commit
lime-config
reboot

DISCLAIMER: You need a recent OpenWrt development version with APuP support (at least e80520197c9ca7bced50d3605d6baba6dead6e35 )

local protoModule = "lime.proto."..args[1]
if utils.isModuleAvailable(protoModule) then
local proto = require(protoModule)
xpcall(function() proto.runOnDevice(linuxBaseIfname, args) end,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above, inside network.configure, there is a very similar piece of code running configure on all protocols. What is the intended difference between runOnDevice and configure? Do we need both?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, maybe I understood: configure writes the configuration for the interface to get up at the next reboot or restart of the service while runOnDevice makes the thing happen right now in a volatile way. I think this should be explained more extensively in the comments for the future readers.

@@ -538,4 +580,94 @@ function network.createMacvlanIface(baseIfname, linuxName, argsDev, argsIf)
return owrtInterfaceName, linuxName, owrtDeviceName
end

--! Create a static interface at runtime via ubus
function network.createStatic(linuxBaseIfname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add as a comment in the code the difference between network.createStatic and network.createStaticIface?

From what I can understand, the difference is that createStaticIface creates the configuration file (the actual interface will appear upon reboot or restart of the networking service), with a custom netmask, gateway and IPv4 or IPv6. While this creates the interface in the running system (not in the configuration file) with fixed netmask, no gateway and the primary IPv4. Right? If this is right, I think that it should have a more informative name than just createStatic.

end

--! Create a vlan at runtime via ubus
function network.createVlan(linuxBaseIfname, vid, vlanProtocol)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above: it would be convenient to rename this function so that the difference with createVlanIface is more clear.

@ilario
Copy link
Member

ilario commented Feb 21, 2025

A few comments:

lime-curtigghio only has the test.sh and the readme files, right?
In this case, I would not place it in the packages folder, as it does not have a Makefile.
The test.sh file could be renamed and go in the tests folder.
The readme could be renamed (lime-curtigghio is not clear to me, does it mean anything?) and go either to the website libremesh.org as a new page or in the root of this repository.

In the readme there is no clear explanation about how to compile an image including your modifications of hostapd, for taking advantage of all the new code you propose here.

When a peer gets disconnected and connects back after a while (e.g. for bad signal or the remote device rebooted), the same APuP interface gets used or a new one gets created? In the second case, we would accumulate many interfaces. I could not spot anything that removes created interfaces, but this could be ok as the user can wipe them rebooting the device.

@ilario
Copy link
Member

ilario commented Feb 21, 2025

In the readme there is no clear explanation about how to compile an image including your modifications of hostapd, for taking advantage of all the new code you propose here.

Just realized that your edits have already been upstreamed :D amazing!
openwrt/openwrt@e805201

@@ -59,6 +59,7 @@ config lime wifi
option apname_ssid 'LibreMesh.org/%H'
option adhoc_ssid 'LiMe'
option adhoc_bssid 'ca:fe:00:c0:ff:ee'
option apup_ssid 'LibreMesh.org'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resulting AP interface(s) will simply be named LibreMesh.org? Will there be one new of such interfaces for each peer? Will the normal clients see a long list of identical ESSIDs? If they try to connect to it, will they succeed? In order for APuP to work, the apup_ssid and the ap_ssid have to be identical? Or should this string be identical just between the APuP peers?

Also, can you add this line to packages/lime-docs/files/www/lime-example.txt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for ap mode anymore, APuP create just one SSID both for clients and peers, and everyone will see just one SSID not many

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow ok, this is big. Nice!
In this case, we have to redefine ap_ssid.
I would propose this:

  • remove both apup_ssid and ap_ssid (anyway ap_ssid was not a good description of what this variable was for, see In lime-defaults ap_ssid required even when list_modes 'ap' is removed #430 (comment))
  • the AP interface SSID, both for normal AP and for APuP, gets taken from a new ssid variable.
  • the %N1 things (e.g. Batman-adv VLAN ID) gets set in a new variable, maybe broadcast_id, instead than being calculated from another setting.

So we would move from:

config lime network
[...]
	option main_ipv4_address '10.%N1.0.0/16'
[...]
	option main_ipv6_address 'fd%N1:%N2%N3:%N4%N5::/64'
[...]
	list protocols batadv:%N1
[...]
	option anygw_mac "aa:aa:aa:%N1:%N2:aa"
[...]

config lime wifi
[...]
	option ap_ssid 'LibreMesh.org'
[...]
	option apup_ssid 'LibreMesh.org'

To:

config lime network
	option broadcast_id 13
[...]
	option main_ipv4_address '10.%D1.0.0/16'
[...]
	option main_ipv6_address 'fd%D1:%D2%D3:%D4%D5::/64'
[...]
	list protocols batadv:%D1
[...]
	option anygw_mac "aa:aa:aa:%D1:%D2:aa"
[...]

config lime wifi
[...]
	option ssid 'LibreMesh.org'
[...]

Where %D1 is broadcast_id % 256, %D2 is the next 8 bits etc etc (so by default they would be all zeros except %D1 which is 13).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not against changing this, as I agree ap_ssid is used for too many things that should not depends on it. But that is a major change, so let's keep it for another PR and merge this one AS-IS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense.
Could you comment on the first and last paragraph of this comment? #1134 (comment)

Then in the next weeks I hope I will manage to test it on some hardware... Should this work with OpenWrt 24.10?

G10h4ck and others added 6 commits February 21, 2025 14:05
APuP WiFi is now usable and testeable in LiMe
Some part of the code have workaround to OpenWrt ubus and netifd current
bugs/limitations and will be modified once upstream limitations are solved

To use APuP just set 'apup' as LiMe wifi mode
as an example using the followings commands

```
uci del lime-community.wifi.modes
uci add_list lime-community.wifi.modes=apup
uci commit
lime-config
reboot
```
This project is GREAT! I found it from a brief discussion at https://forum.openwrt.org/t/docs-need-review-wiki-article-on-wi-fi-extender-with-relayd/201803/23

I have taken the liberty of making an editorial pass over the README file to fix misspellings ("it's" is always a contraction for "it is" - all other uses are just "its") and clarify some of the language.

Please forgive me for changing the meaning of anything - I didn't mean to. I hope this becomes an important part of OpenWrt.

Best regards,
Rich Brown
Lyme, NH USA
@G10h4ck
Copy link
Member Author

G10h4ck commented Feb 21, 2025

I did respond to comments inline, but now cannot see my replies anymore, anyway the recent push should address them

@ilario
Copy link
Member

ilario commented Feb 25, 2025

lime-curtigghio only has the test.sh and the readme files, right? In this case, I would not place it in the packages folder, as it does not have a Makefile. The test.sh file could be renamed and go in the tests folder. The readme could be renamed (lime-curtigghio is not clear to me, does it mean anything?) and go either to the website libremesh.org as a new page or in the root of this repository.

When a peer gets disconnected and connects back after a while (e.g. for bad signal or the remote device rebooted), the same APuP interface gets used or a new one gets created? In the second case, we would accumulate many interfaces. I could not spot anything that removes created interfaces, but this could be ok as the user can wipe them rebooting the device.

Also, could you check what happened that caused the unit testing to start failing at this PR? It seems that something got broken, and not for external reasons (I tried to run the tests on another branch and they still work: https://github.com/ilario/lime-packages/actions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants