Now that NetworkManager on Fedora 33 and RHEL 9 no longer writes
ifcfg-rh files by default ([1]), ifup/ifdown became less useful.
Possibly users shouldn't use it and it would be fine that new-style profiles
(keyfile) no longer work with these commands. But this is deemed as too
disruptive for users.
Note that our previous ifup/ifdown compat scripts only honored the argument
to be part of the ifcfg filename. That was not what initscripts were doing,
which called `need_config()` function that searched also the contents of
the files. With this extension, ifup/ifdown gets smarter too, to better
guess what the user might have wanted.
Extend the script by making it smarter, and to work with connection profile
names.
With this extension we further solidify ifup/ifdown as part of NetworkManager
command line API. That is problematic, because these tools pollute the
$PATH, by not having a clear NM-specific name. Also, these scripts
should only exist on Fedora/RHEL, which makes their usage non-portable
to other distros.
Also, other distros already ship different tools with name ifup/ifdown.
Extending the use of these scripts is thus undesirable, as it furthers
distro-specific commands.
Still, these arguments seem to not hold and users need to be "helped".
As Fedora users cannot be expected to unlearn "ifup" today, there is no
reason to assume they could in a few years. This likely means we will
never get rid of these scripts.
Also, if we truly would make ifup/ifdown part of NetworkManager, then a better
implementation would be that nmcli honors being called with these names.
That is not done, because nmcli's implementation currently is not as
nice to make that extension trivial (as it should be). It also would
mean to embrace ifup/ifdown officially. A shell script works well enough
as a hack.
[1] https://fedoraproject.org/wiki/Changes/NetworkManager_keyfile_instead_of_ifcfg_rhhttps://bugzilla.redhat.com/show_bug.cgi?id=1954607https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/936
As we keep track of the current name owner, use its unique name
for the D-Bus requests.
We also track when the name owner changes, so at the point when we make
the D-Bus call, the current name owner was still running. We should talk
to it directly. If at the same time, firewalld restarts, we go through
our usual tracking of the name owner and will retry -- but always
talking to the unique name.
nm-sudo and nm-dispatcher are very similar from a high level. Both are D-Bus activated
services that exit on idle and all they do, is to provide a simple D-Bus API with no
objects or properties.
Hence it's not surprising that they follow the same structure.
Rename the code to make them look more similar.
After we released the well-known name (or if we failed to ever request
it), we must exit as fast as possible, so that a new instance can
be started to serve new requests.
At that point, reject new requests because they are targeted against the
unique name, which they should not do (when talking to a D-Bus activated
service that exits on idle, it's important to talk to the well-known
name).
Also, if we receive SIGTERM, start releasing the name. We are told to
shut down, and must do so in a timely manner. Again, new requests shall
not be served by this instance.
Currently we only implmement two operations (Ping() and GetFD()). Both
complete right away. There is no need to register a pending job, if
the job does not get processed asynchronously.
In the future, we may have methods that need asynchronous processing
and where we need to register them as pending job.
If we fail to acquire the well-known name or if we already released it,
we must not accept anymore new requests.
Otherwise, requests directly targeted to the unique name will keep the
process alive, and prevent it from restarting (and serving the
well-known name). Clients really should not talk to the unique name of a
service that exits on idle. If they do, and the service is about to shut
down, then the request will be rejected. After we released the name,
there is now turning back and we should quit fast (only processing the
requests we already have).
Also, if we receive a SIGTERM, then we are requested to quit and should
do so in a timely manner. That means, we will start with releasing the
name. As the service is D-Bus activated, new requests can be served by
the next instance (or if the service is about to be disabled altogether,
they will start failing).
The "only_dirty" parameter to a remove-all() function is odd.
For one, the function is called remove-all, but depending on a parameter
it does not remove all.
Also, setting remove-all(only_dirty=TRUE) means it will remove not
everything, so passing TRUE will remove only parts. That logic seems
confusing.
Avoid that, by removing the parameter from nm_l3cfg_remove_config_all()
and add nm_l3cfg_remove_config_all_dirty().
add a NM_DISPATCHER_ACTION_DHCP_CHANGE_X() macro that can select the
right action based on a parameter.
Also rename the IPv4/IPv6 enum values, so that their naming scheme works
better with the NM_DISPATCHER_ACTION_DHCP_CHANGE_X() macro.
We call add/remove pending actions for every state change.
I think GSList is never the best choice of a data structure. Use a plain
array instead. Keep it sorted, so we can use binary search.
The point is rather special, and the macros themselves are basically
simple wrappers around memmove().
When having a sorted array (for example, a strv array that is searched
using nm_strv_find_binary_search()), then we want to insert/remove
elements at a particular place (via memmove()).
Getting the memmove() arguments is not terribly hard, but hard enough to
add two helper macros for that.
Like nm_utils_addr_family_to_char(), but gives a different treatment to
AF_UNSPEC to return "" instead of 'X'. As such, it also needs to
return a string and not a char.
The advantage of environment variables is that the user can use
`systemctl edit NetworkManager-dispatcher.service` for setting them,
without need to change the ExecStart= line.
Also, enabling debugging from the start is useful, despite that debug
logging can be enabled per-request.
Also, there is a difference whether we want verbose logging or whether
we want to log to stdout. There should be a flag, that only increases the
logging verbosity, but does not change the logging backend.
- exit-on-idle needs to be done correctly. Fix the race, by first
notifying systemd (STOPPING=1), releasing the name, and all the
while continue processing requests.
- don't use g_bus_own_name_on_connection(). That one also listens
to NameLost and NameAcquired signals, but we don't care about those.
systemd will take care to only spawn one process at a time. And
anyway, the well-known name is only important to be reachable, we
don't require it to be functional. We can get the first request
before RequestName completed and we can continue getting requests
after releasing the name.
- use nm_g_timeout_add_source() for millisecond precision of idle timeout.
- schedule the first idle timeout before registering the D-Bus object.
- let the signal handler do nothing, if we are already quitting. In
practice, this only silences the extra logging.
The difference is that nm_g_bus_get_blocking() iterates the GMainContext
of the caller, and thus it can process and handle SIGTERM signals.
Calling g_bus_get_sync() does not iterate the context, and we cannot
handle or detect early cancellation.
Explicitly iterating the context is more flexible, as we can control the
parameters how long we iterate. GMainLoop is essentially a (thread-safe)
iteration around one boolean flag (controlled by g_main_loop_run() and
g_main_loop_quit()). We can maintain that boolean flag ourselves.
GDBus will invoke the method_call callback also for the Get/Set
functions. Thus, we need to check the interface_name and handle
them (actually, there is nothing to handle, no properties exist).
Also, "Ping" method only exists for testing. It is usually not called
in production, so check for "GetFD" first.
