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:

  <trace> [1590511552.3943] ethtool[31]: ETHTOOL_GRINGPARAM, v: failed: Operation not supported
  <trace> [1590511552.3944] ethtool[31]: get-ring: failure getting ring settings
  <warn>  [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.
This commit is contained in:
Thomas Haller 2020-05-26 18:55:26 +02:00
parent da3b534d45
commit 9c236416c8
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728

View file

@ -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;
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 (!nm_platform_ethtool_init_ring (platform,
&ring_new,
name,
g_variant_get_uint32 (variant))) {
_LOGW (LOGD_DEVICE, "ethtool: invalid ring setting %s", name);
return;
}
}
ring_new = ring_old;
if (!_ethtool_init_ring (self,
platform,
s_ethtool,
&ring_new))
if (!has_old)
return;
ethtool_state->ring = nm_memdup (&ring_old, sizeof (ring_old));