From 19b7a4c167c506d72df407495d5a3da21d47a786 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 3 Dec 2018 17:51:43 +0100 Subject: [PATCH 01/11] udev: increase receive buffer size With the default 128KiB buffer size it is easy to lose events. For example when 64 interfaces appear at the same time, we lose events for the last 16. Increase the buffer size to 4MiB. https://bugzilla.redhat.com/show_bug.cgi?id=1651578 (cherry picked from commit 5780a2893b2adc1a3cafd45dcf2d2c09bf7e15c3) --- shared/nm-utils/nm-udev-utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/shared/nm-utils/nm-udev-utils.c b/shared/nm-utils/nm-udev-utils.c index 709f759043..397d575b91 100644 --- a/shared/nm-utils/nm-udev-utils.c +++ b/shared/nm-utils/nm-udev-utils.c @@ -253,6 +253,7 @@ nm_udev_client_new (const char *const*subsystems, /* listen to events, and buffer them */ if (self->monitor) { + udev_monitor_set_receive_buffer_size (self->monitor, 4*1024*1024); udev_monitor_enable_receiving (self->monitor); channel = g_io_channel_unix_new (udev_monitor_get_fd (self->monitor)); self->watch_source = g_io_create_watch (channel, G_IO_IN); From 67994f0e6f0c1250661448adb7d54a6fcd448199 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 3 Dec 2018 18:23:06 +0100 Subject: [PATCH 02/11] udev: remove unneeded NULL checks self->monitor cannot be NULL there. (cherry picked from commit 3e4b30b2b180b069dcd20520192d93bb05c96d4b) --- shared/nm-utils/nm-udev-utils.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/shared/nm-utils/nm-udev-utils.c b/shared/nm-utils/nm-udev-utils.c index 397d575b91..5d0919b302 100644 --- a/shared/nm-utils/nm-udev-utils.c +++ b/shared/nm-utils/nm-udev-utils.c @@ -241,27 +241,23 @@ nm_udev_client_new (const char *const*subsystems, if (self->subsystems) { /* install subsystem filters to only wake up for certain events */ for (n = 0; self->subsystems[n]; n++) { - if (self->monitor) { - gs_free char *to_free = NULL; - const char *subsystem; - const char *devtype; + gs_free char *to_free = NULL; + const char *subsystem; + const char *devtype; - _subsystem_split (self->subsystems[n], &subsystem, &devtype, &to_free); - udev_monitor_filter_add_match_subsystem_devtype (self->monitor, subsystem, devtype); - } + _subsystem_split (self->subsystems[n], &subsystem, &devtype, &to_free); + udev_monitor_filter_add_match_subsystem_devtype (self->monitor, subsystem, devtype); } /* listen to events, and buffer them */ - if (self->monitor) { - udev_monitor_set_receive_buffer_size (self->monitor, 4*1024*1024); - udev_monitor_enable_receiving (self->monitor); - channel = g_io_channel_unix_new (udev_monitor_get_fd (self->monitor)); - self->watch_source = g_io_create_watch (channel, G_IO_IN); - g_io_channel_unref (channel); - g_source_set_callback (self->watch_source, (GSourceFunc)(void (*) (void)) monitor_event, self, NULL); - g_source_attach (self->watch_source, g_main_context_get_thread_default ()); - g_source_unref (self->watch_source); - } + udev_monitor_set_receive_buffer_size (self->monitor, 4*1024*1024); + udev_monitor_enable_receiving (self->monitor); + channel = g_io_channel_unix_new (udev_monitor_get_fd (self->monitor)); + self->watch_source = g_io_create_watch (channel, G_IO_IN); + g_io_channel_unref (channel); + g_source_set_callback (self->watch_source, (GSourceFunc)(void (*) (void)) monitor_event, self, NULL); + g_source_attach (self->watch_source, g_main_context_get_thread_default ()); + g_source_unref (self->watch_source); } } From 082bb02fa015ab2f0dbf082787f6c949a18c74ce Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 1 Dec 2018 17:44:37 +0100 Subject: [PATCH 03/11] cli: strictly validate SR-IOV attributes Report an error when the user tries to add an unknown attribute instead of silently accepting (and ignoring) it. Note that this commit also changes the behavior of public API nm_utils_sriov_vf_from_str() to return an error when an unknown attribute is found. I think the previous behavior was buggy as wrong attributes were simply ignored without any way for the user to know. Fixes: a9b4532fa77d75f2dd40cbbd2a5184df6ec0d387 (cherry picked from commit 769e0726a81bb6a7c9250527ee6bba205e2f23ab) --- libnm-core/nm-core-internal.h | 2 +- libnm-core/nm-keyfile.c | 2 +- libnm-core/nm-utils.c | 14 +++++++++++--- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index b5e82c3a6d..7dc57e502d 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -252,7 +252,7 @@ gboolean _nm_ip_route_attribute_validate_all (const NMIPRoute *route); const char **_nm_ip_route_get_attribute_names (const NMIPRoute *route, gboolean sorted, guint *out_length); GHashTable *_nm_ip_route_get_attributes_direct (NMIPRoute *route); -NMSriovVF *_nm_utils_sriov_vf_from_strparts (const char *index, const char *detail, GError **error); +NMSriovVF *_nm_utils_sriov_vf_from_strparts (const char *index, const char *detail, gboolean ignore_unknown, GError **error); gboolean _nm_sriov_vf_attribute_validate_all (const NMSriovVF *vf, GError **error); static inline void diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index e4ed1899f9..5aa8a4ac71 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -152,7 +152,7 @@ sriov_vfs_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key) keys[i], NULL); - vf = _nm_utils_sriov_vf_from_strparts (rest, value, NULL); + vf = _nm_utils_sriov_vf_from_strparts (rest, value, TRUE, NULL); if (vf) g_ptr_array_add (vfs, vf); } diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 3f8a322acf..f75276c784 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -2737,11 +2737,14 @@ nm_utils_sriov_vf_from_str (const char *str, GError **error) detail++; } - return _nm_utils_sriov_vf_from_strparts (str, detail, error); + return _nm_utils_sriov_vf_from_strparts (str, detail, FALSE, error); } NMSriovVF * -_nm_utils_sriov_vf_from_strparts (const char *index, const char *detail, GError **error) +_nm_utils_sriov_vf_from_strparts (const char *index, + const char *detail, + gboolean ignore_unknown, + GError **error) { NMSriovVF *vf; guint32 n_index; @@ -2761,7 +2764,12 @@ _nm_utils_sriov_vf_from_strparts (const char *index, const char *detail, GError vf = nm_sriov_vf_new (n_index); if (detail) { - ht = nm_utils_parse_variant_attributes (detail, ' ', '=', TRUE, _nm_sriov_vf_attribute_spec, error); + ht = nm_utils_parse_variant_attributes (detail, + ' ', + '=', + ignore_unknown, + _nm_sriov_vf_attribute_spec, + error); if (!ht) { nm_sriov_vf_unref (vf); return NULL; diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 70fffb8882..6e83182cce 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -2261,7 +2261,7 @@ make_sriov_setting (shvarFile *ifcfg) key += NM_STRLEN ("SRIOV_VF"); - vf = _nm_utils_sriov_vf_from_strparts (key, value, &error); + vf = _nm_utils_sriov_vf_from_strparts (key, value, TRUE, &error); if (!vf) { PARSE_WARNING ("ignoring invalid SR-IOV VF '%s %s': %s", key, value, error->message); From 20f38e09ec5ced176c531603f3288dfd7d330d06 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 1 Dec 2018 17:53:22 +0100 Subject: [PATCH 04/11] libnm-core: slightly improve SR-IOV documentation Describe how to specify multiple VFs and which attributes are supported, so that this information is available in the nm-settings manual page. Also, clarify that SR-IOV parameters are managed only when the setting is present. https://bugzilla.redhat.com/show_bug.cgi?id=1651979 (cherry picked from commit 3de25bbc977fa935a77aebf3f8f7d9e9dfaa55b8) --- clients/common/settings-docs.h.in | 4 ++-- libnm-core/nm-setting-sriov.c | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 880353fad6..bc56edae07 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -296,8 +296,8 @@ #define DESCRIBE_DOC_NM_SETTING_SERIAL_SEND_DELAY N_("Time to delay between each byte sent to the modem, in microseconds.") #define DESCRIBE_DOC_NM_SETTING_SERIAL_STOPBITS N_("Number of stop bits for communication on the serial port. Either 1 or 2. The 1 in \"8n1\" for example.") #define DESCRIBE_DOC_NM_SETTING_SRIOV_AUTOPROBE_DRIVERS N_("Whether to autoprobe virtual functions by a compatible driver. If set to NM_TERNARY_TRUE (1), the kernel will try to bind VFs to a compatible driver and if this succeeds a new network interface will be instantiated for each VF. If set to NM_TERNARY_FALSE (0), VFs will not be claimed and no network interfaces will be created for them. When set to NM_TERNARY_DEFAULT (-1), the global default is used; in case the global default is unspecified it is assumed to be NM_TERNARY_TRUE (1).") -#define DESCRIBE_DOC_NM_SETTING_SRIOV_TOTAL_VFS N_("The total number of virtual functions to create.") -#define DESCRIBE_DOC_NM_SETTING_SRIOV_VFS N_("Array of virtual function descriptors. Each VF descriptor is a dictionary mapping attribute names to GVariant values. The 'index' entry is mandatory for each VF. When represented as string a VF is in the form: \"INDEX [ATTR=VALUE[ ATTR=VALUE]...]\". for example: \"2 mac=00:11:22:33:44:55 spoof-check=true\". The \"vlans\" attribute is represented as a semicolor-separated list of VLAN descriptors, where each descriptor has the form \"ID[.PRIORITY[.PROTO]]\". PROTO can be either 'q' for 802.1Q (the default) or 'ad' for 802.1ad.") +#define DESCRIBE_DOC_NM_SETTING_SRIOV_TOTAL_VFS N_("The total number of virtual functions to create. Note that when the sriov setting is present NetworkManager enforces the number of virtual functions on the interface also when it is zero. To prevent any changes to SR-IOV parameters don't add a sriov setting to the connection.") +#define DESCRIBE_DOC_NM_SETTING_SRIOV_VFS N_("Array of virtual function descriptors. Each VF descriptor is a dictionary mapping attribute names to GVariant values. The 'index' entry is mandatory for each VF. When represented as string a VF is in the form: \"INDEX [ATTR=VALUE[ ATTR=VALUE]...]\". for example: \"2 mac=00:11:22:33:44:55 spoof-check=true\". Multiple VFs can be specified using a comma as separator. Currently the following attributes are supported: mac, spoof-check, trust, min-tx-rate, max-tx-rate, vlans. The \"vlans\" attribute is represented as a semicolon-separated list of VLAN descriptors, where each descriptor has the form \"ID[.PRIORITY[.PROTO]]\". PROTO can be either 'q' for 802.1Q (the default) or 'ad' for 802.1ad.") #define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_QDISCS N_("Array of TC queueing disciplines.") #define DESCRIBE_DOC_NM_SETTING_TC_CONFIG_TFILTERS N_("Array of TC traffic filters.") #define DESCRIBE_DOC_NM_SETTING_TEAM_CONFIG N_("The JSON configuration for the team network interface. The property should contain raw JSON configuration data suitable for teamd, because the value is passed directly to teamd. If not specified, the default configuration is used. See man teamd.conf for the format details.") diff --git a/libnm-core/nm-setting-sriov.c b/libnm-core/nm-setting-sriov.c index 7228fb0cc2..0addd972db 100644 --- a/libnm-core/nm-setting-sriov.c +++ b/libnm-core/nm-setting-sriov.c @@ -1248,6 +1248,11 @@ nm_setting_sriov_class_init (NMSettingSriovClass *klass) * * The total number of virtual functions to create. * + * Note that when the sriov setting is present NetworkManager + * enforces the number of virtual functions on the interface + * also when it is zero. To prevent any changes to SR-IOV + * parameters don't add a sriov setting to the connection. + * * Since: 1.14 **/ /* ---ifcfg-rh--- @@ -1283,7 +1288,11 @@ nm_setting_sriov_class_init (NMSettingSriovClass *klass) * * "2 mac=00:11:22:33:44:55 spoof-check=true". * - * The "vlans" attribute is represented as a semicolor-separated + * Multiple VFs can be specified using a comma as separator. + * Currently the following attributes are supported: mac, + * spoof-check, trust, min-tx-rate, max-tx-rate, vlans. + * + * The "vlans" attribute is represented as a semicolon-separated * list of VLAN descriptors, where each descriptor has the form * * "ID[.PRIORITY[.PROTO]]". @@ -1291,6 +1300,7 @@ nm_setting_sriov_class_init (NMSettingSriovClass *klass) * PROTO can be either 'q' for 802.1Q (the default) or 'ad' for * 802.1ad. * + * Since: 1.14 **/ /* ---ifcfg-rh--- From ca45433f4fdcfdde9767aab7c8b4ff429855c9cb Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 4 Dec 2018 11:18:01 +0100 Subject: [PATCH 05/11] device: configure static number of VFs in unavailable state Don't configure the static number of VFs when the device is realized because the device could still be unmanaged. Instead, do it when the device becomes managed. (cherry picked from commit 75024e11b395e5d4910efc407be09e76b6c6ff7c) --- src/devices/nm-device.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 996b8afff9..806a794a98 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4303,8 +4303,6 @@ realize_start_setup (NMDevice *self, nm_device_set_carrier_from_platform (self); - device_init_static_sriov_num_vfs (self); - nm_assert (!priv->stats.timeout_id); real_rate = _stats_refresh_rate_real (priv->stats.refresh_rate_ms); if (real_rate) @@ -14675,6 +14673,7 @@ _set_state_full (NMDevice *self, save_ip6_properties (self); if (priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED) ip6_managed_setup (self); + device_init_static_sriov_num_vfs (self); } if (priv->sys_iface_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED) { From 6ae1f64351f326a18c9654b4b8cf5c0dfedb0951 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 4 Dec 2018 11:36:21 +0100 Subject: [PATCH 06/11] device: reset SR-IOV VFs on deactivation If the connection has a sriov setting we configure SR-IOV VFs on activation. We should also clear resources when the connection deactivates. (cherry picked from commit 529533a50c01fec36e91134c1c44c6b00ef25fd4) --- src/devices/nm-device.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 806a794a98..3527794171 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -14566,6 +14566,7 @@ _set_state_full (NMDevice *self, NMActRequest *req; gboolean no_firmware = FALSE; NMSettingsConnection *sett_conn; + NMSettingSriov *s_sriov; g_return_if_fail (NM_IS_DEVICE (self)); @@ -14762,6 +14763,12 @@ _set_state_full (NMDevice *self, } break; case NM_DEVICE_STATE_DEACTIVATING: + if ( (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV)) + && priv->ifindex > 0) { + nm_platform_link_set_sriov_params (nm_device_get_platform (self), + priv->ifindex, 0, 1); + } + _cancel_activation (self); /* We cache the ignore_carrier state to not react on config-reloads while the connection From 9deca176f86553d08ceee3a4ccd2ae0b8a18c565 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 12 Dec 2018 10:08:28 +0100 Subject: [PATCH 07/11] ifcfg-rh: fix persisting sriov setting The writer should write all properties of the sriov setting when the setting exists without additional logic. Likewise, the reader should instantiate a sriov setting when any sriov key is present and blindly set properties from keys. The old code did not always preserve the presence of a sriov setting after a write/read cycle. Fixes: c02d1c488f69ed6183cb86c80a771c902ea5e397 (cherry picked from commit d48f389cbf6a24f6e58f1422e5b5a8a8d649c973) --- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 25 +++++++++++++------ .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 9 +++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 6e83182cce..5dec474f36 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -2226,20 +2226,19 @@ make_sriov_setting (shvarFile *ifcfg) { gs_unref_hashtable GHashTable *keys = NULL; gs_unref_ptrarray GPtrArray *vfs = NULL; - NMTernary autoprobe_drivers; + int autoprobe_drivers; NMSettingSriov *s_sriov; - int total_vfs; + gint64 total_vfs; - total_vfs = svGetValueInt64 (ifcfg, "SRIOV_TOTAL_VFS", 10, 0, G_MAXINT32, 0); - if (!total_vfs) - return NULL; + + total_vfs = svGetValueInt64 (ifcfg, "SRIOV_TOTAL_VFS", 10, 0, G_MAXUINT32, -1); autoprobe_drivers = svGetValueInt64 (ifcfg, "SRIOV_AUTOPROBE_DRIVERS", 10, - NM_TERNARY_FALSE, + NM_TERNARY_DEFAULT, NM_TERNARY_TRUE, - NM_TERNARY_DEFAULT); + -2); keys = svGetKeys (ifcfg, SV_KEY_TYPE_SRIOV_VF); if (keys) { @@ -2273,11 +2272,21 @@ make_sriov_setting (shvarFile *ifcfg) } } + /* Create the setting when at least one key is set */ + if ( total_vfs < 0 + && !vfs + && autoprobe_drivers < NM_TERNARY_DEFAULT) + return NULL; + s_sriov = (NMSettingSriov *) nm_setting_sriov_new (); + + autoprobe_drivers = NM_MAX (autoprobe_drivers, NM_TERNARY_DEFAULT); + total_vfs = NM_MAX (total_vfs, 0); + g_object_set (s_sriov, NM_SETTING_SRIOV_TOTAL_VFS, total_vfs, NM_SETTING_SRIOV_VFS, vfs, - NM_SETTING_SRIOV_AUTOPROBE_DRIVERS, (int) autoprobe_drivers, + NM_SETTING_SRIOV_AUTOPROBE_DRIVERS, autoprobe_drivers, NULL); return (NMSetting *) s_sriov; diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index b70690cc9f..f5be7520ea 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -2223,16 +2223,15 @@ write_sriov_setting (NMConnection *connection, shvarFile *ifcfg) svUnsetAll (ifcfg, SV_KEY_TYPE_SRIOV_VF); - s_sriov = NM_SETTING_SRIOV (nm_connection_get_setting (connection, NM_TYPE_SETTING_SRIOV)); - if (s_sriov) - num = nm_setting_sriov_get_total_vfs (s_sriov); - if (num == 0) { + s_sriov = NM_SETTING_SRIOV (nm_connection_get_setting (connection, + NM_TYPE_SETTING_SRIOV)); + if (!s_sriov) { svUnsetValue (ifcfg, "SRIOV_TOTAL_VFS"); svUnsetValue (ifcfg, "SRIOV_AUTOPROBE_DRIVERS"); return; } - svSetValueInt64 (ifcfg, "SRIOV_TOTAL_VFS", num); + svSetValueInt64 (ifcfg, "SRIOV_TOTAL_VFS", nm_setting_sriov_get_total_vfs (s_sriov)); b = nm_setting_sriov_get_autoprobe_drivers (s_sriov); if (b != NM_TERNARY_DEFAULT) From 9ff5e3b2a6e40f265623e6b341bf901bfe5774e1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 4 Dec 2018 14:09:50 +0100 Subject: [PATCH 08/11] core: use NMTernary for SR-IOV autoprobe-drivers (cherry picked from commit e01a7c1154e0a580f09b76e7735448c54224b2e8) --- src/devices/nm-device.c | 6 +++--- src/platform/nm-linux-platform.c | 10 +++++----- src/platform/nm-platform.c | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3527794171..52167b7852 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4140,7 +4140,7 @@ device_init_static_sriov_num_vfs (NMDevice *self) num_vfs = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT32, -1); if (num_vfs >= 0) { nm_platform_link_set_sriov_params (nm_device_get_platform (self), - priv->ifindex, num_vfs, -1); + priv->ifindex, num_vfs, NM_TERNARY_DEFAULT); } } } @@ -6206,7 +6206,7 @@ act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason) nm_auto_freev NMPlatformVF **plat_vfs = NULL; gs_free_error GError *error = NULL; NMSriovVF *vf; - int autoprobe; + NMTernary autoprobe; autoprobe = nm_setting_sriov_get_autoprobe_drivers (s_sriov); if (autoprobe == NM_TERNARY_DEFAULT) { @@ -14766,7 +14766,7 @@ _set_state_full (NMDevice *self, if ( (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV)) && priv->ifindex > 0) { nm_platform_link_set_sriov_params (nm_device_get_platform (self), - priv->ifindex, 0, 1); + priv->ifindex, 0, NM_TERNARY_TRUE); } _cancel_activation (self); diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e73d5d8c3d..3283338bf7 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5737,7 +5737,7 @@ static gboolean link_set_sriov_params (NMPlatform *platform, int ifindex, guint num_vfs, - int autoprobe) + NMTernary autoprobe) { nm_auto_pop_netns NMPNetns *netns = NULL; nm_auto_close int dirfd = -1; @@ -5782,7 +5782,7 @@ link_set_sriov_params (NMPlatform *platform, "device/sriov_drivers_autoprobe"), 10, 0, G_MAXUINT, 0); if ( current_num == num_vfs - && (autoprobe == -1 || current_autoprobe == autoprobe)) + && (autoprobe == NM_TERNARY_DEFAULT || current_autoprobe == autoprobe)) return TRUE; if (current_num != 0) { @@ -5800,14 +5800,14 @@ link_set_sriov_params (NMPlatform *platform, if (num_vfs == 0) return TRUE; - if ( autoprobe >= 0 + if ( NM_IN_SET (autoprobe, NM_TERNARY_TRUE, NM_TERNARY_FALSE) && current_autoprobe != autoprobe && !nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "device/sriov_drivers_autoprobe"), - nm_sprintf_buf (buf, "%d", autoprobe))) { - _LOGW ("link: couldn't set SR-IOV drivers-autoprobe to %d: %s", autoprobe, strerror (errno)); + nm_sprintf_buf (buf, "%d", (int) autoprobe))) { + _LOGW ("link: couldn't set SR-IOV drivers-autoprobe to %d: %s", (int) autoprobe, strerror (errno)); return FALSE; } diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index e5a42030b9..757a0f43d8 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1489,25 +1489,25 @@ nm_platform_link_supports_sriov (NMPlatform *self, int ifindex) * @self: platform instance * @ifindex: the index of the interface to change * @num_vfs: the number of VFs to create - * @autoprobe: -1 to keep the current autoprobe-drivers value, - * or {0,1} to set a new value + * @autoprobe: the new autoprobe-drivers value (pass + * %NM_TERNARY_DEFAULT to keep current value) */ gboolean nm_platform_link_set_sriov_params (NMPlatform *self, int ifindex, guint num_vfs, - int autoprobe) + NMTernary autoprobe) { _CHECK_SELF (self, klass, FALSE); g_return_val_if_fail (ifindex > 0, FALSE); - g_return_val_if_fail (NM_IN_SET (autoprobe, -1, 0, 1), FALSE); _LOGD ("link: setting %u total VFs and autoprobe %d for %s (%d)", num_vfs, - autoprobe, + (int) autoprobe, nm_strquote_a (25, nm_platform_link_get_name (self, ifindex)), ifindex); + return klass->link_set_sriov_params (self, ifindex, num_vfs, autoprobe); } From edb6a21b6dde40725db9045dca817f611f6a8af1 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 3 Dec 2018 17:04:34 +0100 Subject: [PATCH 09/11] man: add SR-IOV nmcli example Add an example on how to configure SR-IOV to the nmcli examples man page. https://bugzilla.redhat.com/show_bug.cgi?id=1651979 (cherry picked from commit f606124b6212e7d6cc005ff0147e63a30da09586) --- man/nmcli-examples.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/man/nmcli-examples.xml b/man/nmcli-examples.xml index 0b15f112d8..8be3903a54 100644 --- a/man/nmcli-examples.xml +++ b/man/nmcli-examples.xml @@ -272,6 +272,23 @@ IP4:192.168.1.12/24:192.168.1.1::192.168.1.1:: + Adding an Ethernet connection and configuring SR-IOV VFs +$ nmcli con add type ethernet con-name EthernetPF ifname em1 +$ nmcli con modify EthernetPF sriov.total-vfs 3 sriov.autoprobe-drivers false +$ nmcli con modify EthernetPF sriov.vfs '0 mac=00:11:22:33:44:55 vlans=10, 1 trust=true spoof-check=false' +$ nmcli con modify EthernetPF +sriov.vfs '2 max-tx-rate=20' + + This example demonstrates adding an Ethernet connection for + physical function (PF) ens4 and + configuring 3 SR-IOV virtual functions (VFs) on it. The first + VF is configured with MAC address 00:11:22:33:44:55 and VLAN + 10, the second one has the trust and + spoof-check features respectively enabled + and disabled. VF number 2 has a maximux transmission rate of + 20Mbps. The kernel is instructed to not automatically + instantiate a network interface for the VFs. + + Escaping colon characters in tabular mode $ nmcli -t -f general -e yes -m tab dev show eth0 From f33954f628eb43ec1c9b221fd5052625d442f183 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 6 Dec 2018 15:36:55 +0100 Subject: [PATCH 10/11] meson: add check on settings docs Move the autotools check on settings docs to a shell script and call it from meson too. (cherry picked from commit 81bc218e6d46dcbc798550a7657734c5b3e50181) --- Makefile.am | 12 ++---------- clients/common/meson.build | 14 +++++--------- meson_options.txt | 1 - tools/check-settings-docs.sh | 19 +++++++++++++++++++ 4 files changed, 26 insertions(+), 20 deletions(-) create mode 100755 tools/check-settings-docs.sh diff --git a/Makefile.am b/Makefile.am index 9816e6f73f..e1b442e991 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3701,16 +3701,7 @@ $(clients_common_settings_doc_h): clients/common/settings-docs.xsl libnm/nm-prop $(AM_V_GEN) $(XSLTPROC) --output $@ $< $(word 2,$^) DISTCLEANFILES += $(clients_common_settings_doc_h) check-local-settings-docs: $(clients_common_settings_doc_h) - @if test -z "$$NMTST_NO_CHECK_SETTINGS_DOCS" ; then \ - if ! cmp -s "$(srcdir)/$(clients_common_settings_doc_h).in" "$(builddir)/$(clients_common_settings_doc_h)" ; then \ - if test "$$NM_TEST_REGENERATE" == 1 ; then \ - cp -f "$(builddir)/$(clients_common_settings_doc_h)" "$(srcdir)/$(clients_common_settings_doc_h).in"; \ - else \ - echo "The generated file \"$(builddir)/$(clients_common_settings_doc_h)\" differs from the source file \"$(srcdir)/$(clients_common_settings_doc_h).in\". You probably should copy the generated file over to the source file. You can skip this test by setting \$$NMTST_NO_CHECK_SETTINGS_DOCS=yes". You can also automatically copy the file by rerunning the test with \$$NM_TEST_REGENERATE=1 ; \ - false; \ - fi; \ - fi;\ - fi + $(srcdir)/tools/check-settings-docs.sh "$(srcdir)" "$(builddir)" "$(clients_common_settings_doc_h)" check_local += check-local-settings-docs else $(clients_common_settings_doc_h): $(clients_common_settings_doc_h).in clients/common/.dirstamp @@ -5052,6 +5043,7 @@ EXTRA_DIST += \ \ tools/check-docs.sh \ tools/check-exports.sh \ + tools/check-settings-docs.sh \ tools/create-exports-NetworkManager.sh \ tools/debug-helper.py \ tools/meson-post-install.sh \ diff --git a/clients/common/meson.build b/clients/common/meson.build index 0db2868e7c..30fd2cfa96 100644 --- a/clients/common/meson.build +++ b/clients/common/meson.build @@ -37,15 +37,11 @@ if enable_introspection command: [xsltproc, '--output', '@OUTPUT@', join_paths(meson.current_source_dir(), 'settings-docs.xsl'), '@INPUT@'] ) - # FIXME: if enabled the check happens even if the settings_docs_source is not set - ''' - if get_option('check_settings_docs') - res = run_command(find_program('cmp'), '-s', settings_docs + '.in', settings_docs_source.full_path()) - if res.returncode() != 0 - message('The generated file ' + settings_docs_source.full_path() + ' differs from the source file ' + settings_docs + '.in' + '. You probably should copy the generated file over to the source file. You can skip this test by setting -Dcheck_settings_docs=false') - endif - endif - ''' + test( + 'check-settings-docs', + find_program(join_paths(meson.source_root(), 'tools', 'check-settings-docs.sh')), + args: [meson.source_root(), meson.build_root(), 'clients/common/' + settings_docs] + ) else settings_docs_source = configure_file( input: settings_docs + '.in', diff --git a/meson_options.txt b/meson_options.txt index 0f93d77f30..f4c8e5eeb8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -73,4 +73,3 @@ option('libpsl', type: 'boolean', value: true, description: 'Link against libpsl option('json_validation', type: 'boolean', value: true, description: 'Enable JSON validation in libnm') option('crypto', type: 'combo', choices: ['nss', 'gnutls'], value: 'nss', description: 'Cryptography library to use for certificate and key operations') option('qt', type: 'boolean', value: true, description: 'enable Qt examples') -option('check_settings_docs', type: 'boolean', value: false, description: 'compare check settings-docs.h file') diff --git a/tools/check-settings-docs.sh b/tools/check-settings-docs.sh new file mode 100755 index 0000000000..8695ccc030 --- /dev/null +++ b/tools/check-settings-docs.sh @@ -0,0 +1,19 @@ +#!/bin/sh + +srcdir=$1 +builddir=$2 +doc_h=$3 + +if [ -z "$NMTST_NO_CHECK_SETTINGS_DOCS" ] ; then + if ! cmp -s "${srcdir}/${doc_h}.in" "${builddir}/${doc_h}"; then + if [ "$NM_TEST_REGENERATE" = 1 ] ; then + cp -f "${builddir}/${doc_h}" "${srcdir}/${doc_h}.in" + else + echo "*** Error: the generated file '${builddir}/${doc_h}' differs from the source file '${srcdir}/${doc_h}.in'. You probably should copy the generated file over to the source file. You can skip this test by setting NMTST_NO_CHECK_SETTINGS_DOCS=yes. You can also automatically copy the file by rerunning the test with NM_TEST_REGENERATE=1" + exit 1 + fi + fi +fi + +exit 0 + From ce44056410c227c5a9a7c106ef467b48c0ef8b98 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 12 Dec 2018 14:17:24 +0100 Subject: [PATCH 11/11] platform: sriov: write new values when we can't read old ones Fixes: 7df33338797fc335c245fed65ac1186284afc357 (cherry picked from commit 1e41495d9a1be9cda302a5c5c294d794b7e760a5) --- src/platform/nm-linux-platform.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 3283338bf7..4c190da656 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5741,8 +5741,9 @@ link_set_sriov_params (NMPlatform *platform, { nm_auto_pop_netns NMPNetns *netns = NULL; nm_auto_close int dirfd = -1; - gboolean current_autoprobe; - guint total, current_num; + int current_autoprobe; + guint total; + gint64 current_num; char ifname[IFNAMSIZ]; char buf[64]; @@ -5775,12 +5776,12 @@ link_set_sriov_params (NMPlatform *platform, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "device/sriov_numvfs"), - 10, 0, G_MAXUINT, 0); + 10, 0, G_MAXUINT, -1); current_autoprobe = nm_platform_sysctl_get_int_checked (platform, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "device/sriov_drivers_autoprobe"), - 10, 0, G_MAXUINT, 0); + 10, 0, 1, -1); if ( current_num == num_vfs && (autoprobe == NM_TERNARY_DEFAULT || current_autoprobe == autoprobe)) return TRUE;