realtek: dsa: rtl930x: offload ingress cls_flower to PIE

[mab-wien]

Note: the first four preparatory fixes are being reviewed separately in #25024. This branch still contains them for now and will be rebased on top once that PR lands.

Add hardware offload of ingress tc/cls_flower rules on RTL930x DSA
user ports, using the switch's existing Packet Inspection Engine, on top
of the port rate-policing path that is already wired to the RTL93xx
cls_flower ops.

Changes

• rtldsa_cls_flower_{add,del} route every rule that is not a plain port
  rate limiter to the PIE path; a new rtldsa_cls_flower_stats op
  reports per-rule packet counts.
• The offload is gated on family_id == RTL9300_FAMILY_ID; RTL931x keeps
  its current behaviour and the RTL83xx SoC code is untouched.
• The flow-rule parser rejects unsupported dissector keys and partial
  masks up front, and gains generic EtherType matching alongside the
  ARP/IPv4/IPv6 fast paths. pie_rule.ethertype_m is widened to u16;
  rtl930x_pie_verify_template() refuses an EtherType match when the
  active template lacks the field.
• rtl83xx_validate_flow_actions() rejects nonsensical action
  combinations (multiple forwards; drop mixed with other actions).
• The rule is bound to the physical ingress port via the source-port
  field. On RTL930x the PIE rule ID is also the LOG counter ID, so the
  packet counter needs no separate allocation.
• The flow rhashtable is created in rtldsa_93xx_setup() (RTL9300 only)
  and destroyed RCU-safely in rtl83xx_sw_remove() and the probe error
  path.

The first commit is a standalone fix: rtl930x_packet_cntr_clear() wrote
a shared LOG table entry back without reading it first, clobbering the
adjacent counter. It is latent today and becomes reachable once the
offload clears counters at runtime.

Testing

Built for realtek / rtl930x (kernel 6.18); make target/linux/compile
is clean with no new warnings.

Flashed and tested on a Zyxel XGS1210-12:

Rule	Result
flower protocol 0x8863 ... action drop (PPPoE discovery)	installed, in_hw in_hw_count 1
flower protocol ip ip_proto udp dst_port 67 ... action trap	installed, in_hw in_hw_count 1
flower ... action trap on ARP + tc -s filter show	3 real packets counted over 12 s (Sent hardware 0 bytes 3 pkt)
tc filter del + tc qdisc del clsact	no residue, no dmesg errors

Note

rtl83xx_setup_tc() (the older flow-block-cb path) has no port_setup_tc
op wired to it and is currently unreachable; it is kept building and now
shares rtl83xx_tc_init().

---

Discussion / testing feedback: https://forum.openwrt.org/t/tcfilter-luci-app-tcfilter-persistent-tc-ingress-filters-with-hardware-offload-on-realtek-switches/253331

[plappermaul]

And another surprise out of nowhere. Thanks for the contribution.

[mab-wien]

Addressed in the latest force-push:

• family_id gates replaced by a pie_rule_id_is_log_counter bool on struct rtldsa_config.
• New printouts converted to dev_err()/dev_dbg(priv->dev, ...).
• New functions renamed to the rtldsa_ prefix (rtldsa_tc_init/cleanup,
  rtldsa_pie_cls_flower_{add,del,stats}, etc.).

[plappermaul]

This is too-much-in-one-commit. Start dissecting this ...

[mab-wien]

Split into logical commits as requested:

1. rtl930x: don't clobber adjacent LOG counter (was already separate)
2. rtl930x: bound PIE rule search by the phase block range (block >= max_block)
3. widen pie_rule.ethertype_m to u16
4. rtl83xx: give the tc flow hashtable a real lifecycle (rtldsa_tc_init/cleanup)
5. rtl930x: offload ingress cls_flower to PIE (the feature)

The combined tree is unchanged from the previously built/tested revision.

[mab-wien]

Dissected further into 7 commits:

