summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
* device: let slaves wait in IP_CONFIG until master is readybg/nested-slavesBeniamino Galvani2017-05-041-0/+12
| | | | | | | | | | We call nm_device_activate_stage3_ipX_start() in various places, e.g. after a carrier change or when a master enslaves a new device to configure IP for the device. If the device is a slave in state IP_CONFIG, this makes it transition to IP_CHECK, while it should stay in IP_CONFIG until the master becomes ready. When the master is ready, it will move slaves directly to SECONDARIES, skipping IP configuration entirely.
* device: avoid integer overflow with route-metric penaltyThomas Haller2017-05-021-10/+18
| | | | (cherry picked from commit bd805b7e49849c76e4bd273799ae84c718033dc0)
* policy: fix memleak in lookup_callback() and cancellingThomas Haller2017-05-021-13/+11
| | | | | | | | | | | | | When the operation is cancelled, we must not touch user_data. Note that NM_POLICY_GET_PRIVATE() theoretically doesn't dereference the pointer (does it?) but doing pointer arithmetic on a dangling pointer is a very ugly thing to do. And of course, the memleak. Fixes: 5c716c8af8ddca1d3f7510494754d875b01a8889 Fixes: a2cdf632045d60b26f7aff470dedb56c1f9b938d (cherry picked from commit 3215508293c26e9e8531c2482def598ef1bbbefd)
* dhcp: dhclient: fix timeout greater than 60 secondsBeniamino Galvani2017-05-023-0/+23
| | | | | | | | | | | | The default timeout in dhclient is 60 seconds; if a lease can't be obtained during such interval, dhclient sends to NM a FAIL event and then the IP method fails. Thus, even if user specified a greater dhcp-timeout, NM terminated DHCP after 60 seconds. Fix this by passing an explicit timeout to dhclient. (cherry picked from commit 82ef497cc9e2728e73cb0426efbae85c83bec3fe)
* ifcfg-rh/tests: fix test_write_unknown() after change svGetValue() for ↵Thomas Haller2017-04-271-1/+1
| | | | | | | invalid values Fixes: daaa741a3d4bddcfd01715fd6caf7d0c84107a6d (cherry picked from commit 43c3501f978aeb783b363bff923e781302736985)
* ifcfg-rh: preserve the archaic NETMASK keyLubomir Rintel2017-04-271-2/+11
| | | | | | | | | | py-kickstart writes this out and there apparently are users using this. Let them have one less problem. Co-Authored-By: Thomas Haller <thaller@redhat.com> https://bugzilla.redhat.com/show_bug.cgi?id=1445414 (cherry picked from commit dbe0659ba419a77ad5ff2340bfc93c71a1bec61a)
* ifcfg-rh: preserve the archaic BOOTPROTO=static formLubomir Rintel2017-04-271-4/+9
| | | | | | | | py-kickstart writes this out. Okay -- we don't care on read and it makes sense when there actually are addresses. https://bugzilla.redhat.com/show_bug.cgi?id=1445414 (cherry picked from commit aa50dfc236b3806c6d7161cdea450655a1268f0d)
* ifcfg-rh: treat a wrongly quoted value like empty stringThomas Haller2017-04-271-2/+13
| | | | | | | | | | | | | | | | | | | | For example, if you want to test whether a value is present and reset it to a different value (only if it is present), it would be reasonable to do if (svGetValue (s, key, &tmp)) { svSetValue (s, key, "new-value"); g_free (tmp); } Without this patch, you could not be sure that key is not set to some inparsable value, which svWriteFile() would then write out as empty string. Have invalid values returned by svGetValue() as empty string. That is how svWriteFile() treats them. (cherry picked from commit 6470bed5f1ad25e20df14b333f1b083c9b390ece)
* core: make dad_counter argument guint32 typeThomas Haller2017-04-273-7/+7
| | | | | | | | | | | | | | | | | | The dad_counter is hashed into the resulting address. Since we want the hashing to be independent of the architecture, we always hash 32 bit of dad_counter. Make the dad_counter argument of type guint32 for consistency. In practice this has no effect because: - for all our (current!) architectues, guint is the same as guint32. - all callers of nm_utils_ipv6_addr_set_stable_privacy() keep their dad-counter argument as guint8, so they never even pass numbers larger then 255. - nm_utils_ipv6_addr_set_stable_privacy() limits dad_counter further against RFC7217_IDGEN_RETRIES. (cherry picked from commit 951e5f5bf87df01c603c9cbe69f401bfdfbbd579)
* core: avoid generating reserved IPv6 interface identifiersThomas Haller2017-04-271-2/+37
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | https://tools.ietf.org/html/rfc7217 says: The resulting Interface Identifier SHOULD be compared against the reserved IPv6 Interface Identifiers [RFC5453] [IANA-RESERVED-IID] and against those Interface Identifiers already employed in an address of the same network interface and the same network prefix. In the event that an unacceptable identifier has been generated, this situation SHOULD be handled in the same way as the case of duplicate addresses (see Section 6). In case of conflict, this suggests to create a new address incrementing the DAD counter, etc. Don't do that. If we generate an address of the reserved region, just rehash it right away. Note that the actual address anyway appears random, so this re-hashing is just as good as incrementing the DAD counter and going through the entire process again. Note that now we no longer generate certain addresses like we did previously. But realize that we now merely reject (1 + 16777216 + 128) addresses out of 2^64. So, the likelyhood of of a user accidentally generating an address that is suddenly rejected is in the order of 10e-13 (1 / 1,099,503,173,697). Which is not astronomically, but still extreeeemely unlikely. Also, the whole process is anyway build on the idea that somebody else might generate conflicting addresses (DAD). It means, there was always the extremely tiny chance that the address you generated last time is suddenly taken by somebody else. So, this change appears to a user like these reserved addresses are now claimed by another (non existing) host and a different address gets generated -- business as usual, as far as SLAAC is concerned. (cherry picked from commit f15c4961ad02a1edf766f140dd94e2d5449f8d82)
* core: move NMIPAddr to nm-core-utils.hThomas Haller2017-04-274-22/+23
| | | | (cherry picked from commit 67da0a28db834192d207fb315a3ba1983fe4a79e)
* ifcfg-rh/tests: fix out-of-tree build for cexpected fileThomas Haller2017-04-271-1/+1
| | | | | Fixes: f04bf45e84bddb7d685b03b9b36088848a46e75d (cherry picked from commit 5fc4bfc0e356444c294d1d8d839c62d56e2f4507)
* device: disable delegating prefixes to the device when the IPv6 config is ↵Lubomir Rintel2017-04-271-1/+4
| | | | | | | | | | | removed Fixes a crash where the default DNS domain to be announced together with the prefixes to be delegated is updated at the same time the device is being unrealized. https://bugzilla.redhat.com/show_bug.cgi?id=1425818 (cherry picked from commit 3e076cf8b1eba4937e1462d2ca34b9a65f500aff)
* device: fix restricting Generic connection by interface-nameThomas Haller2017-04-261-9/+17
| | | | | | | | | | | | | | | | | NMDeviceGeneric:check_connection_compatible() doesn't check for a matching interface name. It relies on the parent implementation to do that. The parent implementation calls nm_manager_get_connection_iface(). That fails for NM_SETTING_GENERIC_SETTING_NAME, because that one has no factory. Maybe this imbalance of having no factory for the Generic device is wrong, but usually factories only match a distinct set of device types, while the generic factory would handle them all (as last resort). Without this, activating a generic connection might activate the wrong interface. (cherry picked from commit 3876b10a4749638c3dcfa7e65b12bfee8030334c)
* ifcfg-rh/tests: compare the written files to a expected resultThomas Haller2017-04-2626-27/+575
| | | | | | | | | | | | | | | | | | | | | | We have unit tests for writing and re-reading ifcfg file. Those tests compare whether a file can be successfully read and is semantically identical. However, there were no tests that a certain output is written in a stable format. We aim not to change the output of what we write. For that, add tests to not only check the semantic of the written ifcfg file, but their bits and bytes. Some future changes may well intentionally change the current output. That will require to update the expected result files and can be done via NMTST_IFCFG_RH_UPDATE_EXPECTED=yes src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh Note that alias, route, and key files are not checked. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1445414 (cherry picked from commit f04bf45e84bddb7d685b03b9b36088848a46e75d)
* ifcfg-rh/tests: remove unused macro _writer_update_connection_FIXME()Thomas Haller2017-04-261-13/+0
| | | | | Fixes: 670e088efec40ca5cc3432e347c5226c9fa7cffc (cherry picked from commit e1e5d0d867c1fd0ef686b278e6ae636851713048)
* proxy: send proxy config after creating D-Bus proxyThomas Haller2017-04-231-6/+12
| | | | | | | | | | | | | As NMDevice now creates the NMPacrunnerManager instance as needed, it is even more likely that the initial call to nm_pacrunner_manager_send() will only queue (but not yet send) the new config. Later, when the D-Bus proxy is created, we will not get a name-owner changed signal. We instead have to push the configuration right away. (cherry picked from commit 019b9fbfc0aa2123d1e0a742c0b3d01caaa7d874)
* proxy: unify logging in nm-pacrunner-managerThomas Haller2017-04-231-11/+25
| | | | | | | Give logging lines that are concerned with a certain "config" the same prefix: their call-id. (cherry picked from commit 8c81a4b58b17283596b0dcb45825890dd6722ac9)
* proxy: introduce call-id for clearing pacmanager configurationThomas Haller2017-04-234-104/+137
| | | | | | | | | | | | | | | | | | | | nm_pacrunner_manager_remove() required a "tag" argument. It was a bug for callers trying to remove a configuration for a non-existing tag. That effectively means, the caller must keep track of whether a certain "tag" is pending. The caller also must remember the tag -- a tag that he must choose uniquely in the first place. Turn that around and have nm_pacrunner_manager_send() return a (non NULL) call-id. This call-id may later be used to remove the configuration. Apparently, previously the tracking of the "tag" was not always correct and we hit the assertion in nm_pacrunner_manager_remove(). https://bugzilla.redhat.com/show_bug.cgi?id=1444374 (cherry picked from commit b04a9c90eb0a866f2d730cc4777b4f670220ddc7)
* dbus: allow firewalld to communicate with NetworkManagerThomas Haller2017-04-211-0/+2
| | | | | | | | | | | | | | Usually, this "<allow send_destination="..."/>" part is shipped by firewalld's D-Bus policy. However, if firewalld is initially not installed with NetworkManager already running, dbus-daemon seems to cache the missing permission for the D-Bus connection. As a result, when installing and starting firewalld, NetworkManager requests fail until restart: firewall: [0x7f4b83643890,change:"eth1"]: complete: request failed (Rejected send message, 1 matched rules; type="method_call", sender=":1.3" (uid=0 pid=715 comm="/usr/sbin/NetworkManager --no-daemon ") interface="org.fedoraproject.FirewallD1.zone" member="changeZone" error name="(unset)" requested_reply="0" destination=":1.25" (uid=0 pid=1243 comm="/usr/bin/python -Es /usr/sbin/firewalld --nofork -")) https://bugzilla.redhat.com/show_bug.cgi?id=1436770 (cherry picked from commit cc1d409ba886e8e7c33f845790cfc700fcd2d854)
* org.freedesktop.NetworkManager.conf: don't use tabsThomas Haller2017-04-211-14/+14
| | | | (cherry picked from commit 8583e62276a23a7ea858edf6c71d122e22f41955)
* firewall: fix supressing errors from D-Bus callsThomas Haller2017-04-211-3/+5
| | | | | | | | | | | | | | | | | We want to ignore certain errors from firewalld. In the past, the error message contained only the error code. Since recently ([1], [2]), the error message contains a longer text: NetworkManager[647]: <debug> [1492768494.7475] device[0x7f7f21e78f50] (eth0): Activation: setting firewall zone 'default' NetworkManager[647]: <debug> [1492768494.7475] firewall: [0x7f7f21ed8900,change:"eth0"]: firewall zone change eth0:default ... firewalld[2342]: ERROR: UNKNOWN_INTERFACE: 'eth0' is not in any zone NetworkManager[647]: <warn> [1492768494.7832] firewall: [0x7f7f0400c780,remove:"eth0"]: complete: request failed (UNKNOWN_INTERFACE: 'eth0' is not in any zone) [1] https://github.com/t-woerner/firewalld/commit/c77156d7f688a0be3f0a1b4869b1c659e9e18cd6 [2] https://github.com/t-woerner/firewalld/commit/7c6ab456c5c461ac40cd7bb979a5daec6a13e6e4 (cherry picked from commit 2ad8bb0ce377624eefafe3b626d3fe691a7b9b7c)
* firewall: queue operations while NMFirewallManager instance is initializingThomas Haller2017-04-212-27/+80
| | | | | | | | | | | | We now initialize the NMFirewallManager asynchronously. That means, at first firewalld appears as "not running", for which we usually would fake-success right away. It would be complex for callers to wait for firewall-manager to be ready. So instead, have the asynchronous requests be queued and complete them once the D-Bus proxy is initialized. (cherry picked from commit fb7815df6e691c6e22769a4703204c010836fca9)
* firewall: drop _cb_info_is_idle()Thomas Haller2017-04-211-15/+12
| | | | | | | | Next we will get another mode, so an is-idle doesn't cut it. It can be confusing where the mode is set and where it is only accessed read-only. For that, add mode_mutable. (cherry picked from commit 04f4e327a9be606de5de009e0b5e1cc88abb8d6a)
* firewall: factor out D-Bus call from _start_request()Thomas Haller2017-04-211-26/+46
| | | | | | Will be used in the next commit. (cherry picked from commit d8bf05d3e695f043eeb0fac4646fc6babad1bee3)
* firewall: merge "started" signal and "available" propertyThomas Haller2017-04-213-62/+33
| | | | | | | The GObject property NM_FIREWALL_MANAGER_AVAILABLE is basically unused. Drop it. (cherry picked from commit db576b848ab029b9a232fe9fda2f816660c6a9b8)
* firewall: create firewall D-Bus proxy asynchronouslyThomas Haller2017-04-211-26/+62
| | | | | | | | | | | | | | Creating it asynchronously changes that on the first call to nm_firewall_manager_get() the instance is not yet running. Note that NMPolicy already connects to the "STARTED" signal and reapplies the zones when firewalld appears. So, this delayed change of the running state is handled mostly fine already. One part is still missing, it's to queue add_or_change/remove calls while the firewall manager is initializing. That follows next. (cherry picked from commit 753f39fa826405d4f50793647899059303b0f0cf)
* device: assume matching connections during first startThomas Haller2017-04-201-24/+66
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 2d1b85f (th/assume-vs-unmanaged-bgo746440), we clearly distinguish between two modes when encountering devices with external IP configuration: a) external devices. For those devices we generate a volatile in-memory connection and pretend it's active. However, the device must not be touched by NetworkManager in any way. b) assume, seamless take over. Mostly for restart of NetworkManager, we activate a connection gracefully without going through an down-up cycle. After the device reaches activated state, the device is considered fully managed. For this only an existing, non volatile connection can be used. Before 'th/assume-vs-unmanaged-bgo746440', the behaviors were not clearly separated. Since then, we only choose to assume a connection (b) when the state file indicates a matching connection. Now, extend this to also assume connections when: - during first-start (not after a restart) when there is no state file yet. - and, if we have an existing, non volatile, connection which matches the device's configuration. This patch lets NetworkManager assume connection also on first start. That is for example useful when handing over network configuration from initrd. This only applies to existing, permanent, matching(!) connections, so it is a good guess that the user wants NM to take over this interface. This brings us closer to the previous behavior before 'th/assume-vs-unmanaged-bgo746440'. https://bugzilla.redhat.com/show_bug.cgi?id=1439220 (cherry picked from commit 27b2477cb7dad2410c88c7dfca51f3aad208b881)
* config: add first_start paramter to NMConfig to detect restartThomas Haller2017-04-205-5/+35
| | | | (cherry picked from commit 2131954a190244714e8b27b48567594c7b103fb9)
* config: remove unused NMConfig self argument from nm_config_device_state_*() APIThomas Haller2017-04-204-21/+10
| | | | | | | | | | | nm_config_device_state_*() always access the file system directly, they don't cache data in NMConfig. Hence, they don't use the @self argument. Maybe those functions don't belong to nm-config.h, anyway. For lack of a better place they are there. (cherry picked from commit 1940be410cc0272de2f690542f43ef7dcb7bc4e1)
* device: re-apply sriov_numvfs after SIGHUPBeniamino Galvani2017-04-191-11/+14
| | | | (cherry picked from commit 264624f91dce43859205faa20303ec2d9cddaa64)
* core: allow setting SR-IOV num_vfsBeniamino Galvani2017-04-193-0/+26
| | | | (cherry picked from commit 32975b6aa5b48118ec65a3790e763a530a4ad913)
* all: detect SR-IOV device supportBeniamino Galvani2017-04-191-0/+3
| | | | (cherry picked from commit f13fd4524c8c5ed250530471a27be83f6f0914ae)
* platform: detect SR-IOV support and allow changing the number of VFsBeniamino Galvani2017-04-184-0/+143
| | | | (cherry picked from commit 0a7694cf81d27cd3e73295372065f46a4765f3a1)
* device: leave device up when setting it as unmanaged by userThomas Haller2017-04-182-3/+12
| | | | | | | | | | | | | | | | | | | | | | | | Before, setting a device to unmanaged causes it to go down and clear the interface state. It may be useful to instruct NetworkManager not to touch the device anymore but leave the current state up. Changing behavior for nmcli device set "$DEV" managed no To get the previous behavior, one has to first disconnect the interface via nmcli device disconnect "$DEV" nmcli device set "$DEV" managed no Note that non-permanent addresses like from DHCP will eventually time out because NetworkManager stops the DHCP client. When instructing NetworkManager to let go of the device, you have to take it over in any way you see fit. https://bugzilla.redhat.com/show_bug.cgi?id=1371433 (cherry picked from commit 9e8218f99a2d5a7020703e0fbac1c7c0983930db)
* core: enable "log-with-ptr" by default for platform and route-managerThomas Haller2017-04-183-3/+3
| | | | | | | | | | | | | | | | | | Arguably, we currently only have one instance of NMPlatform, NMRouteManager, NMDefaultRouteManager -- the one owned by the NMNetns singleton. Hence, all these instances we create with "log-with-ptr" set explicitly to false. In the future we want to support namespaces, and it will be be common to have multiple instances. For that we have "log-with-ptr" so we are able to disambiguiate the logging. Change the default to TRUE because it makes more sense. It has currently no effect as the default is never used. (cherry picked from commit 41148caba8dc6df6c9025680e53c1c88f048dc73)
* device: don't use platform singleton getter in device subclassesThomas Haller2017-04-1820-127/+131
| | | | | | | | | | | | | | | | | Reduce the use of NM_PLATFORM_GET / nm_platform_get() to get the platform singleton instance. For one, this is a step towards supporting namespaces, where we need to use different NMNetns/NMPlatform instances depending on in which namespace the device lives. Also, we should reduce our use of singletons. They are difficult to coordinate on shutdown. Instead there should be a clear order of dependencies, expressed by owning a reference to those singelton instances. We already own a reference to the platform singelton, so use it and avoid NM_PLATFORM_GET. (cherry picked from commit 94d9ee129d85ff926a10de78fedcc6b57cdcdb19)
* device: keep NMNetns instance per deviceThomas Haller2017-04-1814-171/+210
| | | | | | | | | | | | | | | | | | | This also ensures that we own a reference to the NMPlatform, NMRouteManager and NMDefaultRouteManager instances. See bug rh#1440089 where we might access the singleton getter after destroing the singleton instance of NMRouteManager. This is prevented by keeping a reference to those instances -- indirectly via the netns instance. Later, we may add support for multiple namespaces. Then it might make sense to swap the NMNetns instance of a device when moving the device between namespaces. Also, drop the use of singelton instances. https://bugzilla.redhat.com/show_bug.cgi?id=1440089 (cherry picked from commit c48a19b7c6009ee85a4d6a6caa96570de6b315ca)
* core: add NMNetns to bundle platform and route managersThomas Haller2017-04-1816-70/+331
| | | | | | | | | | | | | | | | | | | | | | | NMPlatform, NMRouteManager and NMDefaultRouteManager are singletons instances. Users of those are for example NMDevice, which registers to GObject signals of both NMPlatform and NMRouteManager. Hence, as NMDevice:dispose() disconnects the signal handlers, it must ensure that those singleton instances live longer then the NMDevice instance. That is usually accomplished by having users of singleton instances own a reference to those instances. For NMDevice that effectively means that it shall own a reference to several singletons. NMPlatform, NMRouteManager, and NMDefaultRouteManager are all per-namespace. In general it doesn't make sense to have more then one instances of these per name space. Nnote that currently we don't support multiple namespaces yet. If we will ever support multiple namespaces, then a NMDevice would have a reference to all of these manager instances. Hence, introduce a new class NMNetns which bundles them together. (cherry picked from commit 0af2f5c28b7646c687b5180a45a9124d62c0dfac)
* manager: set interface as removed when the link disappearsBeniamino Galvani2017-04-181-0/+1
| | | | | | | | | | | | | | | | | | | | | | Set the device state as removed when the link disappears, so that in the call to unrealize() when the device is unmanaged we also perform a cleanup of it and especially, we terminate any DHCP client instances running on the device. If we keep DHCP clients running, we can hit assertions later when we start another instance on the same interface, because we kill the old dhclient from the pidfile, and the g_child_watch_add() done by the first client instance is not able to waitpid() it, complaining with: GChildWatchSource: Exit status of a child process was requested but ECHILD was received by waitpid(). Most likely the process is ignoring SIGCHLD, or some other thread is invoking waitpid() with a nonpositive first argument; either behavior can break applications that use g_child_watch_add()/g_spawn_sync() either directly or indirectly. https://bugzilla.redhat.com/show_bug.cgi?id=1436602 (cherry picked from commit df537d2eac44cffe4512214c87f3f303c5a350d3)
* platform: fix a typoLubomir Rintel2017-04-181-1/+1
| | | | (cherry picked from commit c76ee5883d6684213fb2fe7bf06c9e4ce016f97a)
* ppp: allow disabling IPv4 tooDan Williams2017-04-181-3/+28
| | | | | https://bugzilla.redhat.com/show_bug.cgi?id=1439360 (cherry picked from commit a12b3f06cb1e38f788d52e605f25a16347ec8695)
* core: ignore host part when comparing routes for route-managerThomas Haller2017-04-154-11/+36
| | | | (cherry picked from commit b78562570ac49ddd31e20336206399613d5604ef)
* route-manager: normalize host part of tracked routes in _vx_route_sync()Thomas Haller2017-04-151-0/+4
| | | | | | | | | | | | | | | | | | | The input list of routes is allowed to contain non-normalized routes, that is, routes which host part is non-zero. Such routes are rejected by kernel, but NM should transparently allow them (by normalizing the host part). The ID comparison function route_id_cmp() already properly ignored the (possibly non-zero) host part. However, in the internal list we also should make sure not to track such routes. We achive that by normalizing the host part to zero. Note that below we check whether the tracked route is idential to the route configured at platform. If we don't normalize the host part, the comparison will always indicate that the route is not yet configured, and thus we will re-sync the route every time. (cherry picked from commit 5c54b7a31e401d48568cc50827bca3c4043da20c)
* src: only compare network parts of routes in nm_utils_match_connection()Thomas Haller2017-04-151-9/+19
| | | | | | | | | | | | Kernel requires that routes have a host part of zero. For NetworkManager configuration we allow non-zero host parts (but ignore them). Fix route_compare() to ignore the host part. This has only effect during assuming connections. That means, on restart NM would fail to match a connection with static routes if it has a non-zero host part. So, the impact is rather small. (cherry picked from commit 034b7fb51c1d16a4002d2902c60aac05e946bb4f)
* platform: only consider net part of routes for route cache's IDThomas Haller2017-04-151-4/+12
| | | | | | | | | | | | | | | | Routes with a non-zero host part are not allowed by kernel and don't really exist. We didn't reject such routes in users configuration, so various part of NM allow such routes. NM should silently strip the host part. Extend the cache's route ID to clear the host part too. Note that NM's handling of routes is fundamentally flawed, as for kernels routes don't have an "id" (or rather: all properties of a route are part of it's ID, not only the family,ifindex, network/plen and metric tuple (see related bug rh#1337855). (cherry picked from commit 57b0dce083a1991530e14735588d873e441f26fa)
* platform: cleanup possibly non-zero host part for route operationsThomas Haller2017-04-151-3/+5
| | | | | | | | | | | | | | Platform's add/remove operations accept a "network" argument. Kernel requires that the host part (based on plen) is all zero. For NetworkManager we are more resilient to user configuration. Cleanup the input argument already before calling _nl_msg_new_route(). Note that we use the same "network" argument to construct a obj_id instance and to find the route in the cache (do_add_addrroute()). Without cleaning the host part, the added object cannot be found and the add-route command seemingly fails. (cherry picked from commit 11d8c41898e43236476dd5f5f26736e2a3fdbea5)
* vpn: inline call_plugin_disconnect()Thomas Haller2017-04-151-17/+11
| | | | | | | There is only one caller. Don't bother moving the logic to a separate function. (cherry picked from commit b23484be720ac72a1a7e176813a179993d97f36e)
* vpn: avoid calling call_plugin_disconnect() without proxyThomas Haller2017-04-151-1/+2
| | | | | | | | | | | | | | | | | | | | | Got an assertion due to priv-proxy unset. NMDevice: - _platform_link_cb_idle() - nm_device_unrealize() [NMDeviceTun] - nm_device_state_changed() - _set_state_full() NMVpnConnection: - _set_vpn_state() - call_plugin_disconnect() It seam to me, that can only happen if the NMVpnConnection never completed on_proxy_acquired() and is still in preparing state when being disconnected. Avoid that be checking whether we have a proxy. https://bugzilla.redhat.com/show_bug.cgi?id=1442064 (cherry picked from commit bc1d1c9df4c1d68233a3743dab030480aeed2831)
* manager: unexport VPN connections when the activation fails earlyBeniamino Galvani2017-04-111-3/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a VPN connection can't be activated we have to unexport and dispose it. Commit f2182fbf9b24 ("core: don't emit double PropertiesChanged signal for new active connections") removed the call to nm_exported_object_unexport() in case of failure because the active connection already gets unreferenced on failure. However, an exported object can't be disposed until it's explicitly unexported because GDBus code keeps a reference to it. The result was that the active connection was kept alive and exported, but without explicit references to it. As soon as the connection was unexported, it was also automatically disposed, causing issues like: (src/nm-exported-object.c:1025):dispose: code should not be reached #0 _g_log_abort () at /lib64/libglib-2.0.so.0 #1 g_logv () at /lib64/libglib-2.0.so.0 #2 g_log () at /lib64/libglib-2.0.so.0 #3 g_warn_message () at /lib64/libglib-2.0.so.0 #4 dispose (object=0xaaf110) at src/nm-exported-object.c:1025 #5 dispose (object=0xaaf110) at src/nm-active-connection.c:1246 #6 dispose (object=0xaaf110) at src/vpn/nm-vpn-connection.c:2642 #7 g_object_unref () at /lib64/libgobject-2.0.so.0 #8 registration_data_free () at /lib64/libgio-2.0.so.0 #9 g_hash_table_remove_internal () at /lib64/libglib-2.0.so.0 #10 g_dbus_object_manager_server_unexport_unlocked () at /lib64/libgio-2.0.so.0 #11 g_dbus_object_manager_server_unexport () at /lib64/libgio-2.0.so.0 #12 nm_bus_manager_unregister_object (self=0x9069e0, object=object@entry=0xaaf110) at src/nm-bus-manager.c:858 #13 nm_exported_object_unexport (self=0xaaf110) at src/nm-exported-object.c:714 #14 _settings_connection_removed (connection=<optimized out>, user_data=0xaaf110) at src/nm-active-connection.c:184 #15 g_closure_invoke () at /lib64/libgobject-2.0.so.0 #16 signal_emit_unlocked_R () at /lib64/libgobject-2.0.so.0 #17 g_signal_emit_valist () at /lib64/libgobject-2.0.so.0 #18 g_signal_emit_by_name () at /lib64/libgobject-2.0.so.0 #19 nm_settings_connection_signal_remove (self=self@entry=0x9e4a80, allow_reuse=allow_reuse@entry=0) at src/settings/nm-settings-connection.c:2085 #20 do_delete (self=0x9e4a80, callback=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/nm-settings-connection.c:768 #21 do_delete (connection=0x9e4a80, callback=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/plugins/keyfile/nms-keyfile-connection.c:127 #22 nm_settings_connection_delete (self=self@entry=0x9e4a80, callback=callback@entry=0x58106a <con_delete_cb>, user_data=0xa84fa0) at src/settings/nm-settings-connection.c:694 #23 delete_auth_cb (self=self@entry=0x9e4a80, context=context@entry=0x7fffd80131e0, subject=0x91fb40, error=<optimized out>, data=data@entry=0x0) at src/settings/nm-settings-connection.c:1879 #24 pk_auth_cb (chain=0x7fffd00024a0, chain_error=<optimized out>, context=0x7fffd80131e0, user_data=<optimized out>) at src/settings/nm-settings-connection.c:1351 #25 auth_chain_finish (user_data=0x7fffd00024a0) at src/nm-auth-utils.c:92 #26 g_idle_dispatch () at /lib64/libglib-2.0.so.0 Restore the unexport upon failure to fix this. Fixes: f2182fbf9b2423bd8509b2f0cf218edd96dac32c https://bugzilla.redhat.com/show_bug.cgi?id=1440077 (cherry picked from commit 69fd96118e9a5e6b613644c2cb61911d554e7f3b)