From owner-freebsd-ipfw@FreeBSD.ORG Mon Apr 4 11:07:02 2011 Return-Path: Delivered-To: freebsd-ipfw@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4558F1065678 for ; Mon, 4 Apr 2011 11:07:02 +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 297DB8FC1A for ; Mon, 4 Apr 2011 11:07:02 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p34B721w028637 for ; Mon, 4 Apr 2011 11:07:02 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p34B71lf028635 for freebsd-ipfw@FreeBSD.org; Mon, 4 Apr 2011 11:07:01 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 4 Apr 2011 11:07:01 GMT Message-Id: <201104041107.p34B71lf028635@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, 04 Apr 2011 11:07:02 -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/155927 ipfw [ipfw] ipfw stops to check bags for compliance with th o kern/153415 ipfw [ipfw] [patch] Port numbers always zero in dynamic IPF o bin/153252 ipfw [ipfw][patch] ipfw lockdown system in subsequent call o kern/153161 ipfw IPFIREWALL does not allow specify rules with ICMP code a kern/152887 ipfw [ipfw] Can not set more then 1024 buckets with buckets o kern/152113 ipfw [ipfw] page fault on 8.1-RELEASE caused by certain amo o kern/150798 ipfw [ipfw] ipfw2 fwd rule matches packets but does not do 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/148157 ipfw [ipfw] IPFW in kernel nat BUG found in FreeBSD 8.1-PRE o kern/148091 ipfw [ipfw] ipfw ipv6 handling broken. o kern/147720 ipfw [ipfw] ipfw dynamic rules and fwd o kern/145305 ipfw [ipfw] ipfw problems, panics, data corruption, ipv6 so o kern/144269 ipfw [ipfw] problem with ipfw tables o kern/144187 ipfw [ipfw] deadlock using multiple ipfw nat and multiple l o kern/143973 ipfw [ipfw] [panic] ipfw forward option causes kernel reboo o kern/143653 ipfw [ipfw] [patch] ipfw nat redirect_port "buf is too smal o kern/143621 ipfw [ipfw] [dummynet] [patch] dummynet and vnet use result o kern/143474 ipfw [ipfw] ipfw table contains the same address f kern/142951 ipfw [dummynet] using pipes&queues gives OUCH! pipe should o kern/139581 ipfw [ipfw] "ipfw pipe" not limiting bandwidth o kern/139226 ipfw [ipfw] install_state: entry already present, done o kern/137346 ipfw [ipfw] ipfw nat redirect_proto is broken o kern/137232 ipfw [ipfw] parser troubles o kern/136695 ipfw [ipfw] [patch] fwd reached after skipto in dynamic rul o kern/135476 ipfw [ipfw] IPFW table breaks after adding a large number o o bin/134975 ipfw [patch] ipfw(8) can't work with set in rule file. o kern/131817 ipfw [ipfw] blocks layer2 packets that should not be blocke o kern/131601 ipfw [ipfw] [panic] 7-STABLE panic in nat_finalise (tcp=0) o kern/131558 ipfw [ipfw] Inconsistent "via" ipfw behavior o bin/130132 ipfw [patch] ipfw(8): no way to get mask from ipfw pipe sho o kern/129103 ipfw [ipfw] IPFW check state does not work =( o kern/129093 ipfw [ipfw] ipfw nat must not drop packets o kern/129036 ipfw [ipfw] 'ipfw fwd' does not change outgoing interface n o 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 o kern/127209 ipfw [ipfw] IPFW table become corrupted after many changes o bin/125370 ipfw [ipfw] [patch] increase a line buffer limit o conf/123119 ipfw [patch] rc script for ipfw does not handle IPv6 o kern/122963 ipfw [ipfw] tcpdump does not show packets redirected by 'ip o kern/122109 ipfw [ipfw] ipfw nat traceroute problem s kern/121807 ipfw [request] TCP and UDP port_table in ipfw o kern/121382 ipfw [dummynet] 6.3-RELEASE-p1 page fault in dummynet (corr o kern/121122 ipfw [ipfw] [patch] add support to ToS IP PRECEDENCE fields o kern/118993 ipfw [ipfw] page fault - probably it's a locking problem o bin/117214 ipfw ipfw(8) fwd with IPv6 treats input as IPv4 o kern/116009 ipfw [ipfw] [patch] Ignore errors when loading ruleset from o docs/113803 ipfw [patch] ipfw(8) - don't get bitten by the fwd rule o kern/112561 ipfw [ipfw] ipfw fwd does not work with some TCP packets o kern/105330 ipfw [ipfw] [patch] ipfw (dummynet) does not allow to set q 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/98831 ipfw [ipfw] ipfw has UDP hickups 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/93300 ipfw [ipfw] ipfw pipe lost packets o kern/91847 ipfw [ipfw] ipfw with vlanX as the device o kern/88659 ipfw [modules] ipfw and ip6fw do not work properly as modul o kern/87032 ipfw [ipfw] [patch] ipfw ioctl interface implementation 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 kern/74104 ipfw [ipfw] ipfw2/1 conflict not detected or reported, manp o kern/73910 ipfw [ipfw] serious bug on forwarding of packets after NAT o kern/72987 ipfw [ipfw] ipfw/dummynet pipe/queue 'queue [BYTES]KBytes ( o kern/71366 ipfw [ipfw] "ipfw fwd" sometimes rewrites destination mac a o kern/69963 ipfw [ipfw] install_state warning about already existing en o kern/60719 ipfw [ipfw] Headerless fragments generate cryptic error mes o kern/55984 ipfw [ipfw] [patch] time based firewalling support for ipfw o kern/51274 ipfw [ipfw] [patch] ipfw2 create dynamic rules with parent 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 78 problems total. From owner-freebsd-ipfw@FreeBSD.ORG Tue Apr 5 03:34:48 2011 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 36730106564A; Tue, 5 Apr 2011 03:34:48 +0000 (UTC) (envelope-from linimon@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 0D0D38FC08; Tue, 5 Apr 2011 03:34:48 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p353YlMK028882; Tue, 5 Apr 2011 03:34:47 GMT (envelope-from linimon@freefall.freebsd.org) Received: (from linimon@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p353YlY8028878; Tue, 5 Apr 2011 03:34:47 GMT (envelope-from linimon) Date: Tue, 5 Apr 2011 03:34:47 GMT Message-Id: <201104050334.p353YlY8028878@freefall.freebsd.org> To: linimon@FreeBSD.org, freebsd-bugs@FreeBSD.org, freebsd-ipfw@FreeBSD.org From: linimon@FreeBSD.org Cc: Subject: Re: kern/156180: [ipfw] [patch] ipfw checks TCP options on non-contiguous header 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: Tue, 05 Apr 2011 03:34:48 -0000 Old Synopsis: ipfw checks TCP options on non-contiguous header New Synopsis: [ipfw] [patch] ipfw checks TCP options on non-contiguous header Responsible-Changed-From-To: freebsd-bugs->freebsd-ipfw Responsible-Changed-By: linimon Responsible-Changed-When: Tue Apr 5 03:34:24 UTC 2011 Responsible-Changed-Why: Over to maintainer(s). http://www.freebsd.org/cgi/query-pr.cgi?pr=156180 From owner-freebsd-ipfw@FreeBSD.ORG Tue Apr 5 21:30:15 2011 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 32088106566C for ; Tue, 5 Apr 2011 21:30:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 233BE8FC19 for ; Tue, 5 Apr 2011 21:30:15 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p35LUFFR030955 for ; Tue, 5 Apr 2011 21:30:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p35LUElF030950; Tue, 5 Apr 2011 21:30:14 GMT (envelope-from gnats) Date: Tue, 5 Apr 2011 21:30:14 GMT Message-Id: <201104052130.p35LUElF030950@freefall.freebsd.org> To: freebsd-ipfw@FreeBSD.org From: Gleb Smirnoff Cc: Subject: kern/156180 X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Gleb Smirnoff List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Apr 2011 21:30:15 -0000 The following reply was made to PR kern/156180; it has been noted by GNATS. From: Gleb Smirnoff To: bug-followup@FreeBSD.org Cc: ae@FreeBSD.org Subject: kern/156180 Date: Wed, 6 Apr 2011 01:07:29 +0400 --5gxpn/Q6ypwruk0T Content-Type: text/plain; charset=koi8-r Content-Disposition: inline What about the following approach? See attached snap, not tested, patch. -- Totus tuus, Glebius. --5gxpn/Q6ypwruk0T Content-Type: text/x-diff; charset=koi8-r Content-Disposition: attachment; filename="156180.diff" Index: ip_fw2.c =================================================================== --- ip_fw2.c (revision 220373) +++ ip_fw2.c (working copy) @@ -913,9 +913,10 @@ * pointer might become stale after other pullups (but we never use it * this way). */ -#define PULLUP_TO(_len, p, T) \ +#define PULLUP_TO(_len, p, T) PULLUP_LEN(_len, p, sizeof(T)) +#define PULLUP_LEN(_len, p, T) \ do { \ - int x = (_len) + sizeof(T); \ + int x = (_len) + T; \ if ((m)->m_len < x) { \ args->m = m = m_pullup(m, x); \ if (m == NULL) \ @@ -1600,6 +1601,7 @@ break; case O_TCPOPTS: + PULLUP_LEN(hlen, ulp, (TCP(ulp)->th_off << 2)); match = (proto == IPPROTO_TCP && offset == 0 && tcpopts_match(TCP(ulp), cmd)); break; @@ -2230,6 +2232,7 @@ } } /* end of inner loop, scan opcodes */ +#undef PULLUP_LEN if (done) break; --5gxpn/Q6ypwruk0T-- From owner-freebsd-ipfw@FreeBSD.ORG Wed Apr 6 06:45:26 2011 Return-Path: Delivered-To: freebsd-ipfw@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9C944106566C for ; Wed, 6 Apr 2011 06:45:26 +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 5A6328FC14 for ; Wed, 6 Apr 2011 06:45:25 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id AF3887300A; Wed, 6 Apr 2011 09:00:00 +0200 (CEST) Date: Wed, 6 Apr 2011 09:00:00 +0200 From: Luigi Rizzo To: Gleb Smirnoff Message-ID: <20110406070000.GA8987@onelab2.iet.unipi.it> References: <201104052130.p35LUElF030950@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201104052130.p35LUElF030950@freefall.freebsd.org> User-Agent: Mutt/1.4.2.3i Cc: freebsd-ipfw@freebsd.org Subject: Re: kern/156180 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: Wed, 06 Apr 2011 06:45:26 -0000 On Tue, Apr 05, 2011 at 09:30:14PM +0000, Gleb Smirnoff wrote: > The following reply was made to PR kern/156180; it has been noted by GNATS. > > From: Gleb Smirnoff > To: bug-followup@FreeBSD.org > Cc: ae@FreeBSD.org > Subject: kern/156180 > Date: Wed, 6 Apr 2011 01:07:29 +0400 > > --5gxpn/Q6ypwruk0T > Content-Type: text/plain; charset=koi8-r > Content-Disposition: inline no objection cheers luigi > What about the following approach? See attached > snap, not tested, patch. > > -- > Totus tuus, Glebius. > > --5gxpn/Q6ypwruk0T > Content-Type: text/x-diff; charset=koi8-r > Content-Disposition: attachment; filename="156180.diff" > > Index: ip_fw2.c > =================================================================== > --- ip_fw2.c (revision 220373) > +++ ip_fw2.c (working copy) > @@ -913,9 +913,10 @@ > * pointer might become stale after other pullups (but we never use it > * this way). > */ > -#define PULLUP_TO(_len, p, T) \ > +#define PULLUP_TO(_len, p, T) PULLUP_LEN(_len, p, sizeof(T)) > +#define PULLUP_LEN(_len, p, T) \ > do { \ > - int x = (_len) + sizeof(T); \ > + int x = (_len) + T; \ > if ((m)->m_len < x) { \ > args->m = m = m_pullup(m, x); \ > if (m == NULL) \ > @@ -1600,6 +1601,7 @@ > break; > > case O_TCPOPTS: > + PULLUP_LEN(hlen, ulp, (TCP(ulp)->th_off << 2)); > match = (proto == IPPROTO_TCP && offset == 0 && > tcpopts_match(TCP(ulp), cmd)); > break; > @@ -2230,6 +2232,7 @@ > } > > } /* end of inner loop, scan opcodes */ > +#undef PULLUP_LEN > > if (done) > break; > > --5gxpn/Q6ypwruk0T-- > _______________________________________________ > freebsd-ipfw@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw > To unsubscribe, send any mail to "freebsd-ipfw-unsubscribe@freebsd.org" From owner-freebsd-ipfw@FreeBSD.ORG Wed Apr 6 17:00:32 2011 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 1DC7D1065707 for ; Wed, 6 Apr 2011 17:00:32 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 468378FC1D for ; Wed, 6 Apr 2011 17:00:21 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p36H0Lk9028837 for ; Wed, 6 Apr 2011 17:00:21 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p36H0LXu028836; Wed, 6 Apr 2011 17:00:21 GMT (envelope-from gnats) Date: Wed, 6 Apr 2011 17:00:21 GMT Message-Id: <201104061700.p36H0LXu028836@freefall.freebsd.org> To: freebsd-ipfw@FreeBSD.org From: Gleb Smirnoff Cc: Subject: Re: kern/156180 X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Gleb Smirnoff List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Apr 2011 17:00:32 -0000 The following reply was made to PR kern/156180; it has been noted by GNATS. From: Gleb Smirnoff To: Karim Fodil-Lemelin Cc: bug-followup@freebsd.org Subject: Re: kern/156180 Date: Wed, 6 Apr 2011 20:59:22 +0400 Hi, Karim! On Wed, Apr 06, 2011 at 10:36:46AM -0400, Karim Fodil-Lemelin wrote: K> Thanks for the patch it does work in FBSD although it does not work in my K> setup since I have extended TCP option checking into another ipfw action and K> while I could add the check you've proposed for tcpop_match I would prefer a K> more generic approach where the m_pullup call is done for all TCP packets K> with options (basically in the case IPPROTO_TCP). K> K> The rationale behind this is such that there is a guarantee that tcpop_match K> will work but also that any future extensions based on TCP options would K> also work saving the hard to debug situation that a missing call to m_pullup K> can create. Currently almost all TCP traffic has TCP options. And currently most, I suppose > 90%, installations, that use ipfw(4) do not have rules with 'tcpoptions' keyword. So, we would add extra pullup, that is not needed in most cases and may have a performance impact. In case of future extensions the hard to debug situation won't happen if a developer analyses the function he modifies thoroughly. So, can you please confirm, that if you adding this string PULLUP_LEN(hlen, ulp, (TCP(ulp)->th_off << 2)); into your new ipfw action, solves the discussed problem? -- Totus tuus, Glebius. From owner-freebsd-ipfw@FreeBSD.ORG Wed Apr 6 18:20:13 2011 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 54A7A106564A for ; Wed, 6 Apr 2011 18:20:13 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 28EB88FC08 for ; Wed, 6 Apr 2011 18:20:13 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p36IKChJ002268 for ; Wed, 6 Apr 2011 18:20:12 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p36IKCg3002267; Wed, 6 Apr 2011 18:20:12 GMT (envelope-from gnats) Date: Wed, 6 Apr 2011 18:20:12 GMT Message-Id: <201104061820.p36IKCg3002267@freefall.freebsd.org> To: freebsd-ipfw@FreeBSD.org From: Karim Fodil-Lemelin Cc: Subject: Re: kern/156180 X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Karim Fodil-Lemelin List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Apr 2011 18:20:13 -0000 The following reply was made to PR kern/156180; it has been noted by GNATS. From: Karim Fodil-Lemelin To: Gleb Smirnoff Cc: Subject: Re: kern/156180 Date: Wed, 6 Apr 2011 14:07:08 -0400 --000e0cd5d1826dc62504a043dcf6 Content-Type: text/plain; charset=ISO-8859-1 Hi Gleb, 2011/4/6 Gleb Smirnoff > Hi, Karim! > > On Wed, Apr 06, 2011 at 10:36:46AM -0400, Karim Fodil-Lemelin wrote: > K> Thanks for the patch it does work in FBSD although it does not work in > my > K> setup since I have extended TCP option checking into another ipfw action > and > K> while I could add the check you've proposed for tcpop_match I would > prefer a > K> more generic approach where the m_pullup call is done for all TCP > packets > K> with options (basically in the case IPPROTO_TCP). > K> > K> The rationale behind this is such that there is a guarantee that > tcpop_match > K> will work but also that any future extensions based on TCP options would > K> also work saving the hard to debug situation that a missing call to > m_pullup > K> can create. > > Currently almost all TCP traffic has TCP options. And currently most, > I suppose > 90%, installations, that use ipfw(4) do not have rules with > 'tcpoptions' keyword. So, we would add extra pullup, that is not needed > in most cases and may have a performance impact. > In my case even if all packets have at least a timestamp option or SACK the actual m_pullup was only called once every 10s of thousands packet as most of the time the lower layers hands out perfectly contiguous headers. What I've found is whenever a netgraph node or some other decapsulation mechanisms that tends to modify headers and due to its internal operations needs to somewhat break down the mbuf chain, the m_pullup call might be needed due to trailing options being in another mbuf. Most other times the headers are just fine and thats what made this problem hard to find in the first place. So I don't think there is a significant performance hit but I do see your point (see my point on multiple TCP microinstructions below). > In case of future extensions the hard to debug situation won't happen > if a developer analyses the function he modifies thoroughly. > One could assume that adding an action to ipfw should be relatively straightforward and the mechanics of doing sanity checks and what not was already handled by the top half of the function. Although I'm sure that with the call to PULLUP_LEN in tcpopt_match developers basing their code on it will not miss it ;) I can understand that since TCP option filtering isn't used much by the community its better to keep its specific processing isolated from IP although that approach has its drawback. You can imagine that if multiple TCP actions are chained (as microinstructions) together (and so are the calls to PULLUP_LEN) the call overhead becomes quickly a performance issue. > > So, can you please confirm, that if you adding this string > > PULLUP_LEN(hlen, ulp, (TCP(ulp)->th_off << 2)); > > into your new ipfw action, solves the discussed problem? > Unfortunately I won't be able to test this exact code, as I mentionned earlier, since it is not applicable to our solution (btw the problem would show up once every 2 days or so). My PR was really just a mere pointer to an issue I have encountered and I perfectly understand the context of your solution and its applicability. You see myself in the impossibility to confirm with testing your proposed patch. If that can be a consolation my first fix was to add a call to m_pullup in every TCP action we had and it worked just fine although the final solution was to integrate it higher up in the function (before the microinstruction loop). Though, I do have to admit I prefer your macro integration to what I've proposed since it 'blends in' better with the previous implementation. Best regards, Karim. --000e0cd5d1826dc62504a043dcf6 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Hi Gleb,

