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

Ignore ipv6 #2622

Merged
merged 4 commits into from
Jun 20, 2017
Merged

Ignore ipv6 #2622

merged 4 commits into from
Jun 20, 2017

Conversation

rade
Copy link
Member

@rade rade commented Jun 20, 2017

There is no point in paying attention to them since scope connection tracking only deals in IPv4.

In Weave Cloud Prod, 138 of the 218 local networks in the combined report are IPv6 addresses. That slows down endpoint projection during rendering since that needs to detect whether an address is local. This PR doesn't solve that completely - matching against 80 networks is still expensive - but I have plans for that :)

rade added 4 commits June 20, 2017 09:04
There is no point in paying attention to them since scope connection
tracking only deals in IPv4.
...by removing them. It was a ridiculous amount of contorted code to
test some utterly trivial functionality that is largely provided by
the golang stdlib.
@rade rade requested a review from 2opremio June 20, 2017 11:15
@2opremio
Copy link
Contributor

2opremio commented Jun 20, 2017

Uhm, my only reservation about this is that we should probably be moving closer to support ipv6 instead of going way from it.

Granted, it doesn't make sense to waste resources on tracking ipv6 networks if we don't support ipv6 right now, but maybe we should be making easy to reenable it instead of simply removing all the code?

@2opremio
Copy link
Contributor

2opremio commented Jun 20, 2017

Although, after thinking a bit more about it, it's just a matter of removing ipv4Nets so I guess that's fine

@rade
Copy link
Member Author

rade commented Jun 20, 2017

(re-adding support for IPv6 here is) just a matter of removing ipv4Nets

Exactly.

@rade rade merged commit 80c2b9d into master Jun 20, 2017
@2opremio 2opremio deleted the ignore-ipv6 branch June 20, 2017 14:58
rade added a commit that referenced this pull request Jun 20, 2017
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