When doing reapply on linux bridge interface, NetworkManager will reset
the VLAN filtering and default PVID which cause PVID been readded to all
bridge ports regardless they are managed by NetworkManager.
This is because Linux kernel will re-add PVID to bridge port upon the
changes of bridge default-pvid value.
To fix the issue, this patch introduce netlink parsing code for
`vlan_filtering` and `default_pvid` of NMPlatformLnkBridge, and use that
to compare desired VLAN filtering settings, skip the reset of VLAN
filter if `default_pvid` and `vlan_filtering` are unchanged.
Signed-off-by: Gris Ge <fge@redhat.com>
Currently the internal DHCP client sets traffic class "CS6" in the DS
field of the IP header for outgoing packets.
dhclient sets the field according to the definition of TOS (RFC 1349),
which was was deprecated in 1998 by RFC 2474 in favor of DSCP.
Introduce a new property IPvX.dhcp-dscp (currently valid only for
IPv4) to specify a custom DSCP value for DHCP backends that support it
(currently, only the internal one).
Define the default value to CS0, because:
- section 4.9 of RFC 4594 specifies that DHCP should use the standard
(CS0 = 0) service class;
- section 3.2 says that class CS6 is for "transmitting packets
between network devices (routers) that require control (routing)
information to be exchanged between nodes", listing "OSPF, BGP,
ISIS, RIP" as examples of such traffic. Furthermore, it says that:
User traffic is not allowed to use this service class. By user
traffic, we mean packet flows that originate from user-controlled
end points that are connected to the network.
- we got reports of some Cisco switches dropping DHCP packets because
of the CS6 marking.
As done for IPv4 in commit 3b145a33b7 ('vpn-connections: allow the
plugin to specify route preferred src'), allow VPN plugins to return a
preferred source address for routes. Still accept the old format.
The decision to configure or not configure routes without addresses only
related to what method is configured - DHCP and non-DHCP cases. For DHCP
case, the deamon waits until addresses appear first before configuring
the static routes to preserve the behavior mentioned in
https://bugzilla.redhat.com/show_bug.cgi?id=2102212, otherwise, the
daemon can configure the routes immediately for non-DHCP case.
If a generated connection matches a connection that uses interface name
as controller, we need to drop the existing value from the settings to
avoid conflicts. Therefore, both of them need to be dropped; controller
and master.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1833
Fixes: 3e4a2ebb3c ('all: use the new NMSettingConnection Controller property')
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Mark the methods/properties deprecated in the D-Bus API (via
org.freedesktop.DBus.Introspectable.Introspect(), [1]).
It affects those properties that are documented as deprecated in
introspection XML.
$ busctl -j call \
org.freedesktop.NetworkManager \
/org/freedesktop/NetworkManager \
org.freedesktop.DBus.Introspectable \
Introspect | \
jq '.data[0]' -r | \
grep -5 Deprecated
[1] https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-introspectable
A IPv4 conflict detected during the probe is a serious problem, as it
prevents the address from being configured. As such, is should be
displayed at warning level.
A conflict detected after the address is already configured
(addr_info->state == NM_L3_ACD_ADDR_STATE_CONFLICT) is less important
because NM will try to defend the address and will keep using it.
A duplicate address is a serious issue which leads to non-working
setups or problems hard to debug. Enable IPv4 duplicate address
detection (aka ACD, RFC 5227) by default to detect such problems.
While the RFC recommends a timeout of 9 seconds, a comment in n-acd
sources says:
A 9s timeout for successful link setups is not acceptable today.
Hence, we will just go forward and ignore the proposed values. On
both wired and wireless local links round-trip latencies of below
3ms are common. We require the caller to set a timeout multiplier,
where 1 corresponds to a total probe time between 0.5 ms and 1.0
ms. On modern networks a multiplier of about 100 should be a
reasonable default. To comply with the RFC select a multiplier of
9000.
Set a default timeout of 200ms, which is the double of the value
suggested in n-acd sources. 200ms sounds quick enough, and gives at
least ~100ms to other hosts to reply.
See also the Fedora change proposal:
https://fedoraproject.org/wiki/Changes/Enable_IPv4_Address_Conflict_Detection
The supervision address is read-only. It is constructed by kernel and
only the last byte can be modified by setting the multicast-spec as
documented indeed.
As 1.46 was not released yet, we still can drop the whole API for this
setting property. We are keeping the NMDeviceHsr property as it is a
nice to have for reading it.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1823
Fixes: 5426bdf4a1 ('HSR: add support to HSR/PRP interface')
Glib format specifiers are not gettext friendly. It even emits a
warning: src/core/main-utils.c:196: warning: Although being used in a
format string position, the msgid is not a valid C format string."
One possible solution is to use the equivalent format specifiers from
<inttypes.h> like PRId64, available since C99.
Even simpler is to cast the value to a type that is big enough to hold
it according to C specs (i.e. for int64: long long).
Fixes: 50f34217f9 ('main: use _nm_utils_ascii_str_to_int64 instead of strtol for reading pid')
Setting
wifi.cloned-mac-address="stable-ssid"
should generate the same SSID as
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
For that to work correctly, we need to post-process the generated stable
id.
Fixes: d210923c0f ('wifi: add "wifi.cloned-mac-address=stable-ssid"')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1813
This option was only introduced only to allow keeping the old behavior
in RHEL7, while the default order was changed from 'ifindex' to 'name'
in RHEL8. The usefulness of this option is questionable, as 'name'
together with predictable interface names should give predictable order.
When not using predictable interface names, the name is unpredictable
but so is the ifindex.
https://issues.redhat.com/browse/NMT-926https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1814
"nm-glib" h contains compat wrappers for older glib versions. This file
used to be copied over to VPN plugins, to use the same compat code. It
was thus interesting to also have compat code for glib versions, that
were no longer supported by NetworkManager itself.
This was fine. But glib 2.42 is more than 8 years old. At this point,
there really is no need to support that, even if you copy the file out
of NetworkManager source tree.
Drop those compat wrappers.
Let's not have unexpected, non-thread-safe functions somewhere deep down.
NML3ConfigData -- as a data structure -- is not thread-safe, nor aims it to
be. However, our code(!) should be thread-safe. That means, it should be
possible to call our code on separate data from multiple threads.
Violating that is a code smell and a foot gun.
This basically means that code should not access global data (unless
thread-local) or that the access to global-data needs to be
synchronized.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1806
This patch add support to HSR/PRP interface. Please notice that PRP
driver is represented as HSR too. They are different drivers but on
kernel they are integrated together.
HSR/PRP is a network protocol standard for Ethernet that provides
seamless failover against failure of any network component. It intends
to be transparent to the application. These protocols are useful for
applications that request high availability and short switchover time
e.g electrical substation or high power inverters.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1791
Dynamic added routes i.e ECMP single-hop routes, are not managed by l3cd
as the other ones. Therefore, they need to be tracked properly and
marked as dirty when they are.
Without this, the state of those ECMP single hop routes is not properly
tracked, and they are for example not removed by NML3Cfg when they
should.
Usually, routes to be configured originate from the combined
NML3ConfigData and are resolved early during a commit. For example,
_obj_states_update_all() creates for each such route an obj_state_hash
entry. Let's call those static, or "non-dynamic".
Later, we can receive additional routes. We get them back from NMNetns
during nm_netns_ip_route_ecmp_commit() (_commit_collect_routes()).
Let's call them "dynamic".
For those routes, we also must track an obj-state.
Now we have two reasons why an obj-state is tracked. The "non-dynamic"
and the "dynamic" one. Add two flags "os_dynamic" and "os_non_dynamic"
to the ObjStateData and consider the flags at the necessary places.
Co-authored-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
When a single-hop route is merged with another single-hop route from a
different interface creating a new ECMP route, NetworkManager needs to
schedule a commit on the l3cfg that is managing the first single-hop
route. Otherwise, the single-hop will continue to be there until a new
commit is scheduled.
Fixes: d9d33e2acc ('netns: fix configuring onlink routes for ECMP routes')
nm_netns_ip_route_ecmp_commit() returns the ECMP routes that didn't find
a merge-partner. It expects NML3Cfg to configure such routes in
platform.
Note that these routes have a positive "weight", which was used for
tracking the ECMP information in NML3Cfg and NMNetns.
But in kernel, single-hop routes don't have a weight. All routes in
NMPlatform will have a zero weight. While it somewhat works to call
nm_platform_ip_route_add() with a single-hop route of non-zero weight,
the result will be a different(!) route. This means for example that
nm_platform_ip_route_sync() will search the NMPlatform cache for whether
the routes exist, but in the cache single-hop routes with positive weight
never exist. Previously this happened and nm_platform_ip_route_sync()
would not delete those routes when it should.
It's also important because NML3Cfg tracks the object states of routes
that it wants to configure, vs routes in kernel (NMPlatform cache). The
route in kernel has a weight of zero. The route we want to configure
must also have a weight of zero.
The solution is that nm_netns_ip_route_ecmp_commit() does not tell
NML3Cfg to add a route, that cannot be added. Instead, it must first
normalize the weight to zero, to have a "regular" single-hop route.
Fixes: 5b5ce42682 ('nm-netns: track ECMP routes')
It wouldn't work otherwise. The object state is used to track routes
and compare them to what we find in platform.
A "metric_any" is useful at higher layers, to track a route where the
metric is decided by somebody else. But at the point when we add such an
object to the object-state, a fixed metric must be chosen.
Assert for that.
Users should not be allowed to start or stop a wifi-p2p scan unless
they have some kind of permission. Since we already have the
"org.freedesktop.NetworkManager.wifi.scan" permission for wifi scans,
check that.
Fixes: dd0c59c468 ('core/devices: Add DBus methods to start/stop a P2P find')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1795
It doesn't scale to export all addresses/routes on D-Bus as properties.
In particular not combined with PropertiesChanged signal. On a busy
system, this causes severe performance issues. It also doesn't seem very
useful. Routes and addresses are complex things (e.g. policy routing).
If you want to do anything serious, you must check netlink (or find
another way to get the information).
Note that NMPlatform already ignores routes of certain protocols
(ip_route_is_alive()). It also does not expose most route attributes,
making the output only useful for very limited cases (e.g. displaying to
the user for information).
Limit the number of exported entries to 100.
Try adding 100K routes one-by-one. Run a `nmcli monitor` instance.
Re-nice the nmcli process and/or keep the CPUs busy. Then start a script
that adds 100k routes. Observe. Glib's D-Bus worker thread receives the
messages and queues them for the main thread. The main thread is too
slow to process them, the memory consumption grows very quickly in Giga
bytes. Afterwards, the memory also is not returned to the operation
system, either because of fragmentation or because the libc allocator
does anyway not return heap memory.
It doesn't work to expose an unlimited number of objects on D-Bus. At
least not with an API, that sends the full list of all routes, whenever
a route changes. Nobody can use that feature either, because the only
use is a quick overview in `nmcli` output or a GUI. If you see 100+
routes there, that becomes unmanageable anyway. Instead use netlink if
you want to handle the full list of addresses/routes (or some other
API).
It doesn't scale. If you add 100k routes one-by-one, then upon each
change from platform, we will send the growing list of the routes on
D-Bus.
That is too expensive. Especially, if you imagine that the receiving end
is a NMClient instance. There is a D-Bus worker thread that queues the
received GVariant messages, while the main thread may not be able to
process them fast enough. In that case, the memory keeps growing very
fast and due to fragmentation it is not freed.
Instead, rate limit updates to 3 per second.
Note that the receive buffer of the netlink socket can fill up and we
loose messages. Therefore, already on the lowest level, we may miss
addresses/routes. Next, on top of NMPlatform, NMIPConfig listens to
NM_L3_CONFIG_NOTIFY_TYPE_PLATFORM_CHANGE_ON_IDLE events. Thereby it
further will miss intermediate state (e.g. a route that exists only for
a short moment).
Now adding another delay and rate limiting on top of that, does not make
that fundamentally different, we anyway didn't get all intermediate states
from netlink. We may miss addresses/routes that only exist for a short
amount of time. This makes "the problem" worse, but not fundamentally
new.
We can only get a (correct) settled state, after all events are
processed. And we never know, whether there isn't the next event
just waiting to be received.
Rate limiting is important to not overwhelm D-Bus clients. In reality,
none of the users really need this information, because it's also
incomplete. Users who really need to know addresses/routes should use
netlink or find another way (a way that scales and where they explicitly
request this information).
_nm_utils_dns_option_validate() allows specifying the address family,
and filters based on that. Note that all options are valid for IPv6,
but some are not valid for IPv4.
It's not obvious, that such filtering is only performed if
"option_descs" argument is provied. Otherwise, the "ipv6" argument is
ignored.
Regardless, it's also confusing to have a boolean "ipv6". When most
callers don't want a filtering based on the address family. They
actually don't want any filtering at all, as they don't pass an
"option_descs". At the same time passing a TRUE/FALSE "ipv6" is
redundant and ignored. It should be possible, to explicitly not select
an address family (as it's ignored anyway).
Instead, make the "gboolean ipv6" argument an "int addr_family".
Selecting AF_UNSPEC means clearly to accept any address family.
When connection down is explicitly called on the controller, the port
connection should also be deactivated with the reason user-requested,
otherwise any following connection update on the controller profile
will unblock the port connection and unnessarily make the port to
autoconnet again.
Fixes: 645a1bb0ef ('core: unblock autoconnect when master profile changes')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1568
Add a new "stable-ssid" mode that generates the MAC address based on the
Wi-Fi's SSID.
Note that this gives the same MAC address as setting
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
The difference is that changing the stable ID of a profile also affects
"ipv6.addr-gen-mode=stable-privacy" and other settings.
For Wi-Fi profiles, this will encode the SSID in the stable-id.
For other profiles, this encodes the connection UUID (but the SSID and
the UUID will always result in distinct stable IDs).
Also escape the SSID, so that the generated stable-id is always valid
UTF-8.
Comparing integers of different signedness gives often unexpected
results. Adjust usages of MIN()/MAX() to ensure that the arguments agree
in signedness.
nm_hash_siphash42() uses a randomized seed like nm_hash*(). In this case,
we want to always generate the same fake timestamp, based on the host-id.
In practice, it doesn't really matter, because this is only the fallback
path for something gone horribly wrong already.
Most profiles don't have "connection.permissions" set. Avoid resolving the
UID to the name via getpwuid() (in nm_auth_is_subject_in_acl()), until we
know that we require it.
I think GSource* is preferable, because it's more type-safe than the
guint numbers. Also, g_source_remove() only works with
g_main_context_default(), while g_source_detach() of a GSource pointer
works with any GMainContext (so it's more general, even if we in this
case only have sources attached to g_main_context_default()).
Handling GSource* pointers is possibly also faster, since it saves one
extra g_main_context_find_source_by_id() -- on the other hand, tracking
the pointer costs 8 bytes while tracking the guint only costs 4 bytes.
Whether it's faster is unproven, but possibly it is. In any case it's
not an argument against using pointers.
Anyway. Update another usage of source-ids to use GSource pointers.
This is also the pattern that "checkpatch.pl" suggests.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1774
Some Applications require to explicitly enable or disable EEE.
Therefore introduce EEE (Energy Efficient Ethernet) support with:
* ethtool.eee on/off
Unit test case included.
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
