Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 2 Jul 2012 14:51:30 +0200
From:      Luigi Rizzo <rizzo@iet.unipi.it>
To:        Alter <alter@alter.org.ua>
Cc:        freebsd-ipfw@freebsd.org, melifaro@freebsd.org, bug-followup@freebsd.org
Subject:   Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions
Message-ID:  <20120702125130.GA73867@onelab2.iet.unipi.it>
In-Reply-To: <602292882.20120702132409@alter.org.ua>
References:  <201207011554.q61FsZ6A039188@freefall.freebsd.org> <20120701190921.GA63663@onelab2.iet.unipi.it> <602292882.20120702132409@alter.org.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
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,
>  Alter                            mailto:alter@alter.org.ua
> 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120702125130.GA73867>