On 01.07.2012 23:09, Luigi Rizzo wrote:
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,
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
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
To unsubscribe, send any mail to "[email protected]"
