netdev - Re: [PATCH] cls_flow: Add tunnel support to the flow classifier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1319419287.20602.21.camel@ganymede>
Date:	Sun, 23 Oct 2011 21:21:26 -0400
From:	Dan Siemon <dan@...erfire.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] cls_flow: Add tunnel support to the flow classifier

On Mon, 2011-10-17 at 08:40 +0200, Eric Dumazet wrote:
> Le dimanche 16 octobre 2011 à 19:06 -0400, Dan Siemon a écrit :
> > When used on an interface carrying tunneled traffic the flow classifier
> > can't look into the tunnels so all of the traffic within the tunnel is
> > treated as a single flow. This does not allow any type of intelligent
> > queuing to occur. This patch adds new keys to the flow classifier which
> > look inside the tunnel. Presently IP-IP, IP-IPv6, IPv6-IPv6 and IPv6-IP
> > tunnels are supported.
> > 
> > If you are interested I have posted some background and experimental
> > results at:
> > http://www.coverfire.com/archives/2011/10/16/making-the-linux-flow-classifier-tunnel-aware/
> > 
> > The related iproute2 patch can be found at the above URL as well.
> > 
> > Signed-off-by: Dan Siemon <dan@...erfire.com>
> > 
> 
> Hi Dan
> 
> You're adding a lot of code (omitting the diffstat :( ) for a specific
> usage, yet GRE tunnels are not supported.

Thanks for the review.

Are you arguing this use case isn't worth addressing or that there is a
more efficient way to implement this with less code?

> IPv6 part is also a bit limited : It assumes TCP/UDP headers are the
> first ones. Maybe its time to use ipv6_skip_exthdr() ?

I noticed this too but the existing src-proto and dst-proto don't handle
this case either. Maybe I can look into fixing those as well.

> Note also that if we pull (with pskb_network_may_pull()) too many bytes,
> we kill routing performance on paged frags devices, wich are now
> becoming very common.

I don't know what paged frag devices means but I trust you are correct :)

The existing keys also use pskb_network_may_pull(). Should they be changed as well?

> Adding tunnel support and deep packet inspection might require the use
> of skb_header_pointer() wich does the copy of needed data without
> requiring expensive reallocation of skb head.

I'll look into this but it may be a while before I have an updated
patch.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)