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: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?000001cf80cd$5dc1d9b0$19458d10$>