ads: support envoy filter local ratelimit. #859

yuanqijing · 2024-09-15T10:09:04Z

What type of PR is this?
support local rate limit.

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kmesh-bot · 2024-09-15T10:09:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nlgwcy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2024-09-15T10:49:50Z

Codecov Report

Attention: Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.

Project coverage is 53.21%. Comparing base (36825ab) to head (33591ee).
Report is 79 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/controller/ads/cache.go	0.00%	3 Missing ⚠️

Files with missing lines	Coverage Δ
pkg/controller/ads/extensions/local_ratelimit.go	`100.00% <100.00%> (ø)`
pkg/controller/ads/cache.go	`47.01% <0.00%> (-0.54%)`	⬇️

... and 24 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53caaa8...33591ee. Read the comment docs.

hzxuzhonghu · 2024-09-18T02:20:37Z

api/filter/ratelimit.proto

+  }
+*/
+message LocalRateLimit {
+  reserved 1 to 2;


why reserve?

reserve for
{
"stat_prefix": ...,
"status": {...},
}
should i start token_bucket with number 1, or keep the number tag ?

You can start from 1

hzxuzhonghu · 2024-09-18T02:21:26Z

api/filter/ratelimit.proto

+      "enable_x_ratelimit_headers": ...,
+      "vh_rate_limits": ...,
+      "always_consume_default_token_bucket": {...},
+      "rate_limited_as_resource_exhausted": ...


if other fields not supported, please remove them. Add once we support it

just remove the comments? i copy the definition from envoy api.

hzxuzhonghu

First we need a proposal to better understand this implement

hzxuzhonghu · 2024-09-18T02:24:47Z

bpf/kmesh/ads/include/local_ratelimit.h

+};
+
+struct ratelimit_value {
+    __u64 last_topup;


can you add a comment about what is this

yuanqijing · 2024-09-19T13:34:15Z

First we need a proposal to better understand this implement

hi i have created a proposal pr in #873

yuanqijing · 2024-09-19T14:52:35Z

@hzxuzhonghu hi, ratelimit use the same way to cut tcp connection as circuit breaker, it can be merged after #570.

hzxuzhonghu · 2024-09-20T04:33:04Z

bpf/kmesh/ads/include/local_ratelimit.h

+    __type(value, struct ratelimit_value);
+    __uint(max_entries, 1000);
+    __uint(map_flags, BPF_F_NO_PREALLOC);
+} ratelimit_map SEC(".maps");


please keep consistent with other map name.

bpf/kmesh/ads/include/local_ratelimit.h

hzxuzhonghu · 2024-09-20T06:21:47Z

bpf/kmesh/ads/include/filter.h

@@ -181,6 +182,10 @@ int filter_chain_manager(ctx_buff_t *ctx)
    if (filter_chain == NULL) {
        return KMESH_TAIL_CALL_RET(-1);
    }
+
+    /* ratelimit check */
+    Local_rate_limit__check_and_take(filter_chain, &addr, ctx);


This is ratelimit on filterchain, where did you support rate limit on listener

Yes, the ratelimit takes on filterchain, maybe i express it incorrect. The EnvoyFilter YAML object applies to the filter chain, and the rate limit configuration is defined inside the filter. A listener includes multiple filter chains. In eBPF, the listener_manager tail calls the filter_chain_manager.

YaoZengzeng · 2024-09-25T01:32:23Z

pkg/controller/ads/extensions/local_ratelimit.go

@@ -0,0 +1,56 @@
+package extensions


Add copyright

YaoZengzeng · 2024-09-25T01:37:00Z

pkg/controller/ads/extensions/local_ratelimit.go

