* [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
@ 2018-06-15 10:27 Konstantin Khlebnikov
2018-06-15 13:13 ` Eric Dumazet
2018-06-16 23:42 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Konstantin Khlebnikov @ 2018-06-15 10:27 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: Cong Wang, Jiri Pirko, Jamal Hadi Salim
When blackhole is used on top of classful qdisc like hfsc it breaks
qlen and backlog counters because packets are disappear without notice.
In HFSC non-zero qlen while all classes are inactive triggers warning:
WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90 [sch_hfsc]
and schedules watchdog work endlessly.
This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
this flag tells upper layer: this packet is gone and isn't queued.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
net/sched/sch_blackhole.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
index c98a61e980ba..9c4c2bb547d7 100644
--- a/net/sched/sch_blackhole.c
+++ b/net/sched/sch_blackhole.c
@@ -21,7 +21,7 @@ static int blackhole_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
qdisc_drop(skb, sch, to_free);
- return NET_XMIT_SUCCESS;
+ return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
}
static struct sk_buff *blackhole_dequeue(struct Qdisc *sch)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
2018-06-15 10:27 [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets Konstantin Khlebnikov
@ 2018-06-15 13:13 ` Eric Dumazet
2018-06-15 13:21 ` Konstantin Khlebnikov
2018-06-16 23:42 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2018-06-15 13:13 UTC (permalink / raw)
To: Konstantin Khlebnikov, netdev, David S. Miller
Cc: Cong Wang, Jiri Pirko, Jamal Hadi Salim
On 06/15/2018 03:27 AM, Konstantin Khlebnikov wrote:
> When blackhole is used on top of classful qdisc like hfsc it breaks
> qlen and backlog counters because packets are disappear without notice.
>
> In HFSC non-zero qlen while all classes are inactive triggers warning:
> WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90 [sch_hfsc]
> and schedules watchdog work endlessly.
>
> This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
> this flag tells upper layer: this packet is gone and isn't queued.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
> net/sched/sch_blackhole.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
> index c98a61e980ba..9c4c2bb547d7 100644
> --- a/net/sched/sch_blackhole.c
> +++ b/net/sched/sch_blackhole.c
> @@ -21,7 +21,7 @@ static int blackhole_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> struct sk_buff **to_free)
> {
> qdisc_drop(skb, sch, to_free);
> - return NET_XMIT_SUCCESS;
> + return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
Why do not we use instead :
return qdisc_drop(skb, sch, to_free);
Although noop_enqueue() seems to use :
return NET_XMIT_CN;
Oh well.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
2018-06-15 13:13 ` Eric Dumazet
@ 2018-06-15 13:21 ` Konstantin Khlebnikov
2018-06-16 4:00 ` Cong Wang
0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Khlebnikov @ 2018-06-15 13:21 UTC (permalink / raw)
To: Eric Dumazet, netdev, David S. Miller
Cc: Cong Wang, Jiri Pirko, Jamal Hadi Salim
On 15.06.2018 16:13, Eric Dumazet wrote:
>
>
> On 06/15/2018 03:27 AM, Konstantin Khlebnikov wrote:
>> When blackhole is used on top of classful qdisc like hfsc it breaks
>> qlen and backlog counters because packets are disappear without notice.
>>
>> In HFSC non-zero qlen while all classes are inactive triggers warning:
>> WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90 [sch_hfsc]
>> and schedules watchdog work endlessly.
>>
>> This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
>> this flag tells upper layer: this packet is gone and isn't queued.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> ---
>> net/sched/sch_blackhole.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
>> index c98a61e980ba..9c4c2bb547d7 100644
>> --- a/net/sched/sch_blackhole.c
>> +++ b/net/sched/sch_blackhole.c
>> @@ -21,7 +21,7 @@ static int blackhole_enqueue(struct sk_buff *skb, struct Qdisc *sch,
>> struct sk_buff **to_free)
>> {
>> qdisc_drop(skb, sch, to_free);
>> - return NET_XMIT_SUCCESS;
>> + return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>
> Why do not we use instead :
>
> return qdisc_drop(skb, sch, to_free);
>
> Although noop_enqueue() seems to use :
>
> return NET_XMIT_CN;
>
> Oh well.
>
>
I suppose "blackhole" should work like "successful" xmit, but counted as drop.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
2018-06-15 13:21 ` Konstantin Khlebnikov
@ 2018-06-16 4:00 ` Cong Wang
2018-06-16 4:03 ` Cong Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2018-06-16 4:00 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Eric Dumazet, Linux Kernel Network Developers, David S. Miller,
Jiri Pirko, Jamal Hadi Salim
On Fri, Jun 15, 2018 at 6:21 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> On 15.06.2018 16:13, Eric Dumazet wrote:
>>
>>
>>
>> On 06/15/2018 03:27 AM, Konstantin Khlebnikov wrote:
>>>
>>> When blackhole is used on top of classful qdisc like hfsc it breaks
>>> qlen and backlog counters because packets are disappear without notice.
>>>
>>> In HFSC non-zero qlen while all classes are inactive triggers warning:
>>> WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90
>>> [sch_hfsc]
>>> and schedules watchdog work endlessly.
>>>
>>> This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
>>> this flag tells upper layer: this packet is gone and isn't queued.
>>>
>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>> ---
>>> net/sched/sch_blackhole.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
>>> index c98a61e980ba..9c4c2bb547d7 100644
>>> --- a/net/sched/sch_blackhole.c
>>> +++ b/net/sched/sch_blackhole.c
>>> @@ -21,7 +21,7 @@ static int blackhole_enqueue(struct sk_buff *skb,
>>> struct Qdisc *sch,
>>> struct sk_buff **to_free)
>>> {
>>> qdisc_drop(skb, sch, to_free);
>>> - return NET_XMIT_SUCCESS;
>>> + return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>>
>>
>> Why do not we use instead :
>>
>> return qdisc_drop(skb, sch, to_free);
>>
>> Although noop_enqueue() seems to use :
>>
>> return NET_XMIT_CN;
>>
>> Oh well.
>>
>>
>
> I suppose "blackhole" should work like "successful" xmit, but counted as
> drop.
But anything !NET_XMIT_SUCCESS is basically same for upper
layer:
err = qdisc_enqueue(skb, cl->qdisc, to_free);
if (unlikely(err != NET_XMIT_SUCCESS)) {
if (net_xmit_drop_count(err)) {
cl->qstats.drops++;
qdisc_qstats_drop(sch);
}
return err;
}
So using NET_XMIT_DROP is same in this case?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
2018-06-16 4:00 ` Cong Wang
@ 2018-06-16 4:03 ` Cong Wang
0 siblings, 0 replies; 6+ messages in thread
From: Cong Wang @ 2018-06-16 4:03 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Eric Dumazet, Linux Kernel Network Developers, David S. Miller,
Jiri Pirko, Jamal Hadi Salim
On Fri, Jun 15, 2018 at 9:00 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Jun 15, 2018 at 6:21 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>> On 15.06.2018 16:13, Eric Dumazet wrote:
>>>
>>>
>>>
>>> On 06/15/2018 03:27 AM, Konstantin Khlebnikov wrote:
>>>>
>>>> When blackhole is used on top of classful qdisc like hfsc it breaks
>>>> qlen and backlog counters because packets are disappear without notice.
>>>>
>>>> In HFSC non-zero qlen while all classes are inactive triggers warning:
>>>> WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90
>>>> [sch_hfsc]
>>>> and schedules watchdog work endlessly.
>>>>
>>>> This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
>>>> this flag tells upper layer: this packet is gone and isn't queued.
>>>>
>>>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>>>> ---
>>>> net/sched/sch_blackhole.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/sched/sch_blackhole.c b/net/sched/sch_blackhole.c
>>>> index c98a61e980ba..9c4c2bb547d7 100644
>>>> --- a/net/sched/sch_blackhole.c
>>>> +++ b/net/sched/sch_blackhole.c
>>>> @@ -21,7 +21,7 @@ static int blackhole_enqueue(struct sk_buff *skb,
>>>> struct Qdisc *sch,
>>>> struct sk_buff **to_free)
>>>> {
>>>> qdisc_drop(skb, sch, to_free);
>>>> - return NET_XMIT_SUCCESS;
>>>> + return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
>>>
>>>
>>> Why do not we use instead :
>>>
>>> return qdisc_drop(skb, sch, to_free);
>>>
>>> Although noop_enqueue() seems to use :
>>>
>>> return NET_XMIT_CN;
>>>
>>> Oh well.
>>>
>>>
>>
>> I suppose "blackhole" should work like "successful" xmit, but counted as
>> drop.
>
> But anything !NET_XMIT_SUCCESS is basically same for upper
> layer:
>
> err = qdisc_enqueue(skb, cl->qdisc, to_free);
> if (unlikely(err != NET_XMIT_SUCCESS)) {
> if (net_xmit_drop_count(err)) {
> cl->qstats.drops++;
> qdisc_qstats_drop(sch);
> }
> return err;
> }
>
> So using NET_XMIT_DROP is same in this case?
Not same for __dev_xmit_skb()... Yeah, we should keep
the NET_XMIT_SUCCESS bit.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets
2018-06-15 10:27 [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets Konstantin Khlebnikov
2018-06-15 13:13 ` Eric Dumazet
@ 2018-06-16 23:42 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2018-06-16 23:42 UTC (permalink / raw)
To: khlebnikov; +Cc: netdev, xiyou.wangcong, jiri, jhs
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Fri, 15 Jun 2018 13:27:31 +0300
> When blackhole is used on top of classful qdisc like hfsc it breaks
> qlen and backlog counters because packets are disappear without notice.
>
> In HFSC non-zero qlen while all classes are inactive triggers warning:
> WARNING: ... at net/sched/sch_hfsc.c:1393 hfsc_dequeue+0xba4/0xe90 [sch_hfsc]
> and schedules watchdog work endlessly.
>
> This patch return __NET_XMIT_BYPASS in addition to NET_XMIT_SUCCESS,
> this flag tells upper layer: this packet is gone and isn't queued.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-16 23:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15 10:27 [PATCH] net_sched: blackhole: tell upper qdisc about dropped packets Konstantin Khlebnikov
2018-06-15 13:13 ` Eric Dumazet
2018-06-15 13:21 ` Konstantin Khlebnikov
2018-06-16 4:00 ` Cong Wang
2018-06-16 4:03 ` Cong Wang
2018-06-16 23:42 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.