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

lc trie: add exclusive flag. #3825

Merged
merged 5 commits into from
Jul 12, 2018

Conversation

PiotrSikora
Copy link
Contributor

@PiotrSikora PiotrSikora commented Jul 10, 2018

While there, replace remaining "tags" with "data".

Risk Level: Low
Testing: bazel test //test/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Piotr Sikora piotrsikora@google.com

*Risk Level*: Low
*Testing*: bazel test //test/...
*Docs Changes*: n/a
*Release Notes*: n/a

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor Author

cc @brian-pane

@htuch htuch requested a review from brian-pane July 10, 2018 17:15
@htuch htuch requested a review from ggreenway July 10, 2018 17:16
@ggreenway ggreenway assigned ccaraman and unassigned ggreenway Jul 10, 2018
ccaraman
ccaraman previously approved these changes Jul 10, 2018
Copy link
Contributor

@ccaraman ccaraman left a comment

Choose a reason for hiding this comment

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

@PiotrSikora should the exclusive flag be exposed in the filter to enable/disable?

@PiotrSikora
Copy link
Contributor Author

@ccaraman You mean in the IP tagging filter? It could be useful there (e.g. if tags are used for geolocation), but - assuming that you guys are using it - you should probably know better than me.

I'm happy to add it if you think it's going to be useful for you, but I'd rather not block this PR on it.

FWIW, I'm going to use this for destination IP matching in the filter chains.

brian-pane
brian-pane previously approved these changes Jul 10, 2018
Copy link
Contributor

@brian-pane brian-pane left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -309,7 +311,7 @@ template <class T> class LcTrie {
*/
template <class IpType, uint32_t address_size = CHAR_BIT * sizeof(IpType)> class BinaryTrie {
public:
BinaryTrie() : root_(std::make_unique<Node>()) {}
BinaryTrie(bool exclusive) : root_(std::make_unique<Node>()), exclusive_(exclusive) {}

/**
* Add a CIDR prefix and associated tag to the binary trie. If an entry already
Copy link
Contributor

Choose a reason for hiding this comment

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

[minor] You might want to s/tag/data/g in this comment for consistency with the new nomenclature.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora dismissed stale reviews from brian-pane and ccaraman via 4b74d34 July 10, 2018 23:01
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor Author

PiotrSikora commented Jul 10, 2018

@brian-pane I did mass replace, looks that I've missed bunch in the original commit.

@PiotrSikora
Copy link
Contributor Author

Friendly ping, since this was already approved twice (sans s/tag/data/g).

@mattklein123 mattklein123 merged commit 37f66d2 into envoyproxy:master Jul 12, 2018
snowp pushed a commit to snowp/envoy that referenced this pull request Jul 12, 2018
* origin/master:
  config: making v2-config-only a boolean flag (envoyproxy#3847)
  lc trie: add exclusive flag. (envoyproxy#3825)
  upstream: introduce PriorityStateManager, refactor EDS (envoyproxy#3783)
  test: deflaking header_integration_test (envoyproxy#3849)
  http: new style WebSockets, where headers and data are processed by the filter chain. (envoyproxy#3776)
  common: minor doc updates (envoyproxy#3845)
  fix master build (envoyproxy#3844)
  logging: Requiring details for RELEASE_ASSERT (envoyproxy#3842)
  test: add test for consistency of RawStatData internal memory representation (envoyproxy#3843)
  common: jittered backoff implementation (envoyproxy#3791)
  format: run buildifier on .bzl files. (envoyproxy#3824)
  Support mutable metadata for endpoints (envoyproxy#3814)
  test: deflaking a test, improving debugability (envoyproxy#3829)
  Update ApiConfigSource docs with grpc_services only for GRPC configs (envoyproxy#3834)
  Add hard-coded /hot_restart_version test (envoyproxy#3832)
  healthchecks: Add interval_jitter_percent healthcheck option (envoyproxy#3816)

Signed-off-by: Snow Pettersen <snowp@squareup.com>
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.

5 participants