Recently, more and more code was refactored to use an addr_family
integer to distinguish between IPv4 and IPv6.
Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method()
to do that too. If we use different identifiers, we need to translate from one to
another and its inconsistent. Also, accessing a GType is an unnecessary function call,
instead of a plain constant.
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.
Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
There's no reason the mesh shouldn't autoconnect. Almost.
The mesh and regular Wi-Fi shares the same radio. There, in the first
place, probably shouldn't have been separate NMDevices. Not sure whether
we can fix it at this point, but we can surely avoid unnecessary
competition between the two devices: give the regular Wi-Fi priority and
only connect mesh if the regular companion stays disconnected.
For the record; connections shipped on XO-1 laptops all have
autoconnect=off and thus are not affected by this.
If the client wants to pinpoint the connection to a particular device
they can just add an appropriate property.
That said, MAC address probably even doesn't count as appropriate; an
interface name is supposed to stay stable and could be used in such
cases.
This fixes the case where "nmcli d wifi connect ..." ends up with a
connection tied to a rather random device that happened to be around
even without the "ifname" argument.
is_available would recently return true after IWD had disconnected
if a connection was active because it would check that
priv->dbus_station_proxy was non-NULL (i.e. that the DBus interface was
still visible, which it wasn't) but that check would be overridden
if the NMDevice state was activated. Now require priv->dbus_obj to be
non-NULL, which would even be enough on its own although I'm leaving the
previous check there too to catch potential IWD states we don't support
in which priv->dbus_station_proxy is NULL without an active connection.
Sanity check networks received from the Station.GetOrderedNetworks()
DBus method. Duplicates shouldn't happen but the code should be safe
against bogus data received over DBus. There was a recent bug in a
library used by IWD causing occasional duplicates to be returned which
would cause invalid memory accesses reported by valgrind in NM because
g_hash_table_insert would free what we passed as the key.
Watch for connection-removed events and delete corresponding IWD network
configs if found. This mainly changes anything for 802.1X networks
where the deleted NM connections might annoyingly re-appear after a
restart. For PSK networks though it'll make IWD forget the password
which, until now, would be remembered by IWD even if it was removed or
changed in the NM profile, which is a bug.
This is still fragile because we don't handle "connection-updated"
events so the data->mirror_connection pointer for a known network record
may after some time point to an NMSettingsConnection with a different
SSID or security type and there are corner cases where the IWD-side
profile will not be forgotten. At least I'm trying to make sure we
don't crash and don't wrongly remove any IWD profile which could also be
annoying for complicated EAP configs.
Something is possibly wrong with the DBus signal handling if a newly
added KnownNetwork interface already has an entry in
priv->known_networks, but since we handle this case add a warning and
update the GDBusProxy pointer for that existing entry.
interface_added expects mirror_8021x_connection() to return the pointer to
the existing connection if one exists, and NULL on error, rather than
NULL if a conneciton exists. While touching that, add logic to return
specifically a connection with EAP method set to 'external' if one
exists even though this should not affect any other logic we have
currently.
Some internal logic causes the secrets in a connection to be
occasionally moved to NMSettingConnection's priv->system_secrets after a
connection attempt so we need to use nm_act_request_get_secrets to get
them added to the device's settings connection and applied connection if
the PSK is missing during an AP or AdHoc mode activation (in
infrastructure mode we already do secret requests though they're cached
by IWD in most cases).
The common steps for the PSK available and unavailable scenarios is moved
from act_stage2_config to act_set_mode.
This simplifies the code by using g_variant_lookup. In this handler
where we parse more than one property this is probably slower although
the number of string comparisons will be the same.
We should not use GAsyncResult. At least, not for internal API.
It's more cumbersome then helpful, in my opinion. It requires
this awkward async_finish() pattern.
Instead, let the caller pass a suitable callback of the right type.
We're hooking the signal on construction, but we only queue a pending
action on reaching UNAVAILABLE state. The signal could fire in between:
<info> [1539282167.9666] manager: (msh0): new 802.11 OLPC Mesh device (/org/freedesktop/NetworkManager/Devices/4)
<info> [1539282168.1440] manager: (wlan0): new 802.11 WiFi device (/org/freedesktop/NetworkManager/Devices/5)
<info> [1539282168.1831] device (msh0): found companion WiFi device wlan0
<warn> [1539282168.2110] device (msh0): remove_pending_action (1): 'waiting-for-companion' not pending
file src/devices/nm-device.c: line 13966 (<dropped>): should not be reached
https://github.com/NetworkManager/NetworkManager/pull/229
Instead of walking through the list all known networks and comparing
name & SSIDs to judge whether a network is an IWD KnownNetwork, look at
the Network.KnownNetwork pre-IWD-0.8 property.
Ensure priv->can_connect is up to date on IWD state changed. If we
exited the function early priv->can_connect would sometimes be wrongly
TRUE and we'd start a new autoconnect too early after IP configuration
had failed for example.
Handle AP mode connections by setting the Mode property on IWD's Device
interface to "ap" (which will make the Station interface go away, the
Powered property -- normally controlled by set_enabled -- to switch to
FALSE and back to TRUE, and then the AccessPoint interface to appear)
and then calling the AccessPoint.Start method. This is all done in the
CONFIG phase in NM. We also attempt to always set Mode back to
"station" and wait for the Station interface to reappear before going to
the NM DISCONNECTED state. All this complicates the code a little.
While making the necessary changes simplify a lot of the checks which
are implied by other things we've checked already, for example
priv->can_scan and priv->can_connect can now only be TRUE when device is
powered up and in station mode (Station interface is present) so we can
skip other checks. Also assume that check_connection_compatible has
been called before other methods are called so we can skip multiple
connection mode checks and checks that a IWD KnownNetwork exsists for
EAP connections.
act_stage1_prepare and act_stage2_config now borrow more code from
nm-device-wifi.c because both backend now handle multiple modes.
Use nm_utils_error_is_cancelled instead of checking
g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED), set
consider_is_disposing false. Also use the DBUS_INTERFACE_PROPERTIES
macro.
Make sure we g_variant_unref() the values returned from
g_dbus_proxy_call_finish. In get_ordered_networks_cb also make sure we
don't access the NMDeviceIwd data until after we know the call has not
been cancelled. Switch from _nm_dbus_proxy_call_finish to
g_dbus_proxy_call_finish where we don't care about the variant type
returned.
Only call nm_platform_wifi_get_capabilities and
nm_platform_wifi_get_mode with the wpa_supplicant backend. They're used
to initialize the wireless-capabilities property and to skip creating
NMDevices for interfaces in unknown wifi mode which IWD handles already.
See the logfile at [1], for how NetworkManager first attempts to connect
using WPS (which takes about 30 seconds). However, early on, the user logs
into KDE and a secret agent would register, which possibly could provide
secrets to connect. I think it is problematic to wait for WPS (which is
unlikely to succeed) if a secret agent shows up in the meantime.
A possible fix would be that when
- WPS is pending
- the secret request already failed
- another secret-agent registers
then the activation (and WPS) is aborted and autoconnect may be tried
again, possibly with secrets provided by the new secret-agent.
However, this patch goes a step further: it always cancels activation
when the secret request fails. That means, WPS only works while the
user is also prompted for a secret. That makes sense to me, because
an action from the user is required. However, without secret prompt,
the user wouldn't be aware of that and is unlikely to press the WPS
push botton.
[1] https://bugzilla.opensuse.org/show_bug.cgi?id=1079672#c33https://github.com/NetworkManager/NetworkManager/pull/216
The net.connman.iwd.Station interface, unlike the Device interface, can
go away and come back when the device changes modes or goes DOWN and UP.
The GDbusProxy for the interface becomes invalid after the interface
disappeared and reappeared. This would make the IWD backend stop
working after rfkill was used or after suspend/resume (provided the
suspend/resume events are detected, without them everything works and is
really fast too).
Redo the handling of the Powered property changes, corresponding to
device UP state, to get a new GDBusProxy when the Station interface
reappears. Simplify some checks knowing that priv->can_scan implies for
example that the Station interface is present, and that priv->enabled
implies the NM device state is >= DISCONNECTED.
https://github.com/NetworkManager/NetworkManager/pull/211
When there are profiles with wifi.hidden=yes, NetworkManager
will actively scan for these SSIDs. This makes the scan request
(and thus the user) recognizable and trackable.
It seems generally a bad idea to use hidden networks, as they
compromise either the privacy or usablity for the clients.
Log a (rate-limited) warning about this.
Also make sure the secrets request callback only send a reply to IWD and
the Connect method return callback executes the device state change to
"disconnected".
state_changed (called when IWD signalled device state change) was
supposed to not change NM device state on connect success or failure and
instead wait for the DBus Connect() method callback but it would
actually still call nm_device_emit_recheck_auto_activate on failure so
refactor state_changed and network_connect_cb to make sure
the state change and nm_device_emit_recheck_auto_activate are only
called from network_connect_cb.
This fixes a race where during a switch from one network to another NM
would immediately start a new activation after state_changed and
network_connect_cb would then handle the Connect failure and mark the
new activation as failed.
When creating the mirror 802.1x connections for IWD 802.1x profiles
set the NM_SETTING_SECRET_FLAG_NOT_SAVED flag on the secrets that
may at some point be requested from our agent. The saved secrets could
not be used anyway because of our use of
NM_SECRET_AGENT_GET_SECRETS_FLAG_REQUEST_NEW in
nm_device_iwd_agent_query. But also try to respect whatever secret
caching policy has been configured in the IWD profile for those secrets,
IWD would be responsible for storing them if it was allowed in the
profile.
In two places stop using g_dbus_proxy_new_* to create whole new
interface proxy objects for net.connman.iwd.Network as this will
normally have a huge overhead compared to asking the ObjectManager
client that we already have in NMIwdManager for those proxies.
dbus-monitor shows that for each network path returned by
GetOrderedNetworks () -- and we call it every 10 or 20 seconds and may
get many dozens of networks back -- gdbus would do the following each
time:
org.freedesktop.DBus.AddMatch("member=PropertiesChanged")
org.freedesktop.DBus.StartServiceByName("net.connman.iwd")
org.freedesktop.DBus.GetNameOwner("net.connman.iwd")
org.freedesktop.DBus.Properties.GetAll("net.connman.iwd.Network")
org.freedesktop.DBus.RemoveMatch("member=PropertiesChanged")
The Station.GetOrderedNetworks dbus method's return type has changed in
IWD commit 0a42f63d42be903a46c595693884772c1c84d39f as the last incompatible
API change before IWD 0.8 (docs change was made earlier in
0453308134a3aadb6a2ec6a78ea642e19427704c) so that network names and
types are no longer included in the reply. Expect this new reply
signature although still handle the old signature if we're using the
Device interface for IWD <= 0.7 compatibility.
It may be good idea to eventually pass the object manager instance from
nm-iwd-manager.c to nm-device-iwd.c to avoid using g_dbus_proxy_new_sync
and g_dbus_proxy_new_for_bus_sync in act_stage2_config, which possibly
generates a lot of DBus property queries.
https://github.com/NetworkManager/NetworkManager/pull/197
(cherry picked from commit 32506c8788)
NM_IN_SET will only compare string pointers and isn't useful for
checking if nm_setting_wireless_get_mode (s_wifi) is infrastructure.
Fixes: 570e1fa75b
Make sure we free our IWD agent objects whenever we're freeing the
IWD Object Manager. We're registering those objects on the same DBus
connection as the Object Manager so that they're visible to IWD, and
our only reference to that connection is through priv->object_manager
so even though the connection isn't changing when we free the object
manager and create a new one, we still need to free the agent object.
We could maybe keep a reference to the connection, but I'm not sure
there's any warranty that it doesn't get closed. We could also use
nm_dbus_manager_get_connection (nm_dbus_manager_get ()) and only
register and free the agent once, since it happens to be the same
connection but it'd perhaps be a hack to rely on this.
- since commit d17d26887c, a
NMSettingsConnection no longer "is-a" NMConnection. Instead,
we must call nm_settings_connection_get_connection() to obtain
the NMConnection instance. Adjust this in mirror_8021x_connection()
- don't leak "ssid" in mirror_8021x_connection()
- move deletion of the mirror-connection to known_network_data_free().
Previously, we must have made sure that every g_hash_table_remove()
and g_hash_table_insert()(!!) first deletes the mirror connection.
Likewise, in got_object_manager() when we call g_hash_table_remove_all(),
delete created mirror connections.
- rework interface_added() to make it robust against calling
interface_added() more than once without removing the interface
in between. Essentially, this just means that we first look into
"priv->known_networks" to see whether the @id is already tracked.
And if so, delete an existing mirror-connection as necessary.