1. rtl930x: don't clobber adjacent LOG counter
2. rtl930x: bound PIE rule search by the phase block range
3. widen pie_rule.ethertype_m to u16
4. rtl83xx: set fwd_sel for the tc flower TRAP action (standalone bug fix)
5. rtl83xx: validate and tighten tc flower match parsing (hardening of the
   existing parser: reject unknown keys/partial masks, fix the ip_proto
   decode, propagate errors)
6. rtl83xx: give the tc flow hashtable a real lifecycle
7. rtl930x: offload ingress cls_flower to PIE (the feature itself)

Combined tree unchanged from the built/tested revision.

[mab-wien]

Done. rtldsa_packet_cntr_clear() plus its use in rtldsa_tc_flow_free()
and rtl83xx_delete_flower(), together with the packet_cntr = -1 init, are
now their own pre-commit.

While at it I also moved two stray pr_* -> dev_* conversions into the
commits that already touch those lines, so the final commit is only the
offload itself.

Full series now:

1 rtl930x: don't clobber adjacent LOG counter
2 rtl930x: bound PIE rule search by the phase block range
3 widen pie_rule.ethertype_m to u16
4 rtl83xx: set fwd_sel for the tc flower TRAP action
5 rtl83xx: validate and tighten tc flower match parsing
6 rtl83xx: give the tc flow hashtable a real lifecycle
7 rtl83xx: fix rtl83xx_configure_flower() error unwinding
8 rtl83xx: drop the tc flow hashtable ref before HW teardown
9 rtl83xx: read the tc flow counter outside the RCU lock
10 rtl83xx: clear a flow's LOG counter when it is destroyed
11 rtl930x: offload ingress cls_flower to PIE

1-5 are standalone fixes/hardening; 6-10 give the existing add/del/stats
helpers a proper lifecycle and locking; 11 is the offload (flag + gate,
the rtldsa_pie_cls_flower_* wrappers, cls_flower routing in dsa.c, generic
EtherType matching, source-port binding, PIE-rule-id-as-counter).

The combined tree is unchanged from the revision built for realtek/rtl930x
and tested on a Zyxel XGS1210-12 (EtherType/PPPoE drop, UDP/67 trap, ARP
trap with hardware packet counts via tc -s filter show, clean teardown).

[mab-wien]

Rebased onto current main (was 3 commits behind; the 3 new upstream
commits are ath79/mediatek, no realtek changes, no conflicts). No code
changes. Also folded two pr_* -> dev_* one-liners into the commits that
already touch those lines (7 and 9), so commit 11 is purely the offload

[openwrt-ai]

Reviewed 11 new commits; commit messages match their diffs.

---

Generated by Claude Code

[mab-wien]

Rebased onto current main now that #25050 has merged: the two commits carried there (drop the tc flow hashtable ref before HW teardown, verify the MAC template against the mask) drop out as already-applied, leaving 11 commits that replay with no conflicts. git range-diff against the pre-rebase tip is clean (11× =).

Retested on an XGS1210-12 (B1): clean boot on the rebased tree, all four flower match types (0x88e1 / 0x8912 EtherType, IPv4 + IPv6 UDP dst_port) install with in_hw, direct tc filter add/del and the service stop/start cycle behave, and dmesg stays clean (no traces / warnings).

The one open review thread (the tc_initialized re-check) is answered inline. Ready for another look.

[openwrt-ai]

Reviewed 1 new commit; no new issues found.

---

Generated by Claude Code

[plappermaul]

Partially reverting patch 1 with patch 10 is bad style. Not so good. Proceed as follows:

• Make a new PR with two patches
• A fix for realtek: rtl930x: PIE action programs a corrupted VID for tc-flower VLAN push/pop #25048
• A squashed version of patch 1 & 10 of this series

[l0rdg3x]

While reading tc.c for #25062 I checked whether its .ndo_setup_tc path can run, and it cannot: rteth_setup_tc() (rtl838x_eth.c:1528) returns -EOPNOTSUPP unless ds->ops->port_setup_tc is set, and nothing in target/linux/realtek sets it. The only two mentions of that op in the whole target are the check and the call inside that function, on main and with this PR applied.

