gboolean is a typedef for "int".
While older compilers might treat such bitfields as unsigned ([1]),
commonly such a bitfield is signed and can only contain the values 0
and -1.
We only want to use numeric 1 for TRUE, hence, creating such bitfields
is wrong, or at least error prone.
In fact, in this case it's a bug, because later we compare
it with a regular gboolean
if (priv->scanning != new_scanning)
[1] https://lgtm.com/rules/1506024027114/
Fixes: e0f9677018
- have two variants of functions to set the SSID of an access point:
one that passes SSID as GBytes, and one that passes it as plain
data with length. Accepting a GBytes allows to share the immutable
GBytes instance.
- both functions now also support clearing the SSID. In
nm_wifi_ap_update_from_properties(), if the GVariant specifies
a "SSID", we always update the access point. We already support
chaging the SSID, so why not support changing it to *no* SSID
(hidden).
GBytes makes more sense, because it's immutable.
Also, since at other places we use GBytes, having
different types is combersome and requires needless
conversions.
Also:
- avoid nm_utils_escape_ssid() instead of _nm_utils_ssid_to_string().
We use nm_utils_escape_ssid() when we want to log the SSID. However, it
does not escape newlines, which is bad.
- also no longer use nm_utils_same_ssid(). Since it no longer
treated trailing NUL special, it is not different from
g_bytes_equal().
- also, don't use nm_utils_ssid_to_utf8() for logging anymore.
For logging, _nm_utils_ssid_escape_utf8safe() is better because
it is loss-less escaping which can be unambigously reverted.
nm_utils_same_ssid() has a comment
* Earlier versions of the Linux kernel added a NULL byte to the end of the
* SSID to enable easy printing of the SSID on the console or in a terminal,
* but this behavior was problematic (SSIDs are simply byte arrays, not strings)
* and thus was changed. This function compensates for that behavior at the
* cost of some compatibility with odd SSIDs that may legitimately have trailing
* NULLs, even though that is functionally pointless.
and the functionality was introduced by commit
ccb13f0bdd.
There was only place left that calls nm_utils_same_ssid().
I really don't think this is the right approach, nor is it clear
that this is still necessary. Also, it seems to only matter with
WEXT, and we should not have such an ugly hack in all cases.
We already have nm_utils_str_utf8safe_escape() to convert a
NUL termianted string to an UTF-8 string. nm_utils_str_utf8safe_escape()
operates under the assumption, that the input strig is already valid UTF-8
and returns the input string verbatim. That way, in the common expected
cases, the string just looks like a regular UTF-8 string.
However, in case there are invalid UTF-8 sequences (or a backslash
escape characters), the function will use backslash escaping to encode
the input string as a valid UTF-8 sequence. Note that the escaped
sequence, can be reverted to the original non-UTF-8 string via
unescape.
An example, where this is useful are file names or interface names.
Which are not in a defined encoding, but NUL terminated and commonly ASCII or
UTF-8 encoded.
Extend this, to also handle not NUL terminated buffers. The same
applies, except that the process cannot be reverted via g_strcompress()
-- because the NUL character cannot be unescaped.
This will be useful to escape a Wi-Fi SSID. Commonly we expect the SSID
to be in UTF-8/ASCII encoding and we want to print it verbatim. Only
if that is not the case, we fallback to backslash escaping. However, the
orginal value can be fully recovered via unescape(). The difference
between an SSID and a filename is, that the former can contain '\0'
bytes.
Use GBytes instead of GBytesArray. GBytes is immutable and
can be shared.
It is also the type that we natively get from
nm_setting_wireless_get_ssid(). This way we avoid some
conversions.
These should be logged on DEBUG level:
<warn> platform-linux: do-change-link[2]: failure changing link: failure 97 (Address family not supported by protocol)
<warn> device (wlo1): failed to enable userspace IPv6LL address handling (unspecified)
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/10
Add a new 'match' setting containing properties to match a connection
to devices. At the moment only the interface-name property is present
and, contrary to connection.interface-name, it allows the use of
wildcards.
As of upstream kernel v4.18-rc8.
Note that we name the features like they are called in ethtool's
ioctl API ETH_SS_FEATURES.
Except, for features like "tx-gro", which ethtool utility aliases
as "gro". So, for those features where ethtool has a built-in,
alternative name, we prefer the alias.
And again, note that a few aliases of ethtool utility ("sg", "tso", "tx")
actually affect more than one underlying kernel feature.
Note that 3 kernel features which are announced via ETH_SS_FEATURES are
explicitly exluded because kernel marks them as "never_changed":
#define NETIF_F_NEVER_CHANGE (NETIF_F_VLAN_CHALLENGED | \
NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
We will add a large number of offload features. That means, the output
of `nmcli connection show "$PROFILE"` would be very verbose, in case
the profile has a [ethtool] option.
Since this is newly added API, don't do that. Don't show ethtool properties
that are left unset.
A minor problem here is, that it becomes no longer obvious which
properties exist. We should however counter that by documentation.
Also, one could do:
$ nmcli connection modify "$PROFILE" ethtool.xxx x
Error: invalid property 'xxx': 'xxx' not among [feature-gro, feature-gso, feature-lro, feature-ntuple, feature-rx, feature-rxhash, feature-rxvlan, feature-sg, feature-tso, feature-tx, feature-txvlan, feature-tx-tcp6-segmentation, feature-tx-tcp-segmentation].
Likewise, bash completion still works as one would expect.
$ nmcli --complete-args connection modify "$PROFILE" ethtool.
ethtool.feature-gro
ethtool.feature-gso
ethtool.feature-lro
[...]
Note the output of
$ nmcli -f ethtool.feature-gro connection show "$PROFILE"
gives now nothing (if there is an ethtool section, but not this
particular feature). Maybe this shouldn't be like that. On the other
hand, specifying a connection setting that doesn't exist also gives
no output:
$ nmcli -f bond connection show "$PROFILE"
So, maybe this behavior is fine.
Historically, nmcli printed all fields during operations like
`nmcli connection show "$PROFILE"`. As we supported more and
more options, this resulted in a verbose output, of most properties
just being the default values.
To counter that, we added the '-overview' option. When given,
it would hide options that are set at their default. This option
was not the default, to preserve established behavior.
However, for new options, we can afford to hide them. Add a mechanism,
that property getters can mark their value to be hidden. At the moment,
there is no way to show these properties. However, we could add a
'-verbose' option, with the opposite meaning of '-overview'. Anyway,
that does not seem necessary at the moment.
Hiding properties from output is only acceptable for new properties
(otherwise we badly change behavior), and if the properties are set
at their default values (otherwise, we hide important information).
Also, add two more features "tx-tcp-segmentation" and
"tx-tcp6-segmentation". There are two reasons for that:
- systemd-networkd supports setting these two features,
so lets support them too (apparently they are important
enough for networkd).
- these two features are already implicitly covered by "tso".
Like for the "ethtool" program, "tso" is an alias for several
actual features. By adding two features that are already
also covered by an alias (which sets multiple kernel names
at once), we showcase how aliases for the same feature can
coexist. In particular, note how setting
"tso on tx-tcp6-segmentation off" will behave as one would
expect: all 4 tso features covered by the alias are enabled,
except that particular one.
There is little difference in practice because there is only one caller.
Still re-use the SocketHandle also for mii. If only, to make it clear
that SocketHandle is not only suitable for ethtool, but also mii.
Previously, each call to ethtool_get() would resolve the ifindex and
create a new socket for the ethtool request.
This is partly done, because ethtool only supports making requests by
name. Since interfaces can be renamed, this is inherrently racy. So,
we want to fetch the latest name shortly before making the request.
Some functions like nmp_utils_ethtool_supports_vlans() require multiple
ioctls. And next, we will introduce more ethtool functions, that make an
even larger number of individual requests.
Add a simple SocketHandle struct, to create the socket once and reuse
it for multiple requests. This is still entirely internal API in
"nm-platform-utils.c".
ethtool_get_stringset() will be used later, independently.
Also, don't trust and ensure that the block of strings
returned by ETHTOOL_GSTRINGS are NUL terminated.
Note that in NetworkManager API (D-Bus, libnm, and nmcli),
the features are called "feature-xyz". The "feature-" prefix
is used, because NMSettingEthtool possibly will gain support
for options that are not only -K|--offload|--features, for
example -C|--coalesce.
The "xzy" suffix is either how ethtool utility calls the feature
("tso", "rx"). Or, if ethtool utility specifies no alias for that
feature, it's the name from kernel's ETH_SS_FEATURES ("tx-tcp6-segmentation").
If possible, we prefer ethtool utility's naming.
Also note, how the features "feature-sg", "feature-tso", and
"feature-tx" actually refer to multiple underlying kernel features
at once. This too follows what ethtool utility does.
The functionality is not yet implemented server-side.
