Imported from systemd:
The DHCP client should not pre-filter addresses beyond what RFC
requires. If a client's user (like networkd) wishes to skip/filter
certain addresses, it's their responsibility.
The point of this is that the DHCP library does not hide/abstract
information that might be relevant for certain users. For example,
NetworkManager exposes DHCP options in its API. When doing that, the
options should be close to the actual lease.
This is related to commit d9ec2e632df4905201facf76d6a205edc952116a
(dhcp4: filter bogus DNS/NTP server addresses silently).
072320eab0
Imported from systemd:
The Router DHCP option may contain a list of one or more
routers ([1]). Extend the API of sd_dhcp_lease to return a
list instead of only the first.
Note that networkd still only uses the first router (if present).
Aside from extending the internal API of the DHCP client, there
is almost no change in behavior. The only visible difference in
behavior is that the "ROUTER" variable in the lease file is now a
list of addresses.
Note how RFC 2132 does not define certain IP addresses as invalid for the
router option. Still, previously sd_dhcp_lease_get_router() would never
return a "0.0.0.0" address. In fact, the previous API could not
differenciate whether no router option was present, whether it
was invalid, or whether its first router was "0.0.0.0". No longer let
the DHCP client library impose additional restrictions that are not
part of RFC. Instead, the caller should handle this. The patch does
that, and networkd only consideres the first router entry if it is not
"0.0.0.0".
[1] https://tools.ietf.org/html/rfc2132#section-3.5
This also required adjusting "src/dhcp/nm-dhcp-systemd.c" due to the
changed internal API.
f8862395e8
Imported from systemd:
deserialize_in_addrs() allocates the buffer before trying to parse
the IP address. Since a parsing error is silently ignored, the returned
size might be zero. In such a case we shouldn't return any buffer.
Anyway, there was no leak, because there are only two callers like
r = deserialize_in_addrs(&lease->dns, dns);
which both keep the unused buffer and later release it.
Note that deserialize_in_addrs() doesn't free the pointer before
reassigning the new output. The caller must take care to to pass
"ret" with an allocated buffer that would be leaked when returning
the result.
c24b682162
... and lease_to_ip6_config().
- Handle reasons that render the lease invalid first, before logging
anything. This way, upon invalid lease we don't have partially logged
about the lease.
- prefer logging one line for options that contain multiple values, for
example for search domains.
- reorder statements to consistently log first before calling add_option().
- prefer
g_string_append (nm_gstring_add_space_delimiter (str), ...
over
g_string_append_printf (str, "%s%s", str->len ? " " : "", ...
- use @addr_str buffer directly, instead of assigning to another
temporary variable.
The newly set interface may already be in a READY state. In that case,
the device should progress into the DISCONNECTED state rather than
remaining in the UNAVAILABLE state.
Put the device into UNAVAILABLE state when the corresponding WPA
supplicant management interface is unset. This is important to
explicitly clear any pending state changes that are not permissible when
there is no management interface.
../src/devices/wifi/nm-device-iwd.c: In function ‘powered_changed’:
../src/devices/wifi/nm-device-iwd.c:2336:15: warning: assignment from incompatible pointer type [enabled by default]
interface = g_object_ref (priv->dbus_device_proxy);
^
We will need more flags.
WireGuard internal tools solve this by embedding the change flags inside
the structure that corresponds to NMPlatformLnkWireGuard. We don't do
that, NMPlatformLnkWireGuard is only for containing the information about
the link.
Usually, for external/assume we skip calling act_stage2_config().
Add a flag that allows the device to indicate that it always wants
to be called. This is useful, if the device wants to do some initialization
also for external/assume cases.
Instead of performing a series of steps inside one check for
"!nm_device_sys_iface_state_is_external_or_assume (self)", perform
all steps individually (under the same check).
There is no change in behavior, but this is more logical to me.
We perform a series of steps, depending on condition. Each step
individually depends on a set of conditions, instead of checking
for a set of conditions and doing a series of independent steps.
WireGuard devices are (will be) regular NMDevice implementations,
but NMDnsManager should treat them like VPN.
For that, reuse the device's type and nm_device_get_route_metric_default().
Previously, we might have a pending action 'waiting-for-supplicant'
registered, although the device was not waiting:
<info> [1549611177.5815] device (wlan0): supplicant interface state: starting -> ready
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): P2P: Releasing WPA supplicant interfaces.
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): P2P: WPA supplicant management interface changed to /fi/w1/wpa_supplicant1/Interfaces/1.
<trace> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): remove_pending_action (0): 'waiting-for-supplicant' not pending (expected)
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): constructed (NMDeviceWifiP2P)
<debug> [1549611177.5816] device[0x55d1781ae5d0] (p2p-dev-wlan0): add_pending_action (1): 'waiting-for-supplicant'
The previous commit already fixed this bug by dropping the constructor
property for NM_DEVICE_WIFI_P2P_MGMT_IFACE.
Still, refactor handling of pending actions to keep track of whether we
have a pending action registered.
We already have a setter function nm_device_wifi_p2p_set_mgmt_iface()
as we may need to change the mgmt-iface later on. Use that to set the
supplicant interface instead of a constructor property.
That makes the object creation simpler, because nothing noteworthy
happens, until the very last statement in constructed() to add the
pending action.
Use the NM_ERRNO_NATIVE() macro that asserts that these errno numbers are
indeed positive. Using the macro also serves as a documentation of what
the meaning of these numbers is.
That is often not obvious, whether we have an nm_errno(), an nm_errno_native()
(from <errno.h>), or another error number (e.g. WaitForNlResponseResult). This
situation already improved by merging netlink error codes (nle),
NMPlatformError enum and <errno.h> as nm_errno(). But we still must
always be careful about not to mix error codes from different
domains or transform them appropriately (like nm_errno_from_native()).
Using strtol() correctly proves to be hard.
Usually, we want to also check that the end pointer is points to the end
of the string. Othewise, we silently accept trailing garbage.
When the logging level is DEBUG or TRACE, we keep all the sysctl
values we read in a cache to log how they change. Currently there is
no limit on the size of this cache and it can take a large amount of
memory.
Implement a LRU cache where the oldest entries are deleted to make
space for new ones.
https://github.com/NetworkManager/NetworkManager/pull/294
nm_ip_route_get_prefix() and plen are guint type, hence the following
is not correct:
plen = nm_ip_route_get_prefix (route1);
r = plen - nm_ip_route_get_prefix (route2);
if (r)
return r > 0 ? 1 : -1;
Use the macro, it gets subtle cases like this right.
Fixes: b32bb36c61
The right way is IN6_ADDR_INIT_ANY.
While at it, don't initialize multiple variables in the same line.
../src/devices/nm-device-ip-tunnel.c:153:29: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
struct in6_addr local6 = { 0 }, remote6 = { 0 };
^
{}
NMIPAddr contains an unnamed union. We have to either explicitly
initialize one field, or omit it.
../shared/nm-utils/nm-shared-utils.c:38:36: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
const NMIPAddr nm_ip_addr_zero = { 0 };
^
{}
The cast is bogus and leads to a compiler warning:
[424/583] Compiling C object src/devices/wifi/914a32e@@nm-device-plugin-wifi@sha/nm-device-iwd.c.o.
In file included from ../shared/nm-default.h:293,
from ../src/devices/wifi/nm-device-iwd.c:21:
../src/devices/wifi/nm-device-iwd.c: In function ‘nm_device_iwd_set_dbus_object’:
../src/devices/wifi/nm-device-iwd.c:2404:28: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
if (!nm_g_object_ref_set ((GObject **) &priv->dbus_obj, (GObject *) object))
../shared/nm-utils/nm-macros-internal.h:1048:13: note: in definition of macro ‘nm_g_object_ref_set’
typeof (*(pp)) *const _pp = (pp); \
^~
The test should check the behavior with "const typeof(a)" in a macro,
where "a" itself is const. For that we don't need a double const
declaration of v2.
Also, that fixes an actual compiler warning:
../src/tests/test-general.c: In function ‘test_duplicate_decl_specifier’:
../src/tests/test-general.c:1669:8: warning: duplicate ‘const’ declaration specifier [-Wduplicate-decl-specifier]
const const int v2 = 3;
^~~~~
For static functions inside a module, the compiler determines on its own
whether to inline the function.
Also, "inline" was used at some places that don't immediatly look like
candidates for inlining. It was most likely a copy&paste error.
