Fixes autoconnect after the device is realized again:
# nmcli c add type team
# nmcli c up team
# nmcli d dis nm-team # autoconnect is blocked
# nmcli c del team # the is unrealized
# nmcli c add type team # the device is realized again, not
# activating with the new connection
After the device is unrealized a lot of its properites are reset. Notably, it
doesn't have an ifindex anymore so there's nothing to unconfigure really. This
makes at least NMDeviceBond unhappy:
(bond device with a slave is removed externally)
NetworkManager[21022]: <info> (bond0): device state change: activated -> unmanaged (reason 'unmanaged') [100 10 3]
NetworkManager[21022]: nm_platform_link_release: assertion 'master > 0' failed
Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=0x5555557592b1 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd370) at gmessages.c:1046
1046 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth));
(gdb) bt
#0 0x00007ffff4ec88c3 in g_logv (log_domain=0x5555557592b1 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd370) at gmessages.c:1046
#1 0x00007ffff4ec8a3f in g_log (log_domain=log_domain@entry=0x5555557592b1 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff4f3673d "%s: assertion '%s' failed")
at gmessages.c:1079
#2 0x00007ffff4ec8a79 in g_return_if_fail_warning (log_domain=log_domain@entry=0x5555557592b1 "NetworkManager", pretty_function=pretty_function@entry=0x55555575ea50 <__FUNCTION__.33801> "nm_platform_link_relea8
#3 0x000055555560559a in nm_platform_link_release (self=0x555555a27bb0 [NMLinuxPlatform], master=master@entry=0, slave=slave@entry=3) at platform/nm-platform.c:1326
#4 0x00005555555b506e in release_slave (device=<optimized out>, slave=0x555555b6d770 [NMDeviceEthernet], configure=<optimized out>) at devices/nm-device-bond.c:423
#5 0x00005555555dab7b in nm_device_master_release_one_slave (self=self@entry=0x555555bf0cc0 [NMDeviceBond], slave=0x555555b6d770 [NMDeviceEthernet], configure=configure@entry=1, reason=reason@entry=
NM_DEVICE_STATE_REASON_NOW_UNMANAGED) at devices/nm-device.c:1137
#6 0x00005555555dadb6 in nm_device_master_release_slaves (self=self@entry=0x555555bf0cc0 [NMDeviceBond]) at devices/nm-device.c:2344
#7 0x00005555555dd12f in nm_device_cleanup (self=self@entry=0x555555bf0cc0 [NMDeviceBond], reason=reason@entry=NM_DEVICE_STATE_REASON_NOW_UNMANAGED, cleanup_type=cleanup_type@entry=CLEANUP_TYPE_DECONFIGURE)
at devices/nm-device.c:9133
#8 0x00005555555de3ea in _set_state_full (self=self@entry=0x555555bf0cc0 [NMDeviceBond], state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=
NM_DEVICE_STATE_REASON_NOW_UNMANAGED, quitting=quitting@entry=0) at devices/nm-device.c:9510
#9 0x00005555555dedb7 in nm_device_state_changed (self=self@entry=0x555555bf0cc0 [NMDeviceBond], state=state@entry=NM_DEVICE_STATE_UNMANAGED, reason=reason@entry=NM_DEVICE_STATE_REASON_NOW_UNMANAGED)
at devices/nm-device.c:9769
#10 0x00005555555e11b4 in nm_device_unrealize (self=self@entry=0x555555bf0cc0 [NMDeviceBond], remove_resources=remove_resources@entry=0, error=error@entry=0x7fffffffd788) at devices/nm-device.c:2062
#11 0x000055555565c9c5 in _platform_link_cb_idle (data=0x555555c6e2b0) at nm-manager.c:2055
#12 0x00007ffff4ec179a in g_main_context_dispatch (context=0x555555a226c0) at gmain.c:3109
#13 0x00007ffff4ec179a in g_main_context_dispatch (context=context@entry=0x555555a226c0) at gmain.c:3708
#14 0x00007ffff4ec1ae8 in g_main_context_iterate (context=0x555555a226c0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3779
#15 0x00007ffff4ec1dba in g_main_loop_run (loop=0x555555a22780) at gmain.c:3973
#16 0x00005555555b3e5f in main (argc=1, argv=0x7fffffffdb18) at main.c:488
An IPv6 address might have been added externally and the device is yet to
traverse to a connected state.
On the other hand, the externally added devices still traverse through
DISCONNECTED state and we don't want to attempt the LL addition there. Let's
check if the link still exists instead.
Link related functions should have a "nm_platform_link" prefix. Rename.
Naming is a subjective matter and one might argue that omitting
the "link" part from the name is shorter and even preferred.
However, I think functions related to links should have a common
prefix as the underlyings are strongly related.
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.
There are the link-types NONE and UNKNOWN. NONE is a linktype that is never
returned by platform, but UNKNOWN is very much a valid (albeit unspecified)
type.
Effectively, create_and_realized() should create a link of a known type,
thus it should never return an UNKNOWN link type at this point. Still
change it because it feels more correct.
Device subclasses (for example NMDeviceWifi) can use the ifindex in
their constructor(), but the value now is set later in
parent class constructed(). This causes the following:
nm_platform_wifi_get_capabilities: assertion 'ifindex > 0' failed
Fix this by initializing ifindex earlier in NMDevice's
constructor(). While at it, remove the
nm_assert (pllink->type != NM_LINK_TYPE_NONE);
assertion, since pllink can be NULL there.
Fixes: 6db04dc206
This is to make it possible for the device factories to indicate the desired
link type and make it possible to avoid matching the unrealized device to a
platform device of different link type.
We often lookup the private data and retrieve it via NM_DEVICE_GET_PRIVATE(),
which in turn calls G_TYPE_INSTANCE_GET_PRIVATE().
Instead cache the pointer to the private data.
There are up- and downsides:
- requries additional sizeof(gpointer) bytes for each NMDevice.
+ retrieving the private pointer will be slightly faster.
+ easier debugging in gdb as it is currently often a pain to
retrieve the private data.
But most importantly, the allows to change our common pattern
to first cache the private data in a variable @priv. That is
often cumbersome to write, especially for short functions.
This change gives us a choice to use self->priv directly.
Such a change should not be aimed for every class. Instead it makes
mostly sense for NMDevice, where it pays off better due to the
class' size and ubiquitous use.
https://mail.gnome.org/archives/networkmanager-list/2015-December/msg00017.html
Showcase for the new macros NM_UTILS_STRING_LOOKUP_TABLE() and
NM_UTILS_STRING_LOOKUP_TABLE_DEFINE_STATIC().
It changes behavior in case of looking up an invalid/unknown
state reason. Previously it would just have returned "unknown"
-- which was indistinguishable from a regular "unknown" value.
Now it returns the numeric id as a string. The string is allocated
with alloca(), which is desired but one should be aware of the pitfalls:
- prevents the caller from being inlined
- bad idea to do in a loop.
Before the device is setup, we call nm_platform_tun_get_properties() without
a valid ifindex. That triggered an assertion [1].
Thereby, change nm_platform_tun_get_properties() to effectively clear
the tun properties when we are unable to fetch them. Also, never modify
the tun-mode of NMDeviceTun.
[1]
#0 0x00007f0a4173e81b in g_logv (breakpoint=1) at gmessages.c:324
#1 0x00007f0a4173e81b in g_logv (log_domain=0x561e9264ccf6 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7ffd71a0d4d0) at gmessages.c:1081
#2 0x00007f0a4173e98f in g_log (log_domain=<optimized out>, log_level=<optimized out>, format=<optimized out>) at gmessages.c:1119
#3 0x0000561e9241ecec in nm_platform_tun_get_properties (self=0x561e9354ba70 [NMLinuxPlatform], ifindex=0, props=0x7ffd71a0d650) at platform/nm-platform.c:2081
#4 0x0000561e923aad9c in reload_tun_properties (self=0x561e937fb080 [NMDeviceTun]) at devices/nm-device-tun.c:68
#5 0x0000561e923aa795 in realize (device=0x561e937fb080 [NMDeviceTun], plink=0x7ffd71a0d818, error=0x7ffd71a0d798) at devices/nm-device-tun.c:225
#6 0x0000561e923bdc06 in nm_device_realize (self=0x561e937fb080 [NMDeviceTun], plink=0x7ffd71a0d818, out_compatible=0x7ffd71a0d77c, error=0x7ffd71a0d798) at devices/nm-device.c:1713
#7 0x0000561e924ad995 in platform_link_added (self=0x561e9356e230 [NMManager], ifindex=33, plink=0x7ffd71a0d818) at nm-manager.c:1947
#8 0x0000561e924ad717 in _platform_link_cb_idle (data=0x561e937eb940) at nm-manager.c:2029
#9 0x00007f0a41737e3a in g_main_context_dispatch (context=0x561e93547530) at gmain.c:3154
#10 0x00007f0a41737e3a in g_main_context_dispatch (context=context@entry=0x561e93547530) at gmain.c:3769
#11 0x00007f0a417381d0 in g_main_context_iterate (context=0x561e93547530, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3840
#12 0x00007f0a417384f2 in g_main_loop_run (loop=0x561e935475f0) at gmain.c:4034
#13 0x0000561e923ba3f3 in main (argc=1, argv=0x7ffd71a0dc68) at main.c:488
Fixes: 4dbaac4ba2
I found the handling of the master-device very confusing because it was
unclear who sets priv->master, and when it should be set.
Now:
- Setting priv->master (in a slave) always goes together with adding
the master to priv->slaves (in the master). Previously, this was
done at separate places, so it was not clear if master and slave
always agree on their relationship -- in fact, they did not.
- There are now three basic functions which do the enslaving/releasing:
(1) nm_device_master_add_slave()
(2) nm_device_master_enslave_slave()
(3) nm_device_master_release_one_slave()
Step 3/release basically undoes the 1/add and 2/enslave steps.
- completing the enslaving/releasing is now done by
(1) nm_device_slave_notify_enslave()
(2) nm_device_slave_notify_release()
These functions also emit signals like NM_DEVICE_MASTER.
- Derived classes no longer emit NM_DEVICE_SLAVES notification. Instead
the notification is emited together with NM_DEVICE_MASTER, whenever a
slaves changes state. Also, NM_DEVICE_SLAVES list now only exposes
slaves that are actually @is_enslaved.
Instead of reimplementing the slave property in bond, bridge
and team, just add the property to the parent class. It's not
that the parent class would be agnostic to the master/slave
implementation, all the slaves are known to the every device
type implementation.
Also, the derived class doesn't know the correct time when
to invoke the notify-changed for the slaves property.
E.g. it should be only invoked after nm_device_slave_notify_enslave()
when other components also consider the slave as enslaved.
Later this will be fixed so that the SLAVES property correspond
to what other master/slave related properties say.
(gdb) bt
#0 0x00007fc1c920681b in g_logv () at /lib64/libglib-2.0.so.0
#1 0x00007fc1c920698f in g_log () at /lib64/libglib-2.0.so.0
#2 0x00007fc1c9523237 in g_type_check_instance_cast () at /lib64/libgobject-2.0.so.0
#3 0x00007fc1bdef10ed in supplicant_connection_timeout_cb (user_data=0x561a52451600) at nm-device-wifi.c:2207
#4 0x00007fc1c9200893 in g_timeout_dispatch () at /lib64/libglib-2.0.so.0
#5 0x00007fc1c91ffe3a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#6 0x00007fc1c92001d0 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
#7 0x00007fc1c92004f2 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#8 0x0000561a511583f3 in main (argc=1, argv=0x7ffc033f1e28) at main.c:488
release_slave() should do the right thing and handle errors as
good as it can. There is no value in propagating the error and
it's not clear what the caller should do in face of a failure
during release.
We have the master/slave related functions
- for master device:
- nm_device_master_add_slave()
- nm_device_master_release_slaves()
- nm_device_release_one_slave()
- nm_device_enslave_slave()
- for slave device:
- nm_device_slave_notify_enslave()
- nm_device_slave_notify_release()
Rename the two that didn't match the pattern to
- nm_device_master_release_one_slave()
- nm_device_master_enslave_slave()
But, of course, only one realized device can have the same
interface name at a time.
This commit effectively reverts most of:
1b37cd0340
core: allow ActiveConnections to be created without a device
But it's not easy to do a separate revert of that code due to
interdependencies with nm-manager.c.
Creating devices when they are defined by a connection also makes
makes it possible to require the NMDevice to be present when
activating it, which means we can remove a bunch of code from
NMManager that had to handle software devices not existing yet at
the time of the activation request.
But it also means we must be more careful when finding master
interfaces during slave activation, since we cannot simply match
by interface name alone. Instead we must find the master which
matches both the interface name and can control slaves of the type
which is being activated.
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.
Unrealized devices aren't backed by kernel resources and so won't know
all of their attributes. That means three things:
1) they must update their attributes when they become realized
2) they must clear those attributes when unrealized
3) they must be looser in checking compatible connections until
they are realized
This requires that the setup() function be split into two parts, start & finish,
because finish must be run after add_device()
Also, we can simplify whether to pay attention to 'recheck-assume', which
is now dependent on priv->is_nm_owned, because the only case where NM should
*not* listen for the 'recheck-assume' signal is when the device is a
software device created by NM itself. That logic was previously spread
across the callers of add_device() but is now consolidated into
nm-manager.c::device_realized() and nm-device.c::nm_device_create_and_realize().
The device properties are obtained via netlink, thus we get proper
platform notifications whenever they change. So the device properties
always reflect the platform state and there is no need to refresh
it during update_connection()/check_connection_compatible().
This property is TRUE for devices that exist either as a kernel device
or are backed by some other resource (eg, ModemManager object, Bluez
device, etc). It will eventually be FALSE for software devices that
are not yet instantiated.
There is only one user of the NM_DEVICE_MASTER property (NMActiveConnection).
device_master_changed() uses the property for change notifications,
but returns early if !nm_device_get_master().
We might emit the "notify::master" signal too often, even when nothing
changed (because currently we always emit it when priv->master changes).
If we would fix that to only emit the signal when nm_device_get_master()
actually changes, it would still be correct, because device_master_changed()
doesn't care about notifications when the master is unset.
Hence, the only user doesn't care about the difference. So change it because
it's unexpected that the property and function are not completely equal.
