Date: Thu, 5 Jun 2014 15:42:56 +0200 From: Luigi Rizzo <rizzo@iet.unipi.it> To: bycn82 <bycn82@gmail.com> Cc: 'Luigi Rizzo' <luigi@FreeBSD.org>, "'Alexander V. Chernikov'" <melifaro@ipfw.ru>, 'FreeBSD Net' <net@FreeBSD.org> Subject: Re: [CFT]: ipfw named tables / different tabletypes Message-ID: <20140605134256.GA81234@onelab2.iet.unipi.it> In-Reply-To: <000c01cf80be$41194370$c34bca50$@gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 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. 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. 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. cheers luigi > 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?20140605134256.GA81234>