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

Actions: add rateLimitScope #1962

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

kevsecurity
Copy link
Contributor

Post actions can have a rateLimit argument that specifies how often identical events from the same hook and thread are generated. There is a use case to rate limit per process or generally.

This commit introduces the rateLimitScope argument, to be used with rateLimit, to specify whether the rate limiting should be limited to the same thread, the same process, or globally, using values "thread" (default), "process", or "global".

Add rateLimitScope to control what rateLimit applies to.

@kevsecurity kevsecurity added the release-note/minor This PR introduces a minor user-visible change label Jan 11, 2024
@kevsecurity kevsecurity requested review from mtardy and a team as code owners January 11, 2024 14:17
Copy link

netlify bot commented Jan 11, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit b68b89b
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/659ff898709fea00080cc80d
😎 Deploy Preview https://deploy-preview-1962--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kevsecurity kevsecurity force-pushed the pr/kevsecurity/apply-rate-limit-to-all-threads branch 2 times, most recently from deaba76 to 78262f0 Compare January 11, 2024 14:58
Copy link
Member

@tixxdz tixxdz left a comment

Choose a reason for hiding this comment

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

LGTM after I guess updating this

const CustomResourceDefinitionSchemaVersion = "1.1.2"
?

Really nice feature was planning to add it. Also we may want to add:
rateLimitBy="function (default) | policy_name"

@@ -1926,9 +1930,20 @@ rate_limit(__u64 ratelimit_interval, struct msg_generic_kprobe *e)
ro_heap = map_lookup_elem(&ratelimit_ro_heap, &zero);

key->func_id = e->func_id;
key->retprobe_id = e->retprobe_id;
Copy link
Member

Choose a reason for hiding this comment

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

I guess this was never used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah was overkill.

@@ -1926,9 +1930,20 @@ rate_limit(__u64 ratelimit_interval, struct msg_generic_kprobe *e)
ro_heap = map_lookup_elem(&ratelimit_ro_heap, &zero);

Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to memset zero those returned heap entries? (didn't check all the logic)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I overwrite the heap with a read only heap that is all zero, using a probe_read. It should cost about the same as a memset but doesn't cost me the instruction / complexity count.

@kevsecurity
Copy link
Contributor Author

LGTM after I guess updating this

const CustomResourceDefinitionSchemaVersion = "1.1.2"

?
Really nice feature was planning to add it. Also we may want to add: rateLimitBy="function (default) | policy_name"

Yeah I forgot the version! I'll do that now.

Post actions can have a rateLimit argument that specifies how often
identical events from the same hook and thread are generated. There is a
use case to rate limit per process or generally.

This commit introduces the rateLimitScope argument, to be used with
rateLimit, to specify whether the rate limiting should be limited to the
same thread, the same process, or globally, using values "thread"
(default), "process", or "global".

Signed-off-by: Kevin Sheldrake <[email protected]>
@kevsecurity kevsecurity force-pushed the pr/kevsecurity/apply-rate-limit-to-all-threads branch from 78262f0 to abbc757 Compare January 11, 2024 16:25
@kevsecurity kevsecurity merged commit 9d93c99 into main Jan 11, 2024
36 checks passed
@kevsecurity kevsecurity deleted the pr/kevsecurity/apply-rate-limit-to-all-threads branch January 11, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants