* Re: [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace
[not found] <20210129120154.316324-1-ovov@yandex-team.ru>
@ 2021-01-29 23:22 ` Cong Wang
0 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2021-01-29 23:22 UTC (permalink / raw)
To: Alexander Ovechkin
Cc: Linux Kernel Network Developers, Paolo Abeni, David Miller,
Jakub Kicinski, Jamal Hadi Salim, zeil, dmtrmonakhov
On Fri, Jan 29, 2021 at 4:02 AM Alexander Ovechkin <ovov@yandex-team.ru> wrote:
>
> Commit e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
> introduced qdisc tree flush/purge helpers, but erroneously used flush helper
> instead of purge helper in qdisc_replace function.
> This issue was found in our CI, that tests various qdisc setups by configuring
> qdisc and sending data through it. Call of invalid helper sporadically leads
> to corruption of vt_tree/cf_tree of hfsc_class that causes kernel oops:
...
> Fixes: e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
> Signed-off-by: Alexander Ovechkin <ovov@yandex-team.ru>
Looks reasonable to me:
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace
@ 2021-02-01 20:00 Alexander Ovechkin
2021-02-02 2:41 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Ovechkin @ 2021-02-01 20:00 UTC (permalink / raw)
To: netdev; +Cc: pabeni, davem, kuba, jhs, xiyou.wangcong, zeil, dmtrmonakhov
Commit e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
introduced qdisc tree flush/purge helpers, but erroneously used flush helper
instead of purge helper in qdisc_replace function.
This issue was found in our CI, that tests various qdisc setups by configuring
qdisc and sending data through it. Call of invalid helper sporadically leads
to corruption of vt_tree/cf_tree of hfsc_class that causes kernel oops:
Oops: 0000 [#1] SMP PTI
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.11.0-8f6859df #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
RIP: 0010:rb_insert_color+0x18/0x190
Code: c3 31 c0 c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 48 8b 07 48 85 c0 0f 84 05 01 00 00 48 8b 10 f6 c2 01 0f 85 34 01 00 00 <48> 8b 4a 08 49 89 d0 48 39 c1 74 7d 48 85 c9 74 32 f6 01 01 75 2d
RSP: 0018:ffffc900000b8bb0 EFLAGS: 00010246
RAX: ffff8881ef4c38b0 RBX: ffff8881d956e400 RCX: ffff8881ef4c38b0
RDX: 0000000000000000 RSI: ffff8881d956f0a8 RDI: ffff8881d956e4b0
RBP: 0000000000000000 R08: 000000d5c4e249da R09: 1600000000000000
R10: ffffc900000b8be0 R11: ffffc900000b8b28 R12: 0000000000000001
R13: 000000000000005a R14: ffff8881f0905000 R15: ffff8881f0387d00
FS: 0000000000000000(0000) GS:ffff8881f8b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 00000001f4796004 CR4: 0000000000060ee0
Call Trace:
<IRQ>
init_vf.isra.19+0xec/0x250 [sch_hfsc]
hfsc_enqueue+0x245/0x300 [sch_hfsc]
? fib_rules_lookup+0x12a/0x1d0
? __dev_queue_xmit+0x4b6/0x930
? hfsc_delete_class+0x250/0x250 [sch_hfsc]
__dev_queue_xmit+0x4b6/0x930
? ip6_finish_output2+0x24d/0x590
ip6_finish_output2+0x24d/0x590
? ip6_output+0x6c/0x130
ip6_output+0x6c/0x130
? __ip6_finish_output+0x110/0x110
mld_sendpack+0x224/0x230
mld_ifc_timer_expire+0x186/0x2c0
? igmp6_group_dropped+0x200/0x200
call_timer_fn+0x2d/0x150
run_timer_softirq+0x20c/0x480
? tick_sched_do_timer+0x60/0x60
? tick_sched_timer+0x37/0x70
__do_softirq+0xf7/0x2cb
irq_exit+0xa0/0xb0
smp_apic_timer_interrupt+0x74/0x150
apic_timer_interrupt+0xf/0x20
</IRQ>
Fixes: e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
Signed-off-by: Alexander Ovechkin <ovov@yandex-team.ru>
Reported-by: Alexander Kuznetsov <wwfq@yandex-team.ru>
Acked-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
Acked-by: Dmitry Yakunin <zeil@yandex-team.ru>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/sch_generic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 3d03756e1069..b2ceec7b280d 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -1158,7 +1158,7 @@ static inline struct Qdisc *qdisc_replace(struct Qdisc *sch, struct Qdisc *new,
old = *pold;
*pold = new;
if (old != NULL)
- qdisc_tree_flush_backlog(old);
+ qdisc_purge_queue(old);
sch_tree_unlock(sch);
return old;
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace
2021-02-01 20:00 Alexander Ovechkin
@ 2021-02-02 2:41 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2021-02-02 2:41 UTC (permalink / raw)
To: Alexander Ovechkin
Cc: netdev, pabeni, davem, jhs, xiyou.wangcong, zeil, dmtrmonakhov
On Mon, 1 Feb 2021 23:00:49 +0300 Alexander Ovechkin wrote:
> Commit e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
> introduced qdisc tree flush/purge helpers, but erroneously used flush helper
> instead of purge helper in qdisc_replace function.
> This issue was found in our CI, that tests various qdisc setups by configuring
> qdisc and sending data through it. Call of invalid helper sporadically leads
> to corruption of vt_tree/cf_tree of hfsc_class that causes kernel oops:
> Fixes: e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
> Signed-off-by: Alexander Ovechkin <ovov@yandex-team.ru>
> Reported-by: Alexander Kuznetsov <wwfq@yandex-team.ru>
> Acked-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
> Acked-by: Dmitry Yakunin <zeil@yandex-team.ru>
> Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
No need to repost just to add the ack, patchwork will pick the tags up
automatically.
Applied, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace
@ 2021-01-26 21:56 Alexander Ovechkin
2021-01-28 21:32 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Ovechkin @ 2021-01-26 21:56 UTC (permalink / raw)
To: netdev; +Cc: zeil, dmtrmonakhov
Commit e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
introduced qdisc tree flush/purge helpers, but erroneously used flush helper
instead of purge helper in qdisc_replace function.
This issue was found in our CI, that tests various qdisc setups by configuring
qdisc and sending data through it. Call of invalid helper sporadically leads
to corruption of vt_tree/cf_tree of hfsc_class that causes kernel oops:
Oops: 0000 [#1] SMP PTI
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.11.0-8f6859df #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
RIP: 0010:rb_insert_color+0x18/0x190
Code: c3 31 c0 c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 48 8b 07 48 85 c0 0f 84 05 01 00 00 48 8b 10 f6 c2 01 0f 85 34 01 00 00 <48> 8b 4a 08 49 89 d0 48 39 c1 74 7d 48 85 c9 74 32 f6 01 01 75 2d
RSP: 0018:ffffc900000b8bb0 EFLAGS: 00010246
RAX: ffff8881ef4c38b0 RBX: ffff8881d956e400 RCX: ffff8881ef4c38b0
RDX: 0000000000000000 RSI: ffff8881d956f0a8 RDI: ffff8881d956e4b0
RBP: 0000000000000000 R08: 000000d5c4e249da R09: 1600000000000000
R10: ffffc900000b8be0 R11: ffffc900000b8b28 R12: 0000000000000001
R13: 000000000000005a R14: ffff8881f0905000 R15: ffff8881f0387d00
FS: 0000000000000000(0000) GS:ffff8881f8b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 00000001f4796004 CR4: 0000000000060ee0
Call Trace:
<IRQ>
init_vf.isra.19+0xec/0x250 [sch_hfsc]
hfsc_enqueue+0x245/0x300 [sch_hfsc]
? fib_rules_lookup+0x12a/0x1d0
? __dev_queue_xmit+0x4b6/0x930
? hfsc_delete_class+0x250/0x250 [sch_hfsc]
__dev_queue_xmit+0x4b6/0x930
? ip6_finish_output2+0x24d/0x590
ip6_finish_output2+0x24d/0x590
? ip6_output+0x6c/0x130
ip6_output+0x6c/0x130
? __ip6_finish_output+0x110/0x110
mld_sendpack+0x224/0x230
mld_ifc_timer_expire+0x186/0x2c0
? igmp6_group_dropped+0x200/0x200
call_timer_fn+0x2d/0x150
run_timer_softirq+0x20c/0x480
? tick_sched_do_timer+0x60/0x60
? tick_sched_timer+0x37/0x70
__do_softirq+0xf7/0x2cb
irq_exit+0xa0/0xb0
smp_apic_timer_interrupt+0x74/0x150
apic_timer_interrupt+0xf/0x20
</IRQ>
Fixes: e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge
helpers")
Signed-off-by: Alexander Ovechkin <ovov@yandex-team.ru>
Reported-by: Alexander Kuznetsov <wwfq@yandex-team.ru>
Acked-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
Acked-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
include/net/sch_generic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 639e465a108f..5b490b5591df 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -1143,7 +1143,7 @@ static inline struct Qdisc *qdisc_replace(struct Qdisc *sch, struct Qdisc *new,
old = *pold;
*pold = new;
if (old != NULL)
- qdisc_tree_flush_backlog(old);
+ qdisc_purge_queue(old);
sch_tree_unlock(sch);
return old;
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace
2021-01-26 21:56 Alexander Ovechkin
@ 2021-01-28 21:32 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2021-01-28 21:32 UTC (permalink / raw)
To: Alexander Ovechkin; +Cc: netdev, zeil, dmtrmonakhov
On Wed, 27 Jan 2021 00:56:41 +0300 Alexander Ovechkin wrote:
> Commit e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge helpers")
> introduced qdisc tree flush/purge helpers, but erroneously used flush helper
> instead of purge helper in qdisc_replace function.
> This issue was found in our CI, that tests various qdisc setups by configuring
> qdisc and sending data through it. Call of invalid helper sporadically leads
> to corruption of vt_tree/cf_tree of hfsc_class that causes kernel oops:
The patch in question indeed does change the code, but I wonder if the
problem isn't in HFSC. Why would the caller depend on the old qdisc
being purged by qdisc_replace()?
Please add some more info on this..
> Fixes: e5f0e8f8e456 ("net: sched: introduce and use qdisc tree flush/purge
> helpers")
... fix the tag (it shouldn't be wrapped), and CC the right people
(especially the author of the patch you're pointing the tag at -
scripts/get_maintainer is your friend), and repost.
> Signed-off-by: Alexander Ovechkin <ovov@yandex-team.ru>
> Reported-by: Alexander Kuznetsov <wwfq@yandex-team.ru>
> Acked-by: Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>
> Acked-by: Dmitry Yakunin <zeil@yandex-team.ru>
> ---
> include/net/sch_generic.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 639e465a108f..5b490b5591df 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -1143,7 +1143,7 @@ static inline struct Qdisc *qdisc_replace(struct Qdisc *sch, struct Qdisc *new,
> old = *pold;
> *pold = new;
> if (old != NULL)
> - qdisc_tree_flush_backlog(old);
> + qdisc_purge_queue(old);
> sch_tree_unlock(sch);
>
> return old;
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-02 2:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210129120154.316324-1-ovov@yandex-team.ru>
2021-01-29 23:22 ` [PATCH net] net: sched: replaced invalid qdisc tree flush helper in qdisc_replace Cong Wang
2021-02-01 20:00 Alexander Ovechkin
2021-02-02 2:41 ` Jakub Kicinski
-- strict thread matches above, loose matches on Subject: below --
2021-01-26 21:56 Alexander Ovechkin
2021-01-28 21:32 ` Jakub Kicinski
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.