Thread (7 messages) 7 messages, 5 authors, 2011-08-01

Re: [Bugme-new] [Bug 39372] New: Problems with HFSC Scheduler

From: Patrick McHardy <hidden>
Date: 2011-07-29 14:11:25

On 29.07.2011 16:00, Eric Dumazet wrote:
quoted hunk ↗ jump to hunk
Le vendredi 29 juillet 2011 à 15:27 +0200, Eric Dumazet a écrit :
quoted
Le vendredi 29 juillet 2011 à 14:29 +0200, Michal Soltys a écrit :
quoted
On 11-07-15 00:14, Andrew Morton wrote:
quoted
(switched to email.  Please respond via emailed reply-to-all, not via
the bugzilla web interface).


Here: WARN_ON(next_time == 0);
From the other thread on netfilter-devel:
quoted
On 11-07-22 11:58, Michal Pokrywka wrote: After bisecting 2.6.39.1 it
turned out that the bug is caused independently by two patches:

commit b262a5da755cc6ed0cb4fba230cd9bf4037e1096 sch_sfq: fix peek()
implementation

and

commit 9df49f2bfe862573911a080c75a6d81113c5c81d sch_sfq: avoid giving
spurious NET_XMIT_CN signals

Reverting these patches makes HFSC work again.
This one (upstream 8efa885406359af300d46910642b50ca82c0fe47) seems to be
the culprit (does reverting only that one cures the problem ?)

It allows SFQ to return success on enqueuing, when the packet really
replaced some other packet in some other flow. This confuses outer qdisc
(in this particular case HFSC) which thinks new packet was actually
added each time such situation happes.
Technically speaking, _this_ packet was successfuly enqueued.

Returning NET_XMIT_CN or NET_XMIT_SUCCESS should not trigger a bug in
caller.
quoted
This in turn causes additional dequeues and ends with attempt
to schedule non-existent packets, and triggers the warning.
Then its probably a bug in HFSC : It doesnt understand SFQ lost a
packet.

I'll take a look, thanks for the report.
Oh well, it seems one qdisc_tree_decrease_qlen(sch, 1) is missing

Maybe following patch would help...

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 4536ee6..2a2d287 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -410,7 +410,12 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	/* Return Congestion Notification only if we dropped a packet
 	 * from this flow.
 	 */
-	return (qlen != slot->qlen) ? NET_XMIT_CN : NET_XMIT_SUCCESS;
+	if (qlen != slot->qlen)
+		return NET_XMIT_CN;
+
+	/* as we dropped a packet, better let upper stack know this */
+	qdisc_tree_decrease_qlen(sch, 1);
+	return NET_XMIT_SUCCESS;
 }
 
Yeah, that seems to be the correct fix, thanks for looking into this.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help