Skip to content

ip tagging: refactor ip tagging to make it lock free#45166

Open
wbpcode wants to merge 4 commits into
envoyproxy:mainfrom
wbpcode:dev-make-ip-tag-better
Open

ip tagging: refactor ip tagging to make it lock free#45166
wbpcode wants to merge 4 commits into
envoyproxy:mainfrom
wbpcode:dev-make-ip-tag-better

Conversation

@wbpcode
Copy link
Copy Markdown
Member

@wbpcode wbpcode commented May 20, 2026

Commit Message: ip tagging: refactor ip tagging to make it lock free
Additional Description:

Make the ip tagging lock free and removed unnecessary update callback of data source.

Risk Level: mid.
Testing: unit.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
@wbpcode wbpcode requested review from nezdolik and zuercher as code owners May 20, 2026 03:06
@wbpcode wbpcode requested a review from Copilot May 20, 2026 03:07
@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 20, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the IP tagging filter to use atomic snapshots for tries and statistics, ensuring that lookups and stat increments remain consistent during file reloads. It also simplifies the general DataSourceProvider by removing the data_update_cb mechanism. A high-severity issue was identified where validation_visitor is captured by reference in a lambda within a singleton-managed provider, potentially leading to use-after-free bugs if the provider outlives the configuration.

Comment thread source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the HTTP IP tagging filter to publish immutable “loaded snapshot” objects (trie + matching stats container) so request-path lookups and stat updates can be performed without locking, and removes the unused/incorrect DataSourceProvider update callback mechanism.

Changes:

  • Introduce LoadedIpTags snapshots and IpTagsStats to keep trie + per-tag builtin stat names consistent across reloads.
  • Rework IpTagsProvider to publish LoadedIpTags atomically from the datasource transform callback (enabling lock-free request path).
  • Remove DataUpdateCb support from Config::DataSource::DataSourceProvider / DynamicData.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/extensions/filters/http/ip_tagging/ip_tagging_filter_test.cc Updates tests to validate sharing behavior via the new LoadedIpTags snapshot/trie access.
source/extensions/filters/http/ip_tagging/ip_tagging_filter.h Introduces IpTagsStats + LoadedIpTags and updates provider/config APIs to use snapshots.
source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc Implements snapshot creation on load/reload and updates filter request path to use a single per-request snapshot.
source/common/config/datasource.h Removes DataUpdateCb plumbing from DynamicData/DataSourceProvider and associated invocation logic.

Comment thread source/extensions/filters/http/ip_tagging/ip_tagging_filter.cc Outdated
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 20, 2026

/retest

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.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.

3 participants