[TEST/REVIEW #2] ng_ipfw: node to glue together ipfw(4) and netgraph(4)

[Andre Oppermann]

Gleb Smirnoff wrote:
> 
>   Dear collegues,
> 
> pls review an updated patch bringing in ng_ipfw node. Differencies against
> previous patch:
> 
> - packets coming from netgraph are queued, and later serviced by netisr
> - "ngtee" keyword introduced. A copy of packet is made, and it is sent
>   into netgraph. No tagging is done. Original packet is either accepted or
>   continues check against rules, depending on net.inet.ip.fw.one_pass.
>   Target users are the ones, who are going to do ip accounting/netflow via
>   ng_ipfw.
> - a bit more comments in code
> 
> URL: http://people.freebsd.org/~glebius/totest/ng_ipfw.patch

Style-wise there is only the space after "(void )..." in ip_fw_pfil.c
for the ng_tee case which is too much.

I don't like the arbitrary back-passing of errors from ng_ipfw.  I'm
fine with EACCES, ENOMEM and ESRCH (if hook not connected) but nothing
else.  Getting back any other error is very confusing and non-intuitive
when looking at the error of an application having packets sunk there.

Why don't you prepend the m_tag within ip_fw2.c as altq and divert are
doing it?  Dummynet should do the same to get it consistent again.

Just to confirm it, NG_SEND_DATA_ONLY() queues the packet unconditionally
to unwind the stack?

PS: I'm out of town until tomorrow afternoon.  I'll have only limited
email access until then.

-- 
Andre


> A sample setup:
> 
> + ls
> There are 6 total nodes:
>   Name: <unnamed>       Type: hole            ID: 00000009   Num hooks: 1
>   Name: netflow         Type: netflow         ID: 00000008   Num hooks: 2
>   Name: ngctl768        Type: socket          ID: 00000007   Num hooks: 0
>   Name: <unnamed>       Type: hole            ID: 00000006   Num hooks: 1
>   Name: <unnamed>       Type: echo            ID: 00000004   Num hooks: 1
>   Name: ipfw            Type: ipfw            ID: 00000001   Num hooks: 3
> + show ipfw:
>   Name: ipfw            Type: ipfw            ID: 00000001   Num hooks: 3
>   Local hook      Peer name       Peer type    Peer ID         Peer hook
>   ----------      ---------       ---------    -------         ---------
>   555             netflow         netflow      00000008        iface0
>   666             <unnamed>       hole         00000006        qqq
>   100             <unnamed>       echo         00000004        qqq
> +
> 
> root at jujik:~:|>ipfw show
> 00100     0        0 allow ip from any to any via lo0
> 00200     0        0 deny ip from any to 127.0.0.0/8
> 00300     0        0 deny ip from 127.0.0.0/8 to any
> 00400 14927 61918948 netgraph 100 ip from any to any
> 00500 14927 61918948 ngtee 666 ip from any to any
> 00600  7477  1067060 ngtee 555 ip from any to any in
> 65000 14927 61918948 allow ip from any to any
> 65535     0        0 deny ip from any to any
> 
> root at jujik:~:|>sysctl net.inet.ip.fw.one_pass
> net.inet.ip.fw.one_pass: 0
> 
> On Mon, Jan 17, 2005 at 11:06:10PM +0300, Gleb Smirnoff wrote:
> >   Dear collegues,
> >
> > here is quite a simple node for direct interaction between ipfw(4)
> > and netgraph(4). It is going to be more effective and error-prone
> > than a complicated construction around divert socket and ng_ksocket[1].
> >
> > The semantics of node operation are quite simple. There is one node
> > per system, which accepts any hooks with numeric names. Packets
> > can be sent to netgraph(4) using ipfw 'netgraph' action, followed
> > by a numeric cookie. Matched packets are sent out from corresponding
> > hook of ng_ipfw node. These packets are tagged with information which
> > helps them later to reenter ipfw processing. Tagged packets received on
> > any node hook reenter IP stack. If net.inet.ip.fw.one_pass sysctl is non
> > zero they are accepted, otherwise they continue with next rule. Non-tagged
> > packets (not originating from ng_ipfw node) are discarded.
> >
> > Here is sample configuration. ng_echo(4) echoes packets back from netgraph
> > to ipfw thru a tee node, which allows to sniff traffic.
> >
> > ngctl
> > + ls
> > There are 4 total nodes:
> >   Name: ngctl6138       Type: socket          ID: 0000000c   Num hooks: 0
> >   Name: ipfw            Type: ipfw            ID: 00000009   Num hooks: 1
> >   Name: <unnamed>       Type: echo            ID: 00000006   Num hooks: 1
> >   Name: tee             Type: tee             ID: 00000005   Num hooks: 2
> > + show ipfw:
> >   Name: ipfw            Type: ipfw            ID: 00000009   Num hooks: 1
> >   Local hook      Peer name       Peer type    Peer ID         Peer hook
> >   ----------      ---------       ---------    -------         ---------
> >   666             tee             tee          00000005        left
> > + show tee:
> >   Name: tee             Type: tee             ID: 00000005   Num hooks: 2
> >   Local hook      Peer name       Peer type    Peer ID         Peer hook
> >   ----------      ---------       ---------    -------         ---------
> >   left            ipfw            ipfw         00000009        666
> >   right           <unnamed>       echo         00000006        echi
> >
> > root at jujik:/usr/src:|>ipfw show
> > 00100    292      40304 allow ip from any to any via lo0
> > 00200      0          0 deny ip from any to 127.0.0.0/8
> > 00300      0          0 deny ip from 127.0.0.0/8 to any
> > 00350 290730  661428793 netgraph 666 ip from any to any
> > 65000 627921 1896034399 allow ip from any to any
> > 65535      0          0 deny ip from any to any
> >
> > The patch [2] is applicable only to HEAD, sorry. The target users are
> > the ones, who are now running ip_accounting/netflow using diverted
> > ng_ksocket, and just netgraph geeks.
> 
> --
> Totus tuus, Glebius.
> GLEBIUS-RIPN GLEB-RIPE