2011/4/6 Gleb Smirnoff <glebius@freebsd.org= >
=A0Hi, Karim!

On Wed, Apr 06, 2011 at 10:36:46AM -0400, Karim Fodil-Lemelin wrote:
K> Thanks for the patch it does work in FBSD although it does not work i= n my
K> setup since I have extended TCP option checking into another ipfw act= ion and
K> while I could add the check you've proposed for tcpop_match I wou= ld prefer a
K> more generic approach where the m_pullup call is done for all TCP pac= kets
K> with options (basically in the case IPPROTO_TCP).
K>
K> The rationale behind this is such that there is a guarantee that tcpo= p_match
K> will work but also that any future extensions based on TCP options wo= uld
K> also work saving the hard to debug situation that a missing call to m= _pullup
K> can create.

Currently almost all TCP traffic has TCP options. And currently most,
I suppose > 90%, installations, that use ipfw(4) do not have rules with<= br> 'tcpoptions' keyword. So, we would add extra pullup, that is not ne= eded
in most cases and may have a performance impact.

I= n my case even if all packets have at least a timestamp option or SACK the = actual m_pullup was only called once every 10s of thousands packet as most = of the time the lower layers hands out perfectly contiguous headers.

What I've found is whenever a netgraph node or some other decapsula= tion mechanisms that tends to modify headers and due to its internal operat= ions needs to somewhat break down the mbuf chain, the m_pullup call might b= e needed due to trailing options being in another mbuf. Most other times th= e headers are just fine and thats what made this problem hard to find in th= e first place.

