This function is not used nor does it seem useful.
Either you only need the names (nm_setting_gendata_get_all_names())
or you need the names and values together (_nm_setting_gendata_get_all()).
Getting the values without knowing the corresponding name makes
little sense. If you need it, call _nm_setting_gendata_get_all()
instead.
Don't duplicate the code that maps the option to the variant type.
Also, only resolve the name to NMEthtoolID once. Multiple calls
to nm_ethtool_optname_is_*() unnecessarily need to convert the
string to the ethtool id multiple times.
We already have NMEthtoolID to handle coalesce options in a way that is
convenient programmatically. That is, we can iterate over all valid
coalesce options (it's just an integer) and use that in a more generic
way.
If NMEthtoolCoalesceState names all fields explicitly, we need explicit
code that names each coalesce option. Especially since NMEthtoolCoalesceState
is an internal and intermediate data structure, this is cumbersome
and unnecessary.
Thereby it also fixes the issue that nm_platform_ethtool_init_coalesce() has a
NMPlatform argument without actually needing it.
nm_platform_ethtool_init_coalesce() does not operate on a NMPlatform
instance, and should not have the appearance of being a method of
NMPlatform.
All other users name a similar variable "coalesce" (or "coalesce_new",
"coalesce_old"). Rename the variables, to don't use different names for
similar uses.
This will be used for nm_setting_option_clear_by_name(), to
filter based on a name. But it is a general purpose typedef
for a predicate, not tied to NMSetting or option.
As confirmed via private email:
From: Rafael Fontenelle <rafaelff(at)gnome.org>
To: Thomas Haller <thaller(at)redhat.com>
Date: Fri, 22 May 2020 10:28:59 -0300
As confirmed via private email:
From: Ikey Doherty <ikey.doherty(at)lispysnake.com>
To: Thomas Haller <thaller(at)redhat.com>
Date: Fri, 22 May 2020 14:43:18 +0100
As discussed via email:
From: Andrew Sinclair <andrew.sinclair(at)canonical.com>
To: Thomas Haller <thaller(at)redhat.com>
Cc: Legal(at)canonical.com <legal(at)canonical.com>, Loïc Minier <loic.minier(at)canonical.com>, Alfonso Sanchez-Beato <alfonso.sanchez-beato(at)canonical.com>
Date: Wed, 20 May 2020 12:08:20 -0400
nm-device now applies ethtool ring settings during stage 2 "device
config" of the connection activation.
ring settings will be then restored (according to what the state
was before the connection got activated on the device) when the
connection is deactivated during the device cleanup.
One thing to be noted is that unset ring settings (in the profile)
will not be touched at all by NetworkManager so that if the NIC driver
sets some default values these will be preserved unless specifically
overridden by the connection profile.
https://bugzilla.redhat.com/show_bug.cgi?id=1614700
The filter function in nm_setting_gendata_clear_all() is useful
for when you want to only clear values according to a predicate,
if no such function is supplied all values will be cleared.
https://bugzilla.redhat.com/show_bug.cgi?id=1614700
Only in one moment we need the old and requested settings together:
during _ethtool_coalesce_set(). But for that we shouldn't track both
states in "NMEthtoolCoalesceState".
Simplify "NMEthtoolCoalesceState" to only contain one set of options.
By tracking less state, the code becomes simpler, because you don't
need to wonder where the old and requested state is used.