network: IPv4LL and ACD fixes #19980

yuwata · 2021-06-20T19:32:47Z

yuwata · 2021-06-22T08:50:52Z

I added this to v249 milestone. If this is too late to be included in v249, feel free to drop it. But this is slightly large, and maybe hard to backport this after v249.

src/libsystemd-network/sd-ipv4acd.c

yuwata · 2021-06-25T14:59:50Z

@poettering Thank you for your review. Now, the callback mechanisms which checks the sender hardware address is introduced to sd-ipv4acd and sd-ipv4ll. PTAL.

keszybz

LGTM. I'd be inclined to merge this for 249.

src/network/networkd-link.c

src/network/networkd-ipv4ll.c

src/network/networkd-address.c

keszybz · 2021-06-30T07:49:29Z

src/network/networkd-ipv4acd.c

+                r = address_remove(address, link);
+                if (r < 0) {
+                        log_link_warning_errno(link, r, "Failed to remove address "IPV4_ADDRESS_FMT_STR": %m",
+                                               IPV4_ADDRESS_FMT_VAL(address->in_addr.in));
+                        link_enter_failed(link);
+                }
+                break;


Hmm, so we remove the address if the acd client is stopped… Or in other words, we effectively add an address by creating the acd client for it, and remove it by stopping the acd client… I wonder if we should then rename things, to emphasize the "address" part and de-emphesize the "conflict detection" part. But I don't have any good suggestions.

You mean about the log messages? The messages in the callback function is updated. PTAL.

See RFC 5227 section 2.1.1. This introduces a callback which intend to a library user, e.g. networkd, checks whether the sender hardware address is a MAC address of the host's intrerface or not.

This also renames link_get() -> link_get_by_index().

…g ipv4ll engine

…e the length of the hardware address is not ETH_ALEN Currently, sd-ipv4acd assumes hardware address is ETH_ALEN.

Currently, networkd does not set tentative flag on create, and kernel ignore the flag on remove. So, this commit does not change any current behaviour. This is just a preparation for later commits.

A preparation for later commits.

Previously, if IPv4 ACD is enabled on an address, then we first assign the address, and start sd-ipv4acd daemon for the address. This is not only RFC incompliant, but also the address is always dropped, as the daemon always considers the address is conflicted. This commit makes networkd first starts sd-ipv4acd daemon to probe the address, and then the address is configured if no conflict is detected. Fixes systemd#17235.

… for IPv4ACD and IPv4LL Fixes systemd#12145.

yuwata · 2021-06-30T15:57:07Z

@keszybz Thank you for your review. Force-pushed a revised version. In addition to the above points you commented, the following changes are applied.

in dhcp_lease_lost(), now the pending request is also dropped.
in on_acd(), DHCPv4 address is not removed in the function, but dhcp_lease_lost() drop it.
split on_acd() into several small functions.

keszybz

s390x: "boot-and-services": test_service (__main__.NspawnTest) ... Failed to dump process list for 'systemd-nspawn@c1.service', ignoring: Unit systemd-nspawn@c1.service not loaded.
Looks unrelated.

LGTM, let's get this merged.

keszybz · 2021-06-30T20:20:17Z

src/network/networkd-ipv4acd.c

+                /* Unlikely, but during probing the address, the lease may be lost. */
+                return 0;
+
+        log_link_warning(link, "Dropping DHCPv4 lease, as an address conflict is detected.");


"is" → "was"

C.f. systemd#19980 (comment)

C.f. systemd/systemd#19980 (comment)

yuwata added network dont-merge 💣 labels Jun 20, 2021

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch 3 times, most recently from 3fd3b72 to 2533069 Compare June 22, 2021 07:45

yuwata added needs-review and removed dont-merge 💣 labels Jun 22, 2021

yuwata changed the title ~~[WIP] network: IPv4LL and ACD fixes~~ network: IPv4LL and ACD fixes Jun 22, 2021

yuwata added this to the v249 milestone Jun 22, 2021

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch 2 times, most recently from 12eaca2 to 0d5df6b Compare June 22, 2021 15:21

poettering reviewed Jun 24, 2021

View reviewed changes

src/libsystemd-network/sd-ipv4acd.c Outdated Show resolved Hide resolved

poettering reviewed Jun 24, 2021

View reviewed changes

src/libsystemd-network/sd-ipv4acd.c Outdated Show resolved Hide resolved

poettering reviewed Jun 24, 2021

View reviewed changes

src/libsystemd-network/sd-ipv4acd.c Outdated Show resolved Hide resolved

poettering reviewed Jun 24, 2021

View reviewed changes

src/libsystemd-network/sd-ipv4acd.c Outdated Show resolved Hide resolved

poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed needs-review labels Jun 24, 2021

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch from 0d5df6b to 4f6cf24 Compare June 25, 2021 14:54

