From: Paolo valente <paolo.valente@unimore.it>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
"David S. Miller" <davem@davemloft.net>,
shemminger@vyatta.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
fchecconi@gmail.com, rizzo@iet.unipi.it,
Paolo Valente <paolo.valente@unimore.it>
Subject: [PATCH BUGFIX 4/6] pkt_sched: prevent budget from wrapping around after a dequeue
Date: Tue, 5 Mar 2013 19:05:00 +0100 [thread overview]
Message-ID: <1362506702-4985-5-git-send-email-paolo.valente@unimore.it> (raw)
In-Reply-To: <1362506702-4985-1-git-send-email-paolo.valente@unimore.it>
Aggregate budgets are computed so as to guarantee that, after an
aggregate has been selected for service, that aggregate has enough
budget to serve at least one maximum-size packet for the classes it
contains. For this reason, after a new aggregate has been selected
for service, its next packet is immediately dequeued, without any
further control.
The maximum packet size for a class, lmax, can be changed through
qfq_change_class(). In case the user sets lmax to a lower value than
the the size of some of the still-to-arrive packets, QFQ+ will
automatically push up lmax as it enqueues these packets. This
automatic push up is likely to happen with TSO/GSO.
In any case, if lmax is assigned a lower value than the size of some
of the packets already enqueued for the class, then the following
problem may occur: the size of the next packet to dequeue for the
class may happen to be larger than lmax, after the aggregate to which
the class belongs has been just selected for service. In this case,
even the budget of the aggregate, which is an unsigned value, may be
lower than the size of the next packet to dequeue. After dequeueing
this packet and subtracting its size from the budget, the latter would
wrap around.
This fix prevents the budget from wrapping around after any packet
dequeue.
Signed-off-by: Paolo Valente <paolo.valente@unimore.it>
Reviewed-by: Fabio Checconi <fchecconi@gmail.com>
---
net/sched/sch_qfq.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 0dbec31..26149e2 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1070,7 +1070,15 @@ static struct sk_buff *qfq_dequeue(struct Qdisc *sch)
qdisc_bstats_update(sch, skb);
agg_dequeue(in_serv_agg, cl, len);
- in_serv_agg->budget -= len;
+ /* If lmax is lowered, through qfq_change_class, for a class
+ * owning pending packets with larger size than the new value
+ * of lmax, then the following condition may hold.
+ */
+ if (unlikely(in_serv_agg->budget < len))
+ in_serv_agg->budget = 0;
+ else
+ in_serv_agg->budget -= len;
+
q->V += (u64)len * IWSUM;
pr_debug("qfq dequeue: len %u F %lld now %lld\n",
len, (unsigned long long) in_serv_agg->F,
--
1.7.9.5
next prev parent reply other threads:[~2013-03-05 18:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-19 17:31 [PATCH BUGFIX] pkt_sched: fix little service anomalies and possible crashes of qfq+ Paolo valente
2012-12-26 23:13 ` David Miller
2013-02-26 17:02 ` Paolo valente
2013-02-26 22:37 ` David Miller
2013-03-05 18:04 ` [PATCH BUGFIX 0/6] " Paolo valente
2013-03-05 18:04 ` [PATCH BUGFIX 1/6] pkt_sched: properly cap timestamps in charge_actual_service Paolo valente
2013-03-05 18:04 ` [PATCH BUGFIX 2/6] pkt_sched: fix the update of eligible-group sets Paolo valente
2013-03-06 10:05 ` David Laight
2013-03-05 18:04 ` [PATCH BUGFIX 3/6] pkt_sched: serve activated aggregates immediately if the scheduler is empty Paolo valente
2013-03-05 18:05 ` Paolo valente [this message]
2013-03-05 18:05 ` [PATCH BUGFIX 5/6] pkt_sched: do not allow virtual time to jump if an aggregate is in service Paolo valente
2013-03-05 18:05 ` [PATCH BUGFIX 6/6] pkt_sched: remove a useless invocation of qfq_update_eligible Paolo valente
2013-03-06 4:50 ` [PATCH BUGFIX 0/6] pkt_sched: fix little service anomalies and possible crashes of qfq+ David Miller
2013-07-10 13:46 ` [PATCH 0/2] pkt_sched: sch_qfq: efficiency and codestyle improvements Paolo Valente
2013-07-10 13:46 ` [PATCH 1/2] pkt_sched: sch_qfq: improve efficiency of make_eligible Paolo Valente
2013-07-10 13:46 ` [PATCH 2/2] pkt_sched: sch_qfq: remove forward declaration of qfq_update_agg_ts Paolo Valente
2013-07-11 20:01 ` [PATCH 0/2] pkt_sched: sch_qfq: efficiency and codestyle improvements David Miller
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=1362506702-4985-5-git-send-email-paolo.valente@unimore.it \
--to=paolo.valente@unimore.it \
--cc=davem@davemloft.net \
--cc=fchecconi@gmail.com \
--cc=jhs@mojatatu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rizzo@iet.unipi.it \
--cc=shemminger@vyatta.com \
/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).