| Message ID | 150280422406.717624.15125218636935056461.stgit@buzz |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2017-08-15 at 16:37 +0300, Konstantin Khlebnikov wrote: > When sfq_enqueue() drops head packet or packet from another queue it > have to update backlog at upper qdiscs too. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too") > --- > net/sched/sch_sfq.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c > index f80ea2cc5f1f..82469ef9655e 100644 > --- a/net/sched/sch_sfq.c > +++ b/net/sched/sch_sfq.c > @@ -437,6 +437,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) > qdisc_drop(head, sch, to_free); > > slot_queue_add(slot, skb); > + qdisc_tree_reduce_backlog(sch, 0, delta); > return NET_XMIT_CN; > } > > @@ -468,8 +469,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) > /* Return Congestion Notification only if we dropped a packet > * from this flow. > */ > - if (qlen != slot->qlen) > + if (qlen != slot->qlen) { > + qdisc_tree_reduce_backlog(sch, 0, dropped - qdisc_pkt_len(skb)); > return NET_XMIT_CN; > + } > > /* As we dropped a packet, better let upper stack know this */ > qdisc_tree_reduce_backlog(sch, 1, dropped); > Are you sure you have tested this patch ?
On 15.08.2017 17:09, Eric Dumazet wrote: > On Tue, 2017-08-15 at 16:37 +0300, Konstantin Khlebnikov wrote: >> When sfq_enqueue() drops head packet or packet from another queue it >> have to update backlog at upper qdiscs too. >> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >> Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too") >> --- >> net/sched/sch_sfq.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c >> index f80ea2cc5f1f..82469ef9655e 100644 >> --- a/net/sched/sch_sfq.c >> +++ b/net/sched/sch_sfq.c >> @@ -437,6 +437,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) >> qdisc_drop(head, sch, to_free); >> >> slot_queue_add(slot, skb); >> + qdisc_tree_reduce_backlog(sch, 0, delta); >> return NET_XMIT_CN; >> } >> >> @@ -468,8 +469,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) >> /* Return Congestion Notification only if we dropped a packet >> * from this flow. >> */ >> - if (qlen != slot->qlen) >> + if (qlen != slot->qlen) { >> + qdisc_tree_reduce_backlog(sch, 0, dropped - qdisc_pkt_len(skb)); >> return NET_XMIT_CN; >> + } >> >> /* As we dropped a packet, better let upper stack know this */ >> qdisc_tree_reduce_backlog(sch, 1, dropped); >> > > Are you sure you have tested this patch ? > Nope. I'm not sure. But we have something similar in our 4.4 kernel for a while. Also fq_codel and pfifo_head_drop do something similar tho this. Probably this might crash without "[PATCH 1/2] net_sched: call qlen_notify only if child qdisc is empty". I hadn't tested them separately.
On Tue, 2017-08-15 at 17:33 +0300, Konstantin Khlebnikov wrote: > Nope. I'm not sure. But we have something similar in our 4.4 kernel > for a while. > > Also fq_codel and pfifo_head_drop do something similar tho this. > > Probably this might crash without "[PATCH 1/2] net_sched: call > qlen_notify only if child qdisc is empty". > I hadn't tested them separately. Thanks for the info. I've read this patch and it looks fine indeed :) Acked-by: Eric Dumazet <edumazet@google.com>
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Date: Tue, 15 Aug 2017 16:37:04 +0300 > When sfq_enqueue() drops head packet or packet from another queue it > have to update backlog at upper qdiscs too. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too") Applied and queued up for -stable.
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index f80ea2cc5f1f..82469ef9655e 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -437,6 +437,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) qdisc_drop(head, sch, to_free); slot_queue_add(slot, skb); + qdisc_tree_reduce_backlog(sch, 0, delta); return NET_XMIT_CN; } @@ -468,8 +469,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free) /* Return Congestion Notification only if we dropped a packet * from this flow. */ - if (qlen != slot->qlen) + if (qlen != slot->qlen) { + qdisc_tree_reduce_backlog(sch, 0, dropped - qdisc_pkt_len(skb)); return NET_XMIT_CN; + } /* As we dropped a packet, better let upper stack know this */ qdisc_tree_reduce_backlog(sch, 1, dropped);
When sfq_enqueue() drops head packet or packet from another queue it have to update backlog at upper qdiscs too. Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too") --- net/sched/sch_sfq.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)