It's useful to track the flags for IPv4 addresses too.
- we might want to sort IPv4 addresses according to whether they
are permanent or dynamic.
- later we want to set IFA_F_NOPREFIXROUTE also for IPv4 addresses.
While the ability to "set" a flag doesn't necessarily require that we
also keep the flags present in NMPlatformIP4Address, it is more consistent.
"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.
"nm-logging.h" is a basic core module that should have few other
dependencies. Instead of letting nm-logging.c directly call a function
from "nm-linux-platform.c", let platform register a handler as needed.
This way, you can build a core library containing nm-logging but no
nm-platform.
There are far too many "flags". Rename the "flags" to "n_ifa_flags"
which reminds to "ifa_flags" in 'struct ifaddrmsg', but with a
distinctive "n_" prefix.
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.
The value written to sysctl is usually a short string. It makes sense
to optimize for this case and avoid allocating a temporary string
on the heap.
An alternative would be to use writev(), which effectively does the same
and also creates a temporary buffer (preferably stack allocated).
https://mail.gnome.org/archives/networkmanager-list/2016-February/msg00070.html
- 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
If we break the loop normally, @err must be already set to zero.
The only other way this can happen is when the credentials are
invalid. Move setting @err to there.
If @handle_events is FALSE, we want to drain the socket. In that case
even when encountering an error error we don't want to abort, but instead
continue reading the next message.
@abort_parsing is set TRUE at two places, which also explicitly
set @err to something. We don't want to reset @err and got to the
next @hdr. Instead error out first.
Often a netlink event doesn't contain enough information to determine
the link type. Then we consult sysctl or ethtool. However, if we already
have the same object cached, we want to reused the (once detected) link-type.
There was a bug in lookup of the cached object.
Due to a kernel bug [1], we sometimes receive spurious NEWLINK
messages after a wifi interface has disappeared. Since the link is not
present anymore we can't determine its type and thus it will show up
as a Ethernet one, with no address specified. Request the link again
to check if it really exists.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1302037https://bugzilla.gnome.org/show_bug.cgi?id=761151
We detect support for IPv6 temporary addresses (IFA_F_MANAGETEMPADDR) or /64 v6 prefixes
(IFA_F_NOPREFIXROUTE) based on the presence of extended address flags. For the most part
this just works, but it fails down if upon initialization no addresses are present.
In such a case we would have assumed no support. Change that to default to available
support as the feature is already 2 years in upstream kernel.
When the receive buffer is too small, we easily can hit ENOBUFS during recvmsg()
and need to resync the platform cache. But even worse, we possibly also loose
ACKs for pending netlink requests so that requests seem to fail (although they
might have succeeded).
Avoid that harder by increasing the buffer size to 8MB. This is also
done by networkd:
be660c371b
When reading from netlink-socket fails with NLE_NOMEM, messages were
lost. In this case the cache must be resynced and all pending messages
from the socket are rejected via "event_handler_recvmsgs (platform, FALSE)".
In this case, we don't want to handle the received messages as the
cache anyway needs to resync. However, we are still interested in
all queued ACKs that are there.
We are also interested in RTM_NEWADDR messages which we use to detect
kernel support via _support_kernel_extended_ifa_flags_detect().
Coverity: CID 59378 (#1 of 1): Nesting level does not match indentation
(NESTING_INDENT_MISMATCH) This statement is indented to column 41, as if
it were nested within the preceding parent statement, but it is not.
When deleting an object, we allow failure to delete a non-existing object.
Thus, the only thing we care about is whether the object is no longer
present after deletion. Adjust the return values to reflect that.
The idea was allowing pending delayed-actions and process them in an
idle handler. We dont want to do that, because whenever platform code
returns, we want to have no pending actions -- because otherwise the
platform cache might be in an inconsistent state.
Just drop it.
We would not expect that delayed_action_handle_all() is called recursively.
Assert against that. If we ever happen to call it recursively, we would
need to take care of properly avoiding infinite loops or deep call
stacks.
Track pending netlink requests and properly report the resulting errno
back.
Currently we send only requests in do_request_link() and do_request_all().
These callers don't actually care about the result, all they care that the
request is answered before returning back from platform code to the caller.
Thus, up to now the tracking of the sequence number was pretty simple.
Later we also want to get the errno from a request, thus rework sending
requests to also remember about outstanding sequence numbers and
properly track muliple parallel requests.
Later the synchronous actions (e.g. add-link) will also be handled
via the asynchronous socket.
