Date: Fri, 14 Nov 2003 15:31:45 -0800 From: Luigi Rizzo <rizzo@icir.org> To: Andre Oppermann <andre@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet in_var.h ip_fastfwd.c ip_flow.c ip_flow.h ip_input.c ip_output.c src/sys/sys mbuf.h src/sys/conf files src/sys/net if_arcsubr.c if_ef.c if_ethersubr.c if_fddisubr.c if_iso88025subr.c if_ppp.c Message-ID: <20031114153145.A54064@xorpc.icir.org> In-Reply-To: <200311142102.hAEL2Nen073186@repoman.freebsd.org>; from andre@FreeBSD.org on Fri, Nov 14, 2003 at 01:02:23PM -0800 References: <200311142102.hAEL2Nen073186@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Nov 14, 2003 at 01:02:23PM -0800, Andre Oppermann wrote:
...
> Introduce ip_fastforward and remove ip_flow.
>
> Short description of ip_fastforward:
>
> o adds full direct process-to-completion IPv4 forwarding code
> o handles ip fragmentation incl. hw support (ip_flow did not)
> o sends icmp needfrag to source if DF is set (ip_flow did not)
> o supports ipfw and ipfilter (ip_flow did not)
> o supports divert, ipfw fwd and ipfilter nat (ip_flow did not)
> o returns anything it can't handle back to normal ip_input
Sorry if i missed part of the discussion leading to this commit...
Let me state first that I do not mean to criticise your work, but
having spent quite a bit of time trying to optimize the forwarding
path myself, and having found a number of surprises on what really
consumes most of the time, i tend to be a bit skeptical when it
comes to optimizations.
Obviously I see it as a very good thing the creation of a fully
featured fast forwarding path. However I would like to understand
if it is really worthwhile doing it, and whether this new code
isn't a replica of already existing optimizations.
In particular, did you measure any significant performance difference
by replacing calls to ip_fastforward() with calls to ip_input() ?
(say in a kernel without "options IPSEC" which used to be highly
inefficient even for non-ipsec packets).
The reason I am asking is that I see some expensive ops (such as
doing route lookups, scanning through lists of local addresses,
input and output firewall processing, fragmentation) in both pieces
of code. Fragmentation may not be the common case, but all the rest
definitely is, and with large routing tables, multiple interfaces
and even modestly complex firewall rulesets, most of the processing
time goes there.
Also, in the initial comments you claim:
* ip_fastforward gets its speed from processing the forwarded packet to
* completion (if_output on the other side) without any queues or netisr's.
now, netisr_dispatch() (which is invoked if ip_fastforward() fails)
does an inline call to ip_input() if netisr_enable=1 (ip_input
is NETISR_MPSAFE). So this seems to fall into the same category
of optimizations.
So to summarize, could you identify what are the differences
that make ip_fastforward() so much faster than
ip_input()/ip_forward()/ip_output() ?
Given that there are large segments of common code between
ip_fastforward() and ip_input()/ip_output() (i am thinking of the
entire ipfw handling code, for one, and also some basic integrity
checks, the fragmentation code, etc.) I also wonder if it wouldn't
be beneficial to put the optimizations into the standard path rather
than create a new (partial) replica of the same code, with the
potential of introducing bugs, and with some substantial I-cache
pollution which might well destroy the benefits of minor optimizations.
-----------
Minor comments on the code:
+ one of the initial comments in the new code states
... The only part of the packet we touch with the CPU is the
IP header. ...
this is not true if you use ipfw because that code touches many
places in the packet (and can also do some expensive computation
like trying to locate the uid/gid of a packet; the fact that we
only deal with packets not for us does not prevent the existence
of such firewall rules).
+ The code at line 344:
if (fw_enable && IPFW_LOADED) {
bzero(&args, sizeof(args));
args.m = m;
ipfw = 0;
ipfw = ip_fw_chk_ptr(&args);
m = args.m;
the bzero is probably more expensive than individually zeroing the
fields of args as done in ip_input() (I think once i measured the
two solutions). The first "ipfw = 0;" is redundant.
+ could you clarify the divert logic ? I am a bit rusty with that
part of the code, but i am under the impression that in
ip_fastforward() you are passing along args.divert_rule and
losing track of divert_info which is instead what you need too.
-------------
cheers
luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031114153145.A54064>