summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* supplicant: chain asynchronous requests for WPSth/wifi-wps-chain-async-bgo783499Thomas Haller2017-06-071-83/+252
| | | | | | | | | | | | | | | | | | | | | | | Don't have pending asynchronous requests in parallel, like setting "ProcessCredentials" and "Start", or "Cancel" and "Start". Instead, "Start" is only scheduled after "ProcessCredentials" completed and "ProcessCredentials" is only scheduled after "Cancel" completed. Also, handle the async response of these requests. For one, to achive the chaining mentioned above and to log what happens and possible errors. Upon new enrollment, a previously created GDBusProxy is now reused, where the first operation is to Cancel the previous action. Also, consistently <trace> log what is happening. Not doing all of this is less lines of code. It's also simpler, and faster. But in my opinion, it is (usually) better to check and wait for return values, instead of firing off async requests uncontrolled. It allows us to better know where we are and to log about each individual step. This also makes all operations cancellable. Undoubtedly, correctness and handling failures conflicts with simplicity in this case -- or at least: what I think is "correctness" conflicts.
* supplicant/trivial: move code aroundThomas Haller2017-06-071-6/+8
|
* bluetooth: merge branch 'th/bluetooth-nap-bgo783326'Thomas Haller2017-06-0750-467/+401
|\ | | | | | | https://bugzilla.gnome.org/show_bug.cgi?id=783326
| * bluetooth: assert against registering same device multiple timesThomas Haller2017-06-071-0/+3
| |
| * bluetooth/trivial: rename NetworkServer's network_servers fieldThomas Haller2017-06-071-6/+6
| | | | | | | | | | | | | | | | | | The list field should have a unique name and not the same as the list head. This avoids c_list_link_before (&priv->network_servers, &network_server->network_servers); c_list_for_each_entry (network_server, &priv->network_servers, network_servers) {
| * bluetooth: split _find_network_server() in two functionsThomas Haller2017-06-071-18/+25
| | | | | | | | | | | | | | | | | | | | | | | | Instead of having a complex _find_network_server() function with several arguments, split it. Having multiple optional arguments to a find() function is fine, if all arguments consistently narrow down the search. For example nmp_cache_lookup_link_full(), where each argument is optional, and it restricts the search. For _find_network_server() that was not the case, because setting "addr" and "device" together would be non-sensical.
| * device: inline bluetooth function in nm-device-bridge.cThomas Haller2017-06-071-53/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 3 bluetooth NAP hooks are called each only once. Inline them. It is still very easy to understand where the bluetooth related functions are invoked: grep for nm_bt_vtable_network_server. In deactivate(), don't bother checking whether the current active connection is a bluetooth type. Just always call unregister_bridge(). It's fast, and does nothing in case the bridge isn't registered. I change it because I disagree with the previous naming. For example bt_network_server_available() would not only call is_available(). Instead, it checks whether the connection can activate regarding availability of the bluetooth connection (meaning, it returns TRUE if it's not a bluetooth connection or if the bluez manager gives green light). In the bridge case, it doesn't check any network-server availability. There is already a function with a meaningful name for this behavior: check_connection_available(). Same with bt_network_server_register(). It would indicate success, if the applied connection is not a bluetooth connection. In cases, where it didn't actually register anything. A function called bt_network_server_register() should only return success if it actually registered anything.
| * libnm: streamline functions in nm-connection.cThomas Haller2017-06-072-140/+76
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Functions call each other, like nm_connection_get_id() nm_connection_get_setting_connection() nm_connection_get_setting() Along the way, each function asserts that the input argument is of type NMConnection via g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL); Avoid such duplicate assertions when we already verifyied the input argument. For example, in case of nm_connection_get_id(), don't check just call nm_connection_get_setting_connection() right away. It already asserts. The downside is, that the assertion no longer fails in the function that immediately called it. But these are assertions after all.
| * all: change handling of connection.type for bluetooth NAP and in generalThomas Haller2017-06-077-109/+103
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Branch f9b1bc16e9e691ab89caf883f33d94be72364671 added bluetooth NAP support. A NAP connection is of connection.type "bluetooth", but it also has a "bridge" setting. Also, it is primarily handled by NMDeviceBridge and NMBridgeDeviceFactory (with help from NMBluezManager). However, don't let nm_connection_get_connection_type() and nm_connnection_is_type() lie about what the connection.type is. The type is "bluetooth" for most purposes -- at least, as far as the client is concerned (and the public API of libnm). This restores previous API behavior, where nm_connection_get_connection_type() and nm_connection_is_type() would be simple accessors to the "connection.type" property. Only a few places care about the bridge aspect, and those places need special treatment. For example NMDeviceBridge needs to be fully aware that it can handle bluetooth NAP connection. That is nothing new: if you handle a connection of any type, you must know which fields matter and what they mean. It's not enough that nm_connection_get_connection_type() for bluetooth NAP connectins is claiming to be a bridge. Counter examples, where the original behavior is right: src/nm-manager.c- g_set_error (error, src/nm-manager.c- NM_MANAGER_ERROR, src/nm-manager.c- NM_MANAGER_ERROR_FAILED, src/nm-manager.c- "NetworkManager plugin for '%s' unavailable", src/nm-manager.c: nm_connection_get_connection_type (connection)); the correct message is: "no bluetooth plugin available", not "bridge". src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: if ( ( nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: && !nm_connection_get_setting_pppoe (connection)) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME)) src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c- return TRUE; the correct behavior is for ifcfg-rh plugin to reject bluetooth NAP connections, not proceed and store it.
| * device: remove duplicate check for NMDeviceFactory's _add_factory()Thomas Haller2017-06-071-14/+2
| | | | | | | | | | | | | | | | | | | | | | The duplicate check is based only on the link-types and setting-types. However, that doesn't really cut it because in case of bluetooth NAP connections, NMBridgeDeviceFactory is responsible for the connection, although connection.type is "bluetooth". This is only here to catch loading invalid plugins. But at the point where we load an invalid plugin, the process is hosed. This is at best an assertion, but rather pointless really.
| * device: hide nm_device_factory_get_supported_types() functionThomas Haller2017-06-072-5/+1
| | | | | | | | | | | | | | | | | | | | The simple link-types/setting-types mechanism doesn't really cut it because for the bluetooth NAP connection.type is "bluetooth", but it shall be primarily handled by the bridge factory. It's internal API that allows for a basic matching of the factory. It is however not sophisticated enough for the full complexity. Make it as internal API only.
| * libnm: avoid assertion with invalid connections in ↵Thomas Haller2017-06-071-4/+7
| | | | | | | | | | | | | | | | | | _nm_connection_find_base_type_setting() Functions should behave gracefully with connections that don't verify. Especially, as _normalize_connection_type() calls _nm_connection_find_base_type_setting() precisely with an unknown connection.type.
| * libnm: downgrade assertions in _nm_register_setting_impl() to nm_assert()Thomas Haller2017-06-071-12/+7
| | | | | | | | | | This is entirely internal API. We have unit tests that execute these code paths. No need to have these assertions in production code.
| * libnm: distinguish INVALID priority setting from NM_SETTING_PRIORITY_CONNECTIONThomas Haller2017-06-071-13/+14
| |
| * libnm: use enum for setting prioritiesThomas Haller2017-06-0734-34/+34
| |
| * libnm: add enum for setting prioritiesThomas Haller2017-06-078-53/+70
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Using plain numbers make it cumbersome to grep for setting types by priority. The only downside is, that with the enum values it is no longer obvious which value has higher or lower priority. Also, introduce NM_SETTING_PRIORITY_INVALID. This is what _nm_setting_type_get_base_type_priority() returns. For the moment it still has the same numerical value 0 as before. Later, that shall be distinct from NM_SETTING_PRIORITY_CONNECTION.
| * libnm/trivial: rename _nm_register_setting function to _nm_register_setting_implThomas Haller2017-06-072-8/+8
| | | | | | | | | | Avoid having the function _nm_register_setting() shadowed by a macro of the same name, but different behavior/arguments.
| * core: minor refactoring condition in validate_activation_request()Thomas Haller2017-06-071-15/+11
| |
| * core: print the connection.type in nm_utils_log_connection_diff()Thomas Haller2017-06-071-7/+29
|/
* libnm-core: fix typo in 802.1x doc commentBeniamino Galvani2017-06-061-1/+1
|
* device: mark device as sys-iface-state=external when assuming connectionThomas Haller2017-06-051-2/+4
| | | | | | | | | | | | | | Since commit 74dac5f (nm-manager: try assuming connections on managed devices), and commit f4226e7 (manager: avoid generating in memory connections during startup for managed devices), recheck_assume_connection() also assumes connections on devices that are currently not in sys-iface-state "external". That is correct, as also for fully managed devices (which are currently in disconnected state), we want to assume external connections. However, when doing that, we must reset the sys-iface-state to external. https://bugzilla.redhat.com/show_bug.cgi?id=1457242
* bond: merge branch 'bg/bond-option-normalize-rh1457909'Beniamino Galvani2017-06-054-2/+106
|\ | | | | | | | | | | | | Normalize bond options to ensure a correct connection matching upon daemon restart. https://bugzilla.redhat.com/show_bug.cgi?id=1457909
| * bond: add only supported options to the generated connectionBeniamino Galvani2017-06-051-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Upstream commit [1] changed in the kernel the default value of tlb_dynamic_lb bond from 1 to 0 when the mode is not tlb. This is not wrong, as the option value doesn't really matter for other modes, but it breaks the connection matching because we read back a 0 value when we expect a default of 1. Fix this in a generic way by ignoring altogether options that are not relevant for the current bond mode, because they are removed from the connection during normalization. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8b426dc54cf4056984bab7dfa48c92ee79a46434 https://bugzilla.redhat.com/show_bug.cgi?id=1457909
| * libnm-core: remove unsupported bond options during normalizationBeniamino Galvani2017-06-053-0/+98
|/ | | | | | | | | | | | | | | In an ideal world, we should not validate connections containing options not valid for the current bond mode. However adding such restriction now means that during an upgrade to the new NM version some connections that were valid before become invalid, possibly disrupting connectivity. Instead, consider invalid options as a normalizable error and remove them during normalization. Converting the setting to a "canonical" form without invalid options is important for the connection matching logic, where such invalid options can cause false mismatches.
* wifi: merge branch 'th/wifi-ap-hidden'Thomas Haller2017-06-051-13/+30
|\ | | | | | | https://github.com/NetworkManager/NetworkManager/pull/22
| * wifi: change logging about probe-scanning SSIDsth/wifi-ap-hiddenThomas Haller2017-06-031-1/+1
| | | | | | | | | | | | The SSID is not "hidden". It is the wildcard SSID. See build_hidden_probe_list().
| * wifi: fix completing Wi-Fi connection for AP modeThomas Haller2017-06-031-10/+23
| | | | | | | | | | | | | | | | | | | | | | In AP mode we should not look up an access point. It is wrong to do, and it ends up marking the connection as hidden. It seems wrong to me that if the client explicitly set hidden=FALSE before AddAndActivate(), that complete_connection() would still set it to TRUE if it cannot find the access point. That is, because complete_connection() does not know whether hidden was omitted or set intentionally by the user.
| * wifi: exclude AP mode wifi connection from hidden-scan listThomas Haller2017-06-031-2/+6
|/ | | | It makes no sense to scan for those.
* device: merge various device cleanupsThomas Haller2017-06-026-69/+72
|\
| * device: transform NM_DEVICE_IS_MASTER gobject property to field in NMDeviceClassThomas Haller2017-06-025-25/+19
| | | | | | | | | | | | We don't need this flexibility of having a full fledged GObject property for is-master. The property value only depends on the device's class.
| * core: add nm_device_spec_match_list_full()Thomas Haller2017-06-022-1/+18
| | | | | | | | | | This gives a third return value: whether the device did not match.
| * device: rename activate_stage5_ip4_config_commit() to ↵Thomas Haller2017-06-021-4/+4
| | | | | | | | | | | | | | | | | | activate_stage5_ip4_config_result() We have nm_device_activate_schedule_ip4_config_result(). The name should match. Note, this affects logging, as we log the function name.
| * device: prefix log messages related to carrierThomas Haller2017-06-021-5/+5
| | | | | | | | It's easier to search in the logfile.
| * device: minor cleanup of NMDeviceEthernet:get_link_speed()Thomas Haller2017-06-021-4/+3
| | | | | | | | | | | | | | | | | | A better name is link_speed_update(), because it re-reads and sets the speed value. Also, move _notfiy() after logging. It doesn't matter in this case, but we should first log, and then do actions that have potentially complex side-effects.
| * device: rework listening to carrier changes for DCB in NMDeviceEthernetThomas Haller2017-06-021-28/+21
| | | | | | | | | | | | Now, that NMDeviceClass:carrier_changed_notify() is no longer called as deferred action, we can check for DCB state there, instead or registering to the NM_DEVICE_CARRIER notifications.
| * device: move carrier_changed_notify() notification to nm_device_set_carrier()Thomas Haller2017-06-021-2/+2
|/ | | | | | | | | | | | | | | | | | | | | | | Note that: - carrier_changed_notify() has only one implementation: NMDeviceEthernet to call get_link_speed() when carrier comes back. - currently, calling carrier_changed_notify() with carrier=FALSE has no effect, because NMDeviceEthernet only acts on carrier=TRUE. - when carrier appears, nm_device_set_carrier() will call carrier_changed() right away. We only call carrier_changed() with carrier=TRUE only at one place. The change merley moves carrier_changed_notify() out of the function. Apart from that it has no effect. - when carrier disappears, previoulsy we would delay action for 4 seconds. Hence, we would delay carrier_changed_notify() as well -- although it has no effect. The last point is at least ugly. Fix it by moving carrier_changed_notify() to nm_device_set_carrier().
* clients: respond to the secret requests that we can't serviceLubomir Rintel2017-06-021-0/+9
| | | | | Otherwise the daemon would hang waiting for us while we respond with awkward silence. That is not a healthy kind of communication.
* settings: refactor nm_settings_connection_read_and_fill_timestamp()Thomas Haller2017-06-021-18/+20
| | | | | | | | | Coverity complains about not checking the return value: src/settings/nm-settings-connection.c:2329: check_return: Calling "g_key_file_load_from_file" without checking return value (as is done elsewhere 6 out of 7 times). While at it, refactor the code and check whether the timestamp is valid.
* tests: work around coverity false-positivesThomas Haller2017-06-021-9/+10
|
* supplicant: work-around coverify false-positive for check_return: ↵Thomas Haller2017-06-021-3/+5
| | | | | | g_async_initable_init_finish() NetworkManager-1.8.0/src/supplicant/nm-supplicant-interface.c:232: check_return: Calling "g_async_initable_init_finish" without checking return value (as is done elsewhere 7 out of 8 times).
* supplicant/tests: work-around coverify false-positive in testThomas Haller2017-06-021-1/+1
| | | | NetworkManager-1.8.0/src/supplicant/tests/test-supplicant-config.c:528: check_return: Calling "nm_setting_802_1x_set_ca_cert" without checking return value (as is done elsewhere 13 out of 16 times).
* connectivity: fix scheduling periodic connectivity checksThomas Haller2017-06-021-14/+15
| | | | | | | | | | | | | | | commit a955639 (connectivity: don't do periodic checks on interval=0) broke scheduling connectivity checks. That is because the timer is on only scheduled if nm_connectivity_check_enabled(), which in turn only returns TRUE if curl_mhandle is set. However, nm_connectivity_init() would only initialize curl_mhandle after update_config(), missing to schedule the periodic task. https://mail.gnome.org/archives/networkmanager-list/2017-May/msg00076.html Fixes: a95563996f07641e9877eb1760cac24415b65070
* all: replace uses of inet_aton() and friendsThomas Haller2017-06-024-45/+57
| | | | | | | | | | | | | | | | | | | | rpmdiff complains about uses of inet_aton, inet_makeaddr, inet_netof, inet_ntoa under the IPv6 section: usr/sbin/NetworkManager on aarch64 i686 x86_64 ppc ppc64 ppc64le s390 s390x uses function inet_aton, which may impact IPv6 support I think the warning is bogus, but refactor our code to avoid it. Note that systemd code still uses them, so it don't avoid the rpmdiff warning. But let's not diverge our systemd import from upstream for this. - for NMSettingBond:validate_ip() also avoid g_strsplit_set() which allocates a full strv. Instead, we can do with one g_strdup(). - for test-resolvconf-capture.c, replace the functions with macros. Macros should be avoided usually, but for test asserts they are more convenient as they preserved the __FILE__:__LINE__ of where the assertion fails.
* all: reject duplicate keys in JSONLubomir Rintel2017-06-012-4/+4
| | | | | | | | Teamd is not happy about them and would fail anyway. Worse even, if we json_loads() such a JSON, which is precisely what happens when we inject the "hwaddr" key, we turn bad JSON into a good one in a lossy matter. Not good. https://bugzilla.redhat.com/show_bug.cgi?id=1455130
* build: fix nm binutils tool when building with LTOThomas Haller2017-06-013-4/+3
| | | | | | | | | | | | | | | | | | | | | When building with -flto, we need to use linker plugins. In case of binutils' nm, it means to prefer gcc-nm if available. Like for ranlib and ar, prefer gcc-nm. - replace AC_PATH_TOOL() by AC_CHECK_TOOLS(). That is consistent with what we do for ar,ranlib and suggested on bgo#783311. - instead of using the variable $BINUTILS_NM, replace it by $NM, which is more common according to bgo#783311. - Keep recognizing $BINUTILS_NM environment, which was introduced by commit 8bc88bcc7c. This is purely to keep previous build scripts working. Originally I named it "$BINUTILS_NM" because using $NM in NetworkManager seemed confusing. But well... https://bugs.gentoo.org/show_bug.cgi?id=620052 https://bugzilla.gnome.org/show_bug.cgi?id=782525 https://bugzilla.gnome.org/show_bug.cgi?id=783311
* build: add a missing test artifact to distLubomir Rintel2017-06-011-0/+1
| | | | Fixes: ba05819c89d913ad1bc6b86e62c7704d173ef534
* merge: branch 'lr/bluetooth-nap'Lubomir Rintel2017-06-0140-174/+556
|\ | | | | | | https://bugzilla.gnome.org/show_bug.cgi?id=783013
| * bridge: move the Bluetooth NAP logic to bridge devicelr/bluetooth-napLubomir Rintel2017-06-015-64/+76
| | | | | | | | | | The Bluetooth NAP functionality seems only useful for the bridges. Move it away from NMDevice.
| * libnm: add _nm_connection_get_setting_bluetooth_for_nap()Thomas Haller2017-06-013-16/+18
| | | | | | | | | | | | If there is value in such a helper function (there is), then it should go alongside the other nm_connection_get_setting*() helpers. NMDevice is already large enough.
| * device: don't include header of bluetooth plugin in nm-device.hThomas Haller2017-06-013-10/+7
| | | | | | | | | | | | | | | | | | | | | | | | The plugins may use stuff from core, but not the other way around. Including "bluetooth/nm-bluez-common.h" is wrong. The UUID argument is always "nap" in the NAP case. We don't need the flexibility that it might be anything else. Just drop it. As far as NMDevice is concerned, it anyway wouldn't (or shouldn't know what the uuid is. It says register, and NMBluez5Manager should figure out the details.