These logging lines are already disabled by default as _LOGt()
is a NOP unless configured --with-more-logging.
However, the logging is still very verbose also for debug-builds
and currently there are no known issues there. Disable the logging
statements (but leave them in so they can easily be enabled).
(cherry picked from commit 4cb845558e)
We handle cloned routes (that have rtm_flags RTM_F_CLONED) differently.
We used to mark such routes by hacking NMIPConfigSource to have a special
value. No longer do this, because it mixes different concepts.
Note that the rt_cloned filed fits into a hole in the aligment
of NMPlatformIPRoute. Thus there is almost no overhead to this
change.
The "source" field of NMPlatformIPRoute (now "rt_source") maps to the
protocol field of the route. The source of NMPlatformIPAddress (now
"addr_source") has no direct equivalent in the kernel.
As their use is different, they should have different names. Also,
the name "source" is used all over the place. Hence give the fields
a more distinct name.
Consider:
unshare -n
ip link add d0 type dummy
ip link add d1 type dummy
ip link set d0 up
ip link set d1 up
ip addr add 192.168.100.5/24 dev d0
ip addr add 192.168.101.5/24 dev d1
ip route add 192.168.200.0/24 via 192.168.100.1
ip monitor &
ip route change 192.168.200.0/24 via 192.168.101.1
#prints 192.168.200.0/24 via 192.168.101.1 dev d1
ip route show
#192.168.100.0/24 dev d0 proto kernel scope link src 192.168.100.5
#192.168.101.0/24 dev d1 proto kernel scope link src 192.168.101.5
#192.168.200.0/24 via 192.168.101.1 dev d1
Note that `ip route change` replaced the exising route. "Replaced" in this
case means: the previous route on device "d0" got removed and a new route
on "d1" was added. However, kernel only sent one RTM_NEWROUTE event, no
RTM_DELROUTE that notifies about this change.
We need to workaround that by re-synching the routes when we receive a
RTM_NEWROUTE notification.
NMPCacheId is a union with fields for all known NMPCacheIdTypes.
Up to now, we always cloned the entire union, computed the hash
over all (possibly unset) fields and used memcmp() unanimously.
That was ok, because NMPCacheId was 16 bytes in total and cache-id
types that consumed less bytes didn't have a large overhead.
Next, we will add a new cache id type which increases the size of
NMPCacheId to 24 bytes. So, while possibly only a fraction of the
instances is that large, they would all have to pay that price.
Change that to consider and clone only those parts of the id
that are actually used.
As we get more NMPCacheIdType values, it's better to have for
each type a pre-declared list of supported types, instead of
iterating over all types and letting _nmp_object_init_cache_id()
figure out that the cache-id-type is unsupported on that object.
Inside container, where we don't use udev we don't receive
any events from udev client. Thus the client only returns
devices when iterating it initially, but no events for newly
added devices that appear later.
Thus, inside containers we don't want to create a udev client
at all.
"NetworkManagerUtils.h" contains a bunch of helper tools for core
daemon ("src/").
Unfortunately, it has dependencies to other parts of core,
such as "nm-device.h" and "nm-platform.h". Split out a part
of tools that are independent so that they can be used without
dragging in other dependencies.
"nm-core-utils.h" should only use libnm-core, "nm-logging.h"
and shared.
"NetworkManagerUtils.h" should provide all "nm-core-utils.h" and
possibly other utilities that have larger dependencies.
There are far too many "flags". Rename the "flags" to "n_ifi_flags"
which reminds to "ifi_flags" in 'struct ifinfomsg', but with a
distinctive "n_" prefix.
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
Downsides:
- Add some additional overhead to manage the index
- The NMPCacheId struct grows to 16 bytes, affecting
hashing performance for all object types.
Still do it, based on the assumption that it doesn't matter
for a low number of interfaces. But the O(1) access time matters
when having lots of interfaces.
We potentially emit a lot of signals. Don't look up the
signal by name because that adds quite some additional
overhead, like peeking for a GQuark.
Instead pass the numeric signal-id directly.
Previsously, _LOGT() could be disabled at compile time. Thus it
was different then the other macros _LOGD(), _LOGI(), etc.
OTOH, _LOGt() was the macro that always was compiled in.
Swap the name of the macros. Now the upper-case macros are always
enabled, while the lower-case macro _LOGt() is enabled depending
on compile configuration.
Previously, we could only set the ingress-qos-mappings/egress-qos-mappings.
Now also cache the mappings and expose them from the platform cache.
Also, support changing the vlan flags not only when creating the vlan
interface.
Instead of using libnl-route-3 library to serialize netlink messages,
construct the netlink messages ourselves.
This has several advantages:
- Creating the netlink message ourself is actually more straight
forward then having an intermediate layer between NM and the kernel.
Now it is immediately clear, how a platform request translates to
a netlink/kernel request.
You can look at the kernel sources how a certain netlink attribute
behaves, and then it's immediately clear how to set that (and vice
versa).
- Older libnl versions might have bugs or missing features for which
we needed to workaround (often by offering a reduced/broken/untested
functionality). Now we can get rid or workaround like _nl_has_capability(),
check_support_libnl_extended_ifa_flags(), HAVE_LIBNL_INET6_TOKEN.
Another example is a libnl bug when setting vlan ingress map which
isn't even yet fixed in libnl upstream.
- We no longer need libnl-route-3 at all and can drop that runtime
requirement, saving some 400k.
Constructing the messages ourselves also gives better performance
because we don't have to create the intermediate libnl object.
- In the future we will add more link-type support which is easier
to support by basing directly on the plain kernel/netlink API,
instead of requiring also libnl3 to expose this functionality.
E.g. adding macvtap support: we already parsed macvtap properties
ourselves because of missing libnl support. To *add* macvtap
support, we also would have to do it ourself (or extend libnl).
Having a static string buffer for convenience is useful not only
for platform. Define the string buffer in NetworkManagerUtils.h,
so that all to-string functions can reuse *one* buffer.
Of course, this has the potential danger, that different
to-string method might reuse the same buffer. Hence, low-level
library functions are adviced to use their own buffer, because
an upper level might already use the global buffer for another
string.
The peer-address (IFA_ADDRESS) can also be all-zero (0.0.0.0).
That is distinct from an usual address without explicit peer-address,
which implicitly has the same peer and local address.
Previously, we treated an all-zero peer_address as having peer and
local address equal. This is especially grave, because the peer is part
of the primary key for an IPv4 address. So we not only get a property of
the address wrong, but we wrongly consider two different addresses as
one and the same.
To properly handle these addresses, we always must explicitly set the peer.
Constructing the libnl3 object only to parse the message
is wasteful. It involves several memory allocations, thread
synchronization and parsing fields that we don't care about.
But moreover, older libnl version might not support all the
fields we are interested in, hence we have workarounds like
_nl_link_parse_info_data(). Certain features might not fully
work unless libnl supports it too (although kernel would).
As we already parse the data ourselves sometimes, just go
all they way and don't construct the intermediate libnl object.
This will allow us to drop the _nl_link_parse_info_data() workarounds
in next commits. That is important, because _nl_link_parse_info_data()
sidesteps our platform cache and is not in sync with the cache (not to
mention the extra work to explicitly refetch the data on every lookup).
Also, it gets us 60% on the way to no longer needing 'libnl-route-3.so'
at all and eventually drop requiring the library.
For link objects, nmp_object_cmp() did not consider the private fields and
thus behaved different then nmp_object_equal(). This is wrong.
Implement nmp_object_equal() based on compare.
Arguably, it is more convenient to use the static buffer as
it saves typing.
But having such a low-level function use a static buffer also
limits the way how to use it. As it was, you could not avoid
using the static buffer.
E.g. you cannot do:
char buf[100];
_LOGD ("nmp-object: %s; platform-link: %s",
nmp_object_to_string (nmpobj, buf, sizeof(buf)),
nm_platform_link_to_string (link));
This will fail for non-obvious reasons because both
to-string functions end up using the same static buffer.
Also change the to-string implementations to accept NULL
as valid and return it as "(null)".
https://bugzilla.gnome.org/show_bug.cgi?id=756427
Kernel allows to add the same IPv4 address that only differs by
peer-address (IFL_ADDRESS):
$ ip link add dummy type dummy
$ ip address add 1.1.1.1 peer 1.1.1.3/24 dev dummy
$ ip address add 1.1.1.1 peer 1.1.1.4/24 dev dummy
RTNETLINK answers: File exists
$ ip address add 1.1.1.1 peer 1.1.2.3/24 dev dummy
$ ip address show dev dummy
2: dummy@NONE: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
link/ether 52:58:a7:1e:e8:93 brd ff:ff:ff:ff:ff:ff
inet 1.1.1.1 peer 1.1.1.3/24 scope global dummy
valid_lft forever preferred_lft forever
inet 1.1.1.1 peer 1.1.2.3/24 scope global dummy
valid_lft forever preferred_lft forever
We must also consider peer-address, otherwise platform will treat
two different addresses as one and the same.
https://bugzilla.gnome.org/show_bug.cgi?id=756356
Allows to enable more-asserts more granularly.
Unfortunately, the old check was "${enable_more_asserts} == "yes", thus
we cannot extend "--enable-more-assert=level" because that would mean
that the same build script cannot set the option on both old and new
NetworkManager.
Thus, add a new option --with-more-asserts=level. If you put the
following in your build script, it will work as expected whether
you build a new or an old version of NetworkManager.
./configure --enable-more-asserts --with-more-asserts=5
The logging macros _LOGD(), etc. are specific to each
file as they format the message according to their context.
Still, they were cumbersome to define and their implementation
was repeated over and over (slightly different at times).
Move the declaration of these macros to "nm-logging.h".
The source file now only needs to define _NMLOG(), and either
_NMLOG_ENABLED() or _NMLOG_DOMAIN.
This reduces code duplication and encourages a common implementation
and usage of these macros.
