bugzilla-daemon at netfilter.org
2018-Jan-10 17:46 UTC
[Bug 1214] New: Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 Bug ID: 1214 Summary: Allow limit to use any value for time unit Product: nftables Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 Component: nft Assignee: pablo at netfilter.org Reporter: boite.pour.spam at gmail.com Currently, it's not possible to set a limit for, let's say, fifteen minutes. This is completely arbitrary, and counter productive because there is a huge gap between "mn" and "hour". The underlying interface use seconds anyway since the specified unit is converted to seconds in http://git.netfilter.org/nftables/tree/src/datatype.c#n1074 , so it should be possible to change: "limit 10/mn" to "limit 10/300" (or "limit 10/300s") This would require an additional line in the code from: static struct error_record *time_unit_parse(const struct location *loc, const char *str, uint64_t *unit) { if (strcmp(str, "second") == 0) *unit = 1ULL; else if (strcmp(str, "minute") == 0) *unit = 1ULL * 60; else if (strcmp(str, "hour") == 0) *unit = 1ULL * 60 * 60; else if (strcmp(str, "day") == 0) *unit = 1ULL * 60 * 60 * 24; else if (strcmp(str, "week") == 0) *unit = 1ULL * 60 * 60 * 24 * 7; else return error(loc, "Wrong rate format"); return NULL; } to this: static struct error_record *time_unit_parse(const struct location *loc, const char *str, uint64_t *unit) { if (strcmp(str, "second") == 0) *unit = 1ULL; else if (strcmp(str, "minute") == 0) *unit = 1ULL * 60; else if (strcmp(str, "hour") == 0) *unit = 1ULL * 60 * 60; else if (strcmp(str, "day") == 0) *unit = 1ULL * 60 * 60 * 24; else if (strcmp(str, "week") == 0) *unit = 1ULL * 60 * 60 * 24 * 7; else if (strchr("123456789", *str)) /* Starts with a non zero number */ *unit = (unsigned long long)strtol(str, NULL, 10); else return error(loc, "Wrong rate format"); return NULL; } The bison code should also be modified to allow number here: limit_stmt : LIMIT RATE limit_mode NUM SLASH time_unit limit_burst { $$ = limit_stmt_alloc(&@$); $$->limit.rate = $4; $$->limit.unit = $6; $$->limit.burst = $7; $$->limit.type = NFT_LIMIT_PKTS; $$->limit.flags = $3; } should read (kind of, untested): limit_stmt : LIMIT RATE limit_mode NUM SLASH STRING limit_burst { struct error_record *erec; uint64_t unit; erec = time_unit_parse(&@$, $5, &unit); xfree($5); if (erec != NULL) { erec_queue(erec, state->msgs); YYERROR; } $$ = limit_stmt_alloc(&@$); $$->limit.rate = $4; $$->limit.unit = unit; $$->limit.burst = $7; $$->limit.type = NFT_LIMIT_PKTS; $$->limit.flags = $3; } -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180110/bf6acdd1/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-10 18:13 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 Pablo Neira Ayuso <pablo at netfilter.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #1 from Pablo Neira Ayuso <pablo at netfilter.org> --- Would you send us a patch for this? Otherwise, I'll try to convince someone to work on this, but I can make any promise on delivery time. Thanks! -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180110/ce7c5fc4/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-10 18:13 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #2 from Pablo Neira Ayuso <pablo at netfilter.org> --- (In reply to Pablo Neira Ayuso from comment #1)> Would you send us a patch for this? > > Otherwise, I'll try to convince someone to work on this, but I can make any > promise on delivery time.I meant, I *cannot* make any promise... :-) -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180110/ec023ebb/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-10 18:31 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #3 from Cyril <boite.pour.spam at gmail.com> --- Once I've installed my system with git based nft, I'll make a patch. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180110/8be96ec4/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-18 22:11 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #4 from Cyril <boite.pour.spam at gmail.com> --- Created attachment 518 --> https://bugzilla.netfilter.org/attachment.cgi?id=518&action=edit Allow any number in time_unit parsing -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180118/4463e937/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-20 12:58 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 Cyril <boite.pour.spam at gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #518 is|0 |1 obsolete| | --- Comment #5 from Cyril <boite.pour.spam at gmail.com> --- Created attachment 519 --> https://bugzilla.netfilter.org/attachment.cgi?id=519&action=edit Fixed patch with correct listing when calling nft list This should solve this bug. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180120/8d748133/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-20 13:02 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #6 from Pablo Neira Ayuso <pablo at netfilter.org> --- (In reply to Cyril from comment #5)> Created attachment 519 [details] > Fixed patch with correct listing when calling nft list > > This should solve this bug.Thanks Cyril. A couple of comments: static char buffer[20]; Can we replace 20 by something more meaningful? strlen("18446744073709551616"); -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180120/d009585d/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-20 13:05 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #7 from Pablo Neira Ayuso <pablo at netfilter.org> --- (In reply to Pablo Neira Ayuso from comment #6)> (In reply to Cyril from comment #5) > > Created attachment 519 [details] > > Fixed patch with correct listing when calling nft list > > > > This should solve this bug. > > Thanks Cyril. > > A couple of comments: > > static char buffer[20]; > > Can we replace 20 by something more meaningful? > > strlen("18446744073709551616");Or add a comment /* maximum string length for 64-bit value. */ static char buffer[20]; So we remember about the 20 char magic number, this actually needs to be 21 since we need to store \0. BTW, would you also add tests to nftables/tests/py/any/limit.t You will also need to update tests/py/any/limit.t.payload to include what nft --debug=netlink generates. Thanks ! -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180120/e8d5c5f7/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-20 13:09 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #8 from Pablo Neira Ayuso <pablo at netfilter.org> --- (In reply to Pablo Neira Ayuso from comment #7)> (In reply to Pablo Neira Ayuso from comment #6) > > (In reply to Cyril from comment #5) > > > Created attachment 519 [details] > > > Fixed patch with correct listing when calling nft list > > > > > > This should solve this bug. > > > > Thanks Cyril. > > > > A couple of comments: > > > > static char buffer[20]; > > > > Can we replace 20 by something more meaningful? > > > > strlen("18446744073709551616"); > > Or add a comment > > /* maximum string length for 64-bit value. */ > static char buffer[20]; > > So we remember about the 20 char magic number, this actually needs to be 21 > since we need to store \0. > > BTW, would you also add tests to nftables/tests/py/any/limit.t > > You will also need to update tests/py/any/limit.t.payload to include what > nft --debug=netlink generates. > > Thanks !Regarding syntax, I wonder if it is worth to express this including time unit, ie. | NUM SECONDS So we leave room to support things like "1 packet every 3 weeks" later on. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180120/16e03c98/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-22 11:23 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #9 from Cyril <boite.pour.spam at gmail.com> --- Yes, that seems more logical. I assume that not using a "value before unit" means 1 for that unit and is not an error. So I intend to do the following: 1. Revert to master's state 2. Add a new rule in Bison to accept "NUM SLASH NUM TIME_UNIT" 3. Change the code in statement.c to print it correctly (as you said, I made a mistake in the worst case length, and add some comment to justify it) However, I'm not sure what's the best way for the following: 1. Modify the time_unit_parse to accept a value before the unit itself and/or 2. Rename it "time_parse" since it's parsing both time & unit or 3. Modify the upper function ("rate_parse" something, IDRC) to parse the value and keep the time_unit_parse as-is (but that imply "3 week" and not "3 weeks" in the textual version of the rule) or 4. Reject any specification that's not "value unit" thus let all bisons rules merge into a single one "NUM STRING" and parse it to accept "345k/3week" as well as "12/hour" or "3/900second" and so on. This however will likely require large modification to bison's rules since STRING is a "default" case, so I need to remove "hour/week/month/second" to avoid failing the grammar. I'm not sure about the test itself and the debug-netlink stuff. I'll try to hack around but I'll need guidance about it if I don't succeed implementing it. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180122/ffc1f7b4/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-27 15:11 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 Cyril <boite.pour.spam at gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #519 is|0 |1 obsolete| | --- Comment #10 from Cyril <boite.pour.spam at gmail.com> --- Created attachment 524 --> https://bugzilla.netfilter.org/attachment.cgi?id=524&action=edit Add support for period parsing This follows option #3 in the previous comment: It adds support for "limit 400/3 minute" or "235kbytes/6 day" Please notice that this patch will only pass tests if the other patch is applied to libnftnl. While libnftnl is sharing the "get_unit" function (same code in both project), I think it'll be better to rename and export the function to "get_period" (since it's now a period and not just an unit anymore) so we can avoid code duplication. This is not part of this bug though. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180127/03e658ac/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-27 15:13 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #11 from Cyril <boite.pour.spam at gmail.com> --- Created attachment 525 --> https://bugzilla.netfilter.org/attachment.cgi?id=525&action=edit Patch for libnftnl to get good dump in tests too. This is required to pass the tests with debug=netfilter since it's this code that's called. However, I guess it will be even better to make this visible and rename it to get_period instead and remove the duplicate in nftables. -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180127/e1808156/attachment.html>
bugzilla-daemon at netfilter.org
2018-Jan-30 13:21 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 --- Comment #12 from Pablo Neira Ayuso <pablo at netfilter.org> --- Thanks a lot for working on this. Would you send these patch to netfilter-devel at vger.kernel.org? Please, include your Signed-off-by tag. Thanks! -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180130/62fa580f/attachment.html>
bugzilla-daemon at netfilter.org
2018-Apr-29 23:41 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 rypervenche <sub at ryper.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sub at ryper.org -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20180429/0c53b37a/attachment.html>
bugzilla-daemon at netfilter.org
2019-Dec-08 02:44 UTC
[Bug 1214] Allow limit to use any value for time unit
https://bugzilla.netfilter.org/show_bug.cgi?id=1214 kfm at plushkava.net changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kfm at plushkava.net -- You are receiving this mail because: You are watching all bug changes. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20191208/dbfb36ca/attachment.html>
Apparently Analagous Threads
- [Bug 1216] New: Error messaging for "interval overlaps with previous one" misidentifies location
- [Bug 1410] New: STATELESS, rules with notrack into a map
- [Bug 1371] New: Concatenations Literal sets
- [Bug 1431] New: flush set doesn't work as expected in script
- [Bug 1422] New: iptables-nft fails to check / delete rules in raw table