device: inline nm_platform_ethtool_init_ring() function

nm_platform_ethtool_init_ring() only has one caller. It's simpler to
drop the function and implement it at the only place where it is needed.

Maybe there could be a place for a function to initialize NMEthtoolRingState,
one option after the other. However, at the moment there is only one
user, so don't implement it.

This fixes various minor issues:

- the function had a NMPlatform argument, although the argument
  is not used. Thus function merely operates on a NMEthtoolRingState
  instance and shouldn't have a nm_platform_*() name.

- nm_platform_ethtool_init_ring() returned a boolean, but all
  code paths (except assertion failures) returned success.

- as the function returned an error status, the caller was compelled
  to handle an error that could never happen.

- the option was specified by name, although we already have a more
  efficient way to express the option: the NMEthtoolID. Also, the
  caller already needed to resolve the name to the NMEthtoolID, so
  there was no need to again lookup the ID by name.
This commit is contained in:
Thomas Haller 2020-05-26 19:02:18 +02:00
parent 9c236416c8
commit 23d0a76b16
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 22 additions and 49 deletions

View file

@ -999,8 +999,12 @@ _ethtool_ring_set (NMDevice *self,
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))
NMEthtoolID ethtool_id = nm_ethtool_id_get_by_name (name);
guint32 u32;
if (!nm_ethtool_id_is_ring (ethtool_id))
continue;
nm_assert (g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32));
if (!has_old) {
@ -1014,12 +1018,23 @@ _ethtool_ring_set (NMDevice *self,
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;
u32 = g_variant_get_uint32 (variant);
switch (ethtool_id) {
case NM_ETHTOOL_ID_RING_RX:
ring_new.rx_pending = u32;
break;
case NM_ETHTOOL_ID_RING_RX_JUMBO:
ring_new.rx_jumbo_pending = u32;
break;
case NM_ETHTOOL_ID_RING_RX_MINI:
ring_new.rx_mini_pending = u32;
break;
case NM_ETHTOOL_ID_RING_TX:
ring_new.tx_pending = u32;
break;
default:
nm_assert_not_reached ();
}
}
@ -1056,8 +1071,6 @@ _ethtool_state_reset (NMDevice *self)
_ethtool_ring_reset (self, platform, ethtool_state);
}
static void
_ethtool_state_set (NMDevice *self)
{

View file

@ -3252,41 +3252,6 @@ nm_platform_ethtool_get_link_ring (NMPlatform *self,
return nmp_utils_ethtool_get_ring (ifindex, ring);
}
gboolean
nm_platform_ethtool_init_ring (NMPlatform *self,
NMEthtoolRingState *ring,
const char *option_name,
guint32 value)
{
NMEthtoolID ethtool_id;
g_return_val_if_fail (ring, FALSE);
g_return_val_if_fail (option_name, FALSE);
ethtool_id = nm_ethtool_id_get_by_name (option_name);
g_return_val_if_fail (nm_ethtool_id_is_ring (ethtool_id), FALSE);
switch (ethtool_id) {
case NM_ETHTOOL_ID_RING_RX:
ring->rx_pending = value;
break;
case NM_ETHTOOL_ID_RING_RX_JUMBO:
ring->rx_jumbo_pending = value;
break;
case NM_ETHTOOL_ID_RING_RX_MINI:
ring->rx_mini_pending = value;
break;
case NM_ETHTOOL_ID_RING_TX:
ring->tx_pending = value;
break;
default:
g_return_val_if_reached (FALSE);
}
return TRUE;
}
gboolean
nm_platform_ethtool_set_ring (NMPlatform *self,
int ifindex,

View file

@ -1975,11 +1975,6 @@ gboolean nm_platform_ethtool_get_link_ring (NMPlatform *self,
int ifindex,
NMEthtoolRingState *ring);
gboolean nm_platform_ethtool_init_ring (NMPlatform *self,
NMEthtoolRingState *ring,
const char *option_name,
guint32 value);
gboolean nm_platform_ethtool_set_ring (NMPlatform *self,
int ifindex,
const NMEthtoolRingState *ring);