The underscore somehow indicated that these would be an internal
function. Which they are in the sense that they are in "shared/nm-glib-aux/".
But they part of our internal helper functions, and in our code base
their use is no discouraged or "private.
Also, next I'll replace the function call with a macro, so, I will
have a need for the underscore name.
Rename.
Run:
./contrib/scripts/nm-code-format.sh -i
./contrib/scripts/nm-code-format.sh -i
Yes, it needs to run twice because the first run doesn't yet produce the
final result.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Seems with LTO the compiler can sometimes think that thes variables are
uninitialized. Usually those code paths are only after an assertion was
hit (g_return*()), but we still need to workaround the warning.
On Fedora/RHEL, the default for main.plugins is "ifcfg-rh". You would
expect that a single configuration file
[main]
plugins-=ifcfg-rh
would result in an empty list of plugins (which subsequently gets
completed with "keyfile").
That didn't happen due to a bug. Fix it.
The _GET_PRIVATE() macros are all implemented based on
_NM_GET_PRIVATE(). That macro tries to be more type safe and uses
_Generic() to do the right thing. Explicitly casting is not only
unnecessary, it defeats these (static) type checks.
Don't do that.
We should use the same "is-valid" function everywhere.
Since nm_utils_ipaddr_valid() is part of libnm, it does not qualify.
Use nm_utils_ipaddr_is_valid() instead.
Keyfile support was initially added under GPL-2.0+ license as part of
core. It was moved to "libnm-core" in commit 59eb5312a5 ('keyfile: merge
branch 'th/libnm-keyfile-bgo744699'').
"libnm-core" is statically linked with by core and "libnm". In
the former case under terms of GPL-2.0+ (good) and in the latter case
under terms of LGPL-2.1+ (bad).
In fact, to this day, "libnm" doesn't actually use the code. The linker
will probably remove all the GPL-2.0+ symbols when compiled with
gc-sections or LTO. Still, linking them together in the first place
makes "libnm" only available under GPL code (despite the code
not actually being used).
Instead, move the GPL code to a separate static library
"shared/nm-keyfile/libnm-keyfile.la" and only link it to the part
that actually uses the code (and which is GPL licensed too).
This fixes the license violation.
Eventually, it would be very useful to be able to expose keyfile
handling via "libnm". However that is not straight forward due to the
licensing conflict.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/381
We always build with PolicyKit support enabled, because it has no
additional dependencies, beside some D-Bus calls.
However, in NetworkManager.conf the user could configure
"main.auth-polkit" to disable PolicyKit. However, previously it would
only allow to disable PolicyKit while granting access to all users.
I think it's useful to have an option that disables PolicyKit and grants
access only to root. I think we should not go too far in implementing
our own authorization mechanisms beside PolicyKit (e.g. you cannot
disable PolicyKit and grant access based on group membership of the
user). However, disabling PolicyKit can be useful sometimes, and it's
simple to implement a "root-only" setup.
Note one change is that when NetworkManager now runs without a D-Bus
connection (in initrd), it would deny all non-root requests. Previously
it would grant access. I think there should be little difference in
practice, because if we have no D-Bus we also don't have any requests to
authenticate.
(cherry picked from commit 6d7446e52f)
- don't let no_auto_default_from_file() do any preprocessing of
the lines that it reads. It merely splits the lines at '\n'
and utf8safe-unescapes them.
This was previously duplicated also by NMConfigData's property
setter. We don't need to do it twice.
- sort the lines. This makes the entire handling O(n*ln(n)) instead
of O(n^2). Also, sorting effectively normalizes the content, and
it's desirable to have one true representation of what we write.
For devices that have no real MAC address (virtual devices) it makes no
sense to store the MAC address to "no-auto-default.state" file. Also,
because we later would not match the MAC address during
nm_match_spec_device().
Instead, extend the format and add a "interface-name:=$IFACE" match-spec.
Maybe we generally should prefer the interface-name over the MAC
address. Anyway, for now, just extend the previously non-working case.
The define is better, because then we can grep for all the occurances
where they are used. The plain text like "mac:" is not at all unique in
our source-tree.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
Each time when enabling/disabling "systemd-resolved" in combination with another
plugin (which is unchanged), another pair of signal handlers was
connected. That's wrong.
Fixes: d4eb4cb45f
Add a new CON_DEFAULT() macro that places a property name into a
special section used at runtime to check whether it is a supported
connection default.
Unfortunately, this mechanism doesn't work for plugins so we have to
enumerate the connection defaults from plugins in the daemon using
another CON_DEFAULT_NOP() macro.
The need for this is the following:
"ipv4.dhcp-client-id" can be specified via global connection defaults.
In absence of any configuration in NetworkManager, the default depends
on the DHCP client plugin. In case of "dhclient", the default further
depends on /etc/dhcp.
For "internal" plugin, we may very well want to change the default
client-id to "mac" by universally installing a configuration
snippet
[connection-use-mac-client-id]
ipv4.dhcp-client-id=mac
However, if we the user happens to enable "dhclient" plugin, this also
forces the client-id and overrules configuration from /etc/dhcp. The real
problem is, that dhclient can be configured via means outside of NetworkManager,
so our defaults shall not overwrite defaults from /etc/dhcp.
With the new device spec, we can avoid this issue:
[connection-dhcp-client-id]
match-device=except:dhcp-plugin:dhclient
ipv4.dhcp-client-id=mac
This will be part of the solution for rh#1640494. Note that merely
dropping a configuration snippet is not yet enough. More fixes for
DHCP will follow. Also, bug rh#1640494 may have alternative solutions
as well. The nice part of this new feature is that it is generally
useful for configuring connection defaults and not specifically for
the client-id issue.
Note that this match spec is per-device, although the plugin is selected
globally. That makes some sense, because in the future we may or may not
configure the DHCP plugin per-device or per address family.
https://bugzilla.redhat.com/show_bug.cgi?id=1640494
Even when the system resolver is configured to something else that
systemd-resolved, it still is a good idea to keep systemd-resolved up to
date. If not anything else, it does a good job at doing per-interface
resolving for connectivity checks.
If for whatever reasons don't want NetworkManager to push the DNS data
it discovers to systemd-resolved, the functionality can be disabled
with:
[main]
systemd-resolved=false
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[@]}"
- consistently set options, searches, domains fields to %NULL,
if there are no values.
- in nm_global_dns_config_update_checksum(), ensure that we uniquely
hash values. E.g. a config with "searches[a], options=[b]" should
hash differently from "searches=[ab], options=[]".
- in nm_global_dns_config_to_dbus(), reuse the sorted domain list.
We already have it, and it guarantees a consistent ordering of
fields.
- in global_dns_domain_from_dbus(), fix memleaks if D-Bus strdict
contains duplicate entries.
nm_match_spec_device_by_pllink() does not support matching on all parameters,
unlike nm_match_spec_device(). The reason is that certain parameters are
only available when having a NMDevice instance.
Add an argument "match_device_type", so that the caller can inject the
device type to be used. Note that for NMDevice, the device-type is
nm_device_get_type_description(), which usually depends on the device
class only. The only caller of nm_match_spec_device_by_pllink() is the
wifi factory, and it already knows that it wants to create a device of
type NMDeviceWifi. Hence, it knows and can specify "wifi" as
match_device_type.
... by platform link.
One caveat is that without having a NMDevice instance, matching by
several paramters won't work. Like, matching against the driver would
require us to look it up via ethtool. When having an NMDevice instance,
the driver is cached there, it's unclear we want to call ethtool for
lookup in this case -- though it could be done.
For other options, it's more complicated. Like, the type basically
depends on the NMDevice class. Usually that also works without a
netdev known to kernel (like bluetooth).
The inconsistency that certain matches are not implemented is ugly
indeed. But the effect is as if the spec doesn't match.
It's not ever going to change(*), and NMPolicy calls reset() a lot.
No need to lookup the configuration in the GKeyFile every time.
(*) per NMConfigData instance. The config may be reloaded, in which
case NMConfig creates a new NMConfigData instance, but the NMConfigData
instance itself is immutable.
It has almost no callers, and it is a bit of a strange API. Let's
not cache the last accessed value inside NMConfigData. Instead, free
it right after use. It was not reused anyway, it only hangs around
as convenience for the caller.
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
Currently, device types like Bond hack around ignore-carrier
setting, as they always want to ignore-carrier.
Prepare so that also for such master types, we rely and honor the
ignore-carrier setting better. In the next commit, bond, bridge and
team devices they will get ignore-carrier turned on by default.
main() should pass the same atomic-section-prefix setting to it's
NMConfig instances. Currently both are NULL, but make it a define
to make this explicit.
Also, make static array @default_values const and sanitize value
when setting PROP_ATOMIC_SECTION_PREFIXES property.