So I don't think there is a significant performance hit but I do se= e your point (see my point on multiple TCP microinstructions below).

In case of future extensions the hard to debug situation won't happen if a developer analyses the function he modifies thoroughly.
=A0
One could assume that adding an action to ipfw should = be relatively straightforward and the mechanics of doing sanity checks and = what not was already handled by the top half of the function. Although I= 9;m sure that with the call to PULLUP_LEN in tcpopt_match developers basing= their code on it will not miss it ;)

I can understand that since TCP option filtering isn't used much by= the community its better to keep its specific processing isolated from IP = although that approach has its drawback. You can imagine that if multiple T= CP actions are chained (as microinstructions) together (and so are the call= s to PULLUP_LEN) the call overhead becomes quickly a performance issue.
=A0

So, can you please confirm, that if you adding this string

PULLUP_LEN(hlen, ulp, (TCP(ulp)->th_off << 2));

into your new ipfw action, solves the discussed problem?

Unfortunately I won't be able to test this exact code, as I men= tionned earlier, since it is not applicable to our solution (btw the proble= m would show up once every 2 days or so). My PR was really just a mere poin= ter to an issue I have encountered and I perfectly understand the context o= f your solution and its applicability. You see myself in the impossibility = to confirm with testing your proposed patch.

If that can be a consolation my first fix was to add a call to m_pullup= in every TCP action we had and it worked just fine although the final solu= tion was to integrate it higher up in the function (before the microinstruc= tion loop). Though, I do have to admit I prefer your macro integration to w= hat I've proposed since it 'blends in' better with the previous= implementation.

