Since commit 5c299454b4 we can configure
multiple default-routes.
That is especially useful with IPv6 to configure multiple routers.
It will also be useful, once we allow configuring manual default-routes,
like regular static routes.
However the problem is, that the default-route for the manual gateway
and the gateway from DHCP both get the same metric. So it's undefined
which route is used. To avoid that problem, and to restore previous
behavior, don't accept any default-routes if a gateway is set.
Fixes: 5c299454b4
After commit 5c299454b4 ("core: rework tracking of
gateway/default-route in ip-config") NM set a default route for VPNs
only based on the "never-default" option reported by the plugin. It
should also consider the connection setting.
Fixes: 5c299454b4https://bugzilla.redhat.com/show_bug.cgi?id=1505886
Setting the MTU might fail when the underlying device's MTU
is not set.
Detect that case, and log a better warning message.
Unfortunately, it's tricky to detect whether this is a complete
failure, or whether we will later try again to change the MTU.
So, we log a failure, altough later we might fix it. It would
be better not to warn about non-errors.
and nm_utils_ip6_property_path(). The API with static buffers
looks a bit nicer. But I think they are dangerous, because
we tend to pass the buffer down several layers of the stack, and
it's not immediately clear, that we don't overwrite the static
buffer again (which we probably did not, but it's hard to verify
that there is no bug there).
Setting the MTU failes under regular conditions, for example when
setting the MTU of a master larger then the MTU of the slaves.
Logging a warning it too alarming.
Kernel does not allow setting the MTU of a VLAN larger
then the MTU of the underlying device. Hence, we might
initially fail to set a large MTU of the VLAN, but we
have to retry when the MTU of the parent changes.
https://bugzilla.redhat.com/show_bug.cgi?id=1414901
When comparing an unsigned and a signed integer, the signed integer
is promoted to unsigned, resulting in a very large number.
See the checks "nwrote < len - 1", where nwrote might be -1
to indicate failure. The condition would not be TRUE due to
promoting -1 to the max int value.
Hence, sysctl_set() was rather wrong.
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.
In many scenarios, we have no use for the file descriptor
after nm_utils_fd_get_contents(). We just want to read it
and close it.
API wise, it would be nice that the get_contents() function never
closes the passed in fd and it's always responsibility of the caller.
However, that costs an additional dup() syscall that could
be avoided, if we allow the function to (optionally) close
the file descriptor.
nm_close() is like close(), but throws an assertion if the input fd is
>=0 and invalid. Passing an invalid (i.e. already closed) fd to
close() is a programming error with potentially catastrophic effects,
as another thread may reuse the closed fd.
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
Fixes the following build error where gcc fails because the
client/common does not exist yet:
CC shared/nm-utils/clients_common_libnmc_base_la-nm-enum-utils.lo
cc1: error: ./clients/common: No such file or directory [-Werror]
cc1: all warnings being treated as errors
make[4]: *** [shared/nm-utils/clients_common_libnmc_base_la-nm-enum-utils.lo] Error 1
#
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I.. -I../shared -I./shared -I../libnm-core -I./libnm-core -I../libnm -I./libnm -I../clients/common -I./clients/common ... -c ../shared/nm-utils/nm-enum-utils.c -fPIC -DPIC -o shared/nm-utils/.libs/clients_common_libnmc_base_la-nm-enum-utils.o
cc1: error: ./clients/common: No such file or directory [-Werror]
Fixes: 3434261811
The function should not close the input file descriptor; however
fdopen() associates the fd to the new stream so that when the stream
is closed, the fd is too. The result is a double close() and the
second call can in certain cases affect a wrong fd.
Use a duplicate fd for the stream.
Fixes: 1d9bdad1dfhttps://bugzilla.redhat.com/show_bug.cgi?id=1451236
libnm-core limits the rande for GATEWAY_PING_TIMEOUT to 0 to 600.
See commit e86f8354a7, "device: restart
ping process when it exits with an error".
The reader must not pass value out of range to g_object_set().
Clamp and warn.
"nm-dhcp-manager.h" forward declares _nm_dhcp_manager_factories.
We need to make the definition aware of the declaration, so
that the compiler can warn if they differ.
We don't need this extra distinguisher. It makes no sense to ever
compare two routes with a different compare-type.
Also, the number of fields that is hashed already differs between each
compare type. If we have a good hashing algorithm, this already suffices
that the hash value looks largely different.
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.
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
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.
==28576== 78 (72 direct, 6 indirect) bytes in 1 blocks are definitely lost in loss record 843 of 1,311
==28576== at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
==28576== by 0x6FD0A38: g_malloc (gmem.c:94)
==28576== by 0x6FE8575: g_slice_alloc (gslice.c:1025)
==28576== by 0x6FE8A08: g_slice_alloc0 (gslice.c:1051)
==28576== by 0x7010261: g_system_thread_new (gthread-posix.c:1152)
==28576== by 0x6FF283E: g_thread_new_internal (gthread.c:874)
==28576== by 0x6FF28E7: g_thread_new (gthread.c:827)
==28576== by 0x6FCC4A3: g_get_worker_context (gmain.c:5851)
==28576== by 0x681A814: g_task_thread_pool_init (gtask.c:1977)
==28576== by 0x681A814: g_task_get_type (gtask.c:592)
==28576== by 0x685CB50: ensure_required_types (gdbusprivate.c:231)
==28576== by 0x685CB50: _g_dbus_initialize (gdbusprivate.c:1927)
==28576== by 0x6850CA0: g_bus_get_sync (gdbusconnection.c:7267)
==28576== by 0x10B6AC: nmtstc_service_init (nm-test-utils-impl.c:91)
==28576== by 0x10D261: test_setup (test-secret-agent.c:249)
==28576== by 0x6FF118B: test_case_run (gtestutils.c:2160)
==28576== by 0x6FF118B: g_test_run_suite_internal (gtestutils.c:2244)
==28576== by 0x6FF139A: g_test_run_suite_internal (gtestutils.c:2256)
==28576== by 0x6FF139A: g_test_run_suite_internal (gtestutils.c:2256)
==28576== by 0x6FF1571: g_test_run_suite (gtestutils.c:2332)
==28576== by 0x6FF1590: g_test_run (gtestutils.c:1599)
==28576== by 0x10D0EE: main (test-secret-agent.c:654)
nmtui determines the activation result by tracking the active
connection state but that is not enough, as the active connection may
disappear or because we need to consider also the device state in some
particular cases.
Use the same logic implemented in nmcli that is now provided by the
nmc_activation_get_effective_state() helper.
