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

WIP: Added ipv6 support #97

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

WIP: Added ipv6 support #97

wants to merge 2 commits into from

Conversation

pamidur
Copy link

@pamidur pamidur commented Oct 18, 2019 •
edited
Loading

With these changes startup script can create AAAA records
Also prevent querying upstream servers when resolution is done from cache.

@VibroAxe
Copy link
Member

We've only had issues with ipv6 so far, DNS is only part of the issue. Currently it's disabled for good reasons.

If this is fully tested with sniproxy and monolithic ipv6 then that might be a different discussion

@pamidur
Copy link
Author

pamidur commented Oct 21, 2019

Well, I currently use it with lancachenet (ipv6 modified) in my ipv6 only network though without sniproxy.
I'm not ready (yet) to make a PR for lancachenet itself.

However I believe that there might more than one use case for lancache-dns. And I believe it should not be tied to lancachenet this hard.

@astrolox
Copy link
Member

Perhaps a enable/disable flag would be a good enough addition to allay any concerns?

@pamidur
Copy link
Author

pamidur commented Oct 22, 2019

@astrolox basically it works the same way as before if you supply only ipv4 addresses.
Now it just supports ipv6 addresses and dns records as well.

@astrolox
Copy link
Member

I'm a huge fan of IPv6 and I love that you're contributing this.
Yet I'm not ready to commit this without further testing and spending some time to understand what issues have arisen in the past. I'll leave this open for now and hopefully we can circle back to it when we have some spare time in the future.

@VibroAxe
Copy link
Member

VibroAxe commented Oct 23, 2019 •
edited
Loading

Sorry @pamidur, I didn't intend to come accross negatively!

I'm also a big fan of ipv6 but we've been caught out numerous times with people not understanding it enough causing huge issues. Currently the DNS disabling ipv6 lookups has been a useful backstop against "my setup is broken" issues.

I agree with astro, I'm interested in this, and we'll definitely keep it open. However I wouldn't be happy to release this without the corresponding generic/monolithic update. If you can submit a WIP PR to the relevant project then we can discuss the route to testing and adoption

@VibroAxe VibroAxe changed the title Added ipv6 support WIP: Added ipv6 support Oct 23, 2019
@pamidur
Copy link
Author

pamidur commented Oct 23, 2019

@VibroAxe , @astrolox , Alright, I'll try to put everything together and make a PR for ipv6 for lancache-generic soon

@astrolox astrolox self-requested a review November 4, 2019 17:58
@astrolox astrolox mentioned this pull request Apr 15, 2020
Open
N1kO23
N1kO23 approved these changes Apr 17, 2022
View reviewed changes
Copy link

@N1kO23 N1kO23 left a comment

Choose a reason for hiding this comment

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

Yes this works, would love to see in prod

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

@N1kO23 N1kO23 N1kO23 approved these changes

@VibroAxe VibroAxe Awaiting requested review from VibroAxe

@astrolox astrolox Awaiting requested review from astrolox

Assignees
No one assigned
Labels
enhancement help wanted testing-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pamidur @VibroAxe @astrolox @N1kO23