Date: Wed, 24 Apr 2013 23:50:48 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: freebsd-ipfw@freebsd.org, luigi@freebsd.org Subject: Re: [patch] ipfw interface tracking and opcode rewriting Message-ID: <51783798.4020004@FreeBSD.org> In-Reply-To: <20130424190930.GA10395@onelab2.iet.unipi.it> References: <517801D3.5040502@FreeBSD.org> <20130424162349.GA8439@onelab2.iet.unipi.it> <51780C49.7000204@FreeBSD.org> <20130424190930.GA10395@onelab2.iet.unipi.it>
next in thread | previous in thread | raw e-mail | index | archive | help
On 24.04.2013 23:09, Luigi Rizzo wrote:
> On Wed, Apr 24, 2013 at 08:46:01PM +0400, Alexander V. Chernikov wrote:
>> On 24.04.2013 20:23, Luigi Rizzo wrote:
> ...
>>>> vesrion) in the middle of the next week.
>>> hmmm.... this is quite a large change, and from the description it
>>> is a bit unclear to me how the "opcode rewriting" thing relates to
>>> the use of strings vs index for name matching.
>> sorry, I havent't describe this explicitly.
>> Index matching is done via storing interface index in in p.glob field of
>> ipfw_insn_if instruction.
> understood. the reasons why i did not use the index is that
> one could specify a non-existing interface name, and also interfaces
> can be renamed. If you want to use indexses, you should add
> (perhaps you do, i haven't checked)
Yes, this is done (without 'good' renaming handling), but still.
> hooks to the interface add/rename/delete code in order to
> update the ruleset upon changes on the if list, and it
> seemed to me a bad idea to add this dependency
> (lockingwise, too).
>
> Really, with 16-byte fixed size interface names, the match
> is as simple as this:
>
> #if CAN_DO_FAST_MATCH && IFNAMSIZ == 16 /* archs with no align requirements */
> {
> uint64_t *a = (uint64_t *)ifp->if_xname;
> uint64_t *b = (uint64_t *)cmd->name;
> if (a[0] == b[0] && a[1] == b[1])
> return 1;
> }
> #else
> if (strncmp(ifp->if_xname, cmd->name, IFNAMSIZ) == 0)
> return 1
> #endif
>
> (assuming the names are zero-padded, which should be the case already).
> Since you have the measurement infrastructure in place, perhaps
> you have an easy way to try this patch and see how effective
> it is in terms of performance.
I'll try this tomorrow, thanks.
>
>>> Additionally, i wonder if there isn't a better way to replace strncmp
>>> with some two 64-bit comparisons (the name is 16 bytes) by making
>>> sure that the fields are zero-padded and suitably aligned.
>>> At this point, on the machines you care about (able to sustain
>>> 1+ Mpps) the two comparison should have the same cost as
>>> the index comparison, without the need to track update in the names.
>> Well, actually I'm thinking of the next 2 steps:
>> 1) making kernel rule header more compact (20 bytes instead of 48) and
>> making it invisible for userland.
>> This involves rule counters to be stored separately (and possibly as
>> pcpu-based ones).
>> 2) since ruleset is now nearly readonly and more or less compact we can
>> try to store it in
>> contiguous address space to optimize cache line usage.
> certainly a worthwhile goal (also using gleb's new counters)
> but i suspect that compacting rules are a second order effect.
> I a bit skeptical they make a big difference on the in-kernel
> version of ipfw. You might see some difference in the
My current numbers are ~5mpps of IPv4 forwarding with ipfw turned on (1
rule) for vlans over ixgbe, with 60% cpu usage (2xE5646).
For lagg with 2x ixgbe it is ~7mpps with the same 60% usage.
(And, say, 70% of CPU usage on our production is ipfw, despite low
number of rules).
> userspace version, which runs on top of netmap.
We are preparing to move forward in this direction (and thinking of
20-30mpps as our goal).
(And I hope some changes of kernel-based version can migrate to userland
one :))
>
> cheers
> luigi
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51783798.4020004>