| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Don't have pending asynchronous requests in parallel, like setting
"ProcessCredentials" and "Start", or "Cancel" and "Start".
Instead, "Start" is only scheduled after "ProcessCredentials" completed
and "ProcessCredentials" is only scheduled after "Cancel" completed.
Also, handle the async response of these requests. For one, to achive the
chaining mentioned above and to log what happens and possible errors.
Upon new enrollment, a previously created GDBusProxy is now reused,
where the first operation is to Cancel the previous action.
Also, consistently <trace> log what is happening.
Not doing all of this is less lines of code. It's also simpler, and
faster. But in my opinion, it is (usually) better to check and wait for
return values, instead of firing off async requests uncontrolled. It
allows us to better know where we are and to log about each individual
step. This also makes all operations cancellable.
Undoubtedly, correctness and handling failures conflicts with simplicity
in this case -- or at least: what I think is "correctness" conflicts.
|
| |
|
|\
| |
| |
| | |
https://bugzilla.gnome.org/show_bug.cgi?id=783326
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The list field should have a unique name and not the same as the
list head. This avoids
c_list_link_before (&priv->network_servers, &network_server->network_servers);
c_list_for_each_entry (network_server, &priv->network_servers, network_servers) {
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Instead of having a complex _find_network_server() function with several
arguments, split it.
Having multiple optional arguments to a find() function is fine,
if all arguments consistently narrow down the search.
For example nmp_cache_lookup_link_full(), where each argument is
optional, and it restricts the search.
For _find_network_server() that was not the case, because setting
"addr" and "device" together would be non-sensical.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The 3 bluetooth NAP hooks are called each only once. Inline them.
It is still very easy to understand where the bluetooth related
functions are invoked: grep for nm_bt_vtable_network_server.
In deactivate(), don't bother checking whether the current active
connection is a bluetooth type. Just always call unregister_bridge().
It's fast, and does nothing in case the bridge isn't registered.
I change it because I disagree with the previous naming.
For example bt_network_server_available() would not only call
is_available(). Instead, it checks whether the connection can
activate regarding availability of the bluetooth connection
(meaning, it returns TRUE if it's not a bluetooth connection or
if the bluez manager gives green light). In the bridge case,
it doesn't check any network-server availability.
There is already a function with a meaningful name for this behavior:
check_connection_available().
Same with bt_network_server_register(). It would indicate success,
if the applied connection is not a bluetooth connection. In cases,
where it didn't actually register anything. A function called
bt_network_server_register() should only return success if it actually
registered anything.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Functions call each other, like
nm_connection_get_id()
nm_connection_get_setting_connection()
nm_connection_get_setting()
Along the way, each function asserts that the input argument
is of type NMConnection via
g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
Avoid such duplicate assertions when we already verifyied the
input argument.
For example, in case of nm_connection_get_id(), don't check just call
nm_connection_get_setting_connection() right away. It already
asserts.
The downside is, that the assertion no longer fails in the function
that immediately called it. But these are assertions after all.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Branch f9b1bc16e9e691ab89caf883f33d94be72364671 added bluetooth NAP
support. A NAP connection is of connection.type "bluetooth", but it
also has a "bridge" setting. Also, it is primarily handled by NMDeviceBridge
and NMBridgeDeviceFactory (with help from NMBluezManager).
However, don't let nm_connection_get_connection_type() and
nm_connnection_is_type() lie about what the connection.type is.
The type is "bluetooth" for most purposes -- at least, as far as
the client is concerned (and the public API of libnm). This restores
previous API behavior, where nm_connection_get_connection_type()
and nm_connection_is_type() would be simple accessors to the
"connection.type" property.
Only a few places care about the bridge aspect, and those places need special
treatment. For example NMDeviceBridge needs to be fully aware that it can
handle bluetooth NAP connection. That is nothing new: if you handle a
connection of any type, you must know which fields matter and what they
mean. It's not enough that nm_connection_get_connection_type() for bluetooth
NAP connectins is claiming to be a bridge.
Counter examples, where the original behavior is right:
src/nm-manager.c- g_set_error (error,
src/nm-manager.c- NM_MANAGER_ERROR,
src/nm-manager.c- NM_MANAGER_ERROR_FAILED,
src/nm-manager.c- "NetworkManager plugin for '%s' unavailable",
src/nm-manager.c: nm_connection_get_connection_type (connection));
the correct message is: "no bluetooth plugin available", not "bridge".
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: if ( ( nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: && !nm_connection_get_setting_pppoe (connection))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c- return TRUE;
the correct behavior is for ifcfg-rh plugin to reject bluetooth NAP
connections, not proceed and store it.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The duplicate check is based only on the link-types and setting-types.
However, that doesn't really cut it because in case of bluetooth
NAP connections, NMBridgeDeviceFactory is responsible for the connection,
although connection.type is "bluetooth".
This is only here to catch loading invalid plugins. But at the point
where we load an invalid plugin, the process is hosed. This is at best
an assertion, but rather pointless really.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The simple link-types/setting-types mechanism doesn't really cut it
because for the bluetooth NAP connection.type is "bluetooth", but
it shall be primarily handled by the bridge factory.
It's internal API that allows for a basic matching of the factory.
It is however not sophisticated enough for the full complexity.
Make it as internal API only.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
_nm_connection_find_base_type_setting()
Functions should behave gracefully with connections that don't verify.
Especially, as _normalize_connection_type() calls
_nm_connection_find_base_type_setting() precisely with an unknown
connection.type.
|
| |
| |
| |
| |
| | |
This is entirely internal API. We have unit tests that execute these
code paths. No need to have these assertions in production code.
|
| | |
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Using plain numbers make it cumbersome to grep for
setting types by priority.
The only downside is, that with the enum values it
is no longer obvious which value has higher or lower
priority.
Also, introduce NM_SETTING_PRIORITY_INVALID. This is what
_nm_setting_type_get_base_type_priority() returns. For the moment
it still has the same numerical value 0 as before. Later, that
shall be distinct from NM_SETTING_PRIORITY_CONNECTION.
|
| |
| |
| |
| |
| | |
Avoid having the function _nm_register_setting() shadowed by a macro
of the same name, but different behavior/arguments.
|
| | |
|
|/ |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since commit 74dac5f (nm-manager: try assuming connections on managed devices),
and commit f4226e7 (manager: avoid generating in memory connections
during startup for managed devices), recheck_assume_connection() also
assumes connections on devices that are currently not in sys-iface-state
"external".
That is correct, as also for fully managed devices (which are currently
in disconnected state), we want to assume external connections. However,
when doing that, we must reset the sys-iface-state to external.
https://bugzilla.redhat.com/show_bug.cgi?id=1457242
|
|\
| |
| |
| |
| |
| |
| | |
Normalize bond options to ensure a correct connection matching upon
daemon restart.
https://bugzilla.redhat.com/show_bug.cgi?id=1457909
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Upstream commit [1] changed in the kernel the default value of
tlb_dynamic_lb bond from 1 to 0 when the mode is not tlb. This is not
wrong, as the option value doesn't really matter for other modes, but
it breaks the connection matching because we read back a 0 value when
we expect a default of 1.
Fix this in a generic way by ignoring altogether options that are not
relevant for the current bond mode, because they are removed from the
connection during normalization.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8b426dc54cf4056984bab7dfa48c92ee79a46434
https://bugzilla.redhat.com/show_bug.cgi?id=1457909
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In an ideal world, we should not validate connections containing
options not valid for the current bond mode. However adding such
restriction now means that during an upgrade to the new NM version
some connections that were valid before become invalid, possibly
disrupting connectivity.
Instead, consider invalid options as a normalizable error and remove
them during normalization.
Converting the setting to a "canonical" form without invalid options
is important for the connection matching logic, where such invalid
options can cause false mismatches.
|
|\
| |
| |
| | |
https://github.com/NetworkManager/NetworkManager/pull/22
|
| |
| |
| |
| |
| |
| | |
The SSID is not "hidden". It is the wildcard SSID.
See build_hidden_probe_list().
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In AP mode we should not look up an access point. It is wrong to
do, and it ends up marking the connection as hidden.
It seems wrong to me that if the client explicitly set
hidden=FALSE before AddAndActivate(), that complete_connection()
would still set it to TRUE if it cannot find the access
point. That is, because complete_connection() does not know
whether hidden was omitted or set intentionally by the user.
|
|/
|
|
| |
It makes no sense to scan for those.
|
|\ |
|
| |
| |
| |
| |
| |
| | |
We don't need this flexibility of having a full fledged GObject
property for is-master. The property value only depends on the
device's class.
|
| |
| |
| |
| |
| | |
This gives a third return value: whether the device did not
match.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
activate_stage5_ip4_config_result()
We have nm_device_activate_schedule_ip4_config_result(). The name
should match.
Note, this affects logging, as we log the function name.
|
| |
| |
| |
| | |
It's easier to search in the logfile.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
A better name is link_speed_update(), because it re-reads and
sets the speed value.
Also, move _notfiy() after logging. It doesn't matter in this
case, but we should first log, and then do actions that have potentially
complex side-effects.
|
| |
| |
| |
| |
| |
| | |
Now, that NMDeviceClass:carrier_changed_notify() is no longer called as
deferred action, we can check for DCB state there, instead or registering
to the NM_DEVICE_CARRIER notifications.
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Note that:
- carrier_changed_notify() has only one implementation: NMDeviceEthernet
to call get_link_speed() when carrier comes back.
- currently, calling carrier_changed_notify() with carrier=FALSE
has no effect, because NMDeviceEthernet only acts on carrier=TRUE.
- when carrier appears, nm_device_set_carrier() will call
carrier_changed() right away. We only call carrier_changed()
with carrier=TRUE only at one place. The change merley moves
carrier_changed_notify() out of the function. Apart from
that it has no effect.
- when carrier disappears, previoulsy we would delay action for
4 seconds. Hence, we would delay carrier_changed_notify() as well
-- although it has no effect.
The last point is at least ugly. Fix it by moving
carrier_changed_notify() to nm_device_set_carrier().
|
|
|
|
|
| |
Otherwise the daemon would hang waiting for us while we respond with awkward
silence. That is not a healthy kind of communication.
|
|
|
|
|
|
|
|
|
| |
Coverity complains about not checking the return value:
src/settings/nm-settings-connection.c:2329: check_return: Calling "g_key_file_load_from_file" without checking return value (as is done elsewhere 6 out of 7 times).
While at it, refactor the code and check whether the timestamp
is valid.
|
| |
|
|
|
|
|
|
| |
g_async_initable_init_finish()
NetworkManager-1.8.0/src/supplicant/nm-supplicant-interface.c:232: check_return: Calling "g_async_initable_init_finish" without checking return value (as is done elsewhere 7 out of 8 times).
|
|
|
|
| |
NetworkManager-1.8.0/src/supplicant/tests/test-supplicant-config.c:528: check_return: Calling "nm_setting_802_1x_set_ca_cert" without checking return value (as is done elsewhere 13 out of 16 times).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
commit a955639 (connectivity: don't do periodic checks on interval=0)
broke scheduling connectivity checks.
That is because the timer is on only scheduled if
nm_connectivity_check_enabled(), which in turn only returns TRUE
if curl_mhandle is set. However, nm_connectivity_init() would only
initialize curl_mhandle after update_config(), missing to schedule
the periodic task.
https://mail.gnome.org/archives/networkmanager-list/2017-May/msg00076.html
Fixes: a95563996f07641e9877eb1760cac24415b65070
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
rpmdiff complains about uses of inet_aton, inet_makeaddr, inet_netof,
inet_ntoa under the IPv6 section:
usr/sbin/NetworkManager on aarch64 i686 x86_64 ppc ppc64 ppc64le s390 s390x uses function inet_aton, which may impact IPv6 support
I think the warning is bogus, but refactor our code to avoid it.
Note that systemd code still uses them, so it don't avoid the rpmdiff
warning. But let's not diverge our systemd import from upstream for this.
- for NMSettingBond:validate_ip() also avoid g_strsplit_set() which
allocates a full strv. Instead, we can do with one g_strdup().
- for test-resolvconf-capture.c, replace the functions with macros.
Macros should be avoided usually, but for test asserts they are
more convenient as they preserved the __FILE__:__LINE__ of where
the assertion fails.
|
|
|
|
|
|
|
|
| |
Teamd is not happy about them and would fail anyway. Worse even, if we
json_loads() such a JSON, which is precisely what happens when we inject the
"hwaddr" key, we turn bad JSON into a good one in a lossy matter. Not good.
https://bugzilla.redhat.com/show_bug.cgi?id=1455130
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When building with -flto, we need to use linker plugins.
In case of binutils' nm, it means to prefer gcc-nm if
available.
Like for ranlib and ar, prefer gcc-nm.
- replace AC_PATH_TOOL() by AC_CHECK_TOOLS(). That is consistent
with what we do for ar,ranlib and suggested on bgo#783311.
- instead of using the variable $BINUTILS_NM, replace it by
$NM, which is more common according to bgo#783311.
- Keep recognizing $BINUTILS_NM environment, which was introduced
by commit 8bc88bcc7c. This is purely to keep previous build
scripts working. Originally I named it "$BINUTILS_NM" because
using $NM in NetworkManager seemed confusing. But well...
https://bugs.gentoo.org/show_bug.cgi?id=620052
https://bugzilla.gnome.org/show_bug.cgi?id=782525
https://bugzilla.gnome.org/show_bug.cgi?id=783311
|
|
|
|
| |
Fixes: ba05819c89d913ad1bc6b86e62c7704d173ef534
|
|\
| |
| |
| | |
https://bugzilla.gnome.org/show_bug.cgi?id=783013
|
| |
| |
| |
| |
| | |
The Bluetooth NAP functionality seems only useful for the bridges. Move
it away from NMDevice.
|
| |
| |
| |
| |
| |
| | |
If there is value in such a helper function (there is), then
it should go alongside the other nm_connection_get_setting*()
helpers. NMDevice is already large enough.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The plugins may use stuff from core, but not the other way around.
Including "bluetooth/nm-bluez-common.h" is wrong.
The UUID argument is always "nap" in the NAP case. We don't need
the flexibility that it might be anything else. Just drop it.
As far as NMDevice is concerned, it anyway wouldn't (or shouldn't
know what the uuid is. It says register, and NMBluez5Manager should
figure out the details.
|