Date: Thu, 10 Feb 2005 11:52:10 +0100 From: Andre Oppermann <oppermann@networx.ch> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sbin/ipfw ipfw2.c src/sys/netgraph ng_ipfw.cng_ipfw.h src/sys/netinet ip_fw.h ip_fw2.c ip_fw_pfil.c Message-ID: <420B3CDA.9033810C@networx.ch> References: <200502051206.j15C6YOY015206@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Gleb Smirnoff wrote:
>
> glebius 2005-02-05 12:06:33 UTC
>
> FreeBSD src repository
>
> Modified files:
> sbin/ipfw ipfw2.c
> sys/netinet ip_fw.h ip_fw2.c ip_fw_pfil.c
> Added files:
> sys/netgraph ng_ipfw.c ng_ipfw.h
> Log:
> Add a ng_ipfw node, implementing a quick and simple interface between
> ipfw(4) and netgraph(4) facilities.
>
> Reviewed by: andre, brooks, julian
^^^^^
I have not withdrawn my objections to the non-decoupling upon entering
into netgraph. I think you should decouple the stack upon entering
netgraph and not when returning back to ng_ipfw. It is not neccessary
to go back the same way and I can imagine several normal setups where
packets may come back through another way leading to recursions and a
very (too) deep stack. NG_SEND_DATA_ONLY() doesn't seem to decouple
it but it's hard to follow the netgraph code and I'm not too used to
it. If you can show that NG_SEND_DATA_ONLY() does in fact decouple
it then I withdraw this objection.
The other thing is the passing back of errors from netgraph. Only
certain kinds of errors should be reported back and others converted
to some default error. It is very confusing for an application
developer to get a very (from his POV) non-sensical error message
like ENOTCONN when writing on a socket. He doesn't have knowledge
of the ipfw/netgraph stuff that happens in the kernel and it makes
debugging extremely confusing. In the he blames FreeBSDs socket
implementation whereas it was only some error in setting up the
netgraph by the administrator.
You've got others to review you stuff and committed it. But it was
pretty clear that I wasn't fully happy with the code yet so please
don't put my name into the reviewed-by line then.
--
Andre
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?420B3CDA.9033810C>