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

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

[Konstantin Khlebnikov]

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

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

[Eric Dumazet]

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 ?

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

[Konstantin Khlebnikov]

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.

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

[Eric Dumazet]

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>

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

[David Miller]

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.