From 5780a2893b2adc1a3cafd45dcf2d2c09bf7e15c3 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 --- 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 3e4b30b2b180b069dcd20520192d93bb05c96d4b 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. --- 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 769e0726a81bb6a7c9250527ee6bba205e2f23ab 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 --- 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 6ffa484465..6c85cc6be4 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 3b981157c2..6ce5bb5388 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 d26e64c75f..547c141850 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 310f8881a0..6dd4d40e8d 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -2263,7 +2263,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 3de25bbc977fa935a77aebf3f8f7d9e9dfaa55b8 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 --- clients/common/settings-docs.h.in | 4 ++-- libnm-core/nm-setting-sriov.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 5fa541975e..2b579c24b7 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 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_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 39b872a225..b7756e3506 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,6 +1288,10 @@ nm_setting_sriov_class_init (NMSettingSriovClass *klass) * * "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 * @@ -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 75024e11b395e5d4910efc407be09e76b6c6ff7c 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. --- 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 930b0ab603..7e1973eb82 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4390,8 +4390,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) @@ -14843,6 +14841,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 529533a50c01fec36e91134c1c44c6b00ef25fd4 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. --- 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 7e1973eb82..e402857244 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -14732,6 +14732,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)); @@ -14930,6 +14931,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 d48f389cbf6a24f6e58f1422e5b5a8a8d649c973 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 --- .../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 6dd4d40e8d..7aa5aa2b07 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -2228,20 +2228,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) { @@ -2275,11 +2274,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 e01a7c1154e0a580f09b76e7735448c54224b2e8 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 --- src/devices/nm-device.c | 6 +++--- src/platform/nm-linux-platform.c | 10 +++++----- src/platform/nm-platform.c | 9 ++++----- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index e402857244..87d168b3e7 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4227,7 +4227,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); } } } @@ -6294,7 +6294,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) { @@ -14934,7 +14934,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 82f872f54e..4189176f9e 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 e119707009..f41517bd41 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1468,21 +1468,20 @@ 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); - _LOG3D ("link: setting %u total VFs and autoprobe %d", num_vfs, autoprobe); + _LOG3D ("link: setting %u total VFs and autoprobe %d", num_vfs, (int) autoprobe); return klass->link_set_sriov_params (self, ifindex, num_vfs, autoprobe); } From f606124b6212e7d6cc005ff0147e63a30da09586 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 --- man/nmcli-examples.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/man/nmcli-examples.xml b/man/nmcli-examples.xml index 32fc567b04..e777c326d1 100644 --- a/man/nmcli-examples.xml +++ b/man/nmcli-examples.xml @@ -285,6 +285,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 81bc218e6d46dcbc798550a7657734c5b3e50181 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. --- 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 d298e54d91..68f4f0a28e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3833,16 +3833,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 @@ -5166,6 +5157,7 @@ EXTRA_DIST += \ tools/check-config-options.sh \ 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 d249567171..221ba2b804 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -74,4 +74,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 1e41495d9a1be9cda302a5c5c294d794b7e760a5 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 --- 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 4189176f9e..8b4b081f64 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;