summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* libnm: drop unused property setter NM_DEVICE_MANAGEDth/libnm-dbus-rework-1Thomas Haller2019-09-041-5/+0
| | | | | | | | | | | NM_DEVICE_MANAGED was intended to work like NM_DEVICE_AUTOCONNECT: namely it would call the D-Bus property setter synchronously. But such behavior is horrendous, we certainly don't want blocking calls during a property getter. Luckily this one instance was unused and never worked as the property was marked as G_PARAM_READABLE. Just drop the setter.
* libnm: inline NMManager's get_permissions_sync()Thomas Haller2019-09-041-24/+10
| | | | | | | | | Synchrnous initialization is problmatic and needs cleanup. get_permissions_sync() is an internal function, that has only one caller. We need to keep track of functions that make synchronous D-Bus calls. Move the synchronous call into the caller, so that it's clearer who calls such API.
* libnm: drop _nm_dbus_is_connection_private()Thomas Haller2019-09-043-24/+3
| | | | | Currently, we don't use private sockets. We are always connected to D-Bus.
* libnm: drop nm_dbus_new_connection() helper APIThomas Haller2019-09-043-63/+6
| | | | | | | | | | We don't need a wrapper around g_bus_get*(). Just use it directly. I guess in the past this had some use when we were using a private socket too. Those days are gone. If we are going to re-introduce private socket support, then we probably should come up with a better solution.
* shared: add NMRefStringThomas Haller2019-09-044-0/+278
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I'd like to refactor libnm's caching. Note that cached D-Bus objects have repeated strings all over the place. For example every object will have a set of D-Bus interfaces (strings) and properties (strings) and an object path (which is referenced by other objects). We can save a lot of redundant strings by deduplicating/interning them. Also, by interning them, we can compare them using pointer equality. Add a NMRefString implementation for this. Maybe an alternative name would be NMInternedString or NMDedupString, because this string gets always interned. There is no way to create a NMRefString that is not interned. Still, NMRefString name sounds better. It is ref-counted after all. Notes: - glib has GQuark and g_intern_string(). However, such strings cannot be unrefered and are leaked indefinitely. It is thus unsuited for anything but a fixed set of well-known strings. - glib 2.58 adds GRefString, but we cannot use that because we currently still use glib 2.40. There are some differences: - GRefString is just a typedef to char. That means, the glib API exposes GRefString like regular character strings. NMRefString intentionally does that not. This makes it slightly less convenient to pass it to API that expects "const char *". But it makes it clear to the reader, that an instance is in fact a NMRefString, which means it indicates that the string is interned and can be referenced without additional copy. - GRefString can be optionally interned. That means you can only use pointer equality for comparing values if you know that the GRefString was created with g_ref_string_new_intern(). So, GRefString looks like a "const char *" pointer and even if you know it's a GRefString, you might not know whether it is interned. NMRefString is always interned, and you can always compare it using pointer equality. - In the past I already proposed a different implementation for a ref-string. That made different choices. For example NMRefString then was a typedef to "const char *", it did not support interning but deduplication (without a global cache), ref/unref was not thread safe (but then there was no global cache so that two threads could still use the API independently). The point is, there are various choices to make. GRefString, the previous NMRefString implementation and the one here, all have pros and cons. I think for the purpose where I intend NMRefString (dedup and efficient comparison), it is a preferable implementation. Ah, and of course NMRefString is an immutable string, which is a nice property.
* shared: add nm_auto_unlock_g_mutex and NM_G_MUTEX_LOCKED() helper macrosThomas Haller2019-09-041-0/+14
|
* core/tests: avoid deprecated g_main_run()/g_main_loop_unref() in testThomas Haller2019-09-031-3/+2
| | | | | | | | | These are deprecated. Also, they are nowadays implemented as macros that expand to #define g_main_run(loop) g_main_loop_run(loop) GLIB_DEPRECATED_MACRO_IN_2_26_FOR(g_main_loop_run) This can cause compilation failure (in some environments).
* core: merge branch 'bg/device-realize-failed-rh1686634'Beniamino Galvani2019-09-032-3/+5
|\ | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/252 https://bugzilla.redhat.com/show_bug.cgi?id=1686634
| * manager: keep device if realize() failsBeniamino Galvani2019-09-031-1/+0
| | | | | | | | | | | | | | | | | | system_create_virtual_device() first creates the device (unrealized) and then, if there a connection for the device with autoconnect=yes, creates the backing resources. If this last step fails the device should continue to exist, even if in an unrealized state. https://bugzilla.redhat.com/show_bug.cgi?id=1686634
| * device: don't set nm-owned flag if realize() failsBeniamino Galvani2019-09-031-2/+5
|/ | | | | | The nm-owned flag indicates whether the device was created by NM. If the realization step fails, the device was not created and so nm-owned should not be updated.
* po: fixed typo in it.poDavide Palma2019-09-031-2/+2
| | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/259
* core: fix a typoLubomir Rintel2019-09-0367-67/+67
| | | | s/grater/greater/
* device: fix crash when master connection failsBeniamino Galvani2019-09-031-5/+5
| | | | | | | | | | | | When the master AC becomes ready, activate_stage1_device_prepare() is called in a idle handler. If the master AC fails in the meantime, it will change state to deactivating or deactivated. We must check for that condition before proceeding with slave activation. Note the the 'master_ready' flag of an AC is never cleared after it is set. Fixes: 5b677d5a3bed ('device: move check for master from nm_device_activate_schedule_stage2_device_config() to end of stage1') https://bugzilla.redhat.com/show_bug.cgi?id=1747998
* libnm/remote-connection: add a pair of curly bracketsLubomir Rintel2019-09-021-2/+2
| | | | ...to aid readability.
* clients: avoid clearing a structure pointer when we're still using itLubomir Rintel2019-09-021-2/+3
| | | | | | | | | We're dereferencing the info pointer in the argument list in the call to nm_client_activate_connection_async(). Stealing it at that point causes a crash. This reverts a chunk of commit b298f2e6058a ('cli: use cleanup macro for freeing AddAndActivateInfo').
* keyfile: reorder printing empty [wireguard] section with peers and fix test ↵Thomas Haller2019-09-021-6/+6
| | | | | | | | | | | | | | failure We want to print the [wireguard] section before printing sections of the peers. It just looks nicer. This also fixes a test failure: /libnm/settings/roundtrip-conversion/wireguard/2: ** test:ERROR:./shared/nm-utils/nm-test-utils.h:2254:nmtst_keyfile_assert_data: assertion failed (d1 == data): ("[connection]\nid=roundtrip-conversion-2\nuuid=63376701-b61e-4318-bf7e-664a1c1eeaab\ntype=wireguard\ninterface-name=ifname2\npermissions=\n\n[wireguard-peer.uoGoXWWRxJvu4jDva8pPGA4nxau8B33S+YR+MfPFjxc=]\nendpoint=192.168.255.180:30429\npreshared-key-flags=2\n\n[wireguard-peer.BED73rH9j3OCHYAeXNrW5y5oia/Ngj+M04e9sG7DQOo=]\nendpoint=192.168.188.253:30407\npreshared-key-flags=1\npersistent-keepalive=5070\nallowed-ips=192.168.215.179/32;192.168.120.249/32;a:b:c::e4:13/128;192.168.157.84/32;a:b:c::1b:df/128;a:b:c::b0:84/128;192.168.168.17/32;\n\n[wireguard]\n\n[ipv4]\ndns-search=\nmethod=disabled\n\n[ipv6]\naddr-gen-mode=stable-privacy\ndns-search=\nmethod=ignore\n\n[proxy]\n" == "[connection]\nid=roundtrip-conversion-2\nuuid=63376701-b61e-4318-bf7e-664a1c1eeaab\ntype=wireguard\ninterface-name=ifname2\npermissions=\n\n[wireguard]\n\n[wireguard-peer.uoGoXWWRxJvu4jDva8pPGA4nxau8B33S+YR+MfPFjxc=]\nendpoint=192.168.255.180:30429\npreshared-key-flags=2\n\n[wireguard-peer.BED73rH9j3OCHYAeXNrW5y5oia/Ngj+M04e9sG7DQOo=]\nendpoint=192.168.188.253:30407\npreshared-key-flags=1\npersistent-keepalive=5070\nallowed-ips=192.168.215.179/32;192.168.120.249/32;a:b:c::e4:13/128;192.168.157.84/32;a:b:c::1b:df/128;a:b:c::b0:84/128;192.168.168.17/32;\n\n[ipv4]\ndns-search=\nmethod=disabled\n\n[ipv6]\naddr-gen-mode=stable-privacy\ndns-search=\nmethod=ignore\n\n[proxy]\n") Fixes: ddd148e02b6b ('keyfile: let keyfile writer serialize setting with all default values')
* merge: branch 'bg/ipv6-accept-ra-rh1734470'Beniamino Galvani2019-08-307-25/+107
|\ | | | | | | | | | | https://bugzilla.redhat.com/show_bug.cgi?id=1734470 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/247
| * ipv6: disable kernel handling of RAs (accept_ra)Beniamino Galvani2019-08-302-25/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With accept_ra set to 1, kernel sends its own router solicitation messages and parses the advertisements. This duplicates what NM already does in userspace and has unwanted consequences like [1] and [2]. The only reason why accept_ra was re-enabled in the past was to apply RA parameters like ReachableTime and RetransTimer [3]; but now NM supports them and so accept_ra can be turned off again. Also, note that previously the option was set in addrconf6_start_with_link_ready(), and so this was done only when the method was 'auto'. Instead, now we clear it for all methods except 'ignore'. [1] https://mail.gnome.org/archives/networkmanager-list/2019-June/msg00027.html [2] https://bugzilla.redhat.com/show_bug.cgi?id=1734470 [3] https://bugzilla.redhat.com/show_bug.cgi?id=1068673
| * ipv6: set neighbor parameters from RAsBeniamino Galvani2019-08-307-0/+103
|/ | | | | | | | | | | | | | | | | | | | | | | | IPv6 router advertisement messages contain the following parameters (RFC 4861): - Reachable time: 32-bit unsigned integer. The time, in milliseconds, that a node assumes a neighbor is reachable after having received a reachability confirmation. Used by the Neighbor Unreachability Detection algorithm. A value of zero means unspecified (by this router). - Retrans Timer: 32-bit unsigned integer. The time, in milliseconds, between retransmitted Neighbor Solicitation messages. Used by address resolution and the Neighbor Unreachability Detection algorithm. A value of zero means unspecified (by this router). Currently NM ignores them; however, since it leaves accept_ra=1, the kernel parses RAs and applies those parameters for us [1]. In the next commit kernel handling of RAs will be disabled, so let NM set those neighbor-related parameters. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/ndisc.c?h=v5.2#n1353
* dhcp: fall back to 'internal' client for IPv6 when using 'nettools'Beniamino Galvani2019-08-291-13/+0
| | | | The 'nettools' client doesn't support IPv6, fall back to 'internal'.
* device: merge branch 'th/act-stage1-re-entrant'Thomas Haller2019-08-2828-689/+699
|\ | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/245
| * device: after stage1 call stage2 synchronouslyThomas Haller2019-08-281-1/+33
| | | | | | | | | | We know we are ready and in a situation where we can handle state changes. Don't schedule stage2 in an idle handler, just invoke it directly.
| * device: move check for master from ↵Thomas Haller2019-08-281-52/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | nm_device_activate_schedule_stage2_device_config() to end of stage1 Note that by now no callers of nm_device_activate_schedule_stage2_device_config() are left. All previous callers now re-schedule stage1 instead of directly scheduling stage2. Note that if stage2 later also gets re-factored to re-enter itself instead of scheduling stage3 right away, the function will be used again. That means, we can move the check for the master where it belongs: as part (and at the end of) stage1. Also, slightly simplify the code. The handler master_ready_cb() no longer directly calls master_ready(). It's enough to always enter stage1 again. Also drop master_ready_handled. We don't need to remember that this condition was satsified. We can just check it always when we reach the place in activate_stage1_device_prepare().
| * device: let devices call stage1 again after being ready to proceedThomas Haller2019-08-288-48/+99
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I am about to change the when stage1 gets postponed, then the way to proceed it is to schedule stage1 again (instead of scheduling stage2). The reason is that stage1 handling should be reentrant and we should keep entering it until there is no more reason to postpone it. If a subclass postpones stage1 and then later progresses it by directly scheduling stage2, then only the subclass is in control over postponing stage 2. Instead, anybody should be able to delay stage2 independently. That can only work if everybody signals readyness to proceed by scheduling stage1 again.
| * device/wifi-p2p: inline and drop local function cleanup_p2p_connect_attempt()Thomas Haller2019-08-281-16/+9
| | | | | | | | It has only one caller. It's clearer to do the cleanup right there.
| * device/team: don't remember connection while killing teamThomas Haller2019-08-281-22/+18
| | | | | | | | | | We don't need this. The applied-connection is already remembered and suitable.
| * device/team: various cleanupsThomas Haller2019-08-281-58/+72
| |
| * device: set failure reason when settings hardware address failsThomas Haller2019-08-282-2/+8
| |
| * device: let NMDevice set hardware address instead of act_stage1_prepare() ↵Thomas Haller2019-08-281-3/+1
| | | | | | | | | | | | | | for NMDeviceEthernet There is a small change in the order of actions. Now we set the MAC address before calling link_negotiation_set(). That shouldn't make a difference.
| * device: let NMDevice set hardware address instead of act_stage1_prepare()Thomas Haller2019-08-282-8/+2
| |
| * device: move redundant act_stage1_prepare() implementations to set hwaddr to ↵Thomas Haller2019-08-287-49/+17
| | | | | | | | | | | | NMDevice This is so common, that NMDevice can handle it for us.
| * device: don't let subclasses call NMDevice's act_stage1_prepare()Thomas Haller2019-08-2819-109/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | NMDevice's act_stage1_prepare() now does nothing. Calling it is not useful and has no effect. In general, when a subclass overwrites a virtual function, it must be defined whether the subclass must, may or must-not call the parents implementation. Likewise, it must be clear when the parents implementation should be chained: first, as last, does it matter? In any case, that very much depends on how the parent is implemented and this can only be solved by documentation and common conventions. It's a forgiving approach to have a parents implementation do nothing, then the subclass may call it at any time (or not call it at all). This is especially useful if classes don't know their parent class well. But in NetworkManager code the relationship between classes are known at compile time, so every of these classes knows it derives directly from NMDevice. This forgingin approach was what NMDevice's act_stage1_prepare() was doing. However, it also adds lines of code resulting in a different kind of complexity. So, it's not clear that this forgiving approach is really better. Note that it also has a (tiny) runtime and code-size overhead. Change the expectation of how NMDevice's act_stage1_prepare() should be called: it is no longer implemented, and subclasses *MUST* not chain up.
| * device: move SR-IOV initialization to activate_stage1_device_prepare()Thomas Haller2019-08-282-83/+149
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Note that all subclasses of NMDevice that implement act_stage1_prepare(), call the parents implementation as very first thing. Previously, NMDevice's act_stage1_prepare() was setting up SR-IOV. But that is problemantic. Note that it may have returned NM_ACT_STAGE_RETURN_POSTPONE, in which case subclasses would just skip their action and return to the caller. Later, sriov_params_cb() would directly call nm_device_activate_schedule_stage2_device_config(), and thus act_stage1_prepare() was never executed for the subclass. That is wrong. First, I don't think it is good to let subclasses decide whether to call a parents implementation (and when). It just causes ambiguity. In the best case we do it always in the same order, in the worst case we call the parent at the wrong time or never. Instead, we want to initialize SR-IOV always and early in stage1, so we should just do it directly from activate_stage1_device_prepare(). Now NMDevice's act_stage1_prepare() does nothing. The bigger problem is that when a device wants to resume a stage that it previously postponed, that it would schedule the next stage! Instead, it should schedule the same stage again instead. That allows to postpone the completion of a stage for multiple reasons, and each call to a certain stage merely notifies that something changed and we need to check whether we can now complete the stage. For that to work, stages must become re-entrant. That means we need to remember whether an action that we care about needs to be started, is pending or already completed. Compare this to nm_device_activate_schedule_stage3_ip_config_start(), which checks whether firewall is configured. That is likewise the wrong approach. Callers that were in stage2 and postponed stage2, and later would schedule stage3 when they are ready. Then nm_device_activate_schedule_stage3_ip_config_start() would check whether firewall is also ready, and do nothing if that's not the case (relying that when the firewall code completes to call nm_device_activate_schedule_stage3_ip_config_start().
| * device: refactor handling of scheduled activation tasks on idleThomas Haller2019-08-282-99/+79
| | | | | | | | | | | | | | | | | | | | | | | | | | | | - use a [2] array for IPv4/IPv6 variants and a IS_IPv4 variable, like we do for other places that have similar implementations for both address families. - drop ActivationHandleData and use the fields directly. Also drop activation_source_get_by_family(). - rename "act_handle*" field to "activation_source_*", to follow the naming of the related accessor functions. - downgrade the severity of some logging messages.
| * device/trivial: rename local variable for device in ↵Thomas Haller2019-08-282-17/+18
| | | | | | | | | | | | "nm-device-{ethernet,macvlan}.c" This variable is commonly called "device", not "dev". Rename.
| * device: various minor style cleanupThomas Haller2019-08-286-22/+31
| |
| * device/wifi: cleanup supplicant_iface_wps_credentials_cb()Thomas Haller2019-08-281-20/+21
| | | | | | | | Restructure code to return early and free resources with nm_auto.
| * device/wifi: various cleanup in act_stage1_prepare()Thomas Haller2019-08-281-36/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The only change in behavior is in act_stage1_prepare(). That changes compared to before that we also set the specific object path if it was already set (and we looked up the AP by specific object to start with). Also, for existing APs that we found with nm_wifi_aps_find_first_compatible(), it changes the order of calling set_current_ap() before nm_active_connection_set_specific_object(). That should not make a different though. I anyway wonder why we even bother to set the specific object on the AC. Maybe that should be revisited.
| * device/wifi-p2p: make act_stage1_prepare() re-entrantThomas Haller2019-08-281-12/+7
| | | | | | | | | | Don't clear and reschedule finding of p2p peer if called multiple times during (the same) activation.
| * device/wpan: cleanup act_stage1_prepare() and don't assert with missing hwaddrThomas Haller2019-08-281-3/+9
| |
| * device/wireguard: drop act_stage1_prepare() implementationThomas Haller2019-08-281-12/+0
| | | | | | | | | | act_stage1_prepare() should become re-entrant. That means, we should not clear the state there. Instead, we clear it where necessary or on deactivate (which we do already).
| * device/ethernet: make NMDeviceEthernet.act_stage1_prepare() reentrant and ↵Thomas Haller2019-08-281-15/+28
| | | | | | | | minor cleanups
| * device/bridge: minor cleanup in NMDeviceBridge's act_stage1_prepare()Thomas Haller2019-08-281-5/+8
| | | | | | | | | | Only reset "vlan_configured" when deactivating. stage1() should be re-entrant.
| * device/bond: cleanup act-stage return values in NMDeviceBond's ↵Thomas Haller2019-08-281-13/+19
| | | | | | | | act_stage1_prepare()
| * device/team: drop unnecessary cast for NM_DEVICE_TEAM_GET_PRIVATE() macroThomas Haller2019-08-281-5/+5
| |
| * device/modem: drop unnecessary cast for NM_DEVICE_MODEM_GET_PRIVATE() macroThomas Haller2019-08-281-13/+13
| | | | | | | | | | | | NM_DEVICE_MODEM_GET_PRIVATE() is based on _NM_GET_PRIVATE(), which has some smarts to check the pointer type, but is fine with well-known parent pointer types like "NMDevice *".
| * modem/trivial: rename virtual function NMModemClass.act_stage1_prepare()Thomas Haller2019-08-284-16/+16
|/ | | | | NMDeviceClass already has a function with this name. It's confusing to have multiple virtual functions named the same. Rename.
* core: fix adding objects to NMIPConfig with @append_forceBeniamino Galvani2019-08-282-4/+4
| | | | | | | If the @append_force argument is set and the object is already in the list, it must be moved at the end. Fixes: 22edeb5b691b ('core: track addresses for NMIP4Config/NMIP6Config via NMDedupMultiIndex')
* core: add test to show nm_ipX_config_replace() bugBeniamino Galvani2019-08-281-0/+44
| | | | | | | | Add test to show a wrong result of ip_ipX_config_replace() due to a bug in _nm_ip_config_add_obj(). When an address is added to the tail of the index and another address with the same id already exists, the existing object is left at the same place, breaking the order of addresses.
* acd: fix memleak in acd_event()Thomas Haller2019-08-271-1/+1
| | | | | | | | Only happens with debug logging enabled. So, not a large problem. Found by Coverity. Fixes: d9a4b59c18e3 ('acd: adapt NM code and build options')
View Lorry Controller Status