This was added by commit 4de8851eca, probably
by copying from NMDeviceVlan. It's not clear why a netlink request to
set the device IFF_UP would fail, or why that warrants a retry.
Instead of letting different subclasses call reset in their
virtual deactivate() function, do it in the parent class.
This works nicely, because the parent know whether the MAC
address is currently modified.
When a user want to explicitly spoof the MAC address, a failure
to do so should fail activation. For one, failing to do so may
be a security problem. In any case, if user asks to configure the
interface in a certain way and we fail to do so that shall result
in a failure to activate.
Extend the "ethernet.cloned-mac-address" and "wifi.cloned-mac-address"
settings. Instead of specifying an explicit MAC address, the additional
special values "permanent", "preserve", "random", "random-bia", "stable" and
"stable-bia" are supported.
"permanent" means to use the permanent hardware address. Previously that
was the default if no explict cloned-mac-address was set. The default is
thus still "permanent", but it can be overwritten by global
configuration.
"preserve" means not to configure the MAC address when activating the
device. That was actually the default behavior before introducing MAC
address handling with commit 1b49f941a6.
"random" and "random-bia" use a randomized MAC address for each
connection. "stable" and "stable-bia" use a generated, stable
address based on some token. The "bia" suffix says to generate a
burned-in address. The stable method by default uses as token the
connection UUID, but the token can be explicitly choosen via
"stable:<TOKEN>" and "stable-bia:<TOKEN>".
On a D-Bus level, the "cloned-mac-address" is a bytestring and thus
cannot express the new forms. It is replaced by the new
"assigned-mac-address" field. For the GObject property, libnm's API,
nmcli, keyfile, etc. the old name "cloned-mac-address" is still used.
Deprecating the old field seems more complicated then just extending
the use of the existing "cloned-mac-address" field, although the name
doesn't match well with the extended meaning.
There is some overlap with the "wifi.mac-address-randomization" setting.
https://bugzilla.gnome.org/show_bug.cgi?id=705545https://bugzilla.gnome.org/show_bug.cgi?id=708820https://bugzilla.gnome.org/show_bug.cgi?id=758301
VLAN and MACVLAN devices consider an ethernet.mac-address setting
to find the parent device. This setting shall be the permanent MAC
address of the device, not the current.
When the entire NMSettingWired setting is missing, it should be treated
exactly the same as each property having the default/unset value.
Otherwise, adding a NMSettingWired setting only to set (say) MTU,
would result in different behavior. Although effectively the
"cloned-mac-address" shall be in both cases the same.
Instead of accessing the singleton getter nm_settings_get(), obtain
the settings instance from the device instance itself via
nm_device_get_settings().
The device creation can be attempted if the name can be determined. It
alone is doesn't mean that there's a parent device -- the name could
just have been hardcoded in the connection.
NetworkManager[21519]: nm_device_get_ifindex: assertion 'NM_IS_DEVICE (self)' failed
Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046
1046 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth));
(gdb) bt
#0 0x00007ffff4ec88c3 in g_logv (log_domain=0x5555557fb2e5 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd3d0) at gmessages.c:1046
#1 0x00007ffff4ec8a3f in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at gmessages.c:1079
#2 0x00005555555d2090 in nm_device_get_ifindex (self=0x0) at devices/nm-device.c:562
#3 0x00005555555ef77a in nm_device_supports_vlans (self=0x0) at devices/nm-device.c:9865
#4 0x00005555555bf2f9 in create_and_realize (device=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, out_plink=0x7fffffffd5f8, error=0x7fffffffd700) at devices/nm-device-vlan.c:225
#5 0x00005555555d5757 in nm_device_create_and_realize (self=0x555555c549b0 [NMDeviceVlan], connection=0x555555b451e0, parent=0x0, error=0x7fffffffd700) at devices/nm-device.c:1783
#6 0x0000555555688601 in system_create_virtual_device (self=0x555555af51c0 [NMManager], connection=0x555555b451e0) at nm-manager.c:1120
#7 0x000055555568894e in connection_changed (settings=0x555555ae8220 [NMSettings], connection=0x555555b451e0, manager=0x555555af51c0 [NMManager]) at nm-manager.c:1172
#8 0x0000555555693448 in nm_manager_start (self=0x555555af51c0 [NMManager], error=0x7fffffffda30) at nm-manager.c:4466
#9 0x00005555555d166f in main (argc=1, argv=0x7fffffffdba8) at main.c:454
(gdb)
Fixes: 332994f1b1
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.
- 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
Get rid of NM_UNMANAGED_DEFAULT and refine the interaction between
unmanaged flags, device state and managed property.
Previously, the NM_UNMANAGED_DEFAULT was special in that a device was
still considered managed if it had solely the NM_UNMANAGED_DEFAULT flag
set and its state was managed. Thus, whether the device (state) was managed,
depended on the device state too.
Now, a device is considered managed (or unmanaged) based on the unmanaged
flags and realization state alone. At the same time, the device state
directly corresponds to the managed property of the device. Of course,
while changing the unmanaged flags, that invariant is shortly violated
until the state transistion is complete.
Introduce more unmanaged flags whereas some of them are non-authorative.
For example, the EXTERNAL_DOWN flag has only effect as long as the user
didn't explicitly manage the device (NM_UNMANAGED_USER_EXPLICIT). In other
words, certain flags can render other flags ineffective. Whether the device
is considered managed depends on the flags but also at the explicitly unset flags.
In a way, this is similar to previous where NM_UNMANAGED_DEFAULT was ignored
(if no other flags were present).
Also, previously a device that was NM_UNMANAGED_DEFAULT and in disconnected
state would transition back to unmanaged. No longer do that. Once a device is
managed, it stays managed as long as the flags indicate it should be managed.
However, the user can also modify the unmanaged flags via the D-Bus API.
Also get rid or nm_device_finish_init(). That was previously called
by NMManager after add_device(). As we now realize devices (possibly
multiple times) this should be handled during realization.
https://bugzilla.gnome.org/show_bug.cgi?id=746566
When we decide to add a new link, we alredy checked that no such link exists
(ignoring race conditions).
It is wrong to accept a EXITS failure when adding the link. There is no guarantee
that the existing link has all the same properties as the one we intend to add.
More importantly, this link was added externally outside of NetworkManager and it
should not be taken over.
Just treat EXISTS as a failure as any other.
The virtual function NMDevice:realize() is only called by
nm_device_realize() and immediately followed by nm_device_setup_start().
Devices already overwrite setup_start_notify() to update their properties.
No need to duplicate that in realize().
All implementations of NMDevice:setup_start() in derived classes
invoke the parent implementation first. Enforce that by moving
NMDevice:setup_start() to realize_start_setup() and only notify
derived classes afterwards via NMDevice:realize_start_notify().
We inconsistently use gulong,guint,int types to store signal handler
id, but the type returned by g_signal_connect() is a gulong.
This has no practical consequences because a int/guint is enough to
store the value, however it is better to use a consistent type, also
because nm_clear_g_signal_handler() accepts a pointer to the signal id
and thus it must be always called with the same pointer type.
Let the link-add functions return the internal pointer to the platform
link object. Similar to link-get, which doesn't copy the link either.
Also adjust the sole users of the add-functions (create-and-realize)
to take the pointer.
Eventually we still copy the returned data, because accessing platform can
invalidate the returned pointer. Thus we don't actually safe any copying
by this (at least every use of the function currently leads to the data
being copied).
Still change it, because I think the API of NMPlatform should look like that.
NMPlatformLink is a plain struct (not a GObject, for which we usually
don't use const). We certainly don't want the functions to modify the
passed-in data.
Ensure the platform link with the same interface name as the
NMDevice is actually compatible with it before using the link
for initialization of device properties. If not, remove the
NMDevice and create a new one since there are kernel resources
with a different type.
The parent of a link (IFLA_LINK) can be in another network namespace and
thus invisible to NM.
This requires the netlink attribute IFLA_LINK_NETNSID which is supported
by recent versions of kernel and libnl.
In this case, set the parent field to NM_PLATFORM_LINK_OTHER_NETNS
and properly handle this special case.
Future patches will create devices long before they are backed by
kernel resources, so we need to split NMDevice object creation from
actual setup based on the backing resources.
This patch combines the NMDeviceFactory's new_link() and
create_virtual_device_for_connection() class methods into a single
create_device() method that simply creates an unrealized NMDevice
object; this method is not expected to fail unless the device is
supposed to be ignored. This also means that the NMDevice
'platform-device' property is removed, because a platform link
object may not be available at NMDevice object creation time.
After the device is created, it is then "realized" at some later
time from a platform link (for existing/hardware devices via the
realize() method) or from an NMConnection (for newly created software
devices via the create_and_realize() NMDeviceClass methods).
https://bugzilla.gnome.org/show_bug.cgi?id=737458
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
Add NMExportedObject, make it the base class of all D-Bus-exported
types, and move the nm-properties-changed-signal logic into it. (Also,
make NMSettings use the same properties-changed code as everything
else, which it was not previously doing, presumably for historical
reasons).
(This is mostly just shuffling code around at this point, but
NMExportedObject will be more important in the gdbus port, since
gdbus-codegen doesn't do a very good job of supporting objects that
export multiple interfaces [as each NMDevice subclass does, for
example], so we will need more glue/helper code in NMExportedObject
then.)
Instead of hacky stuff in the Manager, let plugins themselves indicate
which links should be ignored (because they are really child links that
are controlled by a different device that the plugin handles).
Instead of looping over all plugins and asking each plugin whether it
can handle a link or a connection, have them advertise the link and
connection types they support, and use that when creating new devices.
Most nm_platform_*() functions operate on the platform
singleton nm_platform_get(). That made sense because the
NMPlatform instance was mainly to hook fake platform for
testing.
While the implicit argument saved some typing, I think explicit is
better. Especially, because NMPlatform could become a more usable
object then just a hook for testing.
With this change, NMPlatform instances can be used individually, not
only as a singleton instance.
Before this change, the constructor of NMLinuxPlatform could not
call any nm_platform_*() functions because the singleton was not
yet initialized. We could only instantiate an incomplete instance,
register it via nm_platform_setup(), and then complete initialization
via singleton->setup().
With this change, we can create and fully initialize NMPlatform instances
before/without setting them up them as singleton.
Also, currently there is no clear distinction between functions
that operate on the NMPlatform instance, and functions that can
be used stand-alone (e.g. nm_platform_ip4_address_to_string()).
The latter can not be mocked for testing. With this change, the
distinction becomes obvious. That is also useful because it becomes
clearer which functions make use of the platform cache and which not.
Inside nm-linux-platform.c, continue the pattern that the
self instance is named @platform. That makes sense because
its type is NMPlatform, and not NMLinuxPlatform what we
would expect from a paramter named @self.
This is a major diff that causes some pain when rebasing. Try
to rebase to the parent commit of this commit as a first step.
Then rebase on top of this commit using merge-strategy "ours".
Remove all remaining GParamSpec name and blurb strings (and fix
indentation while we're there), and add G_PARAM_STATIC_STRINGS to all
paramspecs that were lacking it.
NMDeviceGre and NMDeviceMacvlan didn't deal with the possibility that
the virtual device might be created before its parent's NMDevice is
created. Mostly fix this by having them put off the call to
nm_manager_get_device_by_ifindex() until someone actually requests the
device.
This is not perfect; if someone listening to notify::parent checks
right away, they may find that the parent property is still NULL, and
notify::parent will not be emitted again when it gets filled in. But
it's better than what's there now, when parent would remain NULL
forever in this case.
NMDeviceVeth did not have this problem, but it did have another
possible problem because it wasn't cleaning up its weak references
properly.
The link_changed method expects a valid info parameter.
NMDeviceMacvlan and NMDeviceGre calls link_changed
during construction for initialization.
As it was before, NMDeviceMacvlan and NMDeviceGre passed
NULL as NMPlatformLink, causing NM to segfault.
(Regression was introduced in 0e361e894c)
https://bugzilla.redhat.com/show_bug.cgi?id=997396
Signed-off-by: Thomas Haller <thaller@redhat.com>
Rather than passing UDI, ifname, and driver name to the device
constructors as separate arguments, just pass the NMPlatformLink
instead and let it parse them out.
Virtual types still take UDI and ifname separately, since we create
fake NMDevices for them for autoactivating connections. That's weird
in other ways too though, so perhaps this should be revisted.
