summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
...
| * nm-setting-bond: if unset consider bond options with default values for ↵Antonio Cardace2020-03-062-20/+30
|/ | | | | | | | | | | | | | | | validation Doing 'verify()' with options such as 'miimon' and 'num_grat_arp' set to arbitrary values it's not consistent with what NetworkManager does to bond options when activating the bond through 'apply_bonding_config()' (at a later stage) because the said values do not correspond to what the default values for those options are. This leads to an inconsistency with the 'miimon' parameter for example, where 'verify()' is done while assuming it's 0 if not set but its default value is actually 100. Fixes: 8775c25c3318 ('libnm: verify bond option in defined order')
* ifcfg-rh: use binary search for converting string to ethtool IDThomas Haller2020-03-061-31/+87
| | | | | | | | | | | | | | | | | | | | | | Don't do a linear search through all names, but use binary search. Upside: calling nms_ifcfg_rh_utils_get_ethtool_by_name() in a loop (once over all 60 names) is 75% faster. Downside: when adding a new feature, we have yet another line that we need to add. Previously, adding a new feature required adding 7 lines, not it is 8. But we didn't add a single feature since this was added, so that happens very seldom. Possible downside: is this code harder to read? Now we track both how to convert the ID to name and back. This is redundant (and thus harder to maintain). But it's really just one extra line per feature, for which there is a unit test. So, when adding a new NMEthtoolID it would be pretty hard to mess this up, because of all the tests and assertions. So, maybe it's slightly harder to read. On the other hand, it unifies handling for ethtool and kernel names, and the code has less logic and is more descriptive. I don't think this is actually harder to maintain and it should be easy to see that it is correct (readability).
* ifcfg-rh/tests: add test for consistency of ethtool ifcfg namesThomas Haller2020-03-061-0/+57
|
* shared: expose size of nm_ethtool_data array in headerThomas Haller2020-03-061-1/+1
|
* man: show example for `nmcli --ask device wifi connect` in nmcli-example manualThomas Haller2020-03-061-1/+2
|
* man: show example for `nmcli device wifi connect` in nmcli-example manualAuke Kok2020-03-061-0/+4
| | | | | | | | | | | | | Add the only important example that this file should have All other examples are nice. But when you install a console-only machine and read the NM man pages, you are none the wiser, because something as simple like this isn't covered in the man pages. I've seen other users complain about it, and I've torn my hair out over this several times. [thaller@redhat.com: changed subject line of patch]
* core: merge branch 'th/prune-device-state-files'Thomas Haller2020-03-044-33/+70
|\ | | | | | | | | | | https://bugzilla.redhat.com/show_bug.cgi?id=1810153 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/429
| * core: periodically cleanup unused device state files from /runThomas Haller2020-03-041-1/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Otherwise, we only prune unused files when the service terminates. Usually, NetworkManager service doesn't get restarted before shutdown of the system (nor should it be). That means, if you create (and destroy) a large number of software devices, the state files pile up. From time to time, go through the files on disk and delete those that are no longer relevant. In this case, "from time to time" means after we write/update state files 100 times.
| * core: return ifindex from nm_manager_write_device_state()Thomas Haller2020-03-042-14/+22
| | | | | | | | | | | | | | nm_manager_write_device_state() writes the device state to a file. The ifindex is here important, because that is the identifier for the device and is also used as file name. Return the ifindex that was used, instead of letting the caller reimplement the knowledge which ifindex was used.
| * core/trivial: rename nm_config_device_state_prune_unseen() to ↵Thomas Haller2020-03-043-5/+5
| | | | | | | | | | | | nm_config_device_state_prune_stale() It's just a more fitting name after the previous change.
| * core: cleanup nm_config_device_state_prune_unseen() and accept NMPlatform ↵Thomas Haller2020-03-043-18/+29
|/ | | | for skipping pruning
* dns: cleanup update_dns() for returning errorThomas Haller2020-03-041-18/+33
| | | | | | | It was not very clear whether update_dns() would always set the error output variable if (and only if) it would return false. Rework the code to make it clearer.
* dns: use gs_free_error for clearing error from update_dns()Thomas Haller2020-03-041-22/+20
| | | | | | | | | | | | | | Not using cleanup attribute is error prone. In theory, a function should only return a GError if (and only if) it signals a failure. However, for example in commit 324f67956aee ('dns: ensure to log a warning when writing /etc/resolv.conf fails') due to a bug we was violated. In that case, it resulted in a leak. Avoid explicit frees and use the gs_free_error cleanup attribute instead. That would also work correctly in face of such a bug and in general it seems preferable to explicitly assign ownership to auto variables on the stack.
* dns: ensure to log a warning when writing /etc/resolv.conf failsThomas Haller2020-03-041-4/+6
| | | | | | | | | | | | | | | | | | | When setting "main.rc-manager=symlink" (the default) and /etc/resolv.conf is a file, NetworkManager tries to write the file directly. When that fails, we need to make sure to propagate the error so that we log a warning about that. With this change: <debug> [1583320004.3122] dns-mgr: update-dns: updating plugin systemd-resolved <trace> [1583320004.3123] dns-sd-resolved[f9e3febb7424575d]: send-updates: start 8 requests <trace> [1583320004.3129] dns-mgr: update-resolv-no-stub: '/var/run/NetworkManager/no-stub-resolv.conf' successfully written <trace> [1583320004.3130] dns-mgr: update-resolv-conf: write to /etc/resolv.conf failed (rc-manager=symlink, $ERROR_REASON) <trace> [1583320004.3132] dns-mgr: update-resolv-conf: write internal file /var/run/NetworkManager/resolv.conf succeeded <trace> [1583320004.3133] dns-mgr: current configuration: [{ [...] }] <warn> [1583320004.3133] dns-mgr: could not commit DNS changes: $ERROR_REASON <info> [1583320004.3134] device (eth0): Activation: successful, device activated. https://bugzilla.redhat.com/show_bug.cgi?id=1809181
* dhcp: clean source on dispatch failure (fix leak)Thomas Haller2020-03-031-1/+1
| | | | | | | The GSource must also be unrefed. Also, first clear the field before invoking callbacks to the upper layers. Fixes: 843d696e46d9 ('dhcp: clean source on dispatch failure')
* dhcp: clean source on dispatch failureBeniamino Galvani2020-03-031-0/+1
| | | | | | | | | | | | | | | | | | Fix the following warning: NetworkManager[1524461]: Source ID 3844 was not found when attempting to remove it g_logv (log_domain=0x7f2816fa676e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7ffe697374d0) at gmessages.c:1391 g_log (log_domain=log_domain@entry=0x7f2816fa676e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7f2816fae240 "Source ID %u was not found when attempting to remove it") at gmessages.c:1432 g_source_remove (tag=519) at gmain.c:2352 nm_clear_g_source (id=<optimized out>) at ./shared/nm-glib-aux/nm-macros-internal.h:1198 dispose (object=0x55f7289b1ca0) at src/dhcp/nm-dhcp-nettools.c:1433 g_object_unref (_object=<optimized out>) at gobject.c:3303 g_object_unref (_object=0x55f7289b1ca0) at gobject.c:3232 dhcp4_cleanup (self=self@entry=0x55f728af3b20, cleanup_type=cleanup_type@entry=CLEANUP_TYPE_DECONFIGURE, release=release@entry=0) at src/devices/nm-device.c:7565 ... Fixes: 45521b1b3872 ('dhcp: nettools: move to failed state if event dispatch fails')
* Revert "dispatcher/systemd: order NetworkManager-dispatcher.service ↵Beniamino Galvani2020-03-021-6/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before=NetworkManager.service" The 'Before' dependency between NM-dispatcher and NM causes a deadlock when stopping the NM service. When terminating, NM wants to D-Bus activate NM-dispatcher to synchronously handle pre-down events; but NM-dispatcher start is ordered after NM shutdown due to the following behavior described in systemd.unit(5) man page: Given two units with any ordering dependency between them, if one unit is shut down and the other is started up, the shutdown is ordered before the start-up. It doesn't matter if the ordering dependency is After= or Before=, in this case. It also doesn't matter which of the two is shut down, as long as one is shut down and the other is started up; the shutdown is ordered before the start-up in all cases. So, NM is waiting NM-dispatcher to start and NM-dispatcher is queued by systemd, waiting that NM is stopped. The result is a 90 seconds delay, after which systemd kills NM and continues. The dependency was added so that during shutdown NM-dispatcher would be stopped after NM. I don't think it worked as expected because NM-dispatcher is not supposed to be active most of the times, and so it doesn't need a dependency that delays its stop after NM. This reverts commit acc335aad4c310fef3760d43b2fb137e5206615c.
* shared: move assertion in _NM_UTILS_STRING_TABLE_LOOKUP_DEFINE()Thomas Haller2020-03-021-2/+2
| | | | | | | Move the assertion for valid LIST first. It only checks static data, and regardless of the entry_cmd, it should be done first. Fixes: f4d12f7b59b6 ('shared: add NM_UTILS_STRING_TABLE_LOOKUP_STRUCT_DEFINE() macro for lookup of structs')
* shared: fix handling %NULL argument in nm_ref_string_equals_str()Thomas Haller2020-03-021-1/+1
| | | | Fixes: 190a8ed425c1 ('shared: add nm_ref_string_equals_str() helper')
* license: add Tambet to RELICENSE.mdThomas Haller2020-03-021-0/+1
| | | | https://mail.gnome.org/archives/networkmanager-list/2020-March/msg00000.html
* nm-setting-bond: fix '[up|down]delay', 'miimon' validationAntonio Cardace2020-02-282-5/+29
| | | | | | | | | | | | | | | | | | Just looking at the hashtable entry of 'updelay' and 'downdelay' options is wrong, we have to inspect their values to check if they're actually enabled or not. Otherwise bond connections with valid settings will fail when created: $ nmcli c add type bond ifname bond99 bond.options miimon=0,updelay=0,mode=0 Error: Failed to add 'bond-bond99' connection: bond.options: 'updelay' option requires 'miimon' option to be set Also add unit tests. https://bugzilla.redhat.com/show_bug.cgi?id=1805184 Fixes: d595f7843e31 ('libnm: add libnm/libnm-core (part 1)')
* license: add Pavel to RELICENSE.mdThomas Haller2020-02-281-0/+1
| | | | https://mail.gnome.org/archives/networkmanager-list/2020-February/msg00040.html
* device/trivial: move code aroundThomas Haller2020-02-261-288/+292
| | | | | Or patterns is to have the property get/set functions before the object's create/destroy code. Move it.
* device: merge branch 'th/device-cleanup-properties'Thomas Haller2020-02-261-39/+27
|\
| * device/trivial: rename property enums for statistics properties of NMDeviceThomas Haller2020-02-261-13/+13
| | | | | | | | | | The name of the property name should resemble the define for the name.
| * device/trivial: add comment about NMDevice properties writable from D-BusThomas Haller2020-02-261-3/+6
| | | | | | | | | | These are special. Their setter gets called via D-Bus' SetProperty. Mark them with a comment.
| * device: don't make NM_DEVICE_(IP|DHCP)(4|6)_CONFIG properties writableThomas Haller2020-02-261-4/+4
| | | | | | | | It's not necessary, nor used, nor actually implemented.
| * device: don't make NM_DEVICE_DRIVER_VERSION property writableThomas Haller2020-02-261-5/+1
| |
| * device: don't make NM_DEVICE_FIRMWARE_VERSION property writableThomas Haller2020-02-261-5/+1
| |
| * device: don't make NM_DEVICE_FIRMWARE_MISSING property writableThomas Haller2020-02-261-5/+1
| | | | | | | | It's not necessary nor used.
| * device: don't make NM_DEVICE_IP4_ADDRESS property writableThomas Haller2020-02-261-4/+1
|/ | | | It's not necessary nor used.
* libnm,all: merge branch 'th/libnm-interface-name-cleanup'Thomas Haller2020-02-2611-200/+184
|\ | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/416
| * libnm: validate "connection.interface-name" at one place onlyThomas Haller2020-02-262-46/+48
| | | | | | | | | | | | | | | | | | | | | | Don't spread the validation for the interface name between multiple places. There should be one place only, so when you search for how this property gets verified, you can find the single place. That requires to move the special handling for OVS interfaces to NMSettingConnection. Since we already have _nm_setting_ovs_interface_verify_interface_type(), that is easy.
| * shared: add NMU_IFACE_OVS_OR_KERNEL for nm_utils_ifname_valid()Thomas Haller2020-02-262-0/+4
| | | | | | | | | | | | | | | | | | | | Depending on the type, OVS interfaces also have a corresponding netdev in kernel (e.g. type "internal" does, type "patch" does not). Such a case is neither NMU_IFACE_OVS nor NMU_IFACE_KERNEL (alone). There should be a special type to represent those cases. Add NMU_IFACE_OVS_OR_KERNEL for that.
| * shared: add NMU_IFACE_ANY for nm_utils_ifname_valid()Thomas Haller2020-02-262-1/+13
| | | | | | | | | | | | | | | | | | | | | | nm_utils_ifname_valid() is to validate "connection.interface-name" property. But the exact validation depends on the connection type. Add "NMU_IFACE_ANY" to validate the name to check whether it would be valid for any connection type. This is for completeness and for places where the caller might not know the connection type.
| * libnm: always return normalized-type from ↵Thomas Haller2020-02-262-10/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | _nm_setting_ovs_interface_verify_interface_type() We should return the chosen type whenever we can verify the setting. Previously, the normalized-type output argument was only set when normalization was actually necessary. On most cases, the caller cares whether the setting verifies and which interface type is chosen. It's much less likely that a caller cares only about the normalized-type if normalization is actually necessary. Whenever we return TRUE (indicating that the setting is valid), also return the chosen interface-type.
| * libnm: allow _nm_setting_ovs_interface_verify_interface_type() without ↵Thomas Haller2020-02-263-5/+8
| | | | | | | | | | | | | | | | | | | | | | | | NMSettingOvsInterface instance _nm_setting_ovs_interface_verify_interface_type() does verify and normalize both. Especially for verify, it's useful to run the operation without having a NMSettingOvsInterface instance, because we might want to know how normalization would react, if we had a NMSettingOvsInterface instance. Allow for that.
| * ifcfg-rh: use nm_utils_ifname_valid() for validating interface-name in readerThomas Haller2020-02-261-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Maybe the reader should not try to add its own validation. It could just read the value, set it in the profile, and let nm_connection_verify() handle it. However: - in this form the code only logs a warning about invalid setting. If we let it come to nm_connection_verify(), the connection profile will be entirely rejected. I think this makes sense, because ifcfg files may be edited by the user and we don't know what is out there. - it's nicer to show a warning that specifically mentions the DEVICE= variable. There error message we get from nm_connection_verify() is no longer aware of ifcfg peculiarities. Instead: use the appropriate validation function.
| * libnm: remove redundant check from "nm-setting-bond.c"'s validate_ifname()Thomas Haller2020-02-261-3/+0
| |
| * settings: simplify property setter from GVariant to ↵Thomas Haller2020-02-262-30/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | NMSettingConnection:interface-name The interface-name property has several deprecated aliases, like "bridge.interface-name". For backward compatibility, we keep handling them. In particular, the "missing_from_dbus_fcn" handler is set. This handles the case where GVariant only contains the deprecated form, but not "connection.interface-name". Previously, from_dbus_fcn() would check whether the deprecated form was present, and -- only if that form was invalid -- prefer it. The idea was to fail validation if the deprecated property was invalid. I think that is not necessary. Just completely ignore the deprecated property, if the new property is present. What might make sense is to check whether the deprecated and the new form are both present, that they are identical. However, I don't think that is worth the effort.
| * clients: use nm_utils_ifname_valid() to validate interface name in ↵Thomas Haller2020-02-261-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | nm_vpn_wireguard_import() We use the filename of the imported .conf file for "connection.interface-name". That follows what `wg-quick` does. However, we also validate that the interface name is valid UTF-8 (otherwise -- as it currently is -- the setting couldn't be send via D-Bus). As such, we have stricter requirements. We want to fail early and tell the user when the filename is unsuitable. Failing later gives a worse user experience, because the failure message about invalid "connection.interface-name" wouldn't make it clear that the filename is wrong. Use the appropriate function to validate "connection.interface-name". Before: $ touch $'./a\344b.conf' $ nmcli connection import type wireguard file $'./a\344b.conf' Error: failed to import './a?b.conf': Failed to create WireGuard connection: connection.interface-name: 'a?b': interface name must be UTF-8 encoded. Now: $ nmcli connection import type wireguard file $'./a\344b.conf' Error: failed to import './a?b.conf': The name of the WireGuard config must be a valid interface name followed by ".conf".
| * libnm: don't validate "connection.interface-name" from ↵Thomas Haller2020-02-261-32/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | "nm-setting-infiniband.c"'s verify() There should not be multiple places to validate the interface-name. The check in "nm-setting-infiniband.c" is unnecessary and wrong. It's unnecessary, because _nm_connection_verify() takes care to first verify the NMSettingConnection instance. It's wrong, because it does not check the property the same way as NMSettingConnection does (e.g. it does not check for valid UTF-8).
| * libnm: in find_virtual_interface_name() ensure return value stays aliveThomas Haller2020-02-261-4/+10
| | | | | | | | | | | | It's not clear that the returned string is still valid after we unref the GVariant that contains it. Also return the reference to the variant.
| * libnm: validate settings in _nm_connection_verify() in defined orderThomas Haller2020-02-261-29/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Fully sort the settings in _nm_connection_verify(). Previously, only the NMSettingConnection instance was sorted first (as required). The remaining settings were in undefined order. That means, we would validate settings in undefined order, and if multiple settings have an issue, the reported error would be undefined. Instead, use nm_connection_get_settings() which fully sorts the settings (and of course, sorts NMSettingConnection first as we require it). Also, this way we no longer need to allocate multiple GSList instances but only malloc() one array large enough to contain all settings.
| * libnm: use nm_utils_hash_values_to_array() to implement ↵Thomas Haller2020-02-261-38/+17
| | | | | | | | nm_connection_get_settings()
| * shared: reject reserved names from "connection.interface-name"Thomas Haller2020-02-261-0/+13
| | | | | | | | | | | | | | | | | | "all" and "default" never works. "bonding_masters" works if you unload the bonding module. Well, that should not really be called working... Reject these names.
| * shared: reject '%' from nm_utils_ifname_valid() for kernel namesThomas Haller2020-02-261-2/+28
|/ | | | | | | | | | | | | | Generally, it's dangerous to reject values that were accepted previously. This will lead to NetworkManager being unable to load a profile from disk, which was loadable previously. On the other hand, kernel would not have treated this setting as it was intended. So, I would argue that the such a setting was not working (as intended) anyway. We can only hope that users don't configure arbitrary interface names. It generally isn't a good idea to do, so "breaking" such things is less of a concern.
* supplicant: allocate blobs hash table lazily for supplicant configThomas Haller2020-02-262-21/+25
| | | | | | | | | It's very unlikely that we have actual blobs for a Wi-Fi network. That is because the settings plugins (keyfile, ifcfg-rh) convert blobs to files on disk when writing the profile. So, you can only have them by editing the files directly to contain blobs. At that point, don't always create the GHashTable for blobs.
* license: add Sigfox to RELICENSE.mdThomas Haller2020-02-241-0/+4
| | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/365
* ovs: merge branch 'bg/ovs-no-bridge-rh1797696'Beniamino Galvani2020-02-241-2/+11
|\ | | | | | | | | https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/422 https://bugzilla.redhat.com/show_bug.cgi?id=1797696
View Lorry Controller Status