Integrate Pflua #444

Merged
merged 29 commits into from Apr 28, 2015

Projects

None yet

8 participants

@lukego
Member
lukego commented Apr 11, 2015

This branch explores adopting pcap filter expressions to configure packet filtering in Snabb Switch. This is made easy by pflua, a small and embeddable pcap filter compiler based on LuaJIT.

The work outline for this branch is:

  • Add pflua as a submodule and compile it into Snabb Switch.
  • Use pflua for lib.pcap.filter (replacing libpcap)
  • Replace the packet filtering app & API with pflua and pcap filter strings.
  • Translate OpenStack-style Security Group rules into pcap expressions.

I am cherry-picking commits from #442 with the difference that I "burn the boats" by outright replacing the old packet filtering code.

This branch can only merge if the pflua alternative is okay for all of our existing use cases. This seems plausible: I do think that pcap-filter expressions make a fundamentally superior API that is simpler, more flexible, and more familiar to networking people (from using tcpdump).

lukego and others added some commits Apr 11, 2015
@lukego lukego deps/pflua: Add pflua as a Git submodule f5f694f
@dpino @lukego dpino Adapt function filter
(cherry picked from commit a0f2830)
6fc9b62
@lukego lukego lib.pcap.filter: Exclusively use pflua (drop libpcap)
This module is simplified to only use pflua and not support libpcap.

This frees Snabb Switch of an awkward runtime dependency on libpcap
when this lib.pcap.filter module is used.
bcf954e
@lukego lukego Merge pflua-based lib.pcap.filter module a832bbc
@wingo @lukego wingo Bundle pflua into the snabb binary
* src/Makefile: Add appropriate makefile rules.

