diff mbox series

[net] net_sched: sch_fq: fix incorrect behavior for small weights

Message ID 20240824181901.953776-1-edumazet@google.com (mailing list archive)
State Accepted
Commit bc21000e99f92a6b5540d7267c6b22806c5c33d3
Delegated to: Netdev Maintainers
Headers show
Series [net] net_sched: sch_fq: fix incorrect behavior for small weights | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 4 blamed authors not CCed: soheil@google.com dave.taht@gmail.com toke@redhat.com willemb@google.com; 7 maintainers not CCed: jiri@resnulli.us dave.taht@gmail.com willemb@google.com jhs@mojatatu.com xiyou.wangcong@gmail.com soheil@google.com toke@redhat.com
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch warning WARNING: Non-standard signature: Bisected-by: WARNING: Non-standard signature: Diagnosed-by:
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-08-25--21-00 (tests: 714)

Commit Message

Eric Dumazet Aug. 24, 2024, 6:19 p.m. UTC
fq_dequeue() has a complex logic to find packets in one of the 3 bands.

As Neal found out, it is possible that one band has a deficit smaller
than its weight. fq_dequeue() can return NULL while some packets are
elligible for immediate transmit.

In this case, more than one iteration is needed to refill pband->credit.

With default parameters (weights 589824 196608 65536) bug can trigger
if large BIG TCP packets are sent to the lowest priority band.

Bisected-by: John Sperbeck <jsperbeck@google.com>
Diagnosed-by: Neal Cardwell <ncardwell@google.com>
Fixes: 29f834aa326e ("net_sched: sch_fq: add 3 bands and WRR scheduling")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
---
 net/sched/sch_fq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org Aug. 27, 2024, 3:30 p.m. UTC | #1
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 24 Aug 2024 18:19:01 +0000 you wrote:
> fq_dequeue() has a complex logic to find packets in one of the 3 bands.
> 
> As Neal found out, it is possible that one band has a deficit smaller
> than its weight. fq_dequeue() can return NULL while some packets are
> elligible for immediate transmit.
> 
> In this case, more than one iteration is needed to refill pband->credit.
> 
> [...]

Here is the summary with links:
  - [net] net_sched: sch_fq: fix incorrect behavior for small weights
    https://git.kernel.org/netdev/net/c/bc21000e99f9

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 238974725679327b0a0d483c011e15fc94ab0878..19a49af5a9e527ed0371a3bb96e0113755375eac 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -663,7 +663,9 @@  static struct sk_buff *fq_dequeue(struct Qdisc *sch)
 			pband = &q->band_flows[q->band_nr];
 			pband->credit = min(pband->credit + pband->quantum,
 					    pband->quantum);
-			goto begin;
+			if (pband->credit > 0)
+				goto begin;
+			retry = 0;
 		}
 		if (q->time_next_delayed_flow != ~0ULL)
 			qdisc_watchdog_schedule_range_ns(&q->watchdog,