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

Implement EDNS Client Subnet ECS reading and writing #1906

Merged
merged 1 commit into from Mar 19, 2023

Conversation

mokeyish
Copy link
Contributor

@mokeyish mokeyish commented Mar 14, 2023

@djc djc requested a review from bluejekyll March 16, 2023 12:44
Copy link
Member

@bluejekyll bluejekyll left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks for contributing!

@mokeyish
Copy link
Contributor Author

Do you have a better name for ECS? It seems that it is not appropriate to name ECS.

@bluejekyll
Copy link
Member

Do you have a better name for ECS? It seems that it is not appropriate to name ECS.

You could call it "ClientSubnet" if you're referring to the field. That seems like it would be a better name (the EDNS of ECS, EDNS Client Subnet would be redundant).

Another question I have is if we might want to store or at least have conversions with the IpNet type? We already have this dependency in the library: https://docs.rs/ipnet/2.7.1/ipnet/enum.IpNet.html

@mokeyish
Copy link
Contributor Author

mokeyish commented Mar 17, 2023

Another question I have is if we might want to store or at least have conversions with the IpNet type? We already have this dependency in the library: https://docs.rs/ipnet/2.7.1/ipnet/enum.IpNet.html

I have tried ipnet,but it does not satisfy all derived traits.

@mokeyish mokeyish changed the title Implment EDNS Client Subnet ECS reading and writing Implement EDNS Client Subnet ECS reading and writing Mar 17, 2023
@bluejekyll
Copy link
Member

This looks good. Merging, thanks for the PR!

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.

None yet

2 participants