yuwata added needs-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jun 25, 2021

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch from 4f6cf24 to d2158fa Compare June 25, 2021 14:58

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch 5 times, most recently from 9fc741a to a535d93 Compare June 25, 2021 23:04

keszybz reviewed Jun 30, 2021

View reviewed changes

keszybz removed the needs-review label Jun 30, 2021

yuwata added 18 commits July 1, 2021 00:49

sd-ipv4acd: update condition of address conflict

7f77917

See RFC 5227 section 2.1.1. This introduces a callback which intend to a library user, e.g. networkd, checks whether the sender hardware address is a MAC address of the host's intrerface or not.

sd-ipv4ll: allow to set MAC address without stopping sd-ipv4ll engine

9e5aa34

sd-ipv4ll: introduce sd_ipv4ll_set_check_mac_callback()

1e59122

ether-addr-util: introduce hw_addr_hash_ops

907d5ce

network: rename Manager::links -> Manager::links_by_index

6eab614

This also renames link_get() -> link_get_by_index().

network: introduce link_get_by_hw_addr()

fe321d4

network: ipv4ll: make link enter failed state on failure of restartin…

89d8ed9

…g ipv4ll engine

network: ipv4ll: refuse to configure IPv4LL address on interface wher…

a1e35fc

…e the length of the hardware address is not ETH_ALEN Currently, sd-ipv4acd assumes hardware address is ETH_ALEN.

network: ipv4ll: reset MAC address without stopping sd-ipv4ll engine

f8d6397

network: introduce address_dup() and replace address_copy() with it

99b4666

network: introduce link_request_static_address()

8c29f48

network: address: do not send tentative flag

d5e3b3e

Currently, networkd does not set tentative flag on create, and kernel ignore the flag on remove. So, this commit does not change any current behaviour. This is just a preparation for later commits.

network: rename dhcp_lease_lost() -> dhcp4_lease_lost() and expose it

9155da0

A preparation for later commits.

network: acquire address on request configure rather than on configure

c0c04a4

A preparation for later commits.

network: make request_drop() accept NULL

475ec33

network: introduce callback fucntions to check the sender MAC address…

d7ab6ef

… for IPv4ACD and IPv4LL Fixes systemd#12145.

test-network: update tests for ipv4acd

4492443

yuwata force-pushed the sd-ipv4acd-filter-all-hwaddr branch from a535d93 to 4492443 Compare June 30, 2021 15:50

keszybz reviewed Jun 30, 2021

View reviewed changes

keszybz merged commit 807aa0b into systemd:main Jun 30, 2021

keszybz added the ci-failure-appears-unrelated label Jun 30, 2021

yuwata deleted the sd-ipv4acd-filter-all-hwaddr branch June 30, 2021 20:54

yuwata added a commit to yuwata/systemd that referenced this pull request Jun 30, 2021

network: ipv4acd: update log message

7f02e0e

C.f. systemd#19980 (comment)

yuwata added a commit to yuwata/systemd that referenced this pull request Jul 1, 2021

network: ipv4acd: update log message

cd3a828

C.f. systemd#19980 (comment)

keszybz mentioned this pull request Jul 1, 2021

network: use request queue to configure DHCP clients #19939

Merged

mrc0mmand pushed a commit to mrc0mmand/rhel-9 that referenced this pull request Jul 2, 2021

network: ipv4acd: update log message

4d4aee1

C.f. systemd/systemd#19980 (comment)

mrc0mmand pushed a commit to mrc0mmand/rhel-9 that referenced this pull request Jul 2, 2021

network: ipv4acd: update log message

5dd5d03

C.f. systemd/systemd#19980 (comment)

jamacku pushed a commit to redhat-plumbers/systemd-rhel9 that referenced this pull request Jul 20, 2021

network: ipv4acd: update log message

0cff8f5

C.f. systemd/systemd#19980 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

network: IPv4LL and ACD fixes #19980

network: IPv4LL and ACD fixes #19980

yuwata commented Jun 20, 2021 •

edited

yuwata commented Jun 22, 2021

yuwata commented Jun 25, 2021

keszybz left a comment

keszybz Jun 30, 2021

yuwata Jun 30, 2021

yuwata commented Jun 30, 2021

keszybz left a comment

keszybz Jun 30, 2021

network: IPv4LL and ACD fixes #19980

network: IPv4LL and ACD fixes #19980

Conversation

yuwata commented Jun 20, 2021 • edited

yuwata commented Jun 22, 2021

yuwata commented Jun 25, 2021

keszybz left a comment

Choose a reason for hiding this comment

keszybz Jun 30, 2021

Choose a reason for hiding this comment

yuwata Jun 30, 2021

Choose a reason for hiding this comment

yuwata commented Jun 30, 2021

keszybz left a comment

Choose a reason for hiding this comment

keszybz Jun 30, 2021

Choose a reason for hiding this comment

yuwata commented Jun 20, 2021 •

edited