diff mbox series

[2/2] sch_fq_codel: fix running with classifiers that don't set a classid

Message ID 20220307182602.16978-2-nbd@nbd.name (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [1/2] sch_cake: allow setting TCA_CAKE_NAT with value 0 if conntrack is disabled | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers fail 5 maintainers not CCed: jiri@resnulli.us kuba@kernel.org jhs@mojatatu.com xiyou.wangcong@gmail.com davem@davemloft.net
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 23 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Felix Fietkau March 7, 2022, 6:26 p.m. UTC
If no valid classid is provided, fall back to calculating the hash directly,
in order to avoid dropping packets

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/sched/sch_fq_codel.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Toke Høiland-Jørgensen March 7, 2022, 8:41 p.m. UTC | #1
Felix Fietkau <nbd@nbd.name> writes:

> If no valid classid is provided, fall back to calculating the hash directly,
> in order to avoid dropping packets
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

While I agree that this behaviour makes more sense, it's also a
user-facing API change; I suppose there may be filters out there relying
on the fact that invalid (or unset) class ID values lead to dropped
packets?

-Toke
Eric Dumazet March 7, 2022, 10:43 p.m. UTC | #2
On 3/7/22 12:41, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:
>
>> If no valid classid is provided, fall back to calculating the hash directly,
>> in order to avoid dropping packets
>>
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> While I agree that this behaviour makes more sense, it's also a
> user-facing API change; I suppose there may be filters out there relying
> on the fact that invalid (or unset) class ID values lead to dropped
> packets?


Indeed.

This part was copied from SFQ, so if we want to (optionally ?) change 
the behavior,

same change should be applied to SFQ.
diff mbox series

Patch

diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 839e1235db05..b2a13185bb63 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -88,7 +88,7 @@  static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
 
 	filter = rcu_dereference_bh(q->filter_list);
 	if (!filter)
-		return fq_codel_hash(q, skb) + 1;
+		goto out;
 
 	*qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
 	result = tcf_classify(skb, NULL, filter, &res, false);
@@ -104,10 +104,13 @@  static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
 			return 0;
 		}
 #endif
-		if (TC_H_MIN(res.classid) <= q->flows_cnt)
+		if (TC_H_MIN(res.classid) > 0 &&
+		    TC_H_MIN(res.classid) <= q->flows_cnt)
 			return TC_H_MIN(res.classid);
 	}
-	return 0;
+
+out:
+	return fq_codel_hash(q, skb) + 1;
 }
 
 /* helper functions : might be changed when/if skb use a standard list_head */