From: Florian Westphal <fw@strlen.de>
To: netdev@vger.kernel.org
Cc: Florian Westphal <fw@strlen.de>, Miroslav Kratochvil <exa.exa@gmail.com>
Subject: [PATCH] hfsc: ensure class is added to eltree exactly once
Date: Tue, 31 May 2016 12:12:19 +0200 [thread overview]
Message-ID: <1464689539-15162-1-git-send-email-fw@strlen.de> (raw)
Intent is to insert the class into the eligible tree when first packet
is enqueued (its removed from list when class becomes empty again).
Checking for a size of 1 is problematic:
1. child qdisc might have segmented the skb, in which backlog can transition
from 0 to a value > 1.
In this case we can't dequeue anymore as this class is not in the tree.
2. some qdiscs like fq_codel can purge their backlogs when internal
limits are hit and update the parent qlen via qdisc_tree_reduce_backlog(),
so its possible that we end up with a length of 1 after enqueue for a class
that was already on the active list.
If this happens, we add the same class twice (which then results
in qdisc dequeue soft lockup).
Fix it by testing the length before we attempt to enqueue to child qdisc,
if enqueue operation is successful and old qlen was 0, then the
class was not yet inserted into eltree.
Cc: Miroslav Kratochvil <exa.exa@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
This was found while looking at Miroslav Kratochvil bug report
but I don't think this fixes his case (I could trigger the 2nd case
above w. fq_codel+really_stupid_config_knobs but I got softlockup
which did not match his report).
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index d783d7c..0854be3 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1583,6 +1583,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
{
struct hfsc_class *cl;
int uninitialized_var(err);
+ unsigned int qlen;
cl = hfsc_classify(skb, sch, &err);
if (cl == NULL) {
@@ -1592,6 +1593,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
return err;
}
+ qlen = cl->qdisc->q.qlen;
err = qdisc_enqueue(skb, cl->qdisc);
if (unlikely(err != NET_XMIT_SUCCESS)) {
if (net_xmit_drop_count(err)) {
@@ -1601,7 +1603,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch)
return err;
}
- if (cl->qdisc->q.qlen == 1)
+ if (qlen == 0)
set_active(cl, qdisc_pkt_len(skb));
sch->q.qlen++;
--
2.7.3
next reply other threads:[~2016-05-31 10:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-31 10:12 Florian Westphal [this message]
2016-05-31 14:55 ` [PATCH] hfsc: ensure class is added to eltree exactly once Eric Dumazet
2016-05-31 23:00 ` Florian Westphal
2016-05-31 23:33 ` Eric Dumazet
2016-05-31 23:49 ` Florian Westphal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1464689539-15162-1-git-send-email-fw@strlen.de \
--to=fw@strlen.de \
--cc=exa.exa@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).