From owner-freebsd-net@FreeBSD.ORG Thu Jun 5 14:49:38 2014 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 16C5E44C for ; Thu, 5 Jun 2014 14:49:38 +0000 (UTC) Received: from mail-pb0-x232.google.com (mail-pb0-x232.google.com [IPv6:2607:f8b0:400e:c01::232]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D812E25D6 for ; Thu, 5 Jun 2014 14:49:37 +0000 (UTC) Received: by mail-pb0-f50.google.com with SMTP id ma3so1210841pbc.23 for ; Thu, 05 Jun 2014 07:49:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type:content-transfer-encoding:thread-index :content-language; bh=eOC2uWrvrcCRD/scZ5k7EIbk1xCzTfxXoM5wIuePriU=; b=P9NWuk/Sud362jGMQb2X58FUBKUo0dtpd6eNwPETdmhRdvwCojavnznIoeNe8tsfEU aNFcYAk8bvXQd+I3W534arxpjTwqra57xC/dVvM6LNo9A6U4NKaQPIsfy45VMbZmvqKU f8KoJq953CNvVwwz9ica8WNNhSKzAggHPKH8GEA91tmH5TCrTO6++KjGXNikevTbTQ7o K3mPrpY4LF80n79MxcvaVvSM/zn+R6DvUmbzqr6iSYNkrK+xMO202p5WCGOtTom2dBaJ 53J0E2+lESFg0D/mfoEGgF8WmOu9mkLXWC0nvcIiCM8fDEZsRNzi+R8azGfx2e23ZiQ6 fVSw== X-Received: by 10.68.200.10 with SMTP id jo10mr77682553pbc.143.1401979775958; Thu, 05 Jun 2014 07:49:35 -0700 (PDT) Received: from billwin7 (amx-tls2.starhub.net.sg. [203.116.164.12]) by mx.google.com with ESMTPSA id ib5sm23755075pbb.55.2014.06.05.07.49.31 for (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 05 Jun 2014 07:49:34 -0700 (PDT) From: "bycn82" To: "'Luigi Rizzo'" , References: <20140521111002.GB62462@onelab2.iet.unipi.it> <537CEC12.8050404@FreeBSD.org> <20140521204826.GA67124@onelab2.iet.unipi.it> <537E1029.70007@FreeBSD.org> <20140522154740.GA76448@onelab2.iet.unipi.it> <537E2153.1040005@FreeBSD.org> <20140522163812.GA77634@onelab2.iet.unipi.it> <538B2FE5.6070407@FreeBSD.org> <539044E4.1020904@ipfw.ru> <000c01cf80be$41194370$c34bca50$@gmail.com> <20140605134256.GA81234@onelab2.iet.unipi.it> In-Reply-To: <20140605134256.GA81234@onelab2.iet.unipi.it> Subject: RE: [CFT]: ipfw named tables / different tabletypes Date: Thu, 5 Jun 2014 22:49:27 +0800 Message-ID: <000001cf80cd$5dc1d9b0$19458d10$@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQDP7m1eF4Hlla5jUUEWXMQiUH1mRAEgsmRhAjJtK3MCjc1r7wJb8czGAduoovwBa0FP0gFm8TDwAl7mTWQC+qrnHAHOZXswnMEZ9kA= Content-Language: en-us Cc: "'Alexander V. Chernikov'" , 'FreeBSD Net' X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jun 2014 14:49:38 -0000 Hi Luigi, Yes, use string instead of integer for the ID of table, but the same = method cannot apply to the feature `set type of table`. And in the = kernel, compare string will cause more than compare an integer. In my = opinion, actually they are just alias name for the object. Users already = accept that every object has an integer ID. Hi Alex, Why not clean the ipfw_table_handler() function using the switch/case? = Like in my patch, It can be easier to understand the code. Best Regards, bycn82 > -----Original Message----- > From: Luigi Rizzo [mailto:rizzo@iet.unipi.it] > Sent: 05 June, 2014 21:43 > To: bycn82 > Cc: 'Alexander V. Chernikov'; 'Luigi Rizzo'; 'FreeBSD Net' > Subject: Re: [CFT]: ipfw named tables / different tabletypes >=20 > On Thu, Jun 05, 2014 at 09:01:19PM +0800, bycn82 wrote: > > Hi Alex, > > > > Here is my patch, with this patch, the ipfw can support below > > commands, > > > > root@FB10Head:~ # ipfw table 1 name saleteam root@FB10Head:~ # ipfw > > table 1 name show saleteam >=20 > Bill, > as it was discussed previously, exposing the numeric identifier for = the > table is useless and potentially dangerous so the two commands above > must not exist. >=20 > In the new firewall the only names exposed externally are strings, and = a > table name "1" will be interpreted as a string, not a number. >=20 > Only for transitional purposes (new kernel with old firewall), the > kernel will accept commands with the old numeric identifiers and map > them to strings, in and out. >=20 > cheers > luigi >=20 > > root@FB10Head:~ # ipfw table saleteam add 1.2.3.4 root@FB10Head:~ # > > ipfw table saleteam list > > 1.2.3.4/32 0 > > root@FB10Head:~ # ipfw table 1 list > > 1.2.3.4/32 0 > > root@FB10Head:~ # > > > > > > Currently still cleaning the table handling function, and did not = add > the lock in the kernel functions when changing the `mapping chain`. > > > > Regards, > > bycn82 > > > > > > > > > -----Original Message----- > > > From: Alexander V. Chernikov [mailto:melifaro@ipfw.ru] > > > Sent: 05 June, 2014 18:22 > > > To: Luigi Rizzo > > > Cc: Luigi Rizzo; FreeBSD Net; Bill Yuan > > > Subject: Re: [CFT]: ipfw named tables / different tabletypes > > > > > > On 01.06.2014 17:51, Alexander V. Chernikov wrote: > > > > On 22.05.2014 20:38, Luigi Rizzo wrote: > > > > > > > > Long story short, new version is ready. > > > > I've tried to minimize changes in this patch to ease = review/commit. > > > > > > > > Changes: > > > > * Add namedobject set-aware api capable of searching/allocation > > > > objects by their name/idx. > > > > * Switch tables code to use string ids for configuration tasks. > > > > * Change locking model: most configuration changes are protected > > > > with UH lock, runtime-visible are protected with both locks. > > > > * Reduce number of arguments passed to ipfw_table_add/del by = using > > > > separate structure. > > > > * Add internal V_fw_tables_sets tunable (set to 0) to prepare = for > > > > set-aware tables (requires opcodes/client support) > > > > * Implement typed table referencing (and tables are implicitly > > > > allocated with all state like radix ptrs on reference) > > > > * Add "destroy" ipfw(8) using new IP_FW_DELOBJ opcode > > > > > > > > Namedobj more detailed: > > > > * Blackbox api providing methods to add/del/search/enumerate > > > > objects > > > > * Statically-sized hashes for names/indexes > > > > * Per-set bitmask to indicate free indexes > > > > * Separate methods for index alloc/delete/resize > > > > > > > > > > > > Basically, there should not be any user-visible changes except = the > > > > following: > > > > * reducing table_max is not supported > > > > * flush & add change table type won't work if table is = referenced > > > > > > > > > > > > I haven't removed any numbering restrictions to protect the > > > > following > > > > case: > > > > one (with old client) unintentionally references too many tables > (e.g. > > > > 1000-1128), > > > > tries to allocate table from "valid" range and fails. Old client > > > > does not have any ability to destroy any table, so the only way = to > > > > solve this is either module unload or reboot. > > > > > > > > I've uploaded the same patch to phabricator since it provides > > > > quite handy diffs: > > > > https://phabric.freebsd.org/D139 (no login required). > > > A bit cleaner version attached. > > > > > > > >> On Thu, May 22, 2014 at 08:09:55PM +0400, Alexander V. = Chernikov > > > wrote: > > > >>> On 22.05.2014 19:47, Luigi Rizzo wrote: > > > >>>> On Thu, May 22, 2014 at 06:56:41PM +0400, Alexander V. > > > >>>> Chernikov > > > >>>> wrote: > > > >>>>> On 22.05.2014 00:48, Luigi Rizzo wrote: > > > >>>>>> On Wed, May 21, 2014 at 10:10:26PM +0400, Alexander V. > > > >>>>>> Chernikov > > > >>>>>> wrote: > > > >>>> ... > > > >>>>>> we can solve this by using 'low' numbers for the numeric > > > >>>>>> tables (these were limited anyways) and allocate the fake > > > >>>>>> entries in another range. > > > >>>>> Currently we have u16 space available in base opcode. > > > >>>> yes but the standard range for tables is much more limited: > > > >>>> > > > >>>> net.inet.ip.fw.tables_max: 128 > > > >>>> > > > >>>> so one can just (say) use 32k for "old" tables and the rest = for > > > >>>> tables with non numeric names. > > > >>>> Does not seem to be a problem in practice. > > > >>> Well, using upper 32k means that you set this default to 65k > > > >>> which consumes 256k of memory on 32-bit arch. > > > >>> Embedded people won't be very happy about this (and changing > > > >>> table numbers on resize would be a nightmare). > > > >> no no, this is an implementation detail but within the kernel = you > > > >> can just remap the 'old' and 'new' > > > >> table identifiers to a single contiguous range. > > > >> The only thing you need to do is that when you push identifiers > > > >> up to userland, those with 'new' names will be mapped to the = 32- > 64k range. > > > >> > > > >> Example: > > > >> user first specifies tables > > > >> "18, goodguys, 530, badguys" in the same rule > > > >> /sbin/ipfw will generate these numbers: > > > >> 18, 32768, 530, 32769 ; tlv {32768:goodguys, 32769:badguys} > > > >> The kernel will then do a lookup of those identifiers and > > > >> 18: internal index 1, name "18" > > > >> 32768: internal index 2, name "goodguys" > > > >> 530: internal index 3, name "530" > > > >> 32769: internal index 4, name "badguys" > > > >> > > > >> Then the next rule contains tables > > > >> 1, badguys, 18 > > > >> /sbin/ipfw generates > > > >> 1, 32768, 18 ; tlv {32768:badguys} // note different from > before > > > >> Kernel looks up the names and remaps > > > >> 1: internal index 5, name "1" > > > >> 32768: internal index 4, name "badguys" > > > >> 18: internal index 1, name "18" > > > >> > > > >> Finally when you do an 'ipfw show' the kernel will remap names > > > >> between 1 and 32768 to themselves, and other names to 32768+ = (or > > > >> some other large number, say 40k and above) so as they are = found. > > > >> So the rules will be pushed up with > > > >> 18, 40000, 530, 40001 > > > >> 1, 40001, 18 > > > >> > > > >> we can discusso the other details privately > > > >> > > > >> cheers > > > >> luigi > > > >> > > > >> > > > >> 1. first, the > > > >>>>>> maybe i am missing some detail but it seems reasonably easy > > > >>>>>> to implement the atomic swap -- and the use case is when = you > > > >>>>>> want to move from one configuration to a new one: > > > >>>>>> ipfw table foo-new flush // clear initial content > > > >>>>>> ipfw table foo-new add ... > > > >>>>>> ipfw table swap foo-current foo-new // swap the content > > > >>>>>> of the table objects > > > >>>>>> > > > >>>>>> so you preserve the semantic of the name very easily. > > > >>>>> Yes. We can easily add atomic table swap that way. However, > > > >>>>> I'm talking about different use scenario: > > > >>>>> Atomically swap entire ruleset which has some tables = depency: > > > >>>>> > > > >>>>> > > > >>>>> e.g. we have: > > > >>>>> > > > >>>>> " > > > >>>>> 100 allow ip from table(TABLE1) to me > > > >>>>> 200 allow ip from table(TABLE2) to (TABLE3) 80 > > > >>>>> > > > >>>>> table TABLE1 1.1.1.1/32 > > > >>>>> table TABLE1 1.0.0.0/16 > > > >>>>> > > > >>>>> table TABLE2 2.2.2.2/32 > > > >>>>> > > > >>>>> table TABLE3 3.3.3.3/32 > > > >>>>> " > > > >>>>> and we want to _atomically_ change this to > > > >>>>> > > > >>>>> " > > > >>>>> 100 allow ip from table(TABLE1) to me > > > >>>>> +200 allow ip from table(TABLE4) to any > > > >>>>> 300 allow ip from table(TABLE2) to (TABLE3) 80 > > > >>>>> > > > >>>>> table TABLE1 1.1.1.1/32 > > > >>>>> -table TABLE1 1.0.0.0/16 > > > >>>>> > > > >>>>> -table TABLE2 2.2.2.2/32 > > > >>>>> +table TABLE2 77.77.77.0/24 > > > >>>>> > > > >>>>> table TABLE3 3.3.3.3/32 > > > >>>>> > > > >>>>> +table TABLE4 4.4.4.4/32 > > > >>>>> " > > > >>>> aargh, that's too much -- because between changing one table > > > >>>> and all tables there are infinite intermediate points that = all > > > >>>> make sense. > > > >>> It depends. As I said before, we're currently solving this > > > >>> problem > > > by > > > >>> adding new rules (to set X) referencing tables from different > > > >>> range > > > >>> (2048 tables per ruleset) and than doing swap. > > > >>> (And not being able to use named tables to store real names > > > >>> after implementing them is a bit discouraging). > > > >>> > > > >>>> For those cases i think the way to go could be to insert a > > > >>>> 'disabled' new ruleset (however complex it is, so it covers = all > > > >>>> possible cases), and then do the set swap, or disable/enable. > > > >>> We can think of per-set arrays/namespaces of tables: > > > >>> > > > >>> so "ipfw add 100 set X allow ipfw from table(Y) to ..." will > > > reference > > > >>> table Y in set X and > > > >>> "ipfw table ABC list" can differ from "ipfw table ABC set 5 > list". > > > >>> > > > >>> This behavior can break some users setups so we can provide > > > >>> sysctl/tunable to turn this off or on. > > > >>> > > > >>>> cheers > > > >>>> luigi > > > >>>> > > > > > >