+const LocalRateLimit = "envoy.filters.tcp.local_ratelimit"
+
+// NewLocalRateLimit constructs a new LocalRateLimit filter wrapper.
+func NewLocalRateLimit(filter *listenerv3.Filter) (*listener.Filter_LocalRateLimit, error) {


Any UT or E2E test?

hi, i added ut tests.

Signed-off-by: yuan <[email protected]>

tacslon · 2024-10-11T08:35:12Z

lgtm, and your test report(google doc) can be pasted here to show your test results @yuanqijing

kmesh-bot added the do-not-merge/work-in-progress label Sep 15, 2024

kmesh-bot requested review from hzxuzhonghu and kwb0523 September 15, 2024 10:09

kmesh-bot added the size/L label Sep 15, 2024

yuanqijing force-pushed the ads/ratelimit branch 3 times, most recently from 5964c91 to 1c8d1bb Compare September 15, 2024 10:27

kmesh-bot added size/XL and removed size/L labels Sep 15, 2024

yuanqijing force-pushed the ads/ratelimit branch 2 times, most recently from 80747b0 to 17aa8b8 Compare September 15, 2024 10:34

kmesh-bot added size/XXL and removed size/XL labels Sep 15, 2024

yuanqijing force-pushed the ads/ratelimit branch from 17aa8b8 to e064980 Compare September 15, 2024 10:37

yuanqijing force-pushed the ads/ratelimit branch from b7ffc97 to d12d047 Compare September 15, 2024 10:54

yuanqijing changed the title ~~[WIP] ads: support envoy filter local ratelimit.~~ ads: support envoy filter local ratelimit. Sep 15, 2024

kmesh-bot removed the do-not-merge/work-in-progress label Sep 15, 2024

hzxuzhonghu reviewed Sep 18, 2024

View reviewed changes

yuanqijing force-pushed the ads/ratelimit branch 2 times, most recently from 917ece0 to 007baba Compare September 19, 2024 14:49

yuanqijing force-pushed the ads/ratelimit branch 2 times, most recently from cdf2d67 to 8b988d1 Compare September 19, 2024 15:16

hzxuzhonghu reviewed Sep 20, 2024

View reviewed changes

bpf/kmesh/ads/include/local_ratelimit.h Show resolved Hide resolved

yuanqijing force-pushed the ads/ratelimit branch from 8b988d1 to 6d4e280 Compare September 20, 2024 04:40

hzxuzhonghu reviewed Sep 20, 2024

View reviewed changes

yuanqijing force-pushed the ads/ratelimit branch from 6d4e280 to 854c4f6 Compare September 20, 2024 06:43

kmesh-bot added size/XL and removed size/XXL labels Sep 20, 2024

yuanqijing force-pushed the ads/ratelimit branch from 854c4f6 to 89445c8 Compare September 20, 2024 06:58

YaoZengzeng reviewed Sep 25, 2024

View reviewed changes

yuanqijing force-pushed the ads/ratelimit branch from 89445c8 to 4a57a25 Compare September 25, 2024 07:20

kmesh-bot added size/XXL and removed size/XL labels Sep 25, 2024

ads: support envoy filter local ratelimit.

33591ee

Signed-off-by: yuan <[email protected]>

yuanqijing force-pushed the ads/ratelimit branch from 4a57a25 to 33591ee Compare September 25, 2024 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ads: support envoy filter local ratelimit. #859

ads: support envoy filter local ratelimit. #859

yuanqijing commented Sep 15, 2024

kmesh-bot commented Sep 15, 2024

codecov bot commented Sep 15, 2024 •

edited

Loading

hzxuzhonghu Sep 18, 2024

yuanqijing Sep 19, 2024

hzxuzhonghu Sep 20, 2024

yuanqijing Sep 20, 2024

hzxuzhonghu Sep 18, 2024

yuanqijing Sep 19, 2024

hzxuzhonghu left a comment

hzxuzhonghu Sep 18, 2024

yuanqijing Sep 19, 2024

yuanqijing commented Sep 19, 2024

yuanqijing commented Sep 19, 2024

hzxuzhonghu Sep 20, 2024

yuanqijing Sep 20, 2024

hzxuzhonghu Sep 20, 2024

yuanqijing Sep 20, 2024

YaoZengzeng Sep 25, 2024

yuanqijing Sep 25, 2024

YaoZengzeng Sep 25, 2024

yuanqijing Sep 25, 2024

tacslon commented Oct 11, 2024

ads: support envoy filter local ratelimit. #859

Are you sure you want to change the base?

ads: support envoy filter local ratelimit. #859

Conversation

yuanqijing commented Sep 15, 2024

kmesh-bot commented Sep 15, 2024

codecov bot commented Sep 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuanqijing commented Sep 19, 2024

yuanqijing commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tacslon commented Oct 11, 2024

codecov bot commented Sep 15, 2024 •

edited

Loading