[PATCH] net_sched/sfq: update hierarchical backlog when drop packet

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 <redacted>
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);

On Tue, 2017-08-15 at 16:37 +0300, Konstantin Khlebnikov wrote:

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:

quoted

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 <redacted>
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 <redacted>

From: Konstantin Khlebnikov <redacted>
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 <redacted>
Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too")

Applied and queued up for -stable.