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

rfc: wip: switch to netip for go 1.18 and v0.109.0 #4759

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MartB
Copy link

@MartB MartB commented Jul 18, 2022

First pull request to this repo, looking forward to your feedback 🍵

This fixes the issue with link local addresses (LLA) not working due to a limitation in go that discards the zone information and does not consider anything with fe80:: .... %ethX a valid IP address (see #2926)

Sadly I did not find a way to obtain LLAs from the interface list in "zoned" format, but specifying one for BindHosts in the aforementioned format works fine for binding the DNS resolver now!

I also lifted the artificial limitation in CheckPort for "udp" and "tcp" so ipv6 only listening with "udp6" and "tcp6" can also be tested.

Note: This commit will be squashed and rebased in the end.

@MartB MartB changed the title rfc: wip: switch to netip for go 1.18 rfc: wip: switch to netip for go 1.18 and v0.109.0 Jul 18, 2022
@ainar-g
Copy link
Contributor

ainar-g commented Jul 28, 2022

Hello, thank you for the contribution, and apologies for the delay.

It's generally better to find or file an issue to ask if a PR is necessary. Because often there is already a WIP branch somewhere. A lot of this code will be fully rewritten for the v0.108 cycle, so I'll be honest and say that this PR is likely to be overwritten by our own work. I'll add it to the milestone, but unfortunately I can't give any time frames.

@ainar-g ainar-g added this to the v0.108.0 milestone Jul 28, 2022
@ainar-g ainar-g self-assigned this Jul 28, 2022
@MartB
Copy link
Author

MartB commented Jul 28, 2022

@ainar-g Hey Ainar! Thanks for your reply :)

I made this PR bcs i urgently needed this change for my home network (dynamic IPv6 prefixes, only stable addresses are LLA).
So i had to port it anyway and it served as a learning opportunity to get to know AdGuardHomes code base and the new go IP implementation.

It is perfectly fine if it gets overwritten in a future release as theres no work "wasted" here. Just wanted to give back after fixing my issue :) If you guys end up having a somewhat production ready WIP branch, just drop me a hint here 👍

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

Successfully merging this pull request may close these issues.

2 participants