Best regards,

Karim.
--000e0cd5d1826dc62504a043dcf6-- From owner-freebsd-ipfw@FreeBSD.ORG Thu Apr 7 11:20:12 2011 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 3891F1065670 for ; Thu, 7 Apr 2011 11:20:12 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 2A95F8FC08 for ; Thu, 7 Apr 2011 11:20:12 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p37BKC8j060952 for ; Thu, 7 Apr 2011 11:20:12 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p37BKCqE060951; Thu, 7 Apr 2011 11:20:12 GMT (envelope-from gnats) Date: Thu, 7 Apr 2011 11:20:12 GMT Message-Id: <201104071120.p37BKCqE060951@freefall.freebsd.org> To: freebsd-ipfw@FreeBSD.org From: dfilter@FreeBSD.ORG (dfilter service) Cc: Subject: Re: kern/153415: commit references a PR X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: dfilter service List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Apr 2011 11:20:12 -0000 The following reply was made to PR kern/153415; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/153415: commit references a PR Date: Thu, 7 Apr 2011 11:14:05 +0000 (UTC) Author: ae Date: Thu Apr 7 11:13:50 2011 New Revision: 220415 URL: http://svn.freebsd.org/changeset/base/220415 Log: MFC r220211: Fill up src_port and dst_port variables for SCTP over IPv4. PR: kern/153415 Modified: stable/8/sys/netinet/ipfw/ip_fw2.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/netinet/ipfw/ip_fw2.c ============================================================================== --- stable/8/sys/netinet/ipfw/ip_fw2.c Thu Apr 7 08:32:53 2011 (r220414) +++ stable/8/sys/netinet/ipfw/ip_fw2.c Thu Apr 7 11:13:50 2011 (r220415) @@ -1123,6 +1123,12 @@ do { \ args->f_id._flags = TCP(ulp)->th_flags; break; + case IPPROTO_SCTP: + PULLUP_TO(hlen, ulp, struct sctphdr); + src_port = SCTP(ulp)->src_port; + dst_port = SCTP(ulp)->dest_port; + break; + case IPPROTO_UDP: PULLUP_TO(hlen, ulp, struct udphdr); dst_port = UDP(ulp)->uh_dport; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From owner-freebsd-ipfw@FreeBSD.ORG Thu Apr 7 11:23:02 2011 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 19891106566B; Thu, 7 Apr 2011 11:23:02 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id E6C738FC0C; Thu, 7 Apr 2011 11:23:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p37BN1JP068978; Thu, 7 Apr 2011 11:23:01 GMT (envelope-from ae@freefall.freebsd.org) Received: (from ae@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p37BN1MK068974; Thu, 7 Apr 2011 11:23:01 GMT (envelope-from ae) Date: Thu, 7 Apr 2011 11:23:01 GMT Message-Id: <201104071123.p37BN1MK068974@freefall.freebsd.org> To: szander@swin.edu.au, ae@FreeBSD.org, freebsd-ipfw@FreeBSD.org From: ae@FreeBSD.org Cc: Subject: Re: kern/153415: [ipfw] [patch] Port numbers always zero in dynamic IPFW rules for SCTP over IPv4 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: Thu, 07 Apr 2011 11:23:02 -0000 Synopsis: [ipfw] [patch] Port numbers always zero in dynamic IPFW rules for SCTP over IPv4 State-Changed-From-To: open->closed State-Changed-By: ae State-Changed-When: Thu Apr 7 11:22:13 UTC 2011 State-Changed-Why: Committed to head/ and stable/8. Thanks! http://www.freebsd.org/cgi/query-pr.cgi?pr=153415