So the series leaves two dispatchers for the same three commands: the live one you add through cls_flower_add/_del/_stats, and rtl83xx_setup_tc_cls_flower(), which this PR keeps current — it now calls rtldsa_configure_flower(), rtldsa_delete_flower() and rtldsa_stats_flower() — but which nothing reaches. Your call what to do with it; a marker comment would at least stop the next reader taking it for a live path.

Related: the !priv->r->packet_cntr_read || !priv->r->packet_cntr_clear guard in rtldsa_configure_flower() covers RTL931x, which has n_counters = 2048 and neither accessor. Without it that would have been a NULL call as soon as the path went live.

Analysis assisted by Claude Opus 5 (Anthropic AI).

[mab-wien]

Rebased onto current main now that #25070 is merged. Its four commits (VID encoding fix, packet counter allocator rework + free helper, configure_flower error unwinding, match/action validation) drop out of this series - patches 1, 3 and 10 were superseded and are gone; the offload commit picks up the rtldsa_packet_cntr_alloc() rename and folds the RTL930x pie-rule-id reservation into the reworked allocator.

Also added your dead-dispatcher point as its own commit ("drop the unreachable tc flower block dispatcher"): rtl83xx_setup_tc() / _block_cb() / cls_flower() are removed, nothing sets ds->ops->port_setup_tc so that whole path was dead; the live path stays the DSA .cls_flower{add,del,stats} ops. rtldsa_tc_init/cleanup() stay.

9 commits, only the five rtl83xx DSA files touched. Builds clean for realtek/rtl930x.

[plappermaul]

Next step: Make patch 7/9 and 9/9 a separate PR.

[mab-wien]

Rebased again onto current main - it picked up the realtek: table: refactor (otto_table_read / _otto_table{read,write} replacing rtl_table_read + sw_r32(rtl_table_data())). Only "read a PIE rule's LOG packet counter correctly" conflicted; the counter < RTL930X_PIE_RULE_IDS split is re-expressed in the new API (PIE rule id -> LOG entry at that index, count in word 1; route counter -> halved counter/2 layout). Same table, same index, same word - 1:1 translation. Builds clean for realtek/rtl930x. MERGEABLE now.

[mab-wien]

Done - patch 7/9 (read a PIE rule's LOG packet counter correctly) and 9/9 are now #25076.

9/9 couldn't move as-is: on main rtl83xx_setup_tc_cls_flower() is the only caller of rtl83xx_{configure,delete,stats}flower(), so just dropping the dispatcher orphans them (cascading through add_flow / parse_flow_rule). The whole of tc.c on main is that one never-reachable call graph, so #25076 removes the file outright (tc.o, struct rtl83xx_flow, priv->tc_ht, the decl). This series then adds a working tc.c back through the DSA .cls_flower* ops.

#24994 is 7 commits now (7/9 + 9/9 gone), still builds clean for realtek/rtl930x. Once #25076 lands I'll rebase - tc.c will be a fresh add rather than a diff, so the early lifecycle/locking commits fold into the offload.

[openwrt-ai]

Commit checks

• 1be54a5 "realtek: dsa: rtl930x: offload ingress cls_flower to PIE" — after the rebase this commit only adds the EtherType check to rtl930x_pie_verify_template(); the inner-VLAN and L4 source/destination-port checks the third bullet claims, and the dst_port story in the testing paragraph, are already in the base. It also still names the allocator rtl83xx_packet_cntr_alloc(), now rtldsa_packet_cntr_alloc().
• 5d0f274 "realtek: dsa: rtl83xx: clear a flow's LOG counter when it is destroyed" — the message says it initialises rule.packet_cntr to -1 in rtldsa_configure_flower(), but that init is already in the base and the diff does not touch it; drop that sentence.

---

Generated by Claude Code

[openwrt-ai]

tc_ht is keyed on the filter cookie alone, but the caller is now a per-port DSA op that also programs spn. With a shared block (tc qdisc add dev … ingress_block N clsact on two user ports) DSA registers one flow_block_cb per netdev (dsa_user_setup_tc_block(), cb_priv = dev), so the same cookie is offloaded to each port and the second one lands here with -EEXIST — no PIE rule on that port, and a hard failure of the add under skip_sw. Should the hash key be (cookie, ingress port)?

---

Generated by Claude Code

[mab-wien]

Real gap. A shared ingress_block across two DSA user ports registers one flow_block_cb per netdev, so the same cookie reaches rtldsa_configure_flower() twice and the second add gets -EEXIST - a hard failure under skip_sw, with no PIE rule on that port. The fix is to key tc_ht on (cookie, ingress_port). I'd rather land that as a follow-up on top of the merged offload than grow this series again; for now a per-port tc qdisc add dev <p> clsact works and the shared-block case fails loudly instead of silently. Will note it as a known limitation.

[openwrt-ai]

With the block dispatcher gone, rtldsa_pie_cls_flower_add() is the only caller and it rejects !ingress, so ingress_port is always a real DSA port and this guard is dead. The same caller also requires pie_rule_id_is_log_counter, which makes the else arm at lines 456-462 (rtldsa_packet_cntr_alloc() + log_data) unreachable too — worth dropping in the same commit, since the "keep it correct if the flow-block-cb path is ever revived" rationale went away with that path.

Suggested change

	if (ingress_port >= 0) {
	flow->rule.spn = ingress_port;
	flow->rule.spn_m = 0x7f;
	}
	flow->rule.spn = ingress_port;
	flow->rule.spn_m = 0x7f;

---

Generated by Claude Code

[mab-wien]

Done - folded into "drop the unreachable tc flower block dispatcher". With that gone rtldsa_configure_flower() has one caller (rtldsa_pie_cls_flower_add(), RTL930x-only, always a real ingress port), so the ingress_port >= 0 wrapper and the !pie_rule_id_is_log_counter / rtldsa_packet_cntr_alloc() + log_data branch are both removed there (tc.c: -91 lines).

[openwrt-ai]

nit: patches 1, 2 and 5 each rework rtl83xx_setup_tc() and its helpers — the first_time removal, the rtldsa_ rename, and the ingress_port = -1 call site added for this parameter — and patch 8 then deletes all of it. Sorting that deletion to the front of the series, as was done for the earlier prep patches, removes the churn.

---

Generated by Claude Code

[mab-wien]

Can't sort it to the front here. On main rtl83xx_setup_tc_cls_flower() is the sole caller of rtl83xx_{configure,delete,stats}flower() -> add_flow -> parse_flow_rule, i.e. the whole tc.c call graph; deleting the dispatcher first orphans all of it (-Wunused-function) until patch 5 rewires rtldsa_configure_flower() to the DSA .cls_flower* ops. Moving the deletion up would need patch 5's wiring to move with it - a larger reshuffle than the churn it removes. The first_time / rename / ingress_port touches in patches 1, 2, 5 are each part of turning that path live, not edits to dead code.

[plappermaul]

New feature seems to start at patch 5. So make patches 1-4 a separate fix-PR.

[mab-wien]

Rebased onto current main now that #25076 is merged. The two counter ops now
take priv; adapted the three call sites (rtldsa_stats_flower, the
rtldsa_packet_cntr_clear wrapper, and the 930x packet_cntr_read/_clear
split) and merged the dev_dbg wording. 9 commits, no functional change; rtl930x
build clean.

[plappermaul]

also patch 8 looks very suspicious as it should be part of this pre-Fix series.

[mab-wien]

Split as requested: patches 1-4 and "hold tc_flow_lock in rtldsa_tc_cleanup()"
are now #25094. This PR keeps only the offload feature - 4 commits:

• offload ingress cls_flower to PIE
• read a PIE rule's LOG packet counter correctly (rtl930x)
• drop redundant rcu_read_lock() in tc flower callbacks
• drop the unreachable tc flower block dispatcher

Rebases cleanly on #25094; content unchanged from the tested 9-commit stack
(range-diff clean, only the commit split differs). Will rebase onto main once
#25094 lands.