Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Jun 2014 22:49:27 +0800
From:      "bycn82" <bycn82@gmail.com>
To:        "'Luigi Rizzo'" <rizzo@iet.unipi.it>, <bycn82@gmail.com>
Cc:        "'Alexander V. Chernikov'" <melifaro@ipfw.ru>, 'FreeBSD Net' <net@FreeBSD.org>
Subject:   RE: [CFT]: ipfw named tables / different tabletypes
Message-ID:  <000001cf80cd$5dc1d9b0$19458d10$@gmail.com>
In-Reply-To: <20140605134256.GA81234@onelab2.iet.unipi.it>
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>

Next in thread | Previous in thread | Raw E-Mail | Index | Archive | Help
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  ... <repeat as needed>
> > > >>>>>>     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
> > > >>>>
> > > >
> >





Want to link to this message? Use this URL: <http://docs.FreeBSD.org/cgi/mid.cgi?000001cf80cd$5dc1d9b0$19458d10$>