Extend the lame-ass dhclient.conf parser to ignore the blocks we can't
do anything useful about: alias{}, pseudo{} and even lease{}.
Note that there's still a lot of cases we can't handle without a
full-fledged dhclient.conf parser -- notably the files that don't use
line breaks to separate the statements.
That is probably okay -- the whole thing is probably mostly useless and
we shall ever bother only about cases that actually cause trouble.
https://github.com/NetworkManager/NetworkManager/pull/153
We used to build with -std=gnu89 so commit 1391bdfa61
added a local patch to systemd code to avoid compilation error due to
missing __STDC_VERSION__ define.
In the meantime, since commit ba2b2de3ad
and commit b9bc20f4da, we also use -std=gnu99
and thus __STDC_VERSION__ is defined.
Revert our local modification.
1) the command line gets shorter. I frequently run `make V=1` to see
the command line arguments for the compiler, and there is a lot
of noise.
2) define each of these variables at one place. This makes it easy
to verify that for all compilation units, a particular
define has the same value. Previously that was not obvious or
even not the case (see commit e5d1a71396
and commit d63cf1ef2f).
The point is to avoid redundancy.
3) not all compilation units need all defines. In fact, most modules
would only need a few of these defines. We aimed to pass the necessary
minium of defines to each compilation unit, but that was non-obvious
to get right and often we set a define that wasn't used. See for example
"src_settings_plugins_ibft_cppflags" which needlessly had "-DSYSCONFDIR".
This question is now entirely avoided by just defining all variables in
a header. We don't care to find the minimum, because every component
gets anyway all defines from the header.
4) this also avoids the situation, where a module that previously did
not use a particular define gets modified to require it. Previously,
that would have required to identify the missing define, and add
it to the CFLAGS of the complation unit. Since every compilation
now includes "config-extra.h", all defines are available everywhere.
5) the fact that each define is now available in all compilation units
could be perceived as a downside. But it isn't, because these defines
should have a unique name and one specific value. Defining the same
name with different values, or refer to the same value by different
names is a bug, not a desirable feature. Since these defines should
be unique accross the entire tree, there is no problem in providing
them to every compilation unit.
6) the reason why we generate "config-extra.h" this way, instead of using
AC_DEFINE() in configure.ac, is due to the particular handling of
autoconf for directory variables. See [1].
With meson, it would be trivial to put them into "config.h.meson".
While that is not easy with autoconf, the "config-extra.h" workaround
seems still preferable to me.
[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html
Also, "src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-common.h"
already has a define IFCFG_DIR, but with a different value.
We shouldn't name different things the same.
We previously kept any acd-manager running if the device was
disconnected. It was possible to trigger a crash by setting a long
dad-timeout and interrupting the activation request:
nmcli con add type ethernet ifname eth0 con-name eth0+ ip4 1.2.3.4/32
nmcli con mod eth0+ ipv4.dad-timeout 10000
nmcli -w 2 con up eth0+
nmcli con down eth0+
After this, the n-acd timer would fire after 10 seconds and try to
disconnect an already disconnected device, throwing the assertion:
NetworkManager:ERROR:src/devices/nm-device.c:9845:
activate_stage5_ip4_config_result: assertion failed: (req)
Fixes: 28f6e8b4d2
We commonly don't use the glib typedefs for char/short/int/long,
but their C types directly.
$ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l
587
$ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l
21114
One could argue that using the glib typedefs is preferable in
public API (of our glib based libnm library) or where it clearly
is related to glib, like during
g_object_set (obj, PROPERTY, (gint) value, NULL);
However, that argument does not seem strong, because in practice we don't
follow that argument today, and seldomly use the glib typedefs.
Also, the style guide for this would be hard to formalize, because
"using them where clearly related to a glib" is a very loose suggestion.
Also note that glib typedefs will always just be typedefs of the
underlying C types. There is no danger of glib changing the meaning
of these typedefs (because that would be a major API break of glib).
A simple style guide is instead: don't use these typedefs.
No manual actions, I only ran the bash script:
FILES=($(git ls-files '*.[hc]'))
sed -i \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\> /\1 /g' \
-e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \
"${FILES[@]}"
If a VPN with default route is activated, the Manager's
PrimaryConnection property is not updated to indicate the VPN as
primary connection.
This happens because the PrimaryConnection property gets updated when
the default_ipX_device property of NMPolicy changes, and the primary
connection is set to the activation request currently pending on the
default device. We select the base (for example, ethernet) device as
best device and therefore the NMActRequest active on it is selected as
primary connection.
This patch fixes the problem by properly selecting the VPN as
primary. It seems a better choice to track best active connections
directly from NMPolicy instead of going through two steps.
Commit 10753c3616 ("manager: merge VPN handling into
_new_active_connection()") added a check to fail the activation of
VPNs when a device is passed to ActivateConnection(), since the device
argument is ignored for VPNs.
This broke activating VPNs from nm-applet as nm-applet sets both the
specific_object (parent-connection) and device arguments in the
activation request.
Note that we already check in _new_active_connection() that when a
device is supplied, it matches the device of the parent
connection. Therefore, the check can be dropped.
Reported-by: Michael Biebl <biebl@debian.org>
Fixes: 10753c3616https://github.com/NetworkManager/NetworkManager/pull/159
The following commit:
b869d9cc0 device: add spec "driver:" to match devices
added two parameters ("driver" and "driver_version") to the
nm_match_spec_device() function.
However, the definition of the function and its declaration are not
consistent.
The prototype shows:
nm_match_spec_device (const GSList *specs,
const char *interface_name,
const char *driver,
const char *driver_version,
const char *device_type,
But the definition shows:
nm_match_spec_device (const GSList *specs,
const char *interface_name,
const char *device_type,
const char *driver,
const char *driver_version,
Since all parameters are pointers to const char, the type checking
succeeds at compile time.
All currently existing invocations of the function are correct and pass
the arguments in the order described in the definition/implementation.
This patch only changes the prototype so that potential future
invocations don't end up buggy.
Fixes: b869d9cc0d
gretap and ip6gretap ip-tunnel interfaces encapsulate L2 packets over
IP. Allow adding a wired setting for such connections so that users
can change the interface MAC.
Add platform support for IP6GRE and IP6GRETAP tunnels. The former is a
virtual tunnel interface for GRE over IPv6 and the latter is the L2
variant.
The platform code internally reuses and extends the same structure
used by IPv6 tunnels.
Certain platform operations are logged both in nm-platform.c and
nm-linux-platform.c, resulting in duplicate messages. Drop log prints
from the latter.
Add support for a new wireguard link type to the platform code. For now
this only covers querying existing links via genetlink and parsing them
into platform objects.
This disables loading the system-wide dnsmasq from /etc/dnsmasq.conf
and defines to use the NMSTATEDIR device-unique dhcp-leasefile,
preventing it from trampling over others, and isolating it to just
the wifi-ap use.
https://github.com/NetworkManager/NetworkManager/pull/156
In device_ipx_changed() we only keep track of dad6_failed_addrs
addresses if the device's state is > DISCONNECTED.
For the same reason, we should also do that in queued_ip_config_change().
But it's worse. If the device is in state disconnected, and the user
externally adds IPv6 addresses, we will end up in queued_ip_config_change().
It is easily possible that "need_ipv6ll" ends up being TRUE, which results
in a call to check_and_add_ipv6ll_addr() and later possibly
ip_config_merge_and_apply (self, AF_INET6, TRUE);
This in turn will modify the IP configuration on the device, although
the device may be externally managed and NetworkManager shouldn't touch it.
https://bugzilla.redhat.com/show_bug.cgi?id=1593210
We first iterate over addresses that might have failed IPv6 DAD and
update the state in NMNDisc.
However, while we do that, don't yet invoke the changed signal.
Otherwise, we will invoke it multiple times (in case multiple addresses
failed). Instead, keep track of whether something changed, and handle
it once a bit later.
Whenever we process queued IP changes, we must handle all pending
dad6_failed_addrs. This is, to ensure we don't accumulate more
and more addresses in the list.
Rework the code, by stealing the entire list once at the beginning
dad6_failed_addrs = g_steal_pointer (&priv->dad6_failed_addrs);
and free it at the end:
g_slist_free_full (dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
This makes it easier to see, that we always process all addresses in
priv->dad6_failed_addrs.
There is no change in behavior, however don't handle dad6_failed_addrs
and dad6_ip6_config in the same block.
While both parts are related to IPv6 DAD, they do something rather
different:
- the first block, checks all candidates from dad6_failed_addrs whether
they actually indicate DAD failed, and handles them by notifying
NMNDisc about failed addresses.
- the second block, checks whether we have now all addresses from
dad6_ip6_config that we are waiting for.
Split the blocks.
We don't need to cancel the current idle-action and schedule a new
one. Just return and wait to be called again.
Also, drop the logging. Similarly, we don't log the postponing for
the previous case either.
We also cancel the idle handler
nm_clear_g_source (&priv->queued_ip_config_id_x[IS_IPv4])
which means, nobody is going to process these addresses (at least
for the moment).
The purpose of "dad6_failed_addrs" is to keep track of addresses that
might be interesting for checking about DAD failures. If we are no
longer reacting on IP changes (because the idle handler was removed),
we also no longer need these addresses.
This simplifies commit 31ca7962f8.
We don't need the boolean flags like "queued_ip4_config_pending" to
track whether we received any platform signals while being not yet
initialized in platform (udev, NM_UNMANAGED_PLATFORM_INIT).
In general, as long as the device is NM_UNMANAGED_PLATFORM_INIT,
all platform signals are ignored. And when the device becomes managed,
we schedule anyway an initial config-change.
