From owner-freebsd-ipfw@FreeBSD.ORG Sun Jul 1 19:09:03 2012 Return-Path: Delivered-To: freebsd-ipfw@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8A739106564A; Sun, 1 Jul 2012 19:09:03 +0000 (UTC) (envelope-from melifaro@FreeBSD.org) Received: from mail.ipfw.ru (unknown [IPv6:2a01:4f8:120:6141::2]) by mx1.freebsd.org (Postfix) with ESMTP id 212328FC0C; Sun, 1 Jul 2012 19:09:03 +0000 (UTC) Received: from v6.mpls.in ([2a02:978:2::5] helo=ws.su29.net) by mail.ipfw.ru with esmtpsa (TLSv1:CAMELLIA256-SHA:256) (Exim 4.76 (FreeBSD)) (envelope-from ) id 1SlPYi-000JVL-GD; Sun, 01 Jul 2012 23:11:48 +0400 Message-ID: <4FF0A04C.5090108@FreeBSD.org> Date: Sun, 01 Jul 2012 23:09:00 +0400 From: "Alexander V. Chernikov" User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120121 Thunderbird/9.0 MIME-Version: 1.0 To: Luigi Rizzo References: <201207011554.q61FsZ6A039188@freefall.freebsd.org> <20120701190921.GA63663@onelab2.iet.unipi.it> In-Reply-To: <20120701190921.GA63663@onelab2.iet.unipi.it> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-ipfw@freebsd.org, alter@alter.org.ua, bug-followup@freebsd.org Subject: Re: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Jul 2012 19:09:03 -0000 On 01.07.2012 23:09, Luigi Rizzo wrote: > On Sun, Jul 01, 2012 at 03:54:35PM +0000, melifaro@freebsd.org 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 >