Re: [PATCH RFC] netfilter: nf_tables: extend tracing infrastructure

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Florian,

Cc'ing Patrick to shake him for ideas :)

On Thu, Nov 12, 2015 at 04:46:49PM +0100, Florian Westphal wrote:
> RFC for the kernel-side of the tracing changes.
> 
> Caveats+Questions are:
>  - should kernel dump entire rule instead of just the handle?
>  PRO of dumping entire rule matched:
>   * Userspace doesn't need to dump rules first to display
>   the rule that matched
>   * nft trace doesn't need to also monitor ruleset for changes
>   * would make it possible to use a new nfnetlink group as Pablo
>   suggested
> 
>  CON:
>   * needs more memory in kernel space (can't use NLMSG_GOODSIZE -
>     allocation uses GFP_ATOMIC)
> 
>  This patch only dumps the rule handle.

Agreed, only rule handle should be fine. We have limited bandwidth
delivering netlink event notifications from the packet path, so
keeping the message small there looks like a good idea to me.

>  As you can see, I reimplemented parts of nfqueue/nfnetlink_log attributes.
>  Problem is that I don't want to add dependencies on nfetlink_log for this,
>  but things like iif and oif are rather imporant, esp. when debugging
>  packets being forwarded.
> 
>  The first 128 bytes of the packet data is also dumped to userspace, currently
>  limited to when we do the initial 'skb->nf_trace=1' assignment in nft_meta.c
>
>  Patrick, I saw your idea wrt. dumping register contents instead, but it
>  seems both complex and 'backwards' to me -- isn't the 'meta set nftrace'
>  rule already a selector, i.e. user would say something like
> 
>  'tcp port 22 limit rate 1/second meta nftrace set 1'
> 
>  So I'm not sure its right to extend nftrace with additional selectors (its
>  also possible that I failed to understand what you were suggesting  8-} )

I can see room to extend this infrastructure later on if we need to
make it more flexible (eg. allow the user to specify what part of the
packets need to be dumped to userspace). We can probably even add a
specific 'trace' expression from the kernel to allow more specific
selection on what packet fields you want to dump to userspace.

BTW, any reason to remove the existing infrastructure? It's been there
since almost the beginning (this would break users that are expecting
the existing behaviour). Moreover, we'll have people using the
iptables-compat infrastructure also expecting a similar output to
native iptables.

You can probably generalize the trace infrastructure through static
key plus indirections to keep both tracing approaches around (allowing
only one at the same time should be enough).

>From userspace, we would need to have a way to indicate that we want
to use the new infrastructure, not the classic tracing. That syntax
would need some thinking. Probably we can introduce a 'trace'
statement, so the syntax looks compact like this:

'tcp port 22 limit rate 1/second trace'

Meaning in this case that the userspace is specifically requesting for
the new kind of tracing.