Date: Fri, 25 Aug 2006 03:32:42 -0700 From: Luigi Rizzo <rizzo@icir.org> To: Ian FREISLICH <if@hetzner.co.za> Cc: freebsd-ipfw@freebsd.org Subject: Re: ipfw performance and random musings. Message-ID: <20060825033242.B3245@xorpc.icir.org> In-Reply-To: <E1GGYT4-000CXO-H7@hetzner.co.za>; from if@hetzner.co.za on Fri, Aug 25, 2006 at 11:59:14AM %2B0200 References: <rizzo@icir.org> <E1GGYT4-000CXO-H7@hetzner.co.za>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 25, 2006 at 11:59:14AM +0200, Ian FREISLICH wrote:
> Luigi Rizzo wrote:
> > On Thu, Aug 24, 2006 at 02:32:04PM +0200, Ian FREISLICH wrote:
> > > skipto 1000 ip from any to any ifhash vlan[1000-1264] offset -1000 delta 100
> > >
> > > Which for matching interfaces calculates the skipto target as:
> > >
> > > 1000 + (iface# + offset) * delta
> > >
> > > If you're happy with this format, I'll update the ipfw manual page
> > > and submit a patch for review and commit.
> >
> > I would suggest a modification to the syntax as follows:
> >
> > skipto @ ... recv|xmit|via foo[A-B] base X delta D
> .................................................^^^^^
>
> This will then conflict with:
>
> ip_fw2.c iface_match():
>
> if (cmd->name[0] != '\0') { /* match by name */
> /* Check name */
> if (cmd->p.glob) {
> if (fnmatch(cmd->name, ifp->if_xname, 0) == 0)
> return(1);
>
> cmd->p.glob as set up by ipfw2.c fill_iface():
>
> else if (!isdigit(*arg)) {
> strlcpy(cmd->name, arg, sizeof(cmd->name));
> cmd->p.glob = strpbrk(arg, "*?[") != NULL ? 1 : 0;
hmmm... i did not know of the regexp support in interface name;
too bad it is not reported on the manpage!
How about compromising to vlanA-B (i.e. do not repeat the basename) ?
This way it does not interfere with the regexp code.
> > where @ is a keyword (meaning "the jump target is computed elsewhere")
> > and "foo[A-B] base X delta D" is an extension of the interface-name
> > option already available in ipfw.
>
> I also don't like 'skipto @' because that complicates the skipto
> syntax. I'd prefer to keep skipto the same and use a rule option
> to modify the skipto target. I'm also not overly enthused with
> putting this data into the ipfw_insn_if type.
but it really belongs there. You have no reason to use the 'delta'
otherwise, and you don't have to complicate the skipto syntax - if you
don't like @, just use '0' and then you know that a jump target
of 0 means an indirect jump.
> I'm happy to compromise since I think what's confusing the issue
> is this feature is really both a rule action and body and not just
> a rule body. Perhaps this is better:
>
> skipto 1000 delta 100 ip from any to any via vlan1002-vlan1264
> This then extends the recv|xmit|via syntax to allow "ranges" of
> like interfaces and the skipto syntax to calculate offsets. "delta"
> being optional and defaulting to zero and implying a value based
> on the interface number.
the problem i see above is that the 'delta' is really an attribute
of the 'vlanA-B' instruction.
Say you have this rule:
skipto 1000 recv vlan1002-vlan1264
does it mean 'skip to 1000 plus the interface number' or
'skip to 1000 unconditionally' (i suppose the former because
otherwise the 'skipto' would have two different meanings
depending on whether or not there is a subsequent vlanA-B specifier) ?
In order to figure out, you need to know a lot more info from
the manpage (if it gets updated :) because the syntax is not telling you.
With the other syntax it is obvious what you do:
skipto 1000 recv vlan1002-vlan1264 -> direct jump
skipto @ recv vlan1002-vlan1264 base 1000 -> indirect jump
>
> > The motivations are the following:
> > 1. "ifhash" is misleading, as it isn't really hashing anything.
>
> That occured to me. You suggested "ifhash" though :)
i never said i never make mistakes :)
> > The real hashing, if you implemented it, is in the
> > rule_number --> rule_ptr lookup table, which is a general mechanism
> > and not a specific one.
>
> I did. But reading my thread in -CURRENT about vlan performance
> it appears there might heavy objection to this since it costs 256k
> of memory, ond they're fighting over 16k to get a ~8% performance
> boost with CPU utilisation down from 75% to 3%.
if you make it a hash table you don't have to worry about static sizes,
and it also removes the multiple number -> pointer entries that are
embedded in the rules (mostly important from an update-cost point of view).
> > I have no idea how you wrote your current implementation but i
> > believe that by using the above syntax even the internal implementation
> > could be quite straightforward.
>
> Are you a context or unified diff man?
diff -u
cheers
luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060825033242.B3245>