From owner-freebsd-net@FreeBSD.ORG Thu May 22 04:02:35 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 549335A2; Thu, 22 May 2014 04:02:35 +0000 (UTC) Received: from mail-lb0-x235.google.com (mail-lb0-x235.google.com [IPv6:2a00:1450:4010:c04::235]) (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 5ABC620B0; Thu, 22 May 2014 04:02:34 +0000 (UTC) Received: by mail-lb0-f181.google.com with SMTP id q8so2186172lbi.26 for ; Wed, 21 May 2014 21:02:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=FulnAJHMQuhxV7tpHfhjC4p4KY3IduwrzR3t05KN570=; b=os0pAqFhlbwMAMOxg3NMeSptYyppUcdUKyesWlgSk5cY7m2q/BDEKSS89WYZf9sz5L 5yT3p/tMGkkAozOKxA8vmdwiqtGfD+SCvAGp19HI1Ys8RMtOLFmci6xgGl+R1DUTRGZt AzJktaMVJLHSNtFZ1ajhtLPx+CNUeU/SCLmvR09qWPO+NrYcuZVYY8yAUO0NKs9wdazO 04cDVBxQ8G9bnRBcuzGLzBemUlOsMtnqNNl9ljIUze50TrJWEYQUWrl7Rf/lvQVPxp2h sxrFvZqMDyOwFQaR9nOII79ifqJ3V3IxYOyeg7MQ/ujIWOL7nIEWJfE9vJJGHulqWGWd PWbw== MIME-Version: 1.0 X-Received: by 10.152.23.6 with SMTP id i6mr41156348laf.24.1400731352334; Wed, 21 May 2014 21:02:32 -0700 (PDT) Received: by 10.112.5.10 with HTTP; Wed, 21 May 2014 21:02:32 -0700 (PDT) In-Reply-To: <20140521204826.GA67124@onelab2.iet.unipi.it> References: <5379FE3C.6060501@FreeBSD.org> <20140521111002.GB62462@onelab2.iet.unipi.it> <537CEC12.8050404@FreeBSD.org> <20140521204826.GA67124@onelab2.iet.unipi.it> Date: Thu, 22 May 2014 12:02:32 +0800 Message-ID: Subject: Re: [CFT]: ipfw named tables / different tabletypes From: Bill Yuan To: Luigi Rizzo Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.18 Cc: Luigi Rizzo , "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, 22 May 2014 04:02:35 -0000 Sorry I am little bit blur now. And I am going to wait for your code, I think it will be a good opportunity to learn as a newbie here. 1. So use alphanumeric strings for table's id is not a good way. (because will loose atomicity). I agree that, all this feature/function, I would like to name them as utilities. It can be more user friendly with these utilities. 2. And instead, you are going to introduce a IP_FW3_ADD and ... > It looks like we can do the following: > 1) Add another IP_FW3_ADD opcode with the following layout: > > { > rule len; > unmodified rule itself; > tlv1 {type=table name;len=..;id=1;"_TABLENAME11_"} > tlv2 {type=table name;len=..;id=2;"_TABLENAME4_"} > .. > } > Values inside appropriate opcodes { O_IP_XXX_LOOKUP and so on } won't be > real values So that is the "database" or "mapping table" which I mentioned. So are you going to translate the name to id before calling kernel-space methods? On Thu, May 22, 2014 at 4:48 AM, Luigi Rizzo wrote: > On Wed, May 21, 2014 at 10:10:26PM +0400, Alexander V. Chernikov wrote: > > On 21.05.2014 15:10, Luigi Rizzo wrote: > > > On Mon, May 19, 2014 at 04:51:08PM +0400, Alexander V. Chernikov wrote: > > >> Hello list! > > >> > > >> This patch adds ability to name tables / reference them by name. > > >> Additionally, it simplifies adding new table types. > > > Hi Alex, > > > at a high level, i think your changes here are in the right direction. > > Hello! > > > > > > However i think part of the design, and also of the patch below, > > > is not sound/correct so i would like to wait to commit at least > > > until some of the problems below are resolved. > > > > > > 1. The patch as is has several issues (variable declarations in the > > > middle of a block, assignment in conditionals, incorrect > > > comments in cut&pasted code, missing checks on strings) > > > that should be corrected. > > ..missing documentation and so on :) > > Of course, I'll fix all these (or most of these :)) > > good thanks > > > > 3. there is no explanation, here or in the code, on how the > > > names are managed. I could try to figure out from the code > > > but it would be the wrong way to understand things so let me > > > ask, and please explain what you have in mind. > > Currently it is very simple non-resizable hash table with fnv hash based > > on table name. > > that is not an issue. The question is whether one needs to lookup > the hash table every time you have a 'table' argument (of course i > think one should not, and the implementation i propose below gives > direct access to the table without name lookups, as the internal > identifier is still poiniter or small integer, just one that is not exposed > to userspace). > > > > > > > Let me address first the name <-> table-id thing. > > > > > > Introducing a symbolic name for tables is a great and useful feature. > > > However the implementation has some tricky issues: > > > > > > - do you want to retain the old numeric identifiers or not ? > > > I think it is a bad idea to have two namespaces for the same thing, > > > so if we are switching to alphanumeric strings for tables we should > > > as well get rid of the numbers (i.e. consider them as strings as > well). > > > > > > I am strongly in favour of not using names as aliases for numbers. > > > It would require no changes for clients issuing ipfw commands > > > from a script, and would not require users to to manually handle > > > the name-id translations. > > Well. I'd prefer not to. However, code we're discussing assumes that > > numeric ids > > are primary ones (e.g. you can't have named, but unnumbered table, > > you have to choose number by yourself). Switching to named-only tables > > can be tricky since we don't want to match them by inside rules and we > > don't want > > to loose atomicity when allocating table ids via separate cmds before > > adding rule. > > i think this is solved by the implementation i proposed below. > > > It looks like we can do the following: > > 1) Add another IP_FW3_ADD opcode with the following layout: > > > > { > > rule len; > > unmodified rule itself; > > tlv1 {type=table name;len=..;id=1;"_TABLENAME11_"} > > tlv2 {type=table name;len=..;id=2;"_TABLENAME4_"} > > .. > > } > > Values inside appropriate opcodes { O_IP_XXX_LOOKUP and so on } won't be > > real values > > but values described in attached TLVs. > > > > After validating/parsing/checking under UH lock we replace fake values > > with real ones (probably, newly allocated) > > and return or rollback atomically. > > yes this should work, probably not even requiring a new IP_FW3_ADD opcode > because the extra tlv entries can appear in a block by themselves. > > > The same thing can be done for displaying ruleset, however I'd prefer > > client to ask for table names and caching them while displaying. > > > > Old clients will retain the ability to operate on tables but will see > > table ids, so nothing will change for them > > (except the case when new binary adds table "5" and new binary sees id > > which is not 5, but that shouldn't be a problem). > > we can solve this by using 'low' numbers for the numeric tables > (these were limited anyways) and allocate the fake entries in > another range. > > > > > > > - The rename command worries me a lot: > > > > > > > ipfw table name XXX > > > > Names (or renames) table. Not the name has to be unique. > > > > > > because it is neither clear nor intuitive whether you want to > > > 1. just rename a table (possibly breaking or creating references > > > for rules in the firewall), or > > > 2. modify the name-id translation preserving existing pointers. > > > > > > Consider the sequence > > > ipfw table 13 name bar > > > ipfw add 100 count dst-ip table bar > > > ipfw table 13 name foo > > > ipfw table 14 name bar > > > ipfw add 200 count src-port 22 dst-ip table bar > > > > > > Approach #1 would detach rule 100 from table 13 and then connect > to 14 > > > (in a non-atomic way), whereas approach #2 would make rule 100 and > 200 > > > point to different tables (which is confusing). > > > > > > Now part of this problem goes away if we get rid of number > altogether. > > > > > > You may legitimately want to swap tables in an atomic way (we have > something > > > similar for rulesets): > > > ipfw set swap number number > > There is some problem here: > > Atomic ruleset changes is a great thing, but tables have no atomic > support. > > We, for example, are solving this by using different table range on > > every new configuration. > > It won't be possible with named-only tables (since names usually care > > some semantic in contrast to numbers). > > The only thing I can think of is separate namespace per each set. > > What do you think? > > 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. > > > > > > > So here is what i would propose: > > > - [gradually] introduce new commands that accept strings for every > place where > > > a number was previously accepted. Internally in the firewall, the > old > > > 16-bit number is interpreted as a string. > > Yup. > > > > > > - within the kernel create a small set of functions (invoked on > insert, list, delete) > > > that do proper checks on the string and translate it to a pointer > (or short integer, > > > i.e. an index in an array) to the proper object. Done properly, we > can reuse > > > this code also for pipes, sets, nat groups and whatnot. > > Yup. > > > > > > When a rule references an object that does not exist, you create an > empty > > > object that a subsequent table/pipe/XXX 'create' would initialize. > > > On 'destroy', no need to touch the ruleset, just clear the object. > > Yes. This looks better than requiring user to create table of given type > > _before_ > > referencing it. > > > > > > - for renames, try to follow the approach used for sets i.e. > > > ipfw table rename old-name new-name > > > changes the name, preserves the object. > > > Does not touch the ruleset, only the translation table > > Well, I'm not sure about renaming. I'd prefer permitting renaming iff > > table is not referenced. > > (and why do we need to rename tables?) > > you are right, let's forget it. And 'ipfw set move' actually does a > different thing > which has no use here. > > > > ipfw table swap first-table second-table > > > atomically swap the content of the two table objects > > > (once again not touching the rulesets) > > cheers > luigi > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >