[CFT]: ipfw named tables / different tabletypes

[Bill Yuan]

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 <rizzo at iet.unipi.it> 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 <num> 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  ... <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.
>
> > >
> > > 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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
>