../src/devices/wifi/nm-device-iwd.c: In function ‘powered_changed’:
../src/devices/wifi/nm-device-iwd.c:2336:15: warning: assignment from incompatible pointer type [enabled by default]
interface = g_object_ref (priv->dbus_device_proxy);
^
Previously, we might have a pending action 'waiting-for-supplicant'
registered, although the device was not waiting:
<info> [1549611177.5815] device (wlan0): supplicant interface state: starting -> ready
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): P2P: Releasing WPA supplicant interfaces.
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): P2P: WPA supplicant management interface changed to /fi/w1/wpa_supplicant1/Interfaces/1.
<trace> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): remove_pending_action (0): 'waiting-for-supplicant' not pending (expected)
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): constructed (NMDeviceWifiP2P)
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): add_pending_action (1): 'waiting-for-supplicant'
The previous commit already fixed this bug by dropping the constructor
property for NM_DEVICE_WIFI_P2P_MGMT_IFACE.
Still, refactor handling of pending actions to keep track of whether we
have a pending action registered.
We already have a setter function nm_device_wifi_p2p_set_mgmt_iface()
as we may need to change the mgmt-iface later on. Use that to set the
supplicant interface instead of a constructor property.
That makes the object creation simpler, because nothing noteworthy
happens, until the very last statement in constructed() to add the
pending action.
The cast is bogus and leads to a compiler warning:
[424/583] Compiling C object src/devices/wifi/914a32e@@nm-device-plugin-wifi@sha/nm-device-iwd.c.o.
In file included from ../shared/nm-default.h:293,
from ../src/devices/wifi/nm-device-iwd.c:21:
../src/devices/wifi/nm-device-iwd.c: In function ‘nm_device_iwd_set_dbus_object’:
../src/devices/wifi/nm-device-iwd.c:2404:28: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (!nm_g_object_ref_set ((GObject **) &priv->dbus_obj, (GObject *) object))
../shared/nm-utils/nm-macros-internal.h:1048:13: note: in definition of macro ‘nm_g_object_ref_set’
typeof (*(pp)) *const _pp = (pp); \
^~
We named the types inconsistently:
- "p2p-wireless" ("libnm-core/nm-setting-p2p-wireless.h")
- "p2p" ("libnm/nm-p2p-peer.h")
- "p2p-wifi" ("src/devices/wifi/nm-device-p2p-wifi.h")
It seems to me, "libnm/nm-p2p-peer.h" should be qualified with a "Wi-Fi"
specific name. It's not just peer-to-peer, it's Wi-Fi P2P.
Yes, there is an inconsistency now, because there is already
"libnm/nm-access-point.h".
It seems to me (from looking at the internet), that the name "Wi-Fi P2P"
is more common than "P2P Wi-Fi" -- although both are used. There is also
the name "Wi-Fi Direct". But it's not clear which name should be
preferred here, so stick to "Wi-Fi P2P".
In this first commit only rename the files. The following commit will
rename the content.
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.
