examples: add sniclassify tc/qos

[sneaky-potato]

Add a tc demo script.

[lneto]

This was already raised on #713 and left unaddressed: the SNI parser must not be duplicated. This PR still ships a verbatim second copy of the ClientHello parser from examples/filter/sni.lua. Please fold that feedback in rather than carry it forward — the duplication should be resolved before this goes in.

The parser belongs in examples/common/sni.lua; PR #721 extracts it and moves filter/sni.lua onto it. Rebase this example there and require("examples.common.sni"). Fixup 11d4916 shows the sniclassify side, which also drops the unpacker closures and flattens the walk.

[lneto]

The SNI-parser duplication I flagged is resolved: this now uses require("examples.common.sni") (the shared parser merged in #721), and the example reads cleanly. Good to go, on top of #713.

[lneto]

Still good after the rebase onto #713 — unchanged, uses the shared examples.common.sni.

[lneto]

@sneaky-potato — re-reviewed against the merged #713.

• Rebase needed. The branch still carries the three original (pre-fixup) #713 commits, so it is not mergeable and would reintroduce the un-fixed binding. Rebase onto master so this PR is just the sniclassify commit. (Rebased on review/717 for reference.)

• flow_cache is never populated. classify.c declares the map and looks it up, but nothing writes to it — every packet misses and re-runs the Lua classifier, so the cache does nothing. Fixed by caching the classified priority after the run: be4f743

• README typo — "the classfier" -> "classifier": f875a3a

• README, double filter — the tc filter is added twice: the HTB block adds it on egress, then the "Attach ... on egress" step adds it again with parent 1: (the heading says egress but the command uses parent 1:). One is redundant; which attach point does the example want?

[sneaky-potato]

README, double filter — the tc filter is added twice: the HTB block adds it on egress, then the "Attach ... on egress" step adds it again with parent 1: (the heading says egress but the command uses parent 1:). One is redundant; which attach point does the example want?

The second block is the redundant one.
The example wants the clsact/egress attach point. I'll make the necessary changes.

[lneto]

@sneaky-potato — all addressed: rebased onto master (one commit), flow_cache populated, the typo fixed, and the redundant filter dropped (nice consolidation of the egress attach under its own heading). Built it here — the modules and the sniclassify BPF compile, tc suite 5/5. LGTM.

[lneto]

@sneaky-potato — re-reviewed at runtime, not just the build: the callback crashes with the merged skb.attr.

• skbattr(ctx:skb()) — skb.attr is now a class; the module table isn't callable ("attempt to call a table value"). Use skbattr.new(ctx:skb()).
• skb:data() on the view — the reshaped skb.attr is a pure attribute view (mark/priority) and does not forward methods, so :data() raises "skb has no attribute 'data'". Keep the raw skb for methods:

local raw    = ctx:skb()
local skb    = skbattr.new(raw)
local packet = raw:data()
-- skb.priority = classid  (the view is for the attribute)

Both confirmed by running it; my earlier approval was premature — the example has no test of its own, so its callback never ran under the suite.

Please run the example end-to-end before the next push — load it and drive an HTTPS flow through it. With nothing covering it, a broken callback like this lands green; the API it leans on changed under it, and only running it surfaces that.

[sneaky-potato]

Hi @lneto, pushed the required changes. I was wondering on the following 2 points:

• Right now README uses eth0 as the interface name. Also the commands are kind of long, so maybe contain the commands in a shell file which takes the interface name as argument.
• There are no cleanup steps in the README, it takes 3 commands to do the cleanup, we could include them in the clean step in Makefile

sudo $TC filter del dev $IF egress 2>/dev/null
sudo $TC qdisc del dev $IF clsact 2>/dev/null
sudo $TC qdisc del dev $IF root 2>/dev/null

Need suggestions on how to improve on these two points.

[lneto]

The current asks are in the visible review comment below: #717 (comment)

[lneto]

@sneaky-potato the two runtime bugs are fixed, thanks.

Right now README uses eth0 as the interface name. Also the commands are kind of long, so maybe contain the commands in a shell file which takes the interface name as argument.

Yes — a small setup.sh <iface> (and cleanup.sh <iface>) taking the interface as an argument, the way tcpreject/cleanup.sh already does. The README then reduces to sudo ./setup.sh eth0.

There are no cleanup steps in the README, it takes 3 commands to do the cleanup, we could include them in the clean step in Makefile

A cleanup.sh <iface>, not the Makefile clean target — make clean removes build artifacts and does not know which interface the qdisc lives on, so teardown that needs a runtime argument does not belong there. tcpreject/cleanup.sh is the precedent.

Two changes from a closer pass, each as a fixup on review/717:

• policy matches by substring, not suffix. host:match("netflix%.com") is an unanchored Lua pattern, so ("evilnetflix.com"):match("netflix%.com") and ("netflix.com.attacker.io"):match("netflix%.com") both match and get classified as netflix. 6dc3c25 switches policy to the set module's labeled flavor: set.labeled{...} plus policy:match(host) does .-delimited suffix matching and returns the member's label (the classid), which rejects those two and is O(d log n) instead of a scan over every pattern. The set is built at load, in process context, so the allocation is fine.

• Magic offsets in tcp_payload. 30f9603 names the +9, & 0x0f, +2/+3 and +12 as IP_PROTO, IHL_MASK, TCP_DPORT, TCP_DOFF, the way examples/common/sni.lua names its parsing offsets.

One more thing, not on you: the Lua re-derives the payload offset the eBPF program already computed — a duplicated parse (I have not measured its cost; it is a handful of byte reads, so this is about the redundancy, not a hot path). bpf_luatc_run had no argument channel, unlike xdp, so the callback had no other way to receive the offset. #744 adds that channel; once it lands, the eBPF side can pass it and tcp_payload can go away, the way examples/filter/sni reads its offset from ctx:argument().

[lneto]

@sneaky-potato both scripts and the README change, as one extra commit (not a fixup, so take or adapt it): c1a92ca

• setup.sh <iface> runs the classifier, builds the HTB tree (the default 20 root and the 1:1/1:10/1:20/1:30 classes) and attaches the filter on egress, all parameterized by the interface. It loads the object with bpftool and attaches it object-pinned, the way the tc test suite does, rather than tc ... bpf da obj: on my machine the latter fails with "failed to find valid kernel BTF" (older iproute2 with no vmlinux ELF), and the suite already avoids it for that reason.
• cleanup.sh <iface> is lunatik stop plus the three tc del lines and the pin, in the shape of tcpreject/cleanup.sh.
• The README's two tc blocks then reduce to sudo ./examples/sniclassify/setup.sh eth0, torn down with sudo ./examples/sniclassify/cleanup.sh eth0.

I ran both end to end on a dummy interface: setup attaches the classifier and starts the runtime cleanly, cleanup tears it all down. I did not drive real TLS through it, so I exercised the setup and teardown, not the SNI classification itself.