The next commit will completely rework NMBusManager and replace
NMExportedObject by a new type NMDBusObject.
Originally, NMDBusObject was added along NMExportedObject to ease
the rework and have compilable, intermediate stages of refactoring. Now,
I think the new name is better, because NMDBusObject is very strongly related
to the bus manager and the old name NMExportedObject didn't make that
clear.
I also slighly prefer the name NMDBusObject over NMBusObject, hence
for consistancy, also rename NMBusManager to NMDBusManager.
This commit only renames the file for a nicer diff in the next commit.
It does not actually update the type name in sources. That will be done
later.
The change doesn't really make a difference. I thought it would, so I
did it. But turns out (as the code correctly assumes), while the
notifications are frozen, it's OK to leave the property still in an
inconsistent state while emitting the notify signal.
Still, it feels slightly more correct this way, so keep the change.
The notify() signal is not emitted while the object properties are
blocked via g_object_freeze_notify(). That makes is unsuitable to
emit a notification for "peer" property whenver the device's "parent"
property changes. Because especially with freeze/thaw, we want to emit
both signals in the same batch, not first emit change signals for "parent",
and then in a second run the signals for "peer".
Use the existing parent_changed_notify() virtual function instead.
The generated code is really just a thin wrapper around direct
GDBusProxy calls. GDBusProxy is reasonably convenient to use directly,
drop this wrapper.
We also don't use a generated wrapper for other cases where
NetworkManager acts as a D-Bus client. There is no reason to
do it in this case.
While the nmdbus_*() functions that we were using are small wrappers,
we also created a NMDBusSecretAgent instance, and hence several other
functions and symbols are used as well. It's unnecessary.
This saves 8552 bytes for NetworkManager binary (2817944 vs. 2809392
bytes for contrib/rpm on x86_64).
On exit during NMManager's dispose(), we must fist remove active connections
via active_connection_remove(), before clearing the volatile-connection-list.
Otheriwise, while deleting the active connection, we schedule a idle action
to delete the volatile connection on idle, but at that point the dispose()
already cleaned up the idle list.
==3150== 72 (24 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 3,411 of 6,079
==3150== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==3150== by 0x6AB7358: g_malloc (gmem.c:94)
==3150== by 0x6ACEF35: g_slice_alloc (gslice.c:1025)
==3150== by 0x1686B1: connection_flags_changed (nm-manager.c:1823)
==3150== by 0x661F73C: g_closure_invoke (gclosure.c:804)
==3150== by 0x66324DD: signal_emit_unlocked_R (gsignal.c:3635)
==3150== by 0x663AD04: g_signal_emit_valist (gsignal.c:3391)
==3150== by 0x663B66E: g_signal_emit (gsignal.c:3447)
==3150== by 0x2EC753: connection_flags_changed (nm-settings.c:824)
==3150== by 0x661F73C: g_closure_invoke (gclosure.c:804)
==3150== by 0x66324DD: signal_emit_unlocked_R (gsignal.c:3635)
==3150== by 0x663AD04: g_signal_emit_valist (gsignal.c:3391)
==3150== by 0x663B66E: g_signal_emit (gsignal.c:3447)
==3150== by 0x6623C03: g_object_dispatch_properties_changed (gobject.c:1080)
==3150== by 0x1DFD47: dispatch_properties_changed (nm-dbus-object.c:237)
==3150== by 0x6626178: g_object_notify_by_spec_internal (gobject.c:1173)
==3150== by 0x6626178: g_object_notify_by_pspec (gobject.c:1283)
==3150== by 0x2E7377: _notify (nm-settings-connection.c:53)
==3150== by 0x2E7377: nm_settings_connection_set_flags_full (nm-settings-connection.c:2346)
==3150== by 0x2E744D: nm_settings_connection_set_flags (nm-settings-connection.c:2316)
==3150== by 0x2E7466: set_visible (nm-settings-connection.c:316)
==3150== by 0x2E7774: nm_settings_connection_delete (nm-settings-connection.c:795)
==3150== by 0x1665A8: _delete_volatile_connection_do (nm-manager.c:598)
==3150== by 0x1668F4: active_connection_remove (nm-manager.c:625)
==3150== by 0x16ABA7: dispose (nm-manager.c:6726)
==3150== by 0x6624607: g_object_unref (gobject.c:3293)
==3150== by 0x1D779B: _nm_singleton_instance_destroy (nm-core-utils.c:138)
==3150== by 0x4011332: _dl_fini (in /usr/lib64/ld-2.26.so)
==3150== by 0x815FB57: __run_exit_handlers (in /usr/lib64/libc-2.26.so)
==3150== by 0x815FBA9: exit (in /usr/lib64/libc-2.26.so)
==3150== by 0x1383C7: main (main.c:467)
NMTST_ASSERT_PLATFORM_NETNS_CURRENT() already checks that the current namespace
is correct. Remove the duplicate assertion.
Also, NMP_CACHE_OPS_UNCHANGED is numerically identical to NM_PLATFORM_SIGNAL_NONE.
Use it in the assertion.
Return the extended ack message from the WaitForNlResponse delayed
action so that the caller can print a detailed reason with the
appropriate logging level.
From v4.12 the kernel appends some attributes to netlink acks
containing a textual description of the error and other fields (see
commit [1]). Parse those attributes and print the error message.
Examples:
platform-linux: netlink: recvmsg: error message from kernel: Network is unreachable (101) "Nexthop has invalid gateway" for request 12
platform-linux: netlink: recvmsg: error message from kernel: Invalid argument (22) "Local address cannot be multicast" for request 21
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2d4bc93368f5a0ddb57c8c885cdad9c9b7a10ed5
NAP connections are a bit special, in that they also have a [bridge]
setting, but their connection.type is "bluetooth".
The canonical way to check whether a bluetooth connection is of NAP type
is by calling _nm_connection_get_setting_bluetooth_for_nap().
So, instead of checking for bluetooth.type "pan" or "dun", check the
opposite and whether the connection is of NAP type. In practice it's the
same, but let'check for NAP consistently via get_setting_bluetooth_for_nap().
Bluetooth tethering using DUN or PANU is a common way to expose a
metered 3G or 4G connection from a phone to a laptop. We deliberately
ignore NAP connections, which is where we’re sharing internet from the
laptop to another device.
We could also set GUESS_YES for WiMAX connections, but NetworkManager
doesn’t support them any more. Add a comment about that.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=794120
The condition was obviosly inverted, blocking autoconnect when
it should not, and not blocking it when it should.
[thaller@redhat.com: modified original patch and rewrite commit message]
Fixes: e2c8ef45achttps://bugzilla.gnome.org/show_bug.cgi?id=794014
IPv4 routes that are a response to RTM_GETROUTE must have the cloned
flag while IPv6 routes don't have to. Don't check the flag for IPv6
routes and add a test case to verify that RTM_GETROUTE works for IPv6.
https://bugzilla.gnome.org/show_bug.cgi?id=793962
If SSID is an empty string there's no need to call nm_wifi_ap_set_ssid
as it won't do anything. It also has an assert checking that NULL is
passed for an empty SSID and we were passing a non-NULL pointer.
This bug was not causing a crash for me because of the !IS_NM_DEVICE_IWD
check and because my glib version probably had the assertion within
NM_IWD_MANAGER_GET_PRIVATE disabled.
While there, change the g_signal_connect line to use the macro for the
signal name.
Make sure .set_enabled uses the Device.Powered property to basically
bring the netdev UP and DOWN as I understand is expected by the
nm_device logic.
Device.Powered should generally reflect the UP state immediately but
just to avoid possible race conditions .is_available() will now return
a value that is an AND of the local "enabled" state and IWD's Powered
property.
Change from the default dbus call timeout (-1) to infinite (G_MAXINT)
because the call may now include the secret requests which have their
own timeout policies.
Remove the code (mostly copied from nm-device-wifi.c) that handles
checking if the secrets were provided and requesting missing secrets
before starting a connection attempt. Instead, request secrets when
we're asked for them by IWD through its agent interface. This happens
while the dbus Connect call is running. We change the NMDevice from
the CONFIG state to NEED_AUTH and then change back to CONFIG once we
sent the secrets back to IWD.
The current code would require the secrets only based on whether a
network is a KnownNetwork but IWD may need a new passwords even for
KnownNetworks if the last connection attempt has failed.
Always read and load ipv4 property values when method is shared also if
they will not be used: instead of dropping them at connection update,
keep their values in the ifcfg file.
Exceptions: ipv4.dns and ipv4.dns-search. They will be not read, otherwise
they may trigger a failure in nm-setting-ip4-config.c:verify() on load.
https://bugzilla.redhat.com/show_bug.cgi?id=1519299
Passing ownership with a function call is confusing. Don't do that.
Since we have the cleanup attribute, it doesn't significantly
complicate the callers, as all they need to do is marking the
@msg variable to free the message when going out of scope.
That results in the function that allocates the message also being
responsible to free it.
The callers expect _nl80211_send_and_recv() to free @msg.
This was broken by the previous commit, which wrongly removed
the nm_auto_nlmsg cleanup attribute.
Fix the compiler warning differently.
Fixes: d7108d9362
systemd commit f11cba7479fe ("libsystemd-network: fix unaligned loads
(issue #7654)") changed the way in which the MAC address is read to
use native endiannes:
htobe32(*((uint32_t *)x) -> unaligned_read_ne32(x)
This is wrong because loads done with BPF_LD + BPF_ABS are big-endian, as it
can be seen for the ethertype and arp-operation loads above in the
filter. Also, the same commit changed:
htobe32(*((unsigned int *)x) -> unaligned_read_be32(x)
in _bind_raw_socket(), which is the correct form.
The commit broke IPv4LL in presence of loops, as the sender now considers its
own packets as conflicting.
systemd commit: 6afe9046d570fd86dbc04526b5685c64d7958086
Now that every call to nm_device_set_ip_iface() and nm_device_set_ip_ifindex()
is checked, and setting an interface that does not exist causes the device
state to fail, we no longer need to allow setting an ip-iface if we are
unable to retrieve the ip-ifindex.
Depending on the bearer's configuration method, the data-port is
either a networking interface, or an tty for ppp.
Let's treat them strictily separate.
Also, rework how NM_MODEM_DATA_PORT was used in both contexts.
Instead, use the that we actually care about.
Also, when nm_device_set_ip_ifindex() fails, fail activation
right away.
Also, we early try to resolve the network interface's name to
an ifindex. If that fails, the device is already gone and we
fail early.
nm_device_modem_new() is only called with a newly created
NMModemBroadband or NMModemOfono instance.
See the callers
- NMModemManager:handle_new_modem()
- NMWwanFactory:modem_added_cb()
- NMDeviceModem:nm_device_modem_new()
Hence, at that point, the modem cannot yet have a data-port
or ip-iface set, because that is only obtained later.
- don't even bother to look into the platform cache, but use
if_indextoname() / if_nametoindex(). In most cases, we obtained
the ifindex/ifname not from the platform cache in the first
place. Hence, there is a race, where the interface might not
exist.
However, try to process events of the platform cache, hoping
that the cache contains an interface for the given ifindex/ifname.
- let set_ip_ifindex() and set_ip_iface() both return a boolean
value to indicate whether a ip-interface is set or not. That is,
whether we have a positive ip_ifindex. That seems more interesting
information, then to return whether anything changed.
- as before, set_ip_ifindex() can only clear an ifindex/ifname,
or error out without doing anything. That is different from
set_ip_iface(), which will also set an ifname if no ifindex
can be resolved. That is curreently ugly, because then ip-ifindex
and ip-iface don't agree. That shall be improved in the future
by:
- trying to set an interface that cannot be resolved shall
lead to a disconnect in any case.
- we shall make less use of the ip-iface and rely more on the
ifindex.
The error should be freed by callback functions, but only
_monitor_bridges_cb() actually does it. Simplify this by letting the
caller own the error.
Fixes: 830a5a14cb
- refactor the loop in event_handler_read_netlink() to mark pending
requests as answered by adding a new helper function
delayed_action_wait_for_nl_response_complete_check()
- delayed_action_wait_for_nl_response_complete_all() can be implemented
in terms of delayed_action_wait_for_nl_response_complete_check()
- if nm_platform_netns_push() fails, also complete all pending requests
with a new error code WAIT_FOR_NL_RESPONSE_RESULT_FAILED_SETNS.
Now that we cleaned up nl_recv(), we have full control over which error
variables are returned when. We no longer need to check "errno"
directly, and we no longer need the NLE_USER_* workaround.
