Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

network: do not enable IPv4 ACD for IPv4 link-local address if ACD is disabled explicitly #22824

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

yuwata
Copy link
Member

@yuwata yuwata commented Mar 22, 2022

The commit yuwata@1cf4ed1 makes the IPv4 ACD
enabled unconditionally for IPv4 link-local addresses even if users
explicitly disable ACD.

This makes the IPv4 ACD is enabled by default, but honor user setting.

Fixes #22763.

@poettering
Copy link
Member

hmm, so far we tried to avoid boolean options that are prefixed with "Enable", as that is pretty much redundant wording. it's boolean after all...

@yuwata yuwata force-pushed the network-dad branch 2 times, most recently from fbf235e to 7db3b48 Compare March 22, 2022 15:04
@yuwata yuwata changed the title network: rename DuplicateAddressDetection= -> EnableDuplicateAddressDetection= network: do not enable IPv4 ACD for IPv4 link-local address if ACD is disabled explicitly Mar 22, 2022
@yuwata
Copy link
Member Author

yuwata commented Mar 22, 2022

hmm, so far we tried to avoid boolean options that are prefixed with "Enable", as that is pretty much redundant wording. it's boolean after all...

OK. The renaming part was dropped. PTAL.

@yuwata
Copy link
Member Author

yuwata commented Mar 23, 2022

The issue is not critical, but the offending commit changed previous behavior. Hence, please backport this to v249 and v250.

… disabled explicitly

The commit 1cf4ed1 makes the IPv4 ACD
enabled unconditionally for IPv4 link-local addresses even if users
explicitly disable ACD.

This makes the IPv4 ACD is enabled by default, but honor user setting.

Fixes systemd#22763.
@keszybz
Copy link
Member

keszybz commented Mar 23, 2022

I think we can simplify this further. I took the liberty to force-push into your branch, because I thought that just a simple cleanup can be done and I started working on a patch for that. In the end I realized that we can simplify the logic. Please drop my patch if you think it's wrong.

@yuwata
Copy link
Member Author

yuwata commented Mar 23, 2022

@keszybz No no, you confused the current unfortunate situation: DuplicateAddressDetection=yes was equivalent to DuplicateAddressDetection=none. That's soooo unfortunate.... For backward compatibility, we cannot safely use boolean string for the setting. Ugh!

I'd like to adopt something like you posted to clean up the current situation. But please drop the second commit, as it does not work as expected.

To make it clear, the very unfortunate code is the following:

n->duplicate_address_detection = r ? ADDRESS_FAMILY_NO : ADDRESS_FAMILY_YES;

@yuwata
Copy link
Member Author

yuwata commented Mar 23, 2022

(The second commit was dropped. PTAL)

@keszybz keszybz merged commit 2859932 into systemd:main Mar 23, 2022
@yuwata yuwata deleted the network-dad branch March 23, 2022 21:12
@keszybz
Copy link
Member

keszybz commented Apr 28, 2022

This does not apply cleanly to v250-stable. Please provide a pull request.

@keszybz
Copy link
Member

keszybz commented Jun 9, 2022

@yuwata ^

@yuwata
Copy link
Member Author

yuwata commented Jun 9, 2022

Oops. Will do.

@yuwata
Copy link
Member Author

yuwata commented Jun 9, 2022

v250 backport: systemd/systemd-stable#209
v249 backport: systemd/systemd-stable#210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Setting DuplicateAddressDetection=none doesn't disable DAD for link-local IPs
3 participants