From 9c236416c854ddfb1e99ddd7215cc197e8b86c64 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 26 May 2020 18:55:26 +0200 Subject: device: only ready existing ethtool ring settings if needed Imagine you have a veth device. That device supports certain offload features (like "ethtool.feature-rx-checksum") but doesn't support any ring options. Even trying to read the current ring settings will fail. If you try to activate that profile, NMDevice previously would always try to fetch the ring options and log a warning and extra debugging messages: [1590511552.3943] ethtool[31]: ETHTOOL_GRINGPARAM, v: failed: Operation not supported [1590511552.3944] ethtool[31]: get-ring: failure getting ring settings [1590511552.3944] device (v): ethtool: failure getting ring settings (cannot read) It does so, although you didn't specify any ring settings and there was no need to fetch the ring settings to begin with. Avoid this extra logging by only fetching the ring option when they are actually required. --- src/devices/nm-device.c | 90 +++++++++++++++++++------------------------------ 1 file changed, 35 insertions(+), 55 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9b018ccc8c..aae1949887 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -950,50 +950,6 @@ _ethtool_coalesce_set (NMDevice *self, _LOGD (LOGD_DEVICE, "ethtool: coalesce settings successfully set"); } -static gboolean -_ethtool_init_ring (NMDevice *self, - NMPlatform *platform, - NMSettingEthtool *s_ethtool, - NMEthtoolRingState *ring) -{ - GHashTable *hash; - GHashTableIter iter; - const char *name; - GVariant *variant; - gsize n_ring_set = 0; - - nm_assert (self); - nm_assert (platform); - nm_assert (ring); - nm_assert (NM_IS_SETTING_ETHTOOL (s_ethtool)); - - hash = _nm_setting_option_hash (NM_SETTING (s_ethtool), FALSE); - if (!hash) - return FALSE; - - g_hash_table_iter_init (&iter, hash); - while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &variant)) { - if (!g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32)) - continue; - if (!nm_ethtool_optname_is_ring (name)) - continue; - - if (!nm_platform_ethtool_init_ring (platform, - ring, - name, - g_variant_get_uint32(variant))) { - _LOGW (LOGD_DEVICE, "ethtool: invalid ring setting %s", name); - return FALSE; - } - ++n_ring_set; - - } - - return !!n_ring_set; -} - - - static void _ethtool_ring_reset (NMDevice *self, NMPlatform *platform, @@ -1025,25 +981,49 @@ _ethtool_ring_set (NMDevice *self, { NMEthtoolRingState ring_old; NMEthtoolRingState ring_new; + GHashTable *hash; + GHashTableIter iter; + const char *name; + GVariant *variant; + gboolean has_old = FALSE; - nm_assert (ethtool_state); nm_assert (NM_IS_DEVICE (self)); nm_assert (NM_IS_PLATFORM (platform)); nm_assert (NM_IS_SETTING_ETHTOOL (s_ethtool)); + nm_assert (ethtool_state); + nm_assert (!ethtool_state->ring); - if (!nm_platform_ethtool_get_link_ring (platform, - ethtool_state->ifindex, - &ring_old)) { - _LOGW (LOGD_DEVICE, "ethtool: failure getting ring settings (cannot read)"); + hash = _nm_setting_option_hash (NM_SETTING (s_ethtool), FALSE); + if (!hash) return; - } - ring_new = ring_old; + g_hash_table_iter_init (&iter, hash); + while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &variant)) { + if (!nm_ethtool_optname_is_ring (name)) + continue; + nm_assert (g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32)); + + if (!has_old) { + if (!nm_platform_ethtool_get_link_ring (platform, + ethtool_state->ifindex, + &ring_old)) { + _LOGW (LOGD_DEVICE, "ethtool: failure setting ring options (cannot read existing setting)"); + return; + } + has_old = TRUE; + ring_new = ring_old; + } - if (!_ethtool_init_ring (self, - platform, - s_ethtool, - &ring_new)) + if (!nm_platform_ethtool_init_ring (platform, + &ring_new, + name, + g_variant_get_uint32 (variant))) { + _LOGW (LOGD_DEVICE, "ethtool: invalid ring setting %s", name); + return; + } + } + + if (!has_old) return; ethtool_state->ring = nm_memdup (&ring_old, sizeof (ring_old)); -- cgit v1.2.1