Skip to content

pkg/ip: add functionality to coalesce CIDR list#3619

Merged
ianvernon merged 1 commit intomasterfrom
1658-coalesce-cidrs-2
Apr 11, 2018
Merged

pkg/ip: add functionality to coalesce CIDR list#3619
ianvernon merged 1 commit intomasterfrom
1658-coalesce-cidrs-2

Conversation

@ianvernon
Copy link
Copy Markdown
Member

@ianvernon ianvernon commented Apr 10, 2018

Add CoalesceCIDRs and associated helper functions, which combines a list of
CIDRs into the smallest equivalent set of CIDRs.

Signed-off by: Ian Vernon ian@cilium.io

This was long-lived in a previous PR, but I wanted to separate the library code being merged from the actual consumption of the code in Cilium.

Partially address #1658 : this is only the library functionality. Cilium can consume this change at a later time (this is tracked as part of #1658).

@ianvernon ianvernon added wip area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. labels Apr 10, 2018
@ianvernon ianvernon requested a review from a team as a code owner April 10, 2018 18:44
@ianvernon ianvernon force-pushed the 1658-coalesce-cidrs-2 branch from d7496de to a4e176c Compare April 10, 2018 20:31
@cilium cilium deleted a comment from houndci-bot Apr 10, 2018
@ianvernon
Copy link
Copy Markdown
Member Author

test-me-please

Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Minor comment, nothing blocking. I expect to start using this library soon, after which point I may be able to provide some better feedback as a user. Either way, the testing looks good and I expect that there will be users in the codebase before too long so it seems reasonable to merge in its current state.

Comment thread pkg/ip/ip.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this is making use of the RFC2765 IPv4-mapped address prefix? Would be nice to have a brief comment mentioning this, and perhaps naming the variable as v4Mappedv6Prefix

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in latest update.

Add CoalesceCIDRs and associated helper functions, which combines a list of
CIDRs into the smallest equivalent set of CIDRs.

Signed-off by: Ian Vernon <ian@cilium.io>
@ianvernon ianvernon force-pushed the 1658-coalesce-cidrs-2 branch from a4e176c to 2705a5e Compare April 11, 2018 05:36
@ianvernon
Copy link
Copy Markdown
Member Author

test-me-please

@ianvernon ianvernon added pending-review kind/enhancement This would improve or streamline existing functionality. and removed wip labels Apr 11, 2018
@ianvernon ianvernon merged commit ee23ad0 into master Apr 11, 2018
@ianvernon ianvernon deleted the 1658-coalesce-cidrs-2 branch April 11, 2018 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. kind/enhancement This would improve or streamline existing functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants