On Sun, Jul 01, 2012 at 03:54:35PM +0000, [email protected] 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 _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to "[email protected]"
