Currently in any of the BOUND, RENEWING and REBINDING states the probe
checks the expiration of T1, T2 and lifetime. This is not correct
because, for example, if the timer fires in the RENEWING state, the
probe must not transition to RENEWING again (i.e. check again that
now >= T1). Note that there is no guarantee that the timer triggers
exactly once for T1, T2 and lifetime expirations because the timer is
also used for the retransmission logic in NDhcp4CConnection.
Therefore, add some checks to ensure that only correct transitions are
allowed.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/341https://bugzilla.redhat.com/show_bug.cgi?id=1773456
Now that we can not only get the expiry timestamp of the lease
(n_dhcp4_client_lease_get_lifetime()), but also the base timestamp,
we can calculate the lifetime exactly.
Previously, we had to guess the base time by assuming that we just
received the lease *now*. This wasn't exact.
The "expiry" is the Unix timestamp when the lease expires.
This is not at all a useful parameter, in particular because
the system's clock can be reset. Instead, we should expose
the lease receive time stamp (in CLOCK_BOOTTIME), and the lease
lifetime.
Anyway. So, we somehow need to express infinite lifetimes. Previously,
we would use the special value 4294967295 (2^32-1). However, that value
does not seem so great, because it's also the Unix timestamp of
2106-02-07T06:28:15+0000. While that is quite far in the future, it's
a valid timestamp still. Of course, the code worked around that by never
setting a timestamp larger than 4294967295-1, but it still limits the
range of what we can expose.
Note that for the lifetime "dhcp_lease_time", we do express infinity
with 4294967295. That's fine, it also does not contradict what we
receive in the DHCP lease on the wire because the lifetime there is
expressed by a 32 bit integer.
Instead, for the "expiry" timestamp, don't perform such triming.
The expiry timestamp is just the start timestamp plus the lease
lifetime. If that is larger than 2106-02-07, so be it.
On the other hand, express infinity by omitting the "expiry" field.
Import the downstream addition for lease-time [1].
These are not merged upstream yet, but let's use
the patches already. If the patches that get merged
upstream eventually differ, we still can adjust our fork
easily.
[1] https://github.com/nettools/n-dhcp4/pull/5
The API already had n_dhcp4_client_lease_get_lifetime(), which is the CLOCK_BOOTTIME
when the lease expires (or ((uint64_t)-1)). But it might be interesting to
know the actual lease duration and when the lease was received (and the
time started to count).
Expose an API for that. With this, one can also calculate the original, exact lease
lifetime, by subtracting n_dhcp4_client_lease_get_basetime() from n_dhcp4_client_lease_get_lifetime(),
while taking care of ((uint64_t)-1).
First of all, from the naming of n_dhcp4_incoming_query_u32() it is
confusing to coerce 0xFFFFFFFF to zero. It should just return the
plain value.
Also note that n_dhcp4_incoming_query_u32() only has three callers:
n_dhcp4_incoming_query_lifetime(), n_dhcp4_incoming_query_t1() and
n_dhcp4_incoming_query_t2().
Looking further, those three functions only have one caller:
n_dhcp4_incoming_get_timeouts(). Note how the code there already tries
to handle UINT32_MAX and interprets it as infinity (UINT64_MAX).
But as it was, UINT32_MAX never actually was returned.
It seems that RFC [1] does not specially define the meanings of
0xFFFFFFFF and 0. It sounds reasonable to assume that 0 just means
0 lifetime, and 0xFFFFFFFF means infinity. On the other hand, compare
this to systemd's code [2], which coerces 0 to 1. This does not seem
right to me though. Note how systemd returns 0xFFFFFFFF as-is.
Drop the special handling of 0xFFFFFFFF from n_dhcp4_incoming_query_u32().
It now just returns the plain value and it's up to n_dhcp4_incoming_get_timeouts()
to make sense of that. This will fix behavior, so that 0xFFFFFFFF will be
reported as infinity, and not as zero.
[1] https://tools.ietf.org/html/rfc2132#section-9.2
[2] 68c2b5ddb1/src/libsystemd-network/sd-dhcp-lease.c (L553)
It is error prone when a function consumes an input only in certain
cases (and telling the caller via the return code). At least in these
cases, the message is never used afterwards, and we can always pass
it on.
libnm has a dependency on 'libnmdbus_dep', which contains 'link_with:
libnmdbus'. This however only enforces that libnm is linked after the
libnmdbus static library is built; it doesn't give any guarantees
about the compilation phase.
We need to make libnm compilation depend on the generated header
files. The output of 'gnome.gdbus_codegen' is an array with the header
file in the second position; use it to add a proper
dependency. Unfortunately this works only with meson >= 0.46.
In the future libnm will no longer use gdbus generated code and this
dependency will not be needed anymore.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/286
This essentially aligns the implementation with the documentation.
It is also rather useful, since it allows us to use the value returned
by nm_setting_wired_get_mac_address() directly, and that one can indeed
be NULL.
It is really not clear what the user could have meant by specifying a
bootdev= argument, and we deal with it just by ensuring a device with
that name whould come up.
We therefore pick a default connection if there's one (that is a
conneciton that we create if the device name is unspecified, as in
"ip=auto"), otherwise we create a new one.
We need to actually read the stdout/stderr of the nmcli programs.
Otherwise, the pipe might fill uup and block to process (eventually
leading to timeout).
Debugging tests that are called by test-client.py is cumbersome.
One way would be to set NM_TEST_CLIENT_NMCLI_PATH to a wrapper script.
However, then we want to know from the wrapper script which test
we are currently calling. Add that to the environment.
During the libnm rework, we might still emit permissions changed
signal while destructing the instance. That triggers an assertion.
Backtrace, with a different libnm:
#0 _g_log_abort (breakpoint=1) at ../glib/gmessages.c:554
#1 0x00007ffff77d09b6 in g_logv (log_domain=0x7ffff7f511cd "libnm", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffcb80) at ../glib/gmessages.c:1373
#2 0x00007ffff77d0b83 in g_log
(log_domain=log_domain@entry=0x7ffff7f511cd "libnm", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff78215df "%s: assertion '%s' failed")
at ../glib/gmessages.c:1415
#3 0x00007ffff77d137d in g_return_if_fail_warning
(log_domain=log_domain@entry=0x7ffff7f511cd "libnm", pretty_function=pretty_function@entry=0x7ffff7f58aa0 <__func__.40223> "nm_client_get_permission_result", expression=expression@entry=0x7ffff7f54830 "NM_IS_CLIENT (client)") at ../glib/gmessages.c:2771
#4 0x00007ffff7e9de9a in nm_client_get_permission_result (client=0x0, permission=permission@entry=NM_CLIENT_PERMISSION_ENABLE_DISABLE_NETWORK) at libnm/nm-client.c:3816
#5 0x0000555555593ba3 in got_permissions (nmc=nmc@entry=0x55555562ec20 <nm_cli>) at clients/cli/general.c:587
#6 0x0000555555593bcb in permission_changed (client=<optimized out>, permission=<optimized out>, result=<optimized out>, nmc=0x55555562ec20 <nm_cli>) at clients/cli/general.c:600
#7 0x00007ffff73b1aa8 in ffi_call_unix64 () at ../src/x86/unix64.S:76
#8 0x00007ffff73b12a4 in ffi_call (cif=cif@entry=0x7fffffffced0, fn=fn@entry=0x555555593bbf <permission_changed>, rvalue=<optimized out>, avalue=avalue@entry=0x7fffffffcde0)
at ../src/x86/ffi64.c:525
#9 0x00007ffff78b4746 in g_cclosure_marshal_generic_va
(closure=<optimized out>, return_value=<optimized out>, instance=<optimized out>, args_list=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=<optimized out>) at ../gobject/gclosure.c:1614
#10 0x00007ffff78b3996 in _g_closure_invoke_va (closure=0x5555556f4330, return_value=0x0, instance=0x55555565a020, args=0x7fffffffd180, n_params=2, param_types=0x555555656f00)
at ../gobject/gclosure.c:873
#11 0x00007ffff78d0228 in g_signal_emit_valist (instance=0x55555565a020, signal_id=<optimized out>, detail=0, var_args=var_args@entry=0x7fffffffd180) at ../gobject/gsignal.c:3306
#12 0x00007ffff78d09d3 in g_signal_emit (instance=instance@entry=0x55555565a020, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3453
#13 0x00007ffff7e8989a in _emit_permissions_changed (self=self@entry=0x55555565a020, permissions=permissions@entry=0x555555690e40 = {...}, force_unknown=force_unknown@entry=1)
at libnm/nm-client.c:2874
#14 0x00007ffff7e9a0c9 in _init_release_all (self=self@entry=0x55555565a020) at libnm/nm-client.c:6092
#15 0x00007ffff7e9bcde in dispose (object=0x55555565a020 [NMClient]) at libnm/nm-client.c:6838
#16 0x00007ffff78b8c28 in g_object_unref (_object=<optimized out>) at ../gobject/gobject.c:3344
#17 g_object_unref (_object=0x55555565a020) at ../gobject/gobject.c:3274
#18 0x00005555555badcf in nmc_cleanup (nmc=0x55555562ec20 <nm_cli>) at clients/cli/nmcli.c:924
#19 0x00005555555bbea7 in main (argc=<optimized out>, argv=0x7fffffffd498) at clients/cli/nmcli.c:987
We cannot first remove the connection (and emit property changed signals),
before replying with the newly added path (that already no longer exists).
Fix the stub implementation.
With this test the stub service simulates a failure to add-and-activate
the connection.
However the implementation of the stub service was not simulating the
real behavior of NetworkManager service. libnm will add the possibility
to assert against invalid server behavior by setting LIBNM_CLIENT_DEBUG=error.
With that change, libnm will complain that the stub service behaves
invalid, and rightly so.
Instead of fixing the test, just drop it.
libnm is gonna change, where it would still emit signals when the
instance gets destructed. In particular, when the device gets removed
from the NMClient cache, the references to other objects would be
cleared (and consequently property changed signals emitted).
This will cause a test failure, because the signal was not unsubscribed:
test:ERROR:libnm/tests/test-nm-client.c:694:device_ac_changed_cb: assertion failed: (nm_device_get_active_connection (NM_DEVICE (device)) != NULL)
The advantage of nmtstc_client_new() is that it randomly either uses the
synchronous or asynchronous constructor. Of course, both should behave
pretty much the same. Hence, this increases our test coverage.
The device interface (org.freedesktop.NetworkManager.Device) has
two properties: "State" and "StateReason". Both of them are supported by
NetworkManager for a very long time.
Note that "StateReason" is a tuple and also exposes the state along the
reason.
When reworking libnm, we will ignore the "State" property and only
consider "StateReason". The advantage is less code and not using
redundant state. This will also work well, because NetworkManager's
D-Bus API supports this property for a very long time.
However, that would then break the CI tests, because currently
"tools/test-networkmanager-service.py" does not expose that property.
Add it.