(cherry picked from commit 3f8233d)
f506408
@lukego lukego Makefile: Updated treatment of submodules (including pflua)
Treat deps/*.vsn files as the "proof" that a submodule is correctly
fetched and built with a specific version.
40b964c
@lukego lukego Merge pflua submodule build (Makefile) support fe7b80b
@plajjan
Contributor
plajjan commented Apr 11, 2015

I haven't read the code but the way I understand this PR it's all about the config format and interpreting that into an internal format, right?

The actual parsing of packets and matching towards filters remains the same?

pcap expressions (aren't they called BPF?) are probably well suited for network people but are they really superior for machine interaction? I imagine another system having an internal representation, that probably looks quite a lot like the current config format of packet_filter, and then translating that into a BPF expression only for snabb to again interpret it.

It's a bit like SQL. RDBMS were fitted with a query language that humans could write and there was a time when users, not developers or system administrators, actually interfaced with databases using SQL. But then we invented web2.0 and wrote pretty user interfaces that anyone, even novice users, could use. All of a sudden it's a machine generating SQL, the language invented for humans, and to make it easy for the programmers, ORMs were developed. So what is the job of the parser and planner in any RDBMS? To turn that pesky human-writeable format into something more suitable for a computer. Full circle.

Newer databases, like Rethinkdb offers a different model, where there is never a format suitable for humans, instead you interface with it directly from your favourite language (see rethinkdb.com, although it's a bit off-topic ;).

I'm all for a BPF style interface to ease human interaction but I'm not convinced it should be the only format.

@lukego
Member
lukego commented Apr 12, 2015

I think that we see things the same way, just that the text in my PR is a bit bombastic :)

The meat of this change is to replace the syntax for end-users to configure packet filtering in the snabbnfv traffic program.

Instead of human users writing verbose configurations like this:

    ingress_filter = { rules = { { ethertype='ipv6',
                                   protocol='tcp',
                                   dest_port_min=24,
                                   dest_port_max=25 } } },

we would write short ones like this:

    ingress_filter = "ip6 and tcp and dst portrange 24-25"

and that this change would have non-trivial implications for the rest of the code base that have to be worked out on this branch.

I do take your point that we must not force developers to shoe-horn new problems into filter expressions. There also needs to be a fully general Lua API and we need to be making that more friendly to use over time e.g. with abstractions like Alex's datagram library. I have spent more than enough of my life writing code to translate abstract problems into elaborate iptables configurations and so on :) and I don't want to see Snabb Switch users having to do that stuff either.

btw: In the YANG universe are there some standard schemas for defining packet filters? (ACLs, etc?) That would be interesting to see as context.

@plajjan
Contributor
plajjan commented Apr 12, 2015

Ok, cool. Sounds like we are on the same page.

Regarding YANG, there's a draft (99% of YANG models are drafts at this point) at least; https://tools.ietf.org/html/draft-huang-netmod-acl-01

@sleinen
sleinen commented Apr 12, 2015

On Sun, Apr 12, 2015 at 10:23 AM, Kristian Larsson <notifications@github.com

wrote:

https://tools.ietf.org/html/draft-huang-netmod-acl-01

Note that this revision has expired in 2013 (though there are newer ones
that have expired only last year :-)

But check out:

https://tools.ietf.org/html/draft-ietf-netmod-acl-model

Simon.

@alexandergall
Contributor

It appears that the logic that installs pflua in the snabbswitch/src directory is missing?

@lukego
Member
lukego commented Apr 13, 2015

@alexandergall Yeah this branch is incomplete. I'll try to push a working version this week. (Let me know if you want this urgently and I will push a work in progress; only issue is that I have one small change to pflua itself that I need to work on what to do with.)

@alexandergall
Contributor

No hurry. I should be working on other stuff anyway :)

dpino and others added some commits Nov 15, 2014
@dpino @lukego dpino Add new pflua packet filtering application
This is an implementation of the current packet filtering application
using pflua.

(cherry picked from commit f2041a7)
8a08d7c
@dpino @lukego dpino Refactor packet_filter benchmark
(cherry picked from commit fdab68a)
d4533db
@dpino @lukego dpino Packet filter pflua in the same dir as packet_filter
(cherry picked from commit 949ede8)
e89e963
@kbara @lukego kbara Removed obsolete core.buffer code
(cherry picked from commit 0a66ad6)
e433557
@lukego lukego apps/packet_filter: Replace packet_filter with pcap_filter
pcap_filter is the new name for the pflua-based packet filter.
(packet_filter module is deleted.)
fa0096c
@lukego lukego lib.pcap: Use anon structs to avoid FFI name clash with pflua
Define FFI pcap record types with anonymous structs instead of named
ones. The names aren't globally unique anymore: pflua uses them too.
a2c71b3
@lukego lukego src/Makefile: Fix pflua build rule. 92d60f8
@lukego lukego PcapFilter app replacing PacketFilter.
This app is simple: it filters packets using one pflua filter
rule. Optionally it also uses conntrack for stateful tracking.

The unit tests are weak: they are intended to detect breakages and
unexpected changes in the behavior of pflua and conntrack, but not to
actually test the correctness of those libraries (which are other units).
e2b157f
@lukego lukego nfvconfig: Use PcapFilter app instead of PacketFilter
This implicitly switches the "snabbnfv traffic" program from its
original filtering configuration syntax:

    ingress_filter = { rules = { { ethertype='ipv6',
                                   protocol='tcp',
                                   dest_port_min=24,
                                   dest_port_max=25 } } }

To a new one:

    ingress_filter = "ip6 and tcp dst portrange 24-25"

Reference configuration files are updated.
c4bed2e
@lukego lukego deps/pflua: Update to be compatible with ljsyscall
This change makes our submodule point to a pflua tree on the SnabbCo
github account i.e. we have our own branch of pflua that we can
directly merge changes that we need into and then sync with upstream
separately.
240954f
@lukego lukego neutron2snabb: Translate security groups into pflua filters
snabbnfv-traffic now requires packet filtering rules to be specified
as pcap-filter(7) expressions. Now neutron2snabb translates the
Security Group ACL rules into such expressions.
950d0bd
@lukego lukego Merge pcap_filter app as replacement to packet_filter 57f2f0d
@lukego
Member
lukego commented Apr 14, 2015

The code is all there now but it is not properly tested yet.

  • snabbnfv traffic config only accepts filter expressions now.
  • Neutron Security Groups are translated into filter expressions.
  • PacketFilter is replaced by the pflua-based PcapFilter.

I don't claim that filter expressions are the universal solution for all applications, but I do think that they are an upgrade from Security Groups and that translation is a reasonable way to stay compatible with Neutron. I especially like the fact that we can easily add new concepts, for example VLAN-aware filter rules for trunk ports, without waiting for OpenStack to reach consensus on an API.

@eugeneia are you interested in helping to test and document this? :-)

@wingo
Contributor
wingo commented Apr 14, 2015

You probably know, but FYI support for vlan is one of the few remaining to-be-implemented operators in pflua.

@eugeneia
Member
@eugeneia
Member

@lukego The failure was due to a missing , and (the important part) using ip6 and icmp instead of icmp6. Apparently ip6 and icmp means "IPv6 and ICMPv4". This is probably also a bug in neutron2snabb. I just checked it, neutron2snabb doesn't implement icmp as a possible protocol value.

@alexandergall
Contributor

Encouraging news. I've tried pflua with my app and I see a significant performance boost. I run my test on interlaken with loadgen. There are 3 calls to the pcap.filter match() method in the decapsulation path (one in nd_light, one in a "dispatcher" that demuxes tunneled packets into the proper pseudowire based on src/dst and a match for the tunnel protocol). The profiler for a typical run shows this:

100% Compiled
-- 15% filter.lua:32 < dispatch.lua:76 < app.lua:71
-- 6% filter.lua:32 < nd_light.lua:251 < nd_light.lua:285
-- 6% packet.lua:71 < datagram.lua:205 < pseudowire.lua:597

The packet-rate is around 2.6Mpps. The performance is dominated by the BPF matches, followed by the actual decapsulation (using AVX memmove).

With pflua, I get
100% Compiled
-- 7% packet.lua:71 < datagram.lua:205 < pseudowire.lua:597
-- 6% datagram.lua:188 < l2tpv3.lua:49 < pseudowire.lua:598
-- 4% datagram.lua:189 < l2tpv3.lua:49 < pseudowire.lua:598
...
-- 3% [string]:5 < nd_light.lua:251 < nd_light.lua:285
...
-- 1% [string]:6 < dispatch.lua:76

The BPF overhead is a lot smaller and the packet-rate is up to ~3.3Mpps!

I'll do more thorough tests, but this is looking very good. Cool!

Oh, the packet rates without the profiler running are 3.0Mpps vs 4.2Mpps :)

@eugeneia
Member

@lukego Not sure if you noticed, I created a PR updating the documentation, the fuzzer and fixing a few bugs on your fork: lukego#1

@lukego
Member
lukego commented Apr 20, 2015

@alexandergall woohoo! 20% speedup in a real application simply by switching libpcap for pflua sounds promising indeed. Great work, pflua hackers!

@lukego
Member
lukego commented Apr 20, 2015

@eugeneia Thanks! I had indeed missed that :)

@wingo
Contributor
wingo commented Apr 20, 2015

The speedups are a pleasant surprise :)

Note that we found a couple bugs recently affecting e.g. icmp6. The expander for "icmp6" had a bug whereby it expanded to the pflang equivalent of ip6[proto_offset] == ICMP, which had the side effect of asserting that the packet was ipv6 -- meaning "icmp6 or icmp" would never see the "icmp" side. It now expands to the equivalent of (ip6 and ip6[proto_offset] == ICMP). Anyway, details, but you might run into this bug and it's probably worth updating pflua.

@dpino
Contributor
dpino commented Apr 22, 2015

FWIW, Igalia/pflua#159 is fixed and extra support for dotted triples, dotted doubles, etc will land soon (Igalia/pflua#176).

@lukego
Member
lukego commented Apr 28, 2015

Groovy. Let's get some more experience with this code :).

@lukego lukego merged commit 5056a55 into snabbco:master Apr 28, 2015

1 check passed

snabb_bot SnabbBot
Details
@lukego lukego deleted the lukego:integrate-pflua-filter branch Feb 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment