Add support to the internal DHCP client for requesting a prefix and
distributing it to interfaces with 'shared' IPv6 mode.
The systemd-networkd API currently allows to request only a single
prefix and so there will be issues when the number of downstream
interfaces is greater than the number of /64 subnets available in the
returned prefix; but this is still an improvement over the previous
situation when no prefix was requested at all.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/247
"nm-setting.c" (and property_to_dbus()) should stay independent of
actualy settings implementations. Instead, the property-info should
control the behavior.
What I like about this change is also that the generic handling is not a
flags "handle_secrets_for_vpn", but it just says to skip checking the
param-spec flags and directly call the to_dbus_fcn(). It's just a
generally useful thing to do, to let the to_dbus_fcn() function also
handle checking the property flags. The fact that only vpn.secrets
properties uses this for a certain pupose, is abstracted in a way that
makes sense.
The idea was that properties that are implemented via GENDATA still
could have a GObject property. As such, they would be marked with
this flag.
Currently, gendata properties are only implemented by NMSettingEthtool,
and there are no GObject properties where this flag is used. While it
might make sense in theory or in the future, it is unused.
Drop it.
We use the "properties_override" GArray to construct the list of property infos.
But as we append values to the GArray, the buffer grows exponentially and likely
is larger than the actually used number of values.
As this data is kept until the end of the program, let's not waste the over-allocated
memory and instead copy it to a buffer of the right size.
We have too many _properties_override_add*() variants. They basically are all the
same. Drop _properties_override_add_dbus_only() and use _properties_override_add_virt()
instead.
Also, I am always confused by the term "synth". We shouldn't treat
non-GObject-based properties as somehow odd that need to be synthesized.
In total, we register 447 property informations. Out of these,
326 are plain, GObject property based without special implementations.
The NMSettInfoProperty had all function pointers directly embedded,
currently this amounts to 5 function pointers and the "dbus_type" field.
That means, at runtime we have 326 times trivial implementations with
waste 326*6*8 bytes of NULL pointers. We can compact these by moving
them to a separate structure.
Before:
447 * 5 function pointers
447 * "dbus_type" pointer
= 2682 pointers
After:
447 * 1 pointers (for NMSettInfoProperty.property_type)
89 * 6 pointers (for the distinct NMSettInfoPropertType data)
= 981 pointers
So, in total this saves 13608 byes of runtime memory (on 64 bit arch).
The 89 NMSettInfoPropertType instances are the remaining distinct instances.
Note that every NMSettInfoProperty has a "property_type" pointer, but most of them are
shared. That is because the underlying type and the operations are the same.
Also nice is that the NMSettInfoPropertType are actually constant,
static fields and initialized very early.
This change also makes sense form a design point of view. Previously,
NMSettInfoProperty contained both per-property data (the "name") but
also the behavior. Now, the "behavioral" part is moved to a separate
structure (where it is also shared). That means, the parts that are
concerned with the type of the property (the behavior) are separate
from the actual data of the property.
The preferred logo and logotype are the ones in the main "logo" folder.
There are also few variants available in the "alternate" folder that are
allowed.
Main color for the logo is blue #32557dff.
The alternatei logo red is #cc0000ff.
The font is Montserrat.
Thanks to Máirín Duffy for all the help and support!
Only reapply the IP configuration on link up if the IP state is CONF
or DONE. Previously we also reapplied it when the device was
disconnected (IP state NONE) and this could lead to a situation where
an incomplete config was applied; then we intersected the desired
configuration with the external - incomplete - one, causing the
removal of part of desired configuration (for example the default
route).
Fixes: d0b16b9283 ('device: unconditionally reapply IP configuration on link up')
https://bugzilla.redhat.com/show_bug.cgi?id=1754511https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/291
While nmi_cmdline_reader_parse() only has one caller, which indeed has the
argv parameter at hand and doesn't care it to be modified, I think it
is ugly.
Arguments preferably are strictly either input or output arguments,
with input arguments not being modified by the call.
- nm_setting_wired_add_s390_option() asserts that a "value" argument
is given. Check that the string contains a '=' where we can split.
- pass the requested NM_SETTING_WIRED_SETTING_NAME type to get_conn().
Otherwise, @s_wired might be %NULL, resulting in an assertion.
I do wonder whether this always retrieves a connection of the
appropriate type for modification, or whether a profile could
be returned that was created for a different purpose. But that
isn't changed.
- avoid "g_strcmp0 (nettype, "ctc") != 0". I find it unexpected, that we add the
3rd subchannel component, if the nettype is "ctc" (intuitively, I'd expect it
to be the opposite). The reasons for this are not documented, but I
presume it is correct.
Anyway, using streq() makes this slightly more clear to me, as with
strcmp() I would wonder whether this was just a typo while with
streq() I'd be more confident that this is indeed intended.
- don't initialize local variables unnecessarily. The compiler would
warn if we would forget about this. Also, don'\''t use { } for a
one-line block.
syntax: rd.znet=<nettype>,<subchannels>,<options>
The s390 specific options used to create the network interface in the kernel
are currently not processed by nm-initrd-generator causing incomplete ifcfg file.
fixes https://bugzilla.redhat.com/show_bug.cgi?id=1753975
The client tests compare the test output with a .expected file that is
commit to git and that contains the expected output.
The expected output contains data like
size: 395
location: clients/tests/test-client.py:842:test_001()/1
cmd: $NMCLI
lang: C
returncode: 0
stdout: 277 bytes
>>>
...
Note that there is the line number (clients/tests/test-client.py:842) of
the source code where nmcli is called. This is to help correlate the output
with the test code.
However, Python 3.8 changes behavior and for function calls that span multiple
lines, frame.f_lineno will give now the starting line (previously, it gave the last
line) (see [1]).
No longer include the line number, as it is not stable accross Python versions.
If you really care, you can set NM_TEST_WITH_LINENO to get the line numbers back.
Of course, then the expected output won't match anymore, and you'd have to regenerate
it first. This is only useful if you debug tests, and want to have it easier to
correlate output with the tests while developing them.
[1] https://bugs.python.org/issue38283https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/292
It's unclear how to workaround this issue, so that the tests
work with older python versions and 3.8-beta.
Let's wait whether this will really be released as 3.8 and
for now just skip the test.
Note that the only DNS plugin that actually emits the FAILED signal was
NMDnsDnsmasq. Let's not handle restart, retry and rate-limiting by
NMDnsManager but by NMDnsDnsmasq itself.
There are three goals here:
(1) we want that when dnsmasq (infrequently) crashes, that we always keep
retrying. A random crash should be automatically resolved and
eventually dnsmasq should be working again.
Note that we anyway cannot fully detect whether something is wrong.
OK, we detect crashes, but if dnsmasq just gets catatonic, it's just
as broken. Point being: our ability to detect non-working dnsmasq is limited.
(2) when dnsmasq keeps crashing all the time, then rate limit the retry.
Of course, at this point there is already something seriously wrong,
but we shouldn't kill the system by respawning the process without rate
limiting.
(3) previously, when NMDnsManager noticed that the pluging was broken
(and rate-limiting kicked in), it would temporarily disable the plugin.
Basically, that meant to write the real name servers to /etc/resolv.conf
directly, instead of setting localhost. This partly conflicts with
(1), because we want to retry and recover automatically. So what good
is it to notice a problem, resort to plain /etc/resolv.conf for a
short time, and then run into the issues again? If something is really
broken, there is no way but to involve the user to investigate and
fix the issue. Hence, we don't need to concern NMDnsManager with this either.
The only thing that the manager notices is when the dnsmasq binary is not
available. In that case, update() fails right away, and the manager falls back
to configure the name servers in /etc/resolv.conf directly.
Also, change the backoff time from 5 minutes to 1 minute (twice the
burst interval). There is not particularly strong reason for either
choice, I think that if the ratelimit kicks in, then something is
already so wrong that it doesn't matter either way. Anyway, also 60
seconds is long enough to not kill the machine otherwise.
Several points.
- We spawn the dnsmasq process directly. That has several downsides:
- The lifetime of the process is tied to NetworkManager's. When
stopping NetworkManager, we usually also stop dnsmasq. Or we keep
the process running, but later the process is no longer a child process
of NetworkManager and we can only kill it using the pidfile.
- We don't do special sandboxing of the dnsmasq process.
- Note that we want to ensure that only one dnsmasq process is running
at any time. We should track that in a singletone. Note that NMDnsDnsmasq
is not a singleton. While there is only one instance active at any time,
the DNS plugin can be swapped (e.g. during SIGHUP). Hence, don't track the
process per-NMDnsDnsmasq instance, but in a global variable "gl_pid".
- Usually, when NetworkManager quits, it also stops the dnsmasq process.
Previously, we would always try to terminate the process based on the
pidfile. That is wrong. Most of the time, NetworkManager spawned the
process itself, as a child process. Hence, the PID is known and NetworkManager
will get a signal when dnsmasq exits. The only moment when NetworkManager should
use the pidfile, is the first time when checking to kill the previous instance.
That is: only once at the beginning, to kill instances that were
intentionally or unintentionally (crash) left running earlier.
This is now done by _gl_pid_kill_external().
- Previously, before starting a new dnsmasq instance we would kill a
possibly already running one, and block while waiting for the process to
disappear. We should never block. Especially, since we afterwards start
the process also in non-blocking way, there is no reason to kill the
existing process in a blocking way. For the most part, starting dnsmasq
is already asynchronous and so should be the killing of the dnsmasq
process.
- Drop GDBusProxy and only use GDBusConnection. It fully suffices.
- When we kill a dnsmasq instance, we actually don't have to wait at
all. That can happen fully in background. The only pecularity is that
when we restart a new instance before the previous instance is killed,
then we must wait for the previous process to terminate first. Also, if
we are about to exit while killing the dnsmasq instance, we must register
nm_shutdown_wait_obj_*() to wait until the process is fully gone.
We only have two real DNS plugins: "dnsmasq" and "systemd-resolved" (the "unbound"
plugin is very incomplete and should eventually be dropped).
Of these two, only "dnsmasq" spawns a child process. A lot of the logic
for that is in the parent class NMDnsPlugin, with the purpose for that
logic to be reusable.
However:
- We are unlikely to add more DNS plugins. Especially because
systemd-resolved seems the way forward.
- If we happen to add more plugins, then probably NetworkManager
should not spawn the process itself. That causes problems with
restarting the service. Rather, we should let the service manager
handle the lifetime of such "child" processes. Aside separating
the lifetime of the DNS plugin process from NetworkManager's,
this also would allow to sandbox NetworkManager and the DNS plugin
differently. Currently, NetworkManager itself may might need
capabilities only to pass them on to the DNS plugin, or (more likely)
NetworkManager would want to drop additional capabilities for the
DNS plugin (which we would rather not implement ourself, since that
seems job of the service management already).
- The current implementation is far from beautiful. For example,
it does synchronous (blocking) killing of the running process
from the PID file, and it uses PID fils. This is not something
we would want to reuse for other plugins. Also, note that
dnsmasq already spawns the service asynchronosly (of course).
Hence, we should also kill it asynchronously, but that is complicated
by having the logic separated in two different classes while
providing an abstract API between the two.
Move the code to NMDnsDnsmasq. This is the only place that cares about
this. Also, that makes it actually clearer what is happening, by seeing
the lifetime handling of the child proceess all in one place.
For logging, if the plugin fails with update, it should return a reason
that we can log.
Note that both dnsmasq and system-resolved plugins do the update asynchronously
(of course). Hence, usually they never fail right away, and there isn't really
possibility to handle the failure later. Still, we should print something sensible
for that we need information what went wrong.
The plugin name and whether a plugin is caching only depends on the type,
it does not require a virtual function where types would decided depending
on other reasons.
Convert the virtual functions into fields of the class.
The previous to wait-types (NM_SHUTDOWN_WAIT_TYPE_OBJECT and
NM_SHUTDOWN_WAIT_TYPE_CANCELLABLE) both required a GObject/GCancellable,
and the shutdown was automatically unblocked when the object got
destroyed.
Add another wait type NM_SHUTDOWN_WAIT_TYPE_HANDLE, which does not take
an object to wait. Instead, shutdown is indefinitely blocked, until the
user unregisters the handle again. While other wait-types allow to
ignore the handle, this wait-type only makes sense if the user keeps
track of the handle.
Having G_PID_FORMAT macro is useful, but it's only available in
recent glib versions. Add a compat implementation and a test that
our assumptions hold.
