-
Notifications
You must be signed in to change notification settings - Fork 43
feature: rate limit #346
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
base: main
Are you sure you want to change the base?
feature: rate limit #346
Conversation
qdeslandes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went for an early review, mostly for the parsing part. On the BPF side, the map will contain the runtime values for a given rule's rate limit (current burst/allowance, limit, last update time...).
src/bfcli/lexer.l
Outdated
| [0-9]+ { | ||
| BEGIN(INITIAL); | ||
| yylval.sval = strdup(yytext); | ||
| return RATELIMIT_VAL; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should expect any float value (even negative, we'll error out later on). We also need a time window , e.g. 10/second.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really wanna handle float or do we also error them out later on ?
It feels weird writing 12.1/second (even though it is technically correct), and we could have issues with float precision in some cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to handle floats, but it's easier to error out when parsing the matcher payload (e.g. with a clear error message) than in the lexer.
Fix #215
I've started getting into adding this features.
So far, I've managed to add the
ratelimit [int]/[time_unit]keyword to the cli and put it's value into abf_map.However I'm not sure about implementing a "real" rate limiting as it seems to requireEMIT(and that look scary).Added (empty for now) elfstub to handle rate limiting
The idea of the implementation so far is pretty naive (only allow the first X requests to pass through), mostly because it was easier to implement and I hadn't strong feelings on which direction to go.I'll keep this as a draft for now since it's still in its early stages.