From owner-freebsd-ipfw@FreeBSD.ORG Sun Jul 1 15:54:36 2012 Return-Path: Delivered-To: freebsd-ipfw@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 41D6F106564A; Sun, 1 Jul 2012 15:54:36 +0000 (UTC) (envelope-from melifaro@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 146C78FC16; Sun, 1 Jul 2012 15:54:36 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q61FsZSP039192; Sun, 1 Jul 2012 15:54:35 GMT (envelope-from melifaro@freefall.freebsd.org) Received: (from melifaro@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q61FsZ6A039188; Sun, 1 Jul 2012 15:54:35 GMT (envelope-from melifaro) Date: Sun, 1 Jul 2012 15:54:35 GMT Message-Id: <201207011554.q61FsZ6A039188@freefall.freebsd.org> To: melifaro@FreeBSD.org, freebsd-ipfw@FreeBSD.org, melifaro@FreeBSD.org From: melifaro@FreeBSD.org Cc: 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 15:54:36 -0000 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 From owner-freebsd-ipfw@FreeBSD.ORG Sun Jul 1 18:50:06 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 1D257106566B; Sun, 1 Jul 2012 18:50:06 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id C5E2D8FC0A; Sun, 1 Jul 2012 18:50:05 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 26A367300A; Sun, 1 Jul 2012 21:09:21 +0200 (CEST) Date: Sun, 1 Jul 2012 21:09:21 +0200 From: Luigi Rizzo To: melifaro@freebsd.org, bug-followup@freebsd.org, alter@alter.org.ua Message-ID: <20120701190921.GA63663@onelab2.iet.unipi.it> References: <201207011554.q61FsZ6A039188@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201207011554.q61FsZ6A039188@freefall.freebsd.org> User-Agent: Mutt/1.4.2.3i Cc: freebsd-ipfw@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 18:50:06 -0000 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, 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). 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. - 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 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 > From owner-freebsd-ipfw@FreeBSD.ORG Mon Jul 2 10:13:38 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 5C98B106566B; Mon, 2 Jul 2012 10:13:38 +0000 (UTC) (envelope-from alter@alter.org.ua) Received: from homecat.alter.org.ua (homecat.alter.org.ua [IPv6:2a01:d0:0:1c::34]) by mx1.freebsd.org (Postfix) with ESMTP id D02E68FC08; Mon, 2 Jul 2012 10:13:37 +0000 (UTC) Received: from stealth.netassist.ua (stealth.netassist.ua [195.214.211.142]) by homecat.alter.org.ua (8.14.3/8.14.3) with ESMTP id q62ADJtS082854; Mon, 2 Jul 2012 13:13:19 +0300 (EEST) (envelope-from alter@alter.org.ua) Date: Mon, 2 Jul 2012 13:24:09 +0200 From: Alter Organization: AlterWare X-Priority: 3 (Normal) Message-ID: <602292882.20120702132409@alter.org.ua> To: Luigi Rizzo In-Reply-To: <20120701190921.GA63663@onelab2.iet.unipi.it> References: <201207011554.q61FsZ6A039188@freefall.freebsd.org> <20120701190921.GA63663@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.6 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, SUBJ_RE_NUM autolearn=no version=3.2.3 X-Spam-Checker-Version: SpamAssassin 3.2.3 (2007-08-08) on homecat.alter.org.ua Cc: freebsd-ipfw@freebsd.org, melifaro@freebsd.org, bug-followup@freebsd.org Subject: Re[2]: kern/156770: [ipfw] [dummynet] [patch]: performance improvement and several extensions X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Alter List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2012 10:13:38 -0000 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/) 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. 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. my_err() either calls errx() (without -q) or returns proper error code for handling in callee (with -q) -- Best regards, Alter mailto:alter@alter.org.ua From owner-freebsd-ipfw@FreeBSD.ORG Mon Jul 2 11:07:14 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 10ABE106566B for ; Mon, 2 Jul 2012 11:07:14 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id F04368FC0A for ; Mon, 2 Jul 2012 11:07:13 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q62B7Dwx012642 for ; Mon, 2 Jul 2012 11:07:13 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q62B7D9A012640 for freebsd-ipfw@FreeBSD.org; Mon, 2 Jul 2012 11:07:13 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 2 Jul 2012 11:07:13 GMT Message-Id: <201207021107.q62B7D9A012640@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-ipfw@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-ipfw@FreeBSD.org 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: Mon, 02 Jul 2012 11:07:14 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o kern/169206 ipfw [ipfw] ipfw does not flush entries in table o conf/167822 ipfw [ipfw] [patch] start script doesn't load firewall_type o kern/166406 ipfw [ipfw] ipfw does not set ALTQ identifier for ipv6 traf o kern/165190 ipfw [ipfw] [lo] [patch] loopback interface is not marking f kern/163873 ipfw [ipfw] ipfw fwd does not work with 'via interface' in o kern/158066 ipfw [ipfw] ipfw + netgraph + multicast = multicast packets o kern/157796 ipfw [ipfw] IPFW in-kernel NAT nat loopback / Default Route o kern/157689 ipfw [ipfw] ipfw nat config does not accept nonexistent int f kern/155927 ipfw [ipfw] ipfw stops to check packets for compliance with o bin/153252 ipfw [ipfw][patch] ipfw lockdown system in subsequent call o kern/153161 ipfw [ipfw] does not support specifying rules with ICMP cod o kern/152113 ipfw [ipfw] page fault on 8.1-RELEASE caused by certain amo o kern/148827 ipfw [ipfw] divert broken with in-kernel ipfw o kern/148689 ipfw [ipfw] antispoof wrongly triggers on link local IPv6 a o kern/148430 ipfw [ipfw] IPFW schedule delete broken. o kern/148091 ipfw [ipfw] ipfw ipv6 handling broken. o kern/143973 ipfw [ipfw] [panic] ipfw forward option causes kernel reboo o kern/143621 ipfw [ipfw] [dummynet] [patch] dummynet and vnet use result o kern/137346 ipfw [ipfw] ipfw nat redirect_proto is broken o kern/137232 ipfw [ipfw] parser troubles o kern/135476 ipfw [ipfw] IPFW table breaks after adding a large number o o kern/129036 ipfw [ipfw] 'ipfw fwd' does not change outgoing interface n p kern/128260 ipfw [ipfw] [patch] ipfw_divert damages IPv6 packets o kern/127230 ipfw [ipfw] [patch] Feature request to add UID and/or GID l f kern/122963 ipfw [ipfw] tcpdump does not show packets redirected by 'ip s kern/121807 ipfw [request] TCP and UDP port_table in ipfw o kern/121122 ipfw [ipfw] [patch] add support to ToS IP PRECEDENCE fields o kern/116009 ipfw [ipfw] [patch] Ignore errors when loading ruleset from o bin/104921 ipfw [patch] ipfw(8) sometimes treats ipv6 input as ipv4 (a o kern/104682 ipfw [ipfw] [patch] Some minor language consistency fixes a o kern/103454 ipfw [ipfw] [patch] [request] add a facility to modify DF b o kern/103328 ipfw [ipfw] [request] sugestions about ipfw table o kern/102471 ipfw [ipfw] [patch] add tos and dscp support o kern/97951 ipfw [ipfw] [patch] ipfw does not tie interface details to o kern/95084 ipfw [ipfw] [regression] [patch] IPFW2 ignores "recv/xmit/v o kern/86957 ipfw [ipfw] [patch] ipfw mac logging o bin/83046 ipfw [ipfw] ipfw2 error: "setup" is allowed for icmp, but s o kern/82724 ipfw [ipfw] [patch] [request] Add setnexthop and defaultrou o bin/78785 ipfw [patch] ipfw(8) verbosity locks machine if /etc/rc.fir o bin/65961 ipfw [ipfw] ipfw2 memory corruption inside add() o kern/60719 ipfw [ipfw] Headerless fragments generate cryptic error mes s kern/55984 ipfw [ipfw] [patch] time based firewalling support for ipfw o kern/48172 ipfw [ipfw] [patch] ipfw does not log size and flags o kern/46159 ipfw [ipfw] [patch] [request] ipfw dynamic rules lifetime f a kern/26534 ipfw [ipfw] Add an option to ipfw to log gid/uid of who cau 45 problems total. From owner-freebsd-ipfw@FreeBSD.ORG Mon Jul 2 12:32:09 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 0EB9D106564A; Mon, 2 Jul 2012 12:32:09 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id BC83A8FC14; Mon, 2 Jul 2012 12:32:07 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 693AD73027; Mon, 2 Jul 2012 14:51:30 +0200 (CEST) Date: Mon, 2 Jul 2012 14:51:30 +0200 From: Luigi Rizzo To: Alter Message-ID: <20120702125130.GA73867@onelab2.iet.unipi.it> References: <201207011554.q61FsZ6A039188@freefall.freebsd.org> <20120701190921.GA63663@onelab2.iet.unipi.it> <602292882.20120702132409@alter.org.ua> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <602292882.20120702132409@alter.org.ua> User-Agent: Mutt/1.4.2.3i Cc: freebsd-ipfw@freebsd.org, melifaro@freebsd.org, 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: Mon, 02 Jul 2012 12:32:09 -0000 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 >