It is wrong that nm_ip4_config_set_mtu() tries to ~merge~ the new MTU
with the existing. All callers of nm_ip4_config_set_mtu() want that the
new value prevails.
That is also already the case because the DHCP clients and PPP manager set
the MTU on a newly created NMIP4Config instance, thus their value is taken.
Similarly, the final merge with NM_IP_CONFIG_SOURCE_USER also prevails as the
source has the highest priority.
The setter should just set. The only place where we want the merge behavior
is in nm_ip4_config_merge(), where it is now implemented in-place.
For example, nm_ip4_config_replace() very much wants that the new value
wins, regardless of the previous setting. Using nm_ip4_config_set_mtu()
with the merge behavior was wrong because it means that the MTU of NMDevice's
composite can never be raised again (for example with a new DHCP event).
bool:1 bitfields allow for tighter packing and are guaranteed to be
strictly 0 or 1 (contrary to gboolean's typedef for int). Not that it
matters too much, but it's favorable.
Especially, because each device has several of these ip-config instances,
we might save a few bytes for no(?) downsides.
When multiple address are assigned to an interface and the kernel must
decide which one should be used to communicate with a given IP, it
chooses the most specific one in the same subnet as the
destination. In case there are multiple addresses in the same subnet,
the primary address is choosen, which is basically the first one that
was added.
With commit 7197425137 ("device: expose NMIP4Config:addresses in
stable/defined sort order") we sorted all the addresses before
committing the configuration, with the side effect that the order no
longer respected the one in the user configuration.
Instead, change the sort function to keep the subnet order unchanged.
This makes it easier to install the files with proper names.
Also, it makes the makefile rules slightly simpler.
Lastly, the documentation is now generated into docs/api, which makes it
possible to get rid of the awkward relative file names in docbook.
Keep the include paths clean and separate. We use directories to group source
files together. That makes sense (I guess), but then we should use this
grouping also when including files. Thus require to #include files with their
path relative to "src/".
Also, we build various artifacts from the "src/" tree. Instead of having
individual CFLAGS for each artifact in Makefile.am, the CFLAGS should be
unified. Previously, the CFLAGS for each artifact differ and are inconsistent
in which paths they add to the search path. Fix the inconsistency by just
don't add the paths at all.
It's not clear why a route should be suppressed if it is contained
in the subnet of one of the interface's addresses.
I think it is wrong to do this. For example, imagine an ethernet
and a Wi-Fi device both connected to the same subnet 10.0.0.0/8. By
default, ethernet gets higher priority and a better metric of 100.
If the user wants to configure a route "10.0.0.1/32 metric 99"
to reach a certain host explicitly via Wi-Fi, this check will
forbid that.
This condition was added a long time ago (38dbdae266),
but it's unclear what the original intent was.
See also commit 4f7b1cabc0, which
already relaxed this suppression of routes for non-direct routes.
(cherry picked from commit ac5dc1a951)
The DNS priority property of a IP configuration determines how the
configuration compares to others when deciding their order, but
doesn't specify directly parameters to be applied. In other words, two
configurations which differs only for the dns-priority should have the
same hash as applying them will give the same result.
Especially, when the DNS manager computes the hash of IP
configurations, the ones without real configuration data (servers,
domans, options...) should not change the hash value.
Thus, exclude the property from the hash computation and dowgrade any
modification to 'minor change'.
Fixes: bfabfb05ae
Fixes: f09f5e1ec8
Since commit 4c2410bc92 ("platform: extend NMIPConfigSource to
preserve the rtm_protocol field") the rt_source field of a
NMPlatformIP{4,6}Route contains the RTPROT value read from
kernel. Update checks on route source, otherwise existing routes are
not picked up when a generated connection is created, breaking the
connection matching.
Fixes: 4c2410bc92
Usually, our _GET_PRIVATE() macros cast away the const-ness of
the self argument -- also because they cannot do any better in
plain (gcc) C.
Now it is possible to preserve const-ness, it seems more correct to do so.
After all, the const should also help us not modifying arguments that are
not intended to be modified.
Although, the more important use of const is to signal that a function
promises not to modify an argument, like in memcpy(void*,const void*)
it's immediately clear which is source and destination. In C, a const
is anyway not enforcable, but can show intent.
Likewise for NM_IP6_CONFIG_GET_PRIVATE() and NMIP6Config.
With GObject, the object structure and class structure must be public
to be able to inherit from the type. As NMIP4Config is not inherited
(final), we don't need that and we don't expect ever needing that for
this type.
Already now, we want to have the priv pointer directly accessible via
self->priv. The main reason is improved debugging, another reason
is faster lookup.
Now with the struct private, we can directly embed the private data
inside NMIP4Config. This avoids storing the private data outside separately
inside the GObject which involves a small overhead.
It becomes more attractive to do so, as every NMDevice has a multitude of
these NMIP4Config instances.
And likewise for NMIP6Config.
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.
str_if_set() was added to replace the non-standard gcc extension "?:".
However, "?:" is supported by clang as well and we already use it at
several places.
Also, str_if_set() did not follow our naming scheme and renaming to
nm_str_if_set() would be ugly. So just drop it.
Most functions defined in "nm-platform-utils.h" perform a lookup
of link properties, for example via ethtool or sysfs. Those functions
depend on the system configuration, such as the current network namespace.
Move the simple helper functions away to "nm-core-internal.h", so that
all remaining functions from "nm-platform-utils.h" are really related to
somthing that interacts with the system/kernel.
- 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
Like we already do for IPv6 addresses, we should expose addresses
in a defined, stable sort order.
Clients usually show the addresses in the same order as obtained
via D-Bus.
- "gsystem-local-alloc.h" and <gio/gio.h> are already included via
"nm-default.h". No need to include them separately.
- include "nm-macros-internal.h" via "nm-default.h" and drop all
explict includes.
- in the modified files, ensure that we always include "config.h"
and "nm-default.h" first. As second, include the header file
for the current source file (if applicable). Then follow external
includes and finally internal nm includes.
- include nm headers inside source code files with quotes
- internal header files don't need to include default headers.
They can savely assume that "nm-default.h" is already included
and with it glib, nm-glib.h, nm-macros-internal.h, etc.
This is the same as 04c70c76bc for the
NMIP4Config and NMIP6Config structures. The new field makes debugging
of issues related to IP configuration much easier.
dhclient adds a trailing dot to domain search list entries received
from the server, while the same domains received by other means
(dhcpcd on RA) don't have the final dot. The result is that
resolv.conf can be populated with duplicated entries.
Fix this by stripping the trailing dot when a new search domain is
added to a IP configuration.
https://bugzilla.gnome.org/show_bug.cgi?id=758777
When @src didn't have a gateway and @dst did, the function left @dst's
gateway set to 0.0.0.0; fix this and unset the gateway in such case.
Fixes: 063677101a
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.
Usually, the peer-address is the same as the local address.
In case where it is not, it is the peer-address that determines
the IPv4 device-route. So we must use the peer-address.
Also, don't consider device-routes with the first octet of zero,
just like kernel does.
Also, nm_ip4_config_get_subnet_for_host() is effectively the same
as nm_ip4_config_destination_is_direct(). So drop it.
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
Also change the semantic of nm_ip6_config_address_exists()
to ignore the prefix length. It seems more correct this way,
but as there are no users of the function it doesn't actually
matter.
Kernel treats IPv4 addresses with different netmask/prefix-length as
different addresses.
It is wrong to merge them together in nm_ip4_config_add_address().
For IPv6 addresses that is not the case and you cannot configure
two IPv6 addresses that only differ by plen (on the same interface).
The new flags are not yet used, so there is no change in functionality.
The flags NM_IP_CONFIG_MERGE_NO_ROUTES and NM_IP_CONFIG_MERGE_NO_DNS go
together with the 'ignore-auto-routes' and 'ignore-auto-dns' setting.
Note that for IPv4, NM_IP_CONFIG_MERGE_NO_DNS also ignores NIS, WINS, and dns-options.
This is different from current other places that handle 'ignore-auto-dns'
and only care about nameservers, domains, and searches.
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
Add NMExportedObject, make it the base class of all D-Bus-exported
types, and move the nm-properties-changed-signal logic into it. (Also,
make NMSettings use the same properties-changed code as everything
else, which it was not previously doing, presumably for historical
reasons).
(This is mostly just shuffling code around at this point, but
NMExportedObject will be more important in the gdbus port, since
gdbus-codegen doesn't do a very good job of supporting objects that
export multiple interfaces [as each NMDevice subclass does, for
example], so we will need more glue/helper code in NMExportedObject
then.)
