On Sat, Jun 27, 2026 at 12:52 PM [off-list ref] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removal of TC_ACT_REDIRECT support from qevents (tcf_qevent_handle) breaks existing eBPF redirect functionality.
- [Low] pr_warn_once() used inside a static inline function in a header file will print multiple times.
--

commit d96e142c03f9c1a1e9ac2e1d8c8fe26605053b10
Author: Jamal Hadi Salim [off-list ref]

net/sched: Handle TC_ACT_REDIRECT from qdisc filter chains

When a TC filter attached to a qdisc filter chain returns TC_ACT_REDIRECT,
the redirect is silently lost. This patch adds a tcf_classify_qdisc()
wrapper to convert TC_ACT_REDIRECT to TC_ACT_SHOT and drops the packet.
It also removes the TC_ACT_REDIRECT case from tcf_qevent_handle() since
it is handled by the wrapper.

quoted

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 3bd08d7f39c18..3a542a72e9a55 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h

[ ... ]

quoted

@@ -159,6 +159,19 @@ static inline int tcf_classify(struct sk_buff *skb,

 #endif

+static inline int tcf_classify_qdisc(struct sk_buff *skb,
+                                  const struct tcf_proto *tp,
+                                  struct tcf_result *res, bool compat_mode)
+{
+     int ret = tcf_classify(skb, NULL, tp, res, compat_mode);
+
+     if (unlikely(ret == TC_ACT_REDIRECT)) {
+             pr_warn_once("TC_ACT_REDIRECT from qdisc filter chains is not supported; use eBPF with clsact or mirred redirect instead\n");

[Severity: Low]
Is this going to print multiple times instead of strictly once globally?

Because tcf_classify_qdisc() is a static inline function in a header file,
each C file including it (like the various qdisc modules) will create its own
local instance of the static __already_done variable used by pr_warn_once().

This was intentional. I used WARN_ONCE() initially, but NIPA testing
may consider that as something "bad". In any case, this usage is
common, just ignore it.

quoted

+             ret = TC_ACT_SHOT;
+     }
+     return ret;
+}
+
 static inline unsigned long
 __cls_set_class(unsigned long *clp, unsigned long cl)
 {

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3e67600a4a1a1..3ca56d060e28f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c

@@ -4033,7 +4033,7 @@ struct sk_buff *tcf_qevent_handle(struct tcf_qevent *qe, struct Qdisc *sch, stru

      fl = rcu_dereference_bh(qe->filter_chain);

-     switch (tcf_classify(skb, NULL, fl, &cl_res, false)) {
+     switch (tcf_classify_qdisc(skb, fl, &cl_res, false)) {

[Severity: High]
Does this break existing eBPF redirect functionality for qevents?

Again, design intent. I will update the commit message to state this
clearly with the following text:
---
Remove the TC_ACT_REDIRECT case from tcf_qevent_handle().
skb_do_redirect() belongs in the BPF plumbing layer (net/core/), not
in net/sched/. The case was never consistent with the rest of the
qdisc classification infrastructure, where no classify function
handles
TC_ACT_REDIRECT.
---

IMO, it appears to have been TheLinuxWay(tm) i.e a cut-and-paste from
existing code on day 1 of  the qevent introduction rather than a
deliberate design decision.
Ccing Petr to see if he thinks differently.

cheers,
jamal

Prior to this patch, tcf_qevent_handle() correctly handled TC_ACT_REDIRECT
by calling skb_do_redirect(skb). By using the tcf_classify_qdisc() wrapper
here, TC_ACT_REDIRECT is unconditionally converted to TC_ACT_SHOT. This
inadvertently drops packets that were legitimately redirecting from qevents.

quoted

      case TC_ACT_SHOT:
              qdisc_qstats_drop(sch);
              __qdisc_drop(skb, to_free);

@@ -4045,10 +4045,6 @@ struct sk_buff *tcf_qevent_handle(struct tcf_qevent *qe, struct Qdisc *sch, stru
              __qdisc_drop(skb, to_free);
              *ret = __NET_XMIT_STOLEN;
              return NULL;
-     case TC_ACT_REDIRECT:
-             skb_do_redirect(skb);
-             *ret = __NET_XMIT_STOLEN;
-             return NULL;
      case TC_ACT_CONSUMED:
              *ret = __NET_XMIT_STOLEN;
              return NULL;

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626165156.169012-1-jhs@mojatatu.com?part=2