I want to add more such accessors, because they are the base for
the corresponding for-each macros.
Add a helper macro _nm_platform_dedup_multi_iter_next() to do that,
which should make it simpler to add these nm_platform_dedup_multi_iter_next*()
functions.
Note that previously these functions were inline functions, now they are
macros. I think there is very little difference here. Also before those
functions could be entirely inlined. By using the macro the result
doesn't really change.
One difference is that we now require an "out" pointer. Previously that
was not required, but I guess it makes little sense otherwise.
We must set these compiler flags independent as to whether this
is a release build or a debug build.
In most cases, we don't differentiate between release and debug build
anyway. Granted, we have "-D more_asserts=100" and set "-O" CFLAGS,
but that is more granular and not a simple "buildtype".
In particular, these compiler flags apply to all kinds of builds.
This is important, because otherwise we get build failures, because
also in release build we want to build with `-Werror` and `-Wall`.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/692
(cherry picked from commit c0c6470e4d)
We must set these compiler flags independent as to whether this
is a release build or a debug build.
In most cases, we don't differentiate between release and debug build
anyway. Granted, we have "-D more_asserts=100" and set "-O" CFLAGS,
but that is more granular and not a simple "buildtype".
In particular, these compiler flags apply to all kinds of builds.
This is important, because otherwise we get build failures, because
also in release build we want to build with `-Werror` and `-Wall`.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/692
Revert this change. One problem is that none of the current GUIs
(nm-connection-editor, gnome-control-center, plasma-nm) expose the
dns-priority option. So, users tend to have their profile value set to
0. Changing the default means for them not only a change in behavior,
but its hard to fix via the GUI.
Also, what other call DNS leaks, is Split DNS to some. Both uses make
sense, but have conflicting goals. The default cannot accommodate both
at the same time.
Also, with split DNS enabled (dnsmasq, systemd-resolved), the concern
for DNS leaks is smaller. Imagine:
Wi-Fi profile with ipv4.dns-priority (effectively) 100, domain "example.com".
VPN profile with ipv4.dns-priority (effectively) 50 and a default route.
That is a common setup that one gets by default (and what probably many
users have today). In such a case with split DNS enabled, the Wi-Fi's DNS
server only sees requests for "*.example.com". So, it does not leak
everything.
Hence, revert this change before 1.28.0 release to the earlier behavior.
This reverts commit af13081bec.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/688
(cherry picked from commit ff71bbdc42)
Revert this change. One problem is that none of the current GUIs
(nm-connection-editor, gnome-control-center, plasma-nm) expose the
dns-priority option. So, users tend to have their profile value set to
0. Changing the default means for them not only a change in behavior,
but its hard to fix via the GUI.
Also, what other call DNS leaks, is Split DNS to some. Both uses make
sense, but have conflicting goals. The default cannot accommodate both
at the same time.
Also, with split DNS enabled (dnsmasq, systemd-resolved), the concern
for DNS leaks is smaller. Imagine:
Wi-Fi profile with ipv4.dns-priority (effectively) 100, domain "example.com".
VPN profile with ipv4.dns-priority (effectively) 50 and a default route.
That is a common setup that one gets by default (and what probably many
users have today). In such a case with split DNS enabled, the Wi-Fi's DNS
server only sees requests for "*.example.com". So, it does not leak
everything.
Hence, revert this change before 1.28.0 release to the earlier behavior.
This reverts commit af13081bec.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/688
With glib2-2.67.0-1.fc34.x86_64.rpm, clang-11.0.0-2.fc34.x86_64.rpm, the
generated code emits a compiler warning:
libnm-core/tests/nm-core-tests-enum-types.c:17:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
if (g_once_init_enter (&g_define_type_id__volatile))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter'
(!g_atomic_pointer_get (location) && \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get'
__atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \
^~~~~~~~~~~~~~~~~
libnm-core/tests/nm-core-tests-enum-types.c:40:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
if (g_once_init_enter (&g_define_type_id__volatile))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter'
(!g_atomic_pointer_get (location) && \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get'
__atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \
^~~~~~~~~~~~~~~~~
libnm-core/tests/nm-core-tests-enum-types.c:63:7: error: passing 'typeof (*(&g_define_type_id__volatile)) *' (aka 'volatile unsigned long *') to parameter of type 'gsize *' (aka 'unsigned long *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
if (g_once_init_enter (&g_define_type_id__volatile))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gthread.h:260:7: note: expanded from macro 'g_once_init_enter'
(!g_atomic_pointer_get (location) && \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gatomic.h:112:38: note: expanded from macro 'g_atomic_pointer_get'
__atomic_load (gapg_temp_atomic, &gapg_temp_newval, __ATOMIC_SEQ_CST); \
^~~~~~~~~~~~~~~~~
We could pass "-Wincompatible-pointer-types-discards-qualifiers" as CFLAGS
when building this file. However, we have a workaround in our "nm-glib-aux/nm-glib.h",
so we can instead include "nm-default.h". At first glance, that might look like
the less preferable solution. However, this file is only there for unit tests,
and we also include "nm-default.h" for other sources that are generated with
"glib-mkenums". So, doing it also for our tests becomes the preferable solution.
(cherry picked from commit 755d97d38c)
When including <glib.h>, it will always define G_LOG_DOMAIN if it
is not yet defined.
Usually we want to include "nm-default.h" as very first header. In that
case, <glib.h> is not yet included. Then the previous check #error works
well.
However, if we include "nm-default.h" in sources generated by
glib-mkenums, then the generator first already includes <glib.h>,
and thus defines G_LOG_DOMAIN. It does so for "libnm-core/nm-core-enum-types.c"
and "libnm/nm-enum-types.c", where the #error would not trigger.
But we will also include "nm-default.h" for "libnm-core/tests/nm-core-tests-enum-types.c".
That will start triggering this #error.
While in general we want to include "nm-default.h" first, we also need
to support cases where <glib.h> gets included first. Thus this error is
not useful. Remove it.
(cherry picked from commit 42fa8f3d27)
It's not strictly necessary, because contrary to g_atomic_pointer_get()
and g_atomic_pointer_compare_and_exchange(), glib's variant for the
setter is mostly fine.
Still, reimplement it, because we use typeof() eagerly and can thus add
more static checks than glib.
(cherry picked from commit 7c60e984b6)
It seems it can happen that the service is not yet unregistered from the
D-Bus broker, even if we already reaped the PID.
/builds/NetworkManager/NetworkManager/tools/run-nm-test.sh --called-from-make /builds/NetworkManager/NetworkManager/build --launch-dbus=auto /builds/NetworkManager/NetworkManager/build/libnm/tests/test-nm-client
--- stdout ---
/libnm/device-added:
nmtst: initialize nmtst_get_rand() with NMTST_SEED_RAND=0
--- stderr ---
**
test:ERROR:../shared/nm-test-utils-impl.c:216:nmtstc_service_cleanup: assertion failed: (!name_exists(info->bus, "org.freedesktop.NetworkManager"))
Workaround by waiting a bit.
We now iterate the main GMainContext, unlike before. But that
should not cause any problems for the test.
(cherry picked from commit 1b8ccacc5d)
nmtst_main_context_iterate_until() is a macro, and we don't want to restrict the
valid integer type (or range) of the "timeout_msec" argument.
In particular, if the user calculates a timeout with "timestamp_msec -
now_msec", the resulting "timeout_msec" might be a negative gint64.
We should handle that gracefully, and not let it be cast to a huge
unsigned int.
(cherry picked from commit 6cb6888404)
We used to set "~." domains for all devices that should be used for
resolving unknown domains.
Systemd-resolved also supports setting "SetLinkDefaultRoute()".
We should only set the wildcard domain if we want that this
interface is used exclusively. Otherwise, we should only set
DefaultRoute. See ([1], [2], [3], [4]).
Otherwise the bad effect is if other components (wg-quick) want
to set exclusive DNS lookups on their link. That is achieved by
explicitly adding "~." and that is also what resolved's
`/usr/sbin/resolvconf -x` does. If NetworkManager sets "~." for
interfaces that are not important and should not be used exclusively,
then this steals the DNS requests from those external components.
In NetworkManager we know whether a link should get exclusive lookups
based on the "ipv[46].dns-priority" setting.
[1] https://www.freedesktop.org/software/systemd/man/org.freedesktop.resolve1.html
[2] https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html
[3] https://github.com/systemd/systemd/issues/17529#issuecomment-730522444
[4] https://github.com/systemd/systemd/pull/17678
(cherry picked from commit ee9fab0361)
domain_is_shadowed() only works, because we pre-sort all items. When
we call domain_is_shadowed(), then "priority" must be not smaller than
any priority already in the dictionary.
Let's add an nm_assert() for that.
While at it, I also found it ugly to rely on
GPOINTER_TO_INT(g_hash_table_lookup(ht, domain))
returning zero to know whether the domain is tracked. While more
cumbersome, we should check whether the value is in the hash (and not).
Not whether the value does not translate to zero.
Add domain_ht_get_priority() for that.
(cherry picked from commit 5902f1c91f)
We used to set "~." domains for all devices that should be used for
resolving unknown domains.
Systemd-resolved also supports setting "SetLinkDefaultRoute()".
We should only set the wildcard domain if we want that this
interface is used exclusively. Otherwise, we should only set
DefaultRoute. See ([1], [2], [3], [4]).
Otherwise the bad effect is if other components (wg-quick) want
to set exclusive DNS lookups on their link. That is achieved by
explicitly adding "~." and that is also what resolved's
`/usr/sbin/resolvconf -x` does. If NetworkManager sets "~." for
interfaces that are not important and should not be used exclusively,
then this steals the DNS requests from those external components.
In NetworkManager we know whether a link should get exclusive lookups
based on the "ipv[46].dns-priority" setting.
[1] https://www.freedesktop.org/software/systemd/man/org.freedesktop.resolve1.html
[2] https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html
[3] https://github.com/systemd/systemd/issues/17529#issuecomment-730522444
[4] https://github.com/systemd/systemd/pull/17678
domain_is_shadowed() only works, because we pre-sort all items. When
we call domain_is_shadowed(), then "priority" must be not smaller than
any priority already in the dictionary.
Let's add an nm_assert() for that.
While at it, I also found it ugly to rely on
GPOINTER_TO_INT(g_hash_table_lookup(ht, domain))
returning zero to know whether the domain is tracked. While more
cumbersome, we should check whether the value is in the hash (and not).
Not whether the value does not translate to zero.
Add domain_ht_get_priority() for that.
Change the generator to disable by default IP configuration for the
parent connection of a VLAN, because that is what a user would expect
and what the legacy module does. Of course if the user explicitly
configures DHCP or an address for the parent interface, that overrides
the default.
Note that now the generator always creates a connection for the parent
interface. Before this commit, it did only when there was an explicit
ip= argument for the parent interface.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/509
The command line parser looks for a dot or a colon to determine
whether the first token in a ip= argument is a IPv4 address (dot), an
IPv6 address (colon) or an interface name (none). This strategy
doesn't work for interface names containing a dot (typically VLANs).
Instead, try to parse the IPv4/IPv6 address in the token; if this
fails then consider the token as an interface name.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/581
The caller *always* needs to know whether the argument
is an address in binary or text from. At that point,
it's only inconvenient to require the user to either
pass "-1" or ETH_ALEN as size (nothing else was supported
anyway).
Split the function and rename.
