GitLab

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.

- use NM_IN_STRSET_ASCII_CASE().
- don't use else block after we return.
- don't accept the "iface" argument just for logging. The caller
  can do the logging, if they wish.

g_bytes_ref() does not accept NULL. But doing so can be convenient,
add a helper for that.

Note that g_bytes_unref() does accept NULL, so there is no corresponding
helper.

It seems unnecessary, and can cause failure.

Yes, this is currently broken. *sigh*. Ignore any failure.

For details, read the linked sources.

This requires a new supplicant option, but it seems that supplicant
will silently ignore unrecognized options.

https://bugzilla.redhat.com/show_bug.cgi?id=2072070#c48
https://lists.infradead.org/pipermail/hostap/2022-May/040522.html
https://w1.fi/cgit/hostap/commit/?id=566ce69a8d0e64093309cbde80235aa522fbf84e

!1218

NetworkManager-ci!1042

Log messages when invalid DHCP options are found. For example:

  <info> dhcp4 (eth0): error parsing DHCP option 6 (domain_name_servers): address 0.0.0.0 is ignored
  <info> dhcp4 (eth0): error parsing DHCP option 12 (host_name): '.example.com' is not a valid DNS domain
  <info> dhcp4 (eth0): error parsing DHCP option 26 (interface_mtu): value 60 is smaller than 68

!1225