Alex Henrie authored May 19, 2022 and Thomas Haller committed May 27, 2022

#1003

!1231

Related: https://mail.gnome.org/archives/networkmanager-list/2021-April/msg00015.html
Related: #966

!1187

Adrian Freihofer authored Apr 08, 2022 and Thomas Haller committed May 27, 2022

Introduction of a new setting ipv4.link-local, which enables
link-local IP addresses concurrently with other IP address assignment
implementations such as dhcp or manually.
No way is implemented to obtain a link-local address as a fallback when
dhcp does not respond (as dhcpd does, for example). This could be be
added later.

To maintain backward compatibility with ipv4.method ipv4.link-local has
lower priority than ipv4.method. This results in:
* method=link-local overrules link-local=disabled
* method=disabled overrules link-local=enabled

Furthermore, link-local=auto means that method defines whether
link-local is enabled or disabled:
* method=link-local --> link-local=enabled
* else --> link-local=disabled

The upside is, that this implementation requires no normalization.
Normalization is confusing to implement, because to get it really
right, we probably should support normalizing link-local based on
method, but also vice versa. And since the method affects how other
properties validate/normalize, it's hard to normalize that one, so that
the result makes sense. Normalization is also often not great to the
user, because it basically means to modify the profile based on other
settings.

The downside is that the auto flag becomes API and exists because
we need backward compatibility with ipv4.method.
We would never add this flag, if we would redesign "ipv4.method"
(by replacing by per-method-specific settings).

Defining a default setting for ipv4.link-local in the global
configuration is also supported.
The default setting for the new property can be "default", since old
users upgrading to a new version that supports ipv4.link-local will not
have configured the global default in NetworkManager.conf. Therefore,
they will always use the expected "auto" default unless they change
their configuration.
Co-Authored-By: Thomas Haller <thaller@redhat.com>

Alex Henrie authored May 25, 2022 and Thomas Haller committed May 26, 2022

In IPv4, /0 prevents the creation of a device route, making it
effectively the same as /32. However, in IPv6, /0 makes the device route
an all-encompassing default route. This allows, for example, an 'fe80::'
link-local address to be used to communicate with any public or private
address on the local network without any additional configuration.

NetworkManager/NetworkManager#1006

NetworkManager/NetworkManager!1232

bluikko authored May 26, 2022 and Thomas Haller committed May 26, 2022

Fix one occurrence of "ifcfg-rh" being incorrectly typed
as "fcfg-rh" with a missing letter "i".

https://github.com/NetworkManager/NetworkManager/pull/364

Pass a callback and a 4-second timeout to the "StartServiceByName()"
D-Bus call, so that we can detect any failure immediately.

