Re[2]: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
Hello Luigi, Seems, Alex answered most of you questions LR On the negative side: LR - documentation on new features is completely absent. Just a brief mention LR in the manpage of ftag/funtag, a short comment in a C source code. # Fast ipfw tagging (ftag) - you can assign up to 32 ftags on packet. All ftags are stored in single memory block as bitmap. Are faster than usual tags, those allocate separate memory block for each tag. # Local ipfw tagging (ltag) - you can assign up to 32 ltags on packet. Ltags are not preserved when packet leaves ipfw ruleset (e.g. is sent to another interface, diverted or passed through pipe). The benefit is performance - ltag does not require memory allocation at all. (from http://alter.org.ua/soft/fbsd/ipfw/) LR - a large number of changes to the userspace code replaces errx() LR with return my_err(...) . I might agree on the principle, but LR I'd like to see a few notes on why this change is required, LR and whether it can be applied independently of the others. This change is required to let -q work properly in all cases. Because of inclompete error handling ipfw may eventually exit when processing incorrect rule regardless of -q option. Such behavior seems to be dangerous, especially when dealing to remote servers and auto-generated rulesets. E.g. ruleset may become invalid because of removal of some interface from system. Also, incorrect update of external config file (used for ruleset generation) may lead system to inacessible state. my_err() either calls errx() (without -q) or returns proper error code for handling in callee (with -q) -- Best regards, Altermailto:al...@alter.org.ua ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
On Mon, Jul 02, 2012 at 01:24:09PM +0200, Alter wrote: Hello Luigi, Seems, Alex answered most of you questions LR On the negative side: LR - documentation on new features is completely absent. Just a brief mention LR in the manpage of ftag/funtag, a short comment in a C source code. # Fast ipfw tagging (ftag) - you can assign up to 32 ftags on packet. All ftags are stored in single memory block as bitmap. Are faster than usual tags, those allocate separate memory block for each tag. # Local ipfw tagging (ltag) - you can assign up to 32 ltags on packet. Ltags are not preserved when packet leaves ipfw ruleset (e.g. is sent to another interface, diverted or passed through pipe). The benefit is performance - ltag does not require memory allocation at all. (from http://alter.org.ua/soft/fbsd/ipfw/) i understand that the features are nice, however you need to add explanations in the manpage and in the code, not just in this email. Same goes for the rest of the features. So, let's restart the discussion once you have a patch that is referred to HEAD and has the various features split and documented. LR - a large number of changes to the userspace code replaces errx() LR with return my_err(...) . I might agree on the principle, but LR I'd like to see a few notes on why this change is required, LR and whether it can be applied independently of the others. This change is required to let -q work properly in all cases. Because of inclompete error handling ipfw may eventually exit when processing incorrect rule regardless of -q option. ok, that is a good change but then you should again separate the error handling patch from the one adding new features. Such behavior seems to be dangerous, especially when dealing to remote servers and auto-generated rulesets. E.g. ruleset may become invalid because of removal of some interface from system. Also, incorrect update of external config file (used for ruleset generation) may lead system to inacessible state. the previous two sentences have nothing to do with syntax checking (which is all the frontend can do). my_err() either calls errx() (without -q) or returns proper error code for handling in callee (with -q) cheers luigi -- Best regards, Altermailto:al...@alter.org.ua ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
Synopsis: [ipfw] [dummynet] [patch]: performance improvement and several extensions Responsible-Changed-From-To: freebsd-ipfw-melifaro Responsible-Changed-By: melifaro Responsible-Changed-When: Sun Jul 1 15:54:17 UTC 2012 Responsible-Changed-Why: Take http://www.freebsd.org/cgi/query-pr.cgi?pr=156770 ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
On Sun, Jul 01, 2012 at 03:54:35PM +, melif...@freebsd.org wrote: Synopsis: [ipfw] [dummynet] [patch]: performance improvement and several extensions Responsible-Changed-From-To: freebsd-ipfw-melifaro Responsible-Changed-By: melifaro Responsible-Changed-When: Sun Jul 1 15:54:17 UTC 2012 Responsible-Changed-Why: Take http://www.freebsd.org/cgi/query-pr.cgi?pr=156770 Alex, please any ipfw-related patch through me before committing. On this specific PR i have some comments and several concerns. First, as mentioned in the thread, some specific features (e.g. ftags) might be of interest, but the fact that this is a single monolitic patch make it hard to apply and review. Especially, at least judging from the description, i believe some of the changes replicate features that were already inserted around 2009 and later (in then-head). On the negative side: - documentation on new features is completely absent. Just a brief mention in the manpage of ftag/funtag, a short comment in a C source code. - the way some features are implemented is through adding new IOCTLs, which is the wrong way of doing things. In the 2009 rewrite (ipfw3) i tried to use a single ioctl which carries tagged messages for the various requests (similar to the microinstructions which make up a rule) so the code is easier to extend without breaking ABIs. Please follow the new style if you need to add commands. - can you please split the patch in individual components, and make sure that they not replicate functions already existent (or if they do, are they an improvement) ? I am especially referring to indexed skipto - a large number of changes to the userspace code replaces errx() with return my_err(...) . I might agree on the principle, but I'd like to see a few notes on why this change is required, and whether it can be applied independently of the others. cheers luigi ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
On 01.07.2012 23:09, Luigi Rizzo wrote: On Sun, Jul 01, 2012 at 03:54:35PM +, melif...@freebsd.org wrote: Synopsis: [ipfw] [dummynet] [patch]: performance improvement and several extensions Responsible-Changed-From-To: freebsd-ipfw-melifaro Responsible-Changed-By: melifaro Responsible-Changed-When: Sun Jul 1 15:54:17 UTC 2012 Responsible-Changed-Why: Take http://www.freebsd.org/cgi/query-pr.cgi?pr=156770 Alex, Not sure if you're speaking to me, since both submitter and I are Alexanders :) However I'll try to answer some of the questions. please any ipfw-related patch through me before committing. On this specific PR i have some comments and several concerns. First, as mentioned in the thread, some specific features (e.g. ftags) might be of interest, but the fact that this is a single monolitic patch make it hard to apply and review. Especially, at least judging from the description, i believe some of the changes replicate features that were already inserted around 2009 and later (in then-head). We already got private discussion resulting in preparation of some most interesting (at least to me) parts of code to be split into different patches and remade to work on -current. Particularly I'm interested in rule indexes mostly. On the negative side: - documentation on new features is completely absent. Just a brief mention in the manpage of ftag/funtag, a short comment in a C source code. - the way some features are implemented is through adding new IOCTLs, which is the wrong way of doing things. In the 2009 rewrite (ipfw3) i tried to use a single ioctl which carries tagged messages for the various requests (similar to the microinstructions which make up a rule) so the code is easier to extend without breaking ABIs. Please follow the new style if you need to add commands. IP_FW3 is already used in ipv6 tables code, so there are some ipfw(8) and kernel code to reuse. - can you please split the patch in individual components, and make sure that they not replicate functions already existent (or if they do, are they an improvement) ? I am especially referring to indexed skipto - a large number of changes to the userspace code replaces errx() with return my_err(...) . I might agree on the principle, but I'd like to see a few notes on why this change is required, and whether it can be applied independently of the others. cheers luigi ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
The following reply was made to PR kern/156770; it has been noted by GNATS. From: Alexander V. Chernikov melif...@freebsd.org To: bug-follo...@freebsd.org, al...@alter.org.ua Cc: Subject: Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions Date: Fri, 15 Jun 2012 16:35:56 +0400 Hello Alexandr! I'm afraid singe huge patch for legacy release is not the promising start. Since development model assumes new code being committed to -current first, you should probably port these features to -current (it does not differ from 8-STABLE much). It is also much easier to discuss/import features by small chunks instead of single huge change, so splitting every feature into separate diff is possibly a good thing to do. Please note that some of functionality (skipto tablearg, interface tables are already implemented in a different way). Personally for me index table for fast skipto/pipes, mapped tables and io_fast patch looks very promising, so we can discuss directly if you're interested. ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
The following reply was made to PR kern/156770; it has been noted by GNATS. From: Alter al...@alter.org.ua To: bug-follo...@freebsd.org, Alter al...@alter.org.ua Cc: Subject: Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions Date: Thu, 14 Jun 2012 17:11:18 +0200 Hello bug-followup, I've made unified diff for this patch: http://alter.org.ua/soft/fbsd/ipfw/ipfw.72.20120614u.patch.gz About porting: seems, I'll port it to 8.x soon. Also, there were some discussions about various features of this patch and dividing it into separate patches (one for each feature). What I can do almost immediately is making single patch with some subset of new features. Even with some additional sysctls or #define's (to enable/disable feature). I just need to know the final decision and feature list. -- Best regards, Alter mailto:al...@alter.org.ua ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
On Sat, Jan 28, 2012 at 04:00:28PM +, ??? ??? wrote: The following reply was made to PR kern/156770; it has been noted by GNATS. From: =?windows-1251?B?yu7t/Oru4iDF4uPl7ejp?= kes-...@yandex.ru To: bug-follo...@freebsd.org, al...@alter.org.ua Cc: Subject: Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions Date: Sat, 28 Jan 2012 17:58:56 +0200 Hi, Team. Do you plan to port this patch to FreeBSD-10 or 9? It will be veri nice especially this feature: # it is possible to use bmap instead of port list. It gives performance benefit when you have large list of services. Lookup time doesn't depend on list size. Rather useful to QoS game traffic. not as is. Way too many pieces, many are interesting, from the description some of them are already in. If someone can submit smaller patches for the individual features (such as port maps, fast tags etc) then i can certainly consider adding some of them. ___ freebsd-ipfw@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to freebsd-ipfw-unsubscr...@freebsd.org
