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.

Sign up for GitHub

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

Jump to bottom

dhcpcd: 6.11.5 -> 7.0.8 #45912

Merged
merged 2 commits into from Sep 2, 2018
Merged

dhcpcd: 6.11.5 -> 7.0.8 #45912

merged 2 commits into from Sep 2, 2018

Conversation

xeji
Copy link
Contributor

@xeji xeji commented Sep 1, 2018

Motivation for this change

Latest version, supersedes #37173. Note that dhcpcd updates were tried and reverted before because they broke the networking.scripted.macvlan networking test, see #37173, #35622, #33527.

The test still fails with this update. Probably an issue with the test itself, not dhcpcd (even without this PR, the networking.networkd.macvlan test fails in about 50% of the runs because dhcpcd assigns addresses differently to clients).

WIP while I am still investigating the macvlan tests.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@xeji xeji added this to the 18.09 milestone Sep 1, 2018
@xeji xeji mentioned this pull request Sep 1, 2018
Closed
@vcunat
Copy link
Member

vcunat commented Sep 1, 2018

Here was a hypothesis of what happens: #35622 (comment)

@xeji
nixos/tests/networking.*.macvlan: disable reverse path check
13c3986 
Generated reverse path filtering rules for the macvlan interface
seem to be incorrect, causing the test to fail - sometimes or always,
depending on the dhcpcd version used.
- Disable reverse path checking temporarily to avoid blocking the channel
- Print more diagnostic information for debugging
@xeji
Copy link
Contributor Author

xeji commented Sep 2, 2018 •
edited

This took a loot of testing but was very interesting...

What happens in the macvlan test looks different from previous hypotheses (in particular, it doesn't seem related to qemu at all, we're running a newer qemu now):

The generated firewall reverse path filtering rules for the macvlan interface on the client are incorrect, causing the test to fail. This can be reproduced

  • in about 30% of test runs with dhcpcd 6.11.5 and networkd, depending on what interface gets which address, see also hydra
  • "never" with dhcpcd 6.11.5 and scripted network
  • "always" with dhcpcd 7.0.8 for both scripted and networkd

So it looks like a firewall-related issue that also depends on the interaction with dhcpcd. I'm not yet sure if this is a general bug in our firewall code, or just occurs in combination with our specific test setup. This will need (a lot) more investigation, and I'll open an issue to track it.

I have disabled reverse path filtering in the macvlan test for now.
It now reliably passes with dhcpcd 7.0.8.

@vcunat we can go ahead with the dhcpcd update - I don't see anything wrong with it atm. All networking tests succeed.

@xeji xeji changed the title [ WIP] dhcpcd: 6.11.5 -> 7.0.8 dhcpcd: 6.11.5 -> 7.0.8 Sep 2, 2018
@xeji xeji mentioned this pull request Sep 2, 2018
Open
@samueldr samueldr merged commit a92cfb5 into NixOS:master Sep 2, 2018
@xeji xeji deleted the p/dhcpcd branch September 2, 2018 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
18.09
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@xeji @vcunat @samueldr @GrahamcOfBorg