In this way when systemd-resolved fails to start at boot (for example
because it's masked), nm-online doesn't need to wait those additional
4 seconds due to the fixed timeout source.

Fixes-test: @nm_online_wait_for_delayed_device

https://bugzilla.redhat.com/show_bug.cgi?id=2083332
NetworkManager/NetworkManager!1233

During the deactivation of ovs interfaces, ovsdb receives the command to
remove the interface but for OVS system ports the device won't
disappear.

When reconnecting, ovsdb will update first the status and it will notice
that the OVS system interface was removed and it will set the status as
DEACTIVATING. This is incorrect if the status is already DEACTIVATING,
DISCONNECTED, UNMANAGED or UNAVAILABLE because it will block the
activation of the interface.

https://bugzilla.redhat.com/show_bug.cgi?id=2080236

On m68k we get a static assertion, that NMPlatformIP4Address.address
is not at the same offset as NMPlatformIPAddress.address_ptr.

On most architectures, the bitfields fits in a gap between the fields,
but not on m68k, where integers are 2-byte aligned.

On m68k, integers are 2-byte aligned. Hence the assertion was wrong.

What we really want to check, is that NMIPAddr has not a smaller
alignment than in_addr_t and similar.

While at it, also assert the alignment for NMEtherAddr.

On m68k, 32-bit integers are 2-byte aligned, causing the assertion to fail.
Relax the check, it's good enough still.

  git subtree pull --prefix src/c-rbtree git@github.com:c-util/c-rbtree.git main --squash

eb778d39694a c-rbtree: fix alignment assertion on m64k

git-subtree-dir: src/c-rbtree
git-subtree-split: eb778d39694a0f3389f2438bbc45fb21685a047d

David Rheinsberg authored May 18, 2022 and Thomas Haller committed May 18, 2022

We want to assert that our alignment-guarantees do not exceed the
guarantees of the system-linker or system-allocator on the target
platform. Hence, we check against max_align_t. This is a lower bound,
but not the exact check we actually want. And as it turns out, on m64k
it is too low. Add a static check against 4-byte alignment for m64k as
a workaround.

Reported-by: Michael Biebl
Signed-off-by: David Rheinsberg <david.rheinsberg@gmail.com>

https://github.com/c-util/c-rbtree/issues/9
https://github.com/c-util/c-rbtree/commit/eb778d39694a0f3389f2438bbc45fb21685a047d

DHCPv4 requires a hardware address, while DHCPv6 does not.

Anyway, the DHCP manager already checks that an address is available
when needed, so drop the check here.

Fixes: 58287cbc ('core: rework IP configuration in NetworkManager using layer 3 configuration')

!1228

clang-format likes to indent the comment, at the location where it was.
Move it.

!1219

As almost always, there is a point in keeping IPv4 and IPv6 implementations
similar. Behave different where there is an actual difference, at the bottom
of the stack.

No need to do it otherwise.

Technically, g_warn_if_reached() may not be an assertion, according to
glib. However, there is G_DEBUG=fatal-warnings and we want to run with
that.

So this is an assertion to us. Also, logging to stderr/stdout is not a
useful thing to the daemon. Don't do this. Especially, since it depends
on user provided (untrusted) input.

It's unused now.

Optimally we want stateless, pure code. Obviously, NMDhcpClient needs to
keep state to know what it's doing. However, we should well encapsulate
the state inside NMDhcpClient, and only accept events/notifications that
mutate the internal state according to certain rules.

Having a function public set_state(self, new_state) means that other
components (subclasses of NMDhcpClient) can directly mangle the state.
That means, you no longer need to only reason about the internal state
of NMDhcpClient (and the events/notifications/state-changes that it
implements). You also need to reason that other components take part of
maintaining that internal state.

Rename nm_dhcp_client_set_state() to nm_dhcp_client_notify(). Also, add
a new enum NMDhcpClientEventType with notification/event types.

In practice, this is only renaming. But naming is important, because it
suggests the reader how to think about the code.

The "noop" state is almost unused, however, nm_dhcp_set_state()
has a check "if (new_state >= NM_DHCP_STATE_TIMEOUT)", so the order
of the NOOP state matters.

Fix that by reordering.

Also, just return right away from NOOP.

NMDhcpState is very tied to events from dhclient. But most of these
states we don't care about, and NMDhcpClient definitely should abstract
and hide them.

We should repurpose NMDhcpState to simpler state. For that, first drop
the state from nm_dhcp_client_handle_event().

This is only the first step (which arguably makes the code more
complicated, because reason_to_state() gets spread out and the logic
happens more than once). That will be addressed next.

So that it makes more sense, related parts are closer together.

- return early to avoid nested block.
- use NM_STR_HAS_PREFIX() over g_str_has_prefix(), because that
  can be inlined and only accepts a C literal as prefix argument.

- the code comment was unclear/wrong. If something comes from an environment
  variables it is *NOT* UTF-8 safe. Also, we convert all non-ASCII characters,
  not only non UTF-8 characters.

- as we already convert the string to ASCII, the check whether it's UTF-8
  is bogus.

- using GString is unnecessary.