* [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.