When the link is up and goes down link_changed_cb() schedules
device_link_changed() to be run later. If the function is dispatched
when the link is already up again, it does not detect that the link
was down.
Fix this by storing in the device state that we saw the link down so
that device_link_changed() can properly restore the IP configuration.
https://bugzilla.redhat.com/show_bug.cgi?id=1636715https://github.com/NetworkManager/NetworkManager/pull/264
The check condition was inverted. Anyway, we should receive IPv4LL
events only when the method is LINK_LOCAL so turn this into an
assertion.
Fixes: b16e09a707
There is really no problem here, allow it.
Previously we would assert against a non-zero prefix length.
But I am not sure that all callers really ensured that this
couldn't happen. Anyway, there is no problem we such addresses,
really.
Only we need to make sure that nm_ip4_config_add_dependent_routes()
and nm_ip6_config_add_dependent_routes() don't add prefix routes for
such addresses (which is the case now).
Preface: RFC 3442 (The Classless Static Route Option for Dynamic Host
Configuration Protocol (DHCP) version 4) states:
If the DHCP server returns both a Classless Static Routes option and
a Router option, the DHCP client MUST ignore the Router option.
Similarly, if the DHCP server returns both a Classless Static Routes
option and a Static Routes option, the DHCP client MUST ignore the
Static Routes option.
Changes:
- sd_dhcp_lease_get_routes() returns the combination of both option 33
(static routes) and 121 (classless static routes). If classless static
routes are provided, the state routes must be ignored.
- we collect the options hash that we expose on D-Bus. For that purpose,
we must not merge both option types as classless static routes. Instead,
we want to expose the values like we received them originally: as two
different options.
- we continue our deviation from RFC 3442, when receiving classless static
routes with option 3 (Router), we only ignore the router if we didn't
already receive a default route via classless static routes.
- in the past, NetworkManager treated the default route specially, and
one device could only have one default route. That limitation was
already (partly) lifted by commit 5c299454b4
(core: rework tracking of gateway/default-route in ip-config). However,
from DHCP we still would only accept one default route. Fix that for
internal client. Installing multiple default routes might make sense, as
kernel apparently can skip unreachable routers (as it notes via ICMP
messages) (rh#1634657).
https://bugzilla.redhat.com/show_bug.cgi?id=1634657
In ip4_start(), we iterate over @dhcp4_requests array and add the
options that are to be included. We do so, by calling
sd_dhcp_client_set_request_option().
Note that sd_dhcp_client_set_request_option() only appends the options
to a list, not taking special care about the order in which options are
added.
RFC 3442 (The Classless Static Route Option for Dynamic Host Configuration
Protocol (DHCP) version 4) says:
DHCP clients that support this option and send a parameter request
list MAY also request the Static Routes option, for compatibility
with older servers that don't support Classless Static Routes. The
Classless Static Routes option code MUST appear in the parameter
request list prior to both the Router option code and the Static
Routes option code, if present.
Compare to RFC 2132 (DHCP Options and BOOTP Vendor Extensions) which says
about the parameter request list:
The client MAY list the options in order of preference.
Note, with RFC 7844 (Anonymity Profiles for DHCP Clients), the order
should be randomized. But since we don't follow RFC 7844 (yet), let's follow
at least RFC 3442.
Since sd_dhcp_lease_get_routes() returns the list of all routes,
the caller may need to differenciate whether the route was option
33 (static-routes) or 121 (classless-static-route).
Add an accessor for the internal field.
systemd-pull-request: #10951
- check errors when accessing the lease. Some errors, like a failure
from sd_dhcp_lease_get_address() are fatal.
- while parsing the individual options, don't incrementally build the
NMPlatformIP4Address instance (and similar). Instead, parse the
options to individual variales, and only package them as platform
structure at the point where they are needed. It makes the code simpler,
because all individual bits (like "r_plen" variable) are only
initialized and used at one place. That is clearer than incrementally
building a platform structure, where you have to follow the code to
see how the structure mutates.
- drop redundant comments that only serve as a visual separator
for structuring the code. Instead, structure the code.
lease_to_ip4_config() can fail, if the lease is broken. As such, a function
that fails should not modifiy an in/out parameter. Avoid that, by not
having the caller pre-allocate the options hash, but instead allocate it
by the lease_to_ip*_config() functions, and return it only on success.
- use proper data types "guint16" and "bool" in static
option list. It saves a few bytes, but also it's the appropriate
type. Well, at least, it's the appropriate type for DHCPv6,
not for DHCPv4 (which is guint8).
- assert against failure of sd_dhcp_client_set_request_option() and
sd_dhcp6_client_set_request_option().
sd_dhcp6_client_set_request_option() only accepts a white-listed
set of options. Unexpected options are rejected with -EINVAL.
Currently supported are only:
- SD_DHCP6_OPTION_DNS_SERVERS
- SD_DHCP6_OPTION_DOMAIN_LIST
- SD_DHCP6_OPTION_SNTP_SERVERS
- SD_DHCP6_OPTION_NTP_SERVER
- SD_DHCP6_OPTION_RAPID_COMMIT
As such, SD_DHCP6_OPTION_CLIENTID is not accepted and requesting it
was silently ignored.
Fixes: d2dd3b2c90
While nm_utils_inet*_ntop() accepts a %NULL buffer to fallback
to a static buffer, don't do that.
I find the possibility of using a static buffer here error prone
and something that should be avoided. There is of course the downside,
that in some cases it requires an additional line of code to allocate
the buffer on the stack as auto-variable.
- add nm_device_sysctl_ip_conf_get() and nm_device_sysctl_ip_conf_get_int_checked().
These functions don't use nm_device_get_ip_iface(), but resolve the
ifname from the platform cache.
- in general, resolve the name first with nm_device_get_ip_iface_from_platform().
We have a cached nm_device_get_ip_iface() property. However, the interface
name is not an identifier for a link because it can change at any time.
Also, we already have the (ip) ifindex as proper identifier for the
platform link. We shouldn't use two redundant identifiers to refer to
a link.
Clearly, sometimes we need an ifname. For example for ethtool ioctl or
sysctl path names. For ethtool API, we resolve the actual name as late
as possible, and for sysctl API we prefer NMP_SYSCTL_PATHID_NETDIR*().
However, that is not always possible, for example for /proc/sys/net/ipv6/conf/
sysctls.
Add a function that resolves the ifname by looking into the cache. This
of course is still racy, but it minimizes the time.
Also, we should less and less rely on the ifname, and resolve it as late
as possible. This patch adds a small wrapper going into that direction.
Now that we have other helper function on platfrom for setting
IP configuration sysctls, rename the function to set the hop-limit
to match the pattern.
These functions call platform's sysctl getter and setters.
Note that the called platform functions are called nm_platform_sysctl_get()
and nm_platform_sysctl_set(). Also, in this case they use the ip-conf path
via nm_utils_sysctl_ip_conf_path().
Also, next we will add API nm_platform_sysctl_ip_conf_get() and
nm_platform_sysctl_ip_conf_set(), which will be wrappers around
nm_platform_sysctl_get() and nm_platform_sysctl_set(), using the ip-conf
paths as well.
Rename the device functions, to be more similar to the existing and future
naming in platform.
For one, next we will drop setting rp_filter, hence there are no
more users of an IPv4 variant and nm_device_ipv4_sysctl_set() would
have to be dropped anyway.
However, instead of doing that, merge the IPv4 and IPv6 variant.
With this, the fallback to the default is now also supported for IPv6
(though unused).
Also, don't access nm_device_get_ip_iface(). The interface name might
not be right, we should only rely on the ifindex. Load the interface
name from platform cache instead.
I think this is preferred over memset(), because it allows the
compiler to better unstand what is happening.
Also, strictly speaking in the C language, %NULL pointers are not
guaranteed to have an all zero bit pattern. Of course, that is already
required on any architecture where NetworkManager is running.
NMP_SYSCTL_PATHID_NETDIR_unsafe() uses alloca() to allocate the string.
Assert that the "path" argument is reasonably short.
In practice, that is of course the case, because there are only 2 callers
which take care not to pass an untrusted, unbounded path argument.
The 'number' property in GSM settings is a legacy thing that comes
from when ModemManager used user-provided numbers, if any, to connect
3GPP modems.
Since ModemManager 1.0, this property is completely unused for 3GPP
modems, and so it doesn't make sense to use it in the NetworkManager
settings. Ofono does not use it either.
For AT+PPP-based 3GPP modems, the 'number' to call to establish the
data connection is decided by ModemManager itself, e.g. for standard
GSM/UMTS/LTE modems it will connect a given predefined PDP context,
and for other modems like Iridium it will have the number to call
hardcoded in the plugin itself.
https://github.com/NetworkManager/NetworkManager/pull/261
The reasons to block autoconnection at settings level are not the same
as the ones to block autoconnection at device level.
E.g. if the SIM-PIN is wrong, you may want to block autoconnection
both at settings level (as the PIN configured in settings is wrong)
and at device level (so that no other setting is tried automatically).
For some other reasons, you may want to block autoconnection only at
setting level (e.g. wrong APN).
And for some other reasons you may want to block autoconnection at
device level only (e.g. SIM missing), so that the autoconnection
blocking is removed when the device goes away. This is especially
important with SIM hotplug events processed by ModemManager, as a
device without SIM will be removed from MM when a new SIM is
inserted, so that a completely new object is exposed in MM with the
newly detected SIM.
https://github.com/NetworkManager/NetworkManager/pull/259
When updating a connection passing agent-owned secret, they are lost
from @reread_connection after the settings-plugin persists the
connection. Therefore we need to cache and reapply them separately to
the connection so that they can be saved to secret agents later.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/82
The timeout is limited to be in the range of 1-600s. This is arbitrary,
but the point is that a timeout of 0 is not permitted to prevent a
client from making us run a find continuously simply by forgetting to
call the stop method.
wpa_supplicant will create a new interface for P2P devices. In this case
we need to fetch the supplicant interface using the object path and then
fetch the interface name via dbus to setup the IP interface of the P2P
device later.
The wpa_supplicant interface has a P2PDevice when P2P is supported.
Create a proxy for this and wait for it to be ready before marking the
interface as ready.
The signal was not handled, potentially creating corner cases where
NetworkManager may not notice an interface removal. Add a handler and
ensure the supplicant interface is brought down when it is removed from
wpa_supplicant for a reason other than NetworkManager requesting it.
Refactor some code to use nm_streq() and NM_IN_STRSET() instead of
strcmp().
Note that nm_utils_get_ip_config_method() never returns %NULL (not even
with g_return*() assertion failures). nm_streq() is sufficent.
Recently, more and more code was refactored to use an addr_family
integer to distinguish between IPv4 and IPv6.
Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method()
to do that too. If we use different identifiers, we need to translate from one to
another and its inconsistent. Also, accessing a GType is an unnecessary function call,
instead of a plain constant.
For P2P wifi we need to do DHCP if we are a peer or provide DHCP if we
are the group owner. This may only be decided while establishing the
connection, making the meaning of the AUTO method dynamic.
This adds a way for the device subclass to override the meaning of AUTO.
Patch cherry picked early from [1].
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/24
The writer should write all properties of the sriov setting when the
setting exists without additional logic. Likewise, the reader should
instantiate a sriov setting when any sriov key is present and blindly
set properties from keys.
The old code did not always preserve the presence of a sriov setting
after a write/read cycle.
Fixes: c02d1c488f
