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

Backport DAD handling to the stable branch #300

Open
wants to merge 7 commits into
base: stable
Choose a base branch
from

Conversation

raddessi
Copy link

@raddessi raddessi commented May 30, 2024

This backports the functionality of #232 to the stable branch so it can be used soon.

We were experiencing issues on our Debian 10 hosts similar to those in #30 where up route add ... on IPv6 interfaces would fail due to the address still being in a tentative state when the command is ran. I saw the DAD features were added in #232 but those are on the master branch which seems (for me at least) to be broken and probably in the middle of a rewrite.

I only have one outstanding issue that I can think of at the moment, shown below. Thanks!

Testing

  • without enabling ipv6_dad_handling_enabled in a policyconfig file such as /etc/network/ifupdown2/policy.d/dad.json the logic operates as it does currently
  • Enabling the feature flag in the above file path does enable the logic. Contents to enable the feature:
    {
        "address": {
                "module_globals": {
                        "ipv6_dad_handling_enabled": "yes"
                }
        }
    }
    
    • With no additional config, all IPv6 addresses in network config are waited on for the default period (60x 0.1s)
    • Adding the dad-attempts and/or dad-interval to a config file on any address will be picked up correctly
      • Only the first value is picked up here, you can specify it more than once and further instances are ignored. Of course only one value can be used but perhaps there is a better way to deal with this? Seems like the parent change would have this issue also so I don't think it's a big deal.
    • Any address can have dad-attempts 0 set which will forcibly bring up the address with nodad, even if dad-attempts is set somewhere else

Copy link
Contributor

@sohorx sohorx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I saw your backport. Overall, I don't see any issues but I dropped some code-wise nitpicking.

ifupdown2/addons/address.py Outdated Show resolved Hide resolved
ifupdown2/addons/address.py Outdated Show resolved Hide resolved
ifupdown2/addons/address.py Outdated Show resolved Hide resolved
raddessi and others added 3 commits June 24, 2024 11:07
@raddessi
Copy link
Author

No big deal @sohorx I didn't want to drift from the upstream code unless I had a reason to but that is only a minor problem. All 3 are merged.

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

Successfully merging this pull request may close these issues.

2 participants