Re: [PATCH net 0/6] net/sched: Fix packet loops in mirred and netem

[Stephen Hemminger <stephen@networkplumber.org>]

On Sun, 11 Jan 2026 11:39:41 -0500
Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> We introduce a 2-bit global skb->ttl counter.Patch #1 describes how we puti
> together those bits. Patches #2 and patch #5 use these bits.
> I added Fixes tags to patch #1 in case it is useful for backporting.
> Patch #3 and #4 revert William's earlier netem commits. Patch #6 introduces
> tdc test cases.
> 
> Jamal Hadi Salim (5):
>   net: Introduce skb ttl field to track packet loops
>   net/sched: Fix ethx:ingress -> ethy:egress -> ethx:ingress mirred loop
>   Revert "net/sched: Restrict conditions for adding duplicating netems
>     to qdisc tree"
>   Revert "selftests/tc-testing: Add tests for restrictions on netem
>     duplication"
>   net/sched: fix packet loop on netem when duplicate is on
> 
> Victor Nogueira (1):
>   selftests/tc-testing: Add netem/mirred test cases exercising loops
> 
>  drivers/net/ifb.c                             |   2 +-
>  include/linux/skbuff.h                        |  24 +-
>  include/net/sch_generic.h                     |  22 +
>  net/netfilter/nft_fwd_netdev.c                |   1 +
>  net/sched/act_mirred.c                        |  45 +-
>  net/sched/sch_netem.c                         |  47 +-
>  .../tc-testing/tc-tests/actions/mirred.json   | 616 +++++++++++++++++-
>  .../tc-testing/tc-tests/infra/qdiscs.json     |   5 +-
>  .../tc-testing/tc-tests/qdiscs/netem.json     |  96 +--
>  9 files changed, 698 insertions(+), 160 deletions(-)
>

Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

This is a complex patch series so I decided to get a second opinion using AI.
It is worth reading (but not completely trusting). Review prompt is Chris Mason's
Claude review prompts.


Summary: Patch Series Analysis
Patches Reviewed
6-patch series from Jamal Hadi Salim fixing loop detection in mirred and netem:

Patch 1/6: Introduces skb->ttl (2-bit field) for cross-deferral loop tracking, moves from_ingress to qdisc_skb_cb
Patch 2/6: Fixes mirred ingress→egress→ingress loop detection using ttl
Patch 3/6: Reverts netem duplication restrictions (preparation for proper fix)
Patch 4/6: Reverts associated selftests
Patch 5/6: Fixes netem duplicate infinite loop using ttl
Patch 6/6: (Email thread discussion, not code)

Key Findings
No regressions identified. The analysis covered:
AreaResultSKB structure change (ttl field)✓ Safe - properly initialized via zeroingfrom_ingress relocation✓ Safe - written immediately before readLoop detection logic✓ Correct - ttl tracks across async boundariesNetem duplicate fix✓ Improvement over old q->duplicate hackLocking✓ Correct softirq/per-cpu patternsResource management✓ No leaks identified
Design Assessment
The approach is sound:

Egress paths: Continue using per-cpu sched_mirred_dev[] array for immediate loop detection
Ingress paths: Use skb->ttl to track loops across netif_rx() deferral boundaries
Netem: ttl-based dup prevention works across entire qdisc tree (better than old local-only fix)

Recommendation
Yes, the patch is OK to merge.
The series correctly fixes real bugs (CVE-worthy loop conditions) with a minimal, well-designed solution. The 2-bit ttl field is sufficient for the use case (limit of 3 ingress redirects), and the changes maintain backward compatibility for existing configurations while closing the loop detection gaps.