Connman upgrade to 1.34 #68
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
7 times, most recently
from
0f3b896
to
9e6d82e
Compare
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
2 times, most recently
from
a8f1fbc
to
6829b39
Compare
(cherry picked from commit 4c4d51f)
The marking rules are different from each session. The SNAT routing rule is only needed once per devices. Therefore, we share the rule between the session and should only install unique rules. Note the live time management needs to track which session installed/refd the rule to avoid removal from update_nat_rules() on a session create event. (cherry picked from commit ba926d0)
The current firewall API implementation is depending on iptables. In order to support a nftables implementation we move the init call from main.c to firewall.c. (cherry picked from commit 2c0116a)
Instead heaving a generic firewall API, we use an just provide a few features such as NAT, SNAT or MARK. That allows us to push down all the code into firewall.c. There are only two different users in ConnMan for this API: enabling global NAT or for session handling. Fortunately, the NAT handling is pretty simple. We just have one global table and we either have it enabled or disabled. The session handling is slightly more tricky. There are three different rule sets. The first one enables one global rule for CONNMARK save/restore. This one will be installed only when the first session calls __connman_firewall_enable_marking(). For this we use a global context and enable disable it only if there is a session using it. The SNAT rules are shared between sessions and there exists only one per output device. The session tracking is done in session.c. Note the SNAT rules have also their own struct firewall_context. The third set of rule for MARK is owned by the session, that is the rules are tracked it the session->fw context. Due to the separation of the struct firewall_context usage we can git rid of any special tracking. Either we enable all or disable all in one go. We don't have enable indiviual rules anymore with the FW_ALL_RULES. And we are all happy again. (cherry picked from commit 58a763e)
There is little point in keeping the orignal firewall implementation. It is designed based on iptables.c API. For nftables it is just better to start from fresh. The gives the necessary freedom to really exploit the nftables API without fearing to break the working iptables implementation. That should also make testing considerable more simpler. (cherry picked from commit 43cc63d)
Introduce --with-firewall configuration flag which is on default iptables. You can enable nftables by providing --with-firewall=nftables. (cherry picked from commit c09e0c7)
A straight forward implementation for the firewall API. It is based on the libntfnl examples and Daniel Macks' systemd implementation. There are some comment how to use/extend this code, that is 'nft --debug' your friend. Also check the libnftnl examples for more details. (cherry picked from commit 191da76)
(cherry picked from commit 1a3b170)
In 7784ca7 was added a patch to not perform "RemoveNetwork" in interface_disconnect_result() for WPS use cases because "AddNetwork" is not done in these cases. However, the disconnection callback is not called when there was not error (result == 0). Therefore, flags are not clean and feature initially introduced in 43e6e14 to make sure disconnection is completed before trying to connect a new network will not work in WPS use cases. This patch ensures disconnection callback is always called in WPS use cases. Reported by Eunok Lee <ogi337@gmail.com> while testing RFC for WPS proposal. (cherry picked from commit c82fe86)
When bluetooth PAN connection fails, ConnMan can start looping endlessly. This can happen when trying to connect to a device out of reach or when trying to reconnect just after disconnect. The debug output will look like the following: plugins/bluetooth.c:bluetooth_pan_disconnect() network 0xfb8510 plugins/bluetooth.c:pan_disconnect_cb() network 0xfb8510 org.bluez.Error.NotConnected plugins/bluetooth.c:pan_disconnect_cb() network 0xfb8510 src/network.c:connman_network_set_connected() network 0xfb8510 connected 0/0 connecting 0 associating 1 src/network.c:connman_network_set_error() network 0xfb8510 error 4 src/service.c:__connman_service_indicate_error() service 0xfb88e0 error 4 src/network.c:__connman_network_disconnect() network 0xfb8510 plugins/bluetooth.c:bluetooth_pan_disconnect() network 0xfb8510 plugins/bluetooth.c:pan_disconnect_cb() network 0xfb8510 org.bluez.Error.NotConnected ... Fix this by setting network associating to false in the bluetooth plugin. (cherry picked from commit 3dbd940)
(cherry picked from commit c48f2b2)
After disconnecting, wifi->connected value is not coherent with the actual interface state and it does not allow to start auto-scan. This only happens when we are disconnecting from previous successful WPS association. It is because in interface_disconnect_result() in supplicant.c we do not have to remove the network and then disconnect callback (disconnect_callback() in wifi.c) is called faster and before signal "PropertiesChanged" is received which indicates "State" changed to "disconnected". Therefore, when this signal is handled in wifi.c:interface_state() the wifi->network was already removed by disconnect_callback() and then interface_state() will intermediately return without reseting wifi->connected flag to false. The other flags like network->connected and wifi->disconnecting are correctly reseted in wifi.c:disconnect_callback() but wifi->connected is missing. This is a consequence of the fix done in c82fe86 because before it disconnect_callback() in wifi.c was not even called. (cherry picked from commit 87817b0)
wifi->connected is already checked, so the else statement only happens if connected is false. There is no need to set a false value to false. (cherry picked from commit a07c991)
Memory allocated to store Wi-Fi Display information needs to be freed. The number of lost blocks is proportional to the number of found peers. The following valgrind report was got when there were two peers in range: ==10427== 18 bytes in 2 blocks are definitely lost in loss record 48 of 181 ==10427== at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==10427== by 0x4E85668: g_malloc0 (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0) ==10427== by 0x423C7C: peer_property (supplicant.c:2926) ==10427== by 0x42983C: supplicant_dbus_property_foreach (dbus.c:145) ==10427== by 0x429951: property_get_all_reply (dbus.c:184) ==10427== by 0x514A501: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.7.6) ==10427== by 0x514D730: dbus_connection_dispatch (in /lib/x86_64-linux-gnu/libdbus-1.so.3.7.6) ==10427== by 0x483C8F: message_dispatch (mainloop.c:72) ==10427== by 0x4E7FCE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0) ==10427== by 0x4E80047: g_main_context_iterate.isra.24 (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0) ==10427== by 0x4E80309: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4002.0) ==10427== by 0x41001C: main (main.c:706) (cherry picked from commit 51ed8c0)
Even if OfflineMode is the same as before, Connman always saves to the file system. This patch makes ConnMan to save OfflineMode only when it is actually changed in order to avoid redundant data writing. (cherry picked from commit 67664b9)
We don't use the connection tracking for per session statistics, so just drop it. When we started with the Session API the goal was to support routing and statistics per application. So far we implement the routing via policy routing tables but never finished on the statistics side. Also the D-Bus API does not expose counters for TX/RX. It is quite likely we see eBPF support for cgroups in Linux soon. Creating simple counters via eBPF and do some form of enforcement in an extra component seems a better approach than stuffing all of this into ConnMan. So for the time beeing remove the unused bits. (cherry picked from commit d945ddd)
Let's be a bit more userfriendly and report why we think the nftable code is not working. While at it also use connman_warn for the cleanup path instead of the printf. (cherry picked from commit 0a1a5b0)
The networking stack does the rerouting in the route table. There is a check which prevents calling ip_reroute_me_harder when it was not modified in route output chain. By moving our marking rule to the route output chain the rerouting path is taken and we send our packets via the right interface (which is defined in the policy routing table). (cherry picked from commit 2a496a0)
(cherry picked from commit 01cdf74)
If a dhcp lease is lost (expires and dhcp server does not respond) ipconfig object for an interface still exists, but addr is null. When that happens, add_nat_rules() passes a null pointer to iptables which asserts and exits the ConnMan process. Prevent that, by not enabling snat if there is no ip address. (cherry picked from commit 855ac9b)
The value returned by g_key_file_get_start_group has to be deallocated by the caller. (cherry picked from commit b92e8ff)
When the file doesn't exits we get Cannot read /proc/net/pnp Failed to open file '/proc/net/pnp': No such file or directory in the log. (cherry picked from commit 0e79b4b)
(cherry picked from commit c4df81f)
The internal functions __connman_firewall_add_* were dropped by upstream
as nothing uses them anymore. Change to use the generic
firewall_add_rule() within our changes too.
Drop uses of __connman_firewall_{disable,enable}_. Use directly the
firewall_{enable,disable}_rules() or the single rule removals.
Remove uses of FW_ALL_RULES as this is not needed anymore.
The rules cannot be added to firewall outside the firewall implementation. Move the IPv6 forwarding rules enable/disable to firewall-iptables.
Re-apply the same nat functionality that was in nat.c but place it into firewall-iptables.c.
Use the firewall directly and re-apply the old firewall tests.
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
from
6829b39
to
e60bed1
Compare
Adapt to the changes from 1.34, the upstream WiFi plugin does it similarly as the tethering notify now returns the error. Also use bool instead of gboolean for __connman_technology_tethering_notify().
…tion [service] Fix loose mode routing not enabled with EnableOnlineCheck option. JB#51243 When EnableOnlineCheck option is disabled, the rp_filter is not updated. If there are more than one service available, the result is that some packets are blocked. The initial support for loose mode routing was added by cb3e785 ("service: Activate loose mode routing"). Commit 4de35cd ("service: Add EnableOnlineCheck config option") moved the rp_filter under the EnableOnlineCheck condition. (cherry picked from commit 02d45b6)
[firewall-iptables] Fix use-after-free in firewall_remove_rules. JB#51243 g_list_previous was accessing the pointer deallocated by g_list_remove. g_list_free_full does it right. (cherry picked from commit 1e22d70)
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
from
e1c942e
to
1904728
Compare
spiiroin
approved these changes
May 30, 2024
This became more visible after 1.34 changes that retries the unknown uevent type checks and caused the bond device warning to spam the logs. We have improved the rtnl part to separately read the uevent device type and dropping the "DEVTYPE=" completely from the line. Upstream does include the whole read line for checks and, thus, has the +8 for each. Simply compare the read device type as a whole in the style of our approach.
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
4 times, most recently
from
8bde83d
to
a2aca19
Compare
Use the indexes defined in netfilter_ipv4.h for iptables chains in the test. This makes the test more consistent with the implementation and iptables. Do also some cleanup and optimization for the test code. Make the internal lists to be NULL'd also by the __connman_iptables_cleanup() to avoid test crashes.
LaakkonenJussi
force-pushed
the
connman_upgrade_to_1.34
branch
from
a2aca19
to
7835921
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes applied to upgrade ConnMan to 1.34. Empty and already cherry-picked were naturally dropped.