Re: [Patch net] fq_codel: explicitly reset flows in -reset()

2015-08-02 Thread David Miller
From: Cong Wang xiyou.wangc...@gmail.com
Date: Fri, 31 Jul 2015 17:53:39 -0700

 From: Eric Dumazet eric.duma...@gmail.com
 
 Alex reported the following crash when using fq_codel
 with htb:
 ...
 As Jamal pointed out, there is actually no need to call dequeue
 to purge the queued skb's in reset, data structures can be just
 reset explicitly. Therefore, we reset everything except config's
 and stats, so that we would have a fresh start after device flipping.
 
 Fixes: 4b549a2ef4be (fq_codel: Fair Queue Codel AQM)
 Reported-by: Alex Gartrell agartr...@fb.com
 Cc: Alex Gartrell agartr...@fb.com
 Cc: Jamal Hadi Salim j...@mojatatu.com
 Signed-off-by: Eric Dumazet eric.duma...@gmail.com
 [xiyou.wangc...@gmail.com: added codel_vars_init() and 
 qdisc_qstats_backlog_dec()]
 Signed-off-by: Cong Wang xiyou.wangc...@gmail.com

Applied, thanks.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Patch net] fq_codel: explicitly reset flows in -reset()

2015-07-31 Thread Cong Wang
From: Eric Dumazet eric.duma...@gmail.com

Alex reported the following crash when using fq_codel
with htb:

  crash bt
  PID: 630839  TASK: 8823c990d280  CPU: 14  COMMAND: tc
   [... snip ...]
   #8 [8820ceec17a0] page_fault at 8160a8c2
  [exception RIP: htb_qlen_notify+24]
  RIP: a0841718  RSP: 8820ceec1858  RFLAGS: 00010282
  RAX:   RBX:   RCX: 88241747b400
  RDX: 88241747b408  RSI:   RDI: 8811fb27d000
  RBP: 8820ceec1868   R8: 88120cdeff24   R9: 88120cdeff30
  R10: 0bd4  R11: a0840919  R12: a0843340
  R13:   R14: 0001  R15: 8808dae5c2e8
  ORIG_RAX:   CS: 0010  SS: 0018
   #9 [...] qdisc_tree_decrease_qlen at 81565375
  #10 [...] fq_codel_dequeue at a084e0a0 [sch_fq_codel]
  #11 [...] fq_codel_reset at a084e2f8 [sch_fq_codel]
  #12 [...] qdisc_destroy at 81560d2d
  #13 [...] htb_destroy_class at a08408f8 [sch_htb]
  #14 [...] htb_put at a084095c [sch_htb]
  #15 [...] tc_ctl_tclass at 815645a3
  #16 [...] rtnetlink_rcv_msg at 81552cb0
  [... snip ...]

As Jamal pointed out, there is actually no need to call dequeue
to purge the queued skb's in reset, data structures can be just
reset explicitly. Therefore, we reset everything except config's
and stats, so that we would have a fresh start after device flipping.

Fixes: 4b549a2ef4be (fq_codel: Fair Queue Codel AQM)
Reported-by: Alex Gartrell agartr...@fb.com
Cc: Alex Gartrell agartr...@fb.com
Cc: Jamal Hadi Salim j...@mojatatu.com
Signed-off-by: Eric Dumazet eric.duma...@gmail.com
[xiyou.wangc...@gmail.com: added codel_vars_init() and 
qdisc_qstats_backlog_dec()]
Signed-off-by: Cong Wang xiyou.wangc...@gmail.com
---
 net/sched/sch_fq_codel.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 21ca33c..a9ba030 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -288,10 +288,26 @@ static struct sk_buff *fq_codel_dequeue(struct Qdisc *sch)
 
 static void fq_codel_reset(struct Qdisc *sch)
 {
-   struct sk_buff *skb;
+   struct fq_codel_sched_data *q = qdisc_priv(sch);
+   int i;
 
-   while ((skb = fq_codel_dequeue(sch)) != NULL)
-   kfree_skb(skb);
+   INIT_LIST_HEAD(q-new_flows);
+   INIT_LIST_HEAD(q-old_flows);
+   for (i = 0; i  q-flows_cnt; i++) {
+   struct fq_codel_flow *flow = q-flows + i;
+
+   while (flow-head) {
+   struct sk_buff *skb = dequeue_head(flow);
+
+   qdisc_qstats_backlog_dec(sch, skb);
+   kfree_skb(skb);
+   }
+
+   INIT_LIST_HEAD(flow-flowchain);
+   codel_vars_init(flow-cvars);
+   }
+   memset(q-backlogs, 0, q-flows_cnt * sizeof(u32));
+   sch-q.qlen = 0;
 }
 
 static const struct nla_policy fq_codel_policy[TCA_FQ_CODEL_MAX + 1] = {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html