| b.liu | e958203 | 2025-04-17 19:18:16 +0800 | [diff] [blame^] | 1 | From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> |
| 2 | Date: Mon, 21 Aug 2017 11:14:14 +0300 |
| 3 | Subject: [PATCH] net_sched/codel: do not defer queue length update |
| 4 | |
| 5 | When codel wants to drop last packet in ->dequeue() it cannot call |
| 6 | qdisc_tree_reduce_backlog() right away - it will notify parent qdisc |
| 7 | about zero qlen and HTB/HFSC will deactivate class. The same class will |
| 8 | be deactivated second time by caller of ->dequeue(). Currently codel and |
| 9 | fq_codel defer update. This triggers warning in HFSC when it's qlen != 0 |
| 10 | but there is no active classes. |
| 11 | |
| 12 | This patch update parent queue length immediately: just temporary increase |
| 13 | qlen around qdisc_tree_reduce_backlog() to prevent first class deactivation |
| 14 | if we have skb to return. |
| 15 | |
| 16 | This might open another problem in HFSC - now operation peek could fail and |
| 17 | deactivate parent class. |
| 18 | |
| 19 | Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> |
| 20 | Link: https://bugzilla.kernel.org/show_bug.cgi?id=109581 |
| 21 | --- |
| 22 | |
| 23 | --- a/net/sched/sch_codel.c |
| 24 | +++ b/net/sched/sch_codel.c |
| 25 | @@ -95,11 +95,17 @@ static struct sk_buff *codel_qdisc_deque |
| 26 | &q->stats, qdisc_pkt_len, codel_get_enqueue_time, |
| 27 | drop_func, dequeue_func); |
| 28 | |
| 29 | - /* We cant call qdisc_tree_reduce_backlog() if our qlen is 0, |
| 30 | - * or HTB crashes. Defer it for next round. |
| 31 | + /* If our qlen is 0 qdisc_tree_reduce_backlog() will deactivate |
| 32 | + * parent class, dequeue in parent qdisc will do the same if we |
| 33 | + * return skb. Temporary increment qlen if we have skb. |
| 34 | */ |
| 35 | - if (q->stats.drop_count && sch->q.qlen) { |
| 36 | - qdisc_tree_reduce_backlog(sch, q->stats.drop_count, q->stats.drop_len); |
| 37 | + if (q->stats.drop_count) { |
| 38 | + if (skb) |
| 39 | + sch->q.qlen++; |
| 40 | + qdisc_tree_reduce_backlog(sch, q->stats.drop_count, |
| 41 | + q->stats.drop_len); |
| 42 | + if (skb) |
| 43 | + sch->q.qlen--; |
| 44 | q->stats.drop_count = 0; |
| 45 | q->stats.drop_len = 0; |
| 46 | } |
| 47 | --- a/net/sched/sch_fq_codel.c |
| 48 | +++ b/net/sched/sch_fq_codel.c |
| 49 | @@ -305,6 +305,21 @@ begin: |
| 50 | &flow->cvars, &q->cstats, qdisc_pkt_len, |
| 51 | codel_get_enqueue_time, drop_func, dequeue_func); |
| 52 | |
| 53 | + /* If our qlen is 0 qdisc_tree_reduce_backlog() will deactivate |
| 54 | + * parent class, dequeue in parent qdisc will do the same if we |
| 55 | + * return skb. Temporary increment qlen if we have skb. |
| 56 | + */ |
| 57 | + if (q->cstats.drop_count) { |
| 58 | + if (skb) |
| 59 | + sch->q.qlen++; |
| 60 | + qdisc_tree_reduce_backlog(sch, q->cstats.drop_count, |
| 61 | + q->cstats.drop_len); |
| 62 | + if (skb) |
| 63 | + sch->q.qlen--; |
| 64 | + q->cstats.drop_count = 0; |
| 65 | + q->cstats.drop_len = 0; |
| 66 | + } |
| 67 | + |
| 68 | if (!skb) { |
| 69 | /* force a pass through old_flows to prevent starvation */ |
| 70 | if ((head == &q->new_flows) && !list_empty(&q->old_flows)) |
| 71 | @@ -315,15 +330,6 @@ begin: |
| 72 | } |
| 73 | qdisc_bstats_update(sch, skb); |
| 74 | flow->deficit -= qdisc_pkt_len(skb); |
| 75 | - /* We cant call qdisc_tree_reduce_backlog() if our qlen is 0, |
| 76 | - * or HTB crashes. Defer it for next round. |
| 77 | - */ |
| 78 | - if (q->cstats.drop_count && sch->q.qlen) { |
| 79 | - qdisc_tree_reduce_backlog(sch, q->cstats.drop_count, |
| 80 | - q->cstats.drop_len); |
| 81 | - q->cstats.drop_count = 0; |
| 82 | - q->cstats.drop_len = 0; |
| 83 | - } |
| 84 | return skb; |
| 85 | } |
| 86 | |