From owner-cvs-all@FreeBSD.ORG Thu Feb 10 10:52:10 2005 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6F1E816A4D0 for ; Thu, 10 Feb 2005 10:52:10 +0000 (GMT) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6EC2043D4C for ; Thu, 10 Feb 2005 10:52:09 +0000 (GMT) (envelope-from oppermann@networx.ch) Received: (qmail 28441 invoked from network); 10 Feb 2005 10:30:31 -0000 Received: from unknown (HELO networx.ch) ([62.48.0.53]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 10 Feb 2005 10:30:31 -0000 Message-ID: <420B3CDA.9033810C@networx.ch> Date: Thu, 10 Feb 2005 11:52:10 +0100 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: Gleb Smirnoff References: <200502051206.j15C6YOY015206@repoman.freebsd.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: cvs-src@FreeBSD.org cc: src-committers@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 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Feb 2005 10:52:10 -0000 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