The number of authentication retires is useful also for passwords aside
802-1x settings. For example, src/devices/wifi/nm-device-wifi.c also has
a retry counter and uses a hard-coded value of 3.
Move the setting, so that it can be used in general. Although it is still
not implemented for other settings.
This is an API and ABI break.
When the ifcfg-rh plugin writes a 802-1x setting it currently ignores
the password-raw property and so the password disappears when the
connection is saved. Add support for the property.
Normalizing can be complicated, as settings depend on each other and possibly
conflict.
That is, because verify() must exactly anticipate whether normalization will
succeed and how the result will look like. That is because we only want to
modify the connection, if we are sure that the result will verify.
Hence, verify() and normalize() are strongly related. The implementation
should not be spread out between NMSettingOvsInterface:verify(),
NMSettingOvsPatch:verify() and _normalize_ovs_interface_type().
Also, add some unit-tests.
There is no API to get all settings. You can only ask for
settings explicitly, but that requires you to probe for them
and know which ones may exist.
The alternative API might be nm_connection_for_each_setting_value(),
but that only iterates over settings' properties. If a setting has no
properties, it is ignored.
We want to support large number of routes. Reduce the number
of copies, by adding internal accessor functions.
Also, work around a complaint from coverity:
46. NetworkManager-1.9.2/libnm-core/nm-utils.c:1987:
dereference: Dereferencing a null pointer "names".
Previously, nm_setting_diff() would return !(*results), that means,
if the caller passed in a hash table (empty or not), the return value
would always be FALSE, indicating a difference.
That is not documented, and makes no sense.
The return value, should solely indicate whether some difference was
found. The only convenience is, if nm_setting_diff() created a hash
table internally and no difference was found, it would destroy
it again, without returning it to the caller.
NMSettingGeneric has no properties at all. Hence, nm_connection_diff() would report that
a connection A with a generic setting and a connection B without a generic setting are
equal.
They are not. For empty settings, let nm_setting_diff() return also empty difference
hash.
Since kernel commit a4176a9391868bfa87705bcd2e3b49e9b9dd2996 (net:
reject creation of netdev names with colons), kernel rejects any
colons in the interface name.
Since kernel could get away with tightening up the check, we can
too.
The user anyway can not choose arbitrary interface names, like
"all", "default", "bonding_masters" are all going to fail one
way or another.
teamd adds the "tx_hash" property for "lacp" and "loadbalance" runners
when not present. Do the same so that our original configuration
matches with the one reported by teamd.
https://bugzilla.redhat.com/show_bug.cgi?id=1497333
We often want to cascade hashing, meaning, to combine the
outcome of various hash functions in a larger hash.
Instead of having each hash function return a guint hash value,
accept a hash state argument. This saves the overhead of initializing
and completing the intermediate hash states.
It also avoids loosing entropy when we reduce the larger hash state
into the intermediate guint hash value.
By using a macro, we don't cast all the types to guint. Instead,
we use their native types directly. Hence, we don't need
nm_hash_update_uint64() nor nm_hash_update_ptr().
Also, for types smaller then guint like char, we save hashing
the all zero bytes.
siphash24() is wildly used by projects nowadays.
It's certainly slower then our djb hashing that we used before.
But quite likely it's fast enough for us, given how wildly it is
used. I think it would be hard to profile NetworkManager to show
that the performance of hash tables is the issue, be it with
djb or siphash24.
Certainly with siphash24() it's much harder to exploit the hashing
algorithm to cause worst case hash operations (provided that the
seed is kept private). Does this better resistance against a denial
of service matter for us? Probably not, but let's better be safe then
sorry.
Note that systemd's implementation uses a different seed for each hash
table (at least, after the hash table grows to a certain size).
We don't do that and use only one global seed.
The privious NM_HASH_* macros directly operated on a guint value
and were thus close to the actual implementation.
Replace them by adding a NMHashState struct and accessors to
update the hash state. This hides the implementation better
and would allow us to carry more state. For example, we could
switch to siphash24() transparently.
For now, we still do a form basically djb2 hashing, albeit with
differing start seed.
Also add nm_hash_str() and nm_str_hash():
- nm_hash_str() is our own string hashing implementation
- nm_str_hash() is our own string implementation, but with a
GHashFunc signature, suitable to pass it to g_hash_table_new().
Also, it has this name in order to remind you of g_str_hash(),
which it is replacing.
"nm-utils/nm-shared-utils.h" shall contain utility function without other
dependencies. It is intended to be used by other projects as-is.
nm_utils_random_bytes() requires getrandom() and a HAVE_GETRANDOM configure
check. That makes it more cumbersome to re-use "nm-shared-utils.h", in
cases where you don't care about nm_utils_random_bytes().
Split nm_utils_random_bytes() out to a separate file.
Same for hash utils, which depend on nm_utils_random_bytes(). Also, hash
utils will eventually be extended to use siphash24.
Introduce a NM_HASH_INIT() function. It makes the places
where we initialize a hash with a certain seed visually clear.
Also, move them from "shared/nm-utils/nm-shared-utils.h" to
"shared/nm-utils/nm-macros-internal.h". We might want to
have NM_HASH_INIT() non-inline (hence, define it in the
source file).
So that the man page will display:
The permitted values are: NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE_EUI64
(0) or NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE_STABLE_PRIVACY (1).
instead of
The permitted values are: "eui64" or "stable-privacy".
since the latter is not useful at all for a int32 property.
Unfortunately the enum names are quite long and don't look very well
in a table, but that's another problem.
We added "ipv4.route-table-sync" and "ipv6.route-table-sync" to not change
behavior for users that configured policy routing outside of NetworkManager,
for example, via a dispatcher script. Users had to explicitly opt-in
for NetworkManager to fully manage all routing tables.
These settings were awkward. Replace them with new settings "ipv4.route-table"
and "ipv6.route-table". Note that this commit breaks API/ABI on the unstable
development branch by removing recently added API.
As before, a connection will have no route-table set by default. This
has the meaning that policy-routing is not enabled and only the main table
will be fully synced. Once the user sets a table, we recognize that and
NetworkManager manages all routing tables.
The new route-table setting has other important uses: analog to
"ipv4.route-metric", it is the default that applies to all routes.
Currently it only works for static routes, not DHCP, SLAAC,
default-route, etc. That will be implemented later.
For static routes, each route still can explicitly set a table, and
overwrite the per-connection setting in "ipv4.route-table" and
"ipv6.route-table".
We already have nm_strquote_a(). That is useful, but uses alloca(), hence it
is ill suited to be called from a macro, inside a loop, or from a function
that should be inlined.
Instead, add nm_strquote() that has the same purpose but writes to a provided
string buffer.
- clearify in the manual page that setting retry to 1 means to try
once, without retry.
- log the initially set retry value in nm_settings_connection_get_autoconnect_retries().
- use nm_settings_connection_get_autoconnect_retries() in
nm_settings_connection_can_autoconnect().
Rename @error to @out_err_str, because @error is usually used for GError
output arguments.
Also, make the string variables "const char *".
Use nm_assert() in read_field(), because it is a static function
with only four call sites. It's easily verified that the assertion
holds, so no need for a run-time check in production builds.
Expose previously internal function nm_ip_route_equal_full(). It's
just useful API.
However, add a @cmp_flags argument, so that in the future we could
extend it.
For kernel and NetworkManager's core, route identity is a complicated topic
(see NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID). For example, a route
without explity table is treated identical to "table 254" or "table 0".
It would be complicated to have nm_setting_ip_config_add_route()
implement that logic, especially since libnm offers not public API
to expose kernel's logic.
However, previously nm_setting_ip_config_add_route() would only consider
dest/prefix,next_hop,metric when comparing for equality. Hence, with
nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24'
nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24 table=42'
the second route was not actually added, although it is a very different
route. Fix that, and consider attributes too. Note that this allows the user
to add two routes that look different to libnm, but are actually idential:
nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24'
nmcli connection modify "$CON" +ipv4.routes '192.168.5.0/24 table=254'
In the above example, the route instances look different, but
sementically they are both the same route in the main table (254).
This also allows the user to add routes that are semantically different, but
are treated as the same route by kernel:
nmcli connection modify "$CON" +ipv6.routes 'a🅱️c::/120'
nmcli connection modify "$CON" +ipv6.routes 'a🅱️c::/120 mtu=600'
I think libnm should allow to add routes as long as they look different
to libnm. Regardless how kernel and NetworkManager-core thinks about
route identity.
This changes API of nm_setting_ip_config_add_route(). However, I think
the previous behavior was just broken.
Same for nm_setting_ip_config_remove_route_by_value().
GArray's and GPtrArray's plen argument is unsigned. The index variable
to iterate the list, should not have a smaller range (or different data type).
Also, assert against negative idx argument.
At startup the manager tries to create virtual devices without a
specific order and spits warnings when a device can't be realized
because the parent device is not yet created. These failures are not
something the user should worry about because the creation will be
retried when the parent appears.
A better approach is to return an error code from the device's
create_and_realize() telling that it failed because the parent doesn't
exist. In this way, the manager knows that the device isn't ready and
can avoid printing warning messages.
