str_if_set() was added to replace the non-standard gcc extension "?:".
However, "?:" is supported by clang as well and we already use it at
several places.
Also, str_if_set() did not follow our naming scheme and renaming to
nm_str_if_set() would be ugly. So just drop it.
When a VPN connection is active and the parent device becomes
unmanaged (due to device removal or system entering sleep), all
configurations are removed from the interface and the device is taken
down. After that, the VPN is disconnected and tries to remove the old
VPN configuration from device, causing the following assertion to
fail:
_replace_vpn_config_in_list: assertion '!old' failed
Remove the assertion as @old can be missing for valid reasons.
Coverity complains rightly about "strncpy (dst, ifname, IFNAMSIZ)"
because it might leave @dst non-NULL-terminated, in case @ifname
is too long (which already would be a bug in the first place).
Replace the strcpy() uses by a new helper nm_utils_ifname_cpy()
that asserts against valid arguments.
Since 87a3df2e57 ("device: remove default-unmanaged and refactor
unmanaged flags") the device must properly report the IS_SOFTWARE
capability in order to be potentially activated.
GError codes are only unique per domain, so logging the code without
also indicating the domain is not helpful. And anyway, if the error
messages are not distinctive enough to tell the whole story then we
should fix the error messages.
Based-on-patch-by: Dan Winship <danw@gnome.org>
Functions that take a GError** MUST fill it in on error. There is no
need to check whether error is NULL if the function it was passed to
had a failing return value.
Likewise, a proper GError must have a non-NULL message, so there's no
need to double-check that either.
Based-on-patch-by: Dan Winship <danw@gnome.org>
There are far too many "flags". Rename the "flags" to "n_ifa_flags"
which reminds to "ifa_flags" in 'struct ifaddrmsg', but with a
distinctive "n_" prefix.
There are far too many "flags". Rename the "flags" to "n_ifi_flags"
which reminds to "ifi_flags" in 'struct ifinfomsg', but with a
distinctive "n_" prefix.
g_str_hash() can not be called with NULL. Ensure that we don't crash.
Thereby, refactor the hashing algorithm because the chassis-id and
port-id are small numbers and xor-ing can cancel them easily.
Don't rely on what's already on the device. It could be that the MAC address
set on the device is not meaningful -- the NM crashed while two devices were
teamed together and now they have the same hardware address and now it's
impossible to bond them with mode=5.
This saves 110 bytes (which is 1/29986680757 of the size of the books in the
Library of Congress).
Also, the function is static; not using the nm_device_ prefix is preferred.
This fixes the issue where all Ad-Hoc networks try to connect one after one on
NM startup instead of the managed network that has AP available.
Fixes: e2637760f1
Since commit 87a3df2e57, the unmanaged
flag NM_UNMANAGED_USER_SETTINGS could be overwritten via an explict
user decision (NM_UNMANAGED_USER_EXPLICIT).
It makes sense to allow user configuration from file to be changable
by an explict user action via D-Bus at runtime.
However, it also changes behavior for devices that are currently explicitly
managed. Previously, a reload of the NM_UNMANAGED_USER_SETTINGS would
immediately unmanaged the device:
- for keyfile: send SIGHUP to reload NetworkManager.conf
- for ifcfg-rh: `nmcli connection [re]load`
So this change in behavior could negatively affect users who rely
on being able to configure "NM_CONTROLLED=no" and expect to unmanaged
the device immediately. Thus revert the change.
Note that NM_UNMANAGED_USER_SETTINGS is anyway ugly and should be
deprecated:
- for keyfile, why having the option "keyfile.unmanaged-devices"
instead of a generic options?
- for ifcfg-rh, why put per-device configuration in a per-connection
file?
The preferred way is to configure NM_UNMANAGED_USER_UDEV via
"ENV{NM_UNMANAGED}". Maybe we should also add a new configuration
scheme via NetworkManager.conf.
https://bugzilla.gnome.org/show_bug.cgi?id=762331
The NM_UNMANAGED_USER_SETTINGS flags are determined by the settings plugins.
That is, either:
- keyfile's "unmanaged-devices" configuration option
- ifcfg-rh's "NM_CONTROLLED" option
- ifnet's "managed" option
Rename NM_UNMANAGED_USER_CONFIG to NM_UNMANAGED_USER_SETTINGS to reflect
that it this is user configuration determined by the settings plugin.
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
When the device is transitioning from unmanaged to disconnected for "assumed"
reason, bring_up is not called. This is not a good thing in numerous
instances, e.g.:
1.) vlans that we've created need that to set IFF_UP and read carrier
otherwise they won't be available for connections.
2.) veths that are being managed need to start the deferred carrier check
so that the behavior matches real Ethernet.
3.) Hardware devices that were plugged in while NetworkManager is running
that need the IFF_UP for a carrier check, possibly enqueueing a deferred one.
Fixes: 5637d72af2.
Like we already do for IPv6 addresses, we should expose addresses
in a defined, stable sort order.
Clients usually show the addresses in the same order as obtained
via D-Bus.
When executing these commands:
ip link add br0 type bridge
ip link set dev br0 up
ip link add dummy1 type dummy
ip link set dev dummy1 up
ip addr add 1.1.1.1/24 dev br0
brctl addif br0 dummy1
sleep 1
ip link del br0
ip link del dummy1
the following assertion was failing:
nm_device_master_release_one_slave: runtime check failed: (NM_FLAGS_HAS (slave_priv->unmanaged_mask, NM_UNMANAGED_IS_SLAVE))
#0 g_logv()
#1 g_log()
#2 g_warn_message()
#3 nm_device_master_release_one_slave()
#4 nm_device_cleanup()
#5 _set_state_full()
#6 nm_device_state_changed()
#7 nm_device_unrealize()
#8 _platform_link_cb_idle()
#9 g_main_context_dispatch()
#10 g_main_context_dispatch()
#11 g_main_context_iterate()
#12 g_main_loop_run()
#13 main()
Upon slave removal we unrealize the device, resetting the unmanaged
flags to NM_UNMANAGED_PLATFORM_INIT, then we clean up the device and
call nm_device_master_release_one_slave(), which asserts the presence
of NM_UNMANAGED_IS_SLAVE flag cleared just before.
Drop the check.
Fixes: 87a3df2e57
Factories that overwrite this function are not supposed to chain
up the parent implementation. Thus there is no reason to have
a default implementation and it's clearer to inline it.
