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

Luigi Rizzo rizzo at
Fri Nov 14 15:31:49 PST 2003

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.



More information about the cvs-all mailing list