* mlx5 bug in error path of mlx5e_open_channel()
@ 2016-11-01 14:44 Jesper Dangaard Brouer
2016-11-01 15:48 ` Saeed Mahameed
0 siblings, 1 reply; 3+ messages in thread
From: Jesper Dangaard Brouer @ 2016-11-01 14:44 UTC (permalink / raw)
To: Saeed Mahameed, Tariq Toukan, Tariq Toukan, Eran Ben Elisha
Cc: brouer, netdev@vger.kernel.org
In driver mlx5 function mlx5e_open_channel() does not handle error
path correctly. (Tested by letting mlx5e_create_rq fail with -ENOMEM,
propagates to mlx5e_open_rq)
This first seemed related to commit b5503b994ed5 ("net/mlx5e: XDP TX
forwarding support"). As a failed call of mlx5e_open_rq() always
calls mlx5e_close_sq(&c->xdp_sq) on "xdp_sq" even-though a XDP program
is not attached.
Fixing this like:
@@ -1504,24 +1533,38 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
c->xdp = !!priv->xdp_prog;
err = mlx5e_open_rq(c, &cparam->rq, &c->rq);
- if (err)
- goto err_close_xdp_sq;
+ if (err) {
+ if (c->xdp)
+ goto err_close_xdp_sq;
+ else
+ goto err_close_sqs;
+ }
The fix does remove one problem, but the error path still cause the
kernel to crash. This time it seems related to correct disabling of
NAPI polling before disabling the queues.
Now with another error:
XXX: call mlx5e_close_sqs(c)
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [< (null)>] (null)
PGD 401e00067
PUD 40746e067 PMD 0
Oops: 0010 [#1] PREEMPT SMP
Modules linked in: mlx5_core coretemp kvm_intel kvm irqbypass intel_cstate mxm_wmi i2c_i801 i2c_smbus]
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.0-rc3-page_pool04+ #124
Hardware name: To Be Filled By O.E.M./Z97 Extreme4, BIOS P2.10 05/12/2015
task: ffffffff81c0c4c0 task.stack: ffffffff81c00000
RIP: 0010:[<0000000000000000>] [< (null)>] (null)
RSP: 0018:ffff88041fa03e70 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff880401ecc000 RCX: 0000000000000005
RDX: 0000000000000000 RSI: ffff880401c38000 RDI: ffff880401ecc000
RBP: ffff88041fa03e88 R08: 0000000000000001 R09: ffff8803ea6a7230
R10: 0000000000000000 R11: 0000000000000040 R12: ffff880401c38000
R13: ffff880401ecf148 R14: 0000000000000040 R15: ffff880401ecc000
FS: 0000000000000000(0000) GS:ffff88041fa00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000040c468000 CR4: 00000000001406f0
Stack:
ffffffffa02e62e0 0000000000000000 0000000000000001 ffff88041fa03ed0
ffffffffa02e84c2 0000ffff00000000 ffffffff00000040 ffff880401ecf148
0000000000000040 0000000000000000 000000000000012c 0000000000000000
Call Trace:
<IRQ> [ 428.032595] [<ffffffffa02e62e0>] ? mlx5e_post_rx_wqes+0x80/0xc0 [mlx5_core]
[<ffffffffa02e84c2>] mlx5e_napi_poll+0xf2/0x530 [mlx5_core]
[<ffffffff8160e50c>] net_rx_action+0x1fc/0x350
[<ffffffff8172aff8>] __do_softirq+0xc8/0x2c6
[<ffffffff8106728e>] irq_exit+0xbe/0xd0
[<ffffffff8172ad44>] do_IRQ+0x54/0xd0
[<ffffffff8172937f>] common_interrupt+0x7f/0x7f
<EOI> [ 428.075157] [<ffffffff817285d0>] ? _raw_spin_unlock_irq+0x10/0x20
[<ffffffff81086db8>] ? finish_task_switch+0x78/0x200
[<ffffffff81722dfa>] __schedule+0x17a/0x670
[<ffffffff8172332d>] schedule+0x3d/0x90
[<ffffffff817236a5>] schedule_preempt_disabled+0x15/0x20
[<ffffffff810a560c>] cpu_startup_entry+0x12c/0x1c0
[<ffffffff8171c274>] rest_init+0x84/0x90
[<ffffffff81d95f14>] start_kernel+0x3fe/0x40b
[<ffffffff81d9528f>] x86_64_start_reservations+0x2a/0x2c
[<ffffffff81d953f9>] x86_64_start_kernel+0x168/0x176
Code: Bad RIP value.
RIP [< (null)>] (null)
RSP <ffff88041fa03e70>
CR2: 0000000000000000
---[ end trace a871278f0d0523ac ]---
Could you please look at fixing your driver?
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: mlx5 bug in error path of mlx5e_open_channel()
2016-11-01 14:44 mlx5 bug in error path of mlx5e_open_channel() Jesper Dangaard Brouer
@ 2016-11-01 15:48 ` Saeed Mahameed
2016-11-01 16:30 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 3+ messages in thread
From: Saeed Mahameed @ 2016-11-01 15:48 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan, Tariq Toukan,
Eran Ben Elisha
Cc: netdev@vger.kernel.org
On 11/01/2016 04:44 PM, Jesper Dangaard Brouer wrote:
>
> In driver mlx5 function mlx5e_open_channel() does not handle error
> path correctly. (Tested by letting mlx5e_create_rq fail with -ENOMEM,
> propagates to mlx5e_open_rq)
>
> This first seemed related to commit b5503b994ed5 ("net/mlx5e: XDP TX
> forwarding support"). As a failed call of mlx5e_open_rq() always
> calls mlx5e_close_sq(&c->xdp_sq) on "xdp_sq" even-though a XDP program
> is not attached.
>
Indeed, Nice catch.
> Fixing this like:
>
> @@ -1504,24 +1533,38 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
>
> c->xdp = !!priv->xdp_prog;
> err = mlx5e_open_rq(c, &cparam->rq, &c->rq);
> - if (err)
> - goto err_close_xdp_sq;
> + if (err) {
> + if (c->xdp)
> + goto err_close_xdp_sq;
> + else
> + goto err_close_sqs;
> + }
>
> The fix does remove one problem, but the error path still cause the
> kernel to crash. This time it seems related to correct disabling of
> NAPI polling before disabling the queues.
>
Well a more proper fix will be to add a xdp check in close_xdp_sq error flow,
rather than complicating the error handling branching decision.
i.e:
Keep:
err = mlx5e_open_rq(c, &cparam->rq, &c->rq);
if (err)
goto err_close_xdp_sq;
[...]
err_close_xdp_sq:
- mlx5e_close_sq(&c->xdp_sq);
+ if (priv->xdp_prog)
+ mlx5e_close_sq(&c->xdp_sq);
One more thing, the error flow handling is missing mlx5e_close_cq(&c->xdp_sq.cq);
which might be related to the other bug you reported below.
> Now with another error:
>
> XXX: call mlx5e_close_sqs(c)
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [< (null)>] (null)
> PGD 401e00067
> PUD 40746e067 PMD 0
> Oops: 0010 [#1] PREEMPT SMP
> Modules linked in: mlx5_core coretemp kvm_intel kvm irqbypass intel_cstate mxm_wmi i2c_i801 i2c_smbus]
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.0-rc3-page_pool04+ #124
> Hardware name: To Be Filled By O.E.M./Z97 Extreme4, BIOS P2.10 05/12/2015
> task: ffffffff81c0c4c0 task.stack: ffffffff81c00000
> RIP: 0010:[<0000000000000000>] [< (null)>] (null)
> RSP: 0018:ffff88041fa03e70 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: ffff880401ecc000 RCX: 0000000000000005
> RDX: 0000000000000000 RSI: ffff880401c38000 RDI: ffff880401ecc000
> RBP: ffff88041fa03e88 R08: 0000000000000001 R09: ffff8803ea6a7230
> R10: 0000000000000000 R11: 0000000000000040 R12: ffff880401c38000
> R13: ffff880401ecf148 R14: 0000000000000040 R15: ffff880401ecc000
> FS: 0000000000000000(0000) GS:ffff88041fa00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000040c468000 CR4: 00000000001406f0
> Stack:
> ffffffffa02e62e0 0000000000000000 0000000000000001 ffff88041fa03ed0
> ffffffffa02e84c2 0000ffff00000000 ffffffff00000040 ffff880401ecf148
> 0000000000000040 0000000000000000 000000000000012c 0000000000000000
> Call Trace:
> <IRQ> [ 428.032595] [<ffffffffa02e62e0>] ? mlx5e_post_rx_wqes+0x80/0xc0 [mlx5_core]
> [<ffffffffa02e84c2>] mlx5e_napi_poll+0xf2/0x530 [mlx5_core]
> [<ffffffff8160e50c>] net_rx_action+0x1fc/0x350
> [<ffffffff8172aff8>] __do_softirq+0xc8/0x2c6
> [<ffffffff8106728e>] irq_exit+0xbe/0xd0
> [<ffffffff8172ad44>] do_IRQ+0x54/0xd0
> [<ffffffff8172937f>] common_interrupt+0x7f/0x7f
> <EOI> [ 428.075157] [<ffffffff817285d0>] ? _raw_spin_unlock_irq+0x10/0x20
> [<ffffffff81086db8>] ? finish_task_switch+0x78/0x200
> [<ffffffff81722dfa>] __schedule+0x17a/0x670
> [<ffffffff8172332d>] schedule+0x3d/0x90
> [<ffffffff817236a5>] schedule_preempt_disabled+0x15/0x20
> [<ffffffff810a560c>] cpu_startup_entry+0x12c/0x1c0
> [<ffffffff8171c274>] rest_init+0x84/0x90
> [<ffffffff81d95f14>] start_kernel+0x3fe/0x40b
> [<ffffffff81d9528f>] x86_64_start_reservations+0x2a/0x2c
> [<ffffffff81d953f9>] x86_64_start_kernel+0x168/0x176
> Code: Bad RIP value.
> RIP [< (null)>] (null)
> RSP <ffff88041fa03e70>
> CR2: 0000000000000000
> ---[ end trace a871278f0d0523ac ]---
>
> Could you please look at fixing your driver?
>
Will handle it ASAP, Thank you Jesper.
-Saeed.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: mlx5 bug in error path of mlx5e_open_channel()
2016-11-01 15:48 ` Saeed Mahameed
@ 2016-11-01 16:30 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 3+ messages in thread
From: Jesper Dangaard Brouer @ 2016-11-01 16:30 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Tariq Toukan, Tariq Toukan, Eran Ben Elisha,
netdev@vger.kernel.org, brouer
On Tue, 1 Nov 2016 17:48:31 +0200 Saeed Mahameed <saeedm@mellanox.com> wrote:
> On 11/01/2016 04:44 PM, Jesper Dangaard Brouer wrote:
> >
> > In driver mlx5 function mlx5e_open_channel() does not handle error
> > path correctly. (Tested by letting mlx5e_create_rq fail with -ENOMEM,
> > propagates to mlx5e_open_rq)
> >
> > This first seemed related to commit b5503b994ed5 ("net/mlx5e: XDP TX
> > forwarding support"). As a failed call of mlx5e_open_rq() always
> > calls mlx5e_close_sq(&c->xdp_sq) on "xdp_sq" even-though a XDP program
> > is not attached.
> >
>
> Indeed, Nice catch.
>
> > Fixing this like:
> >
> > @@ -1504,24 +1533,38 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
> >
> > c->xdp = !!priv->xdp_prog;
> > err = mlx5e_open_rq(c, &cparam->rq, &c->rq);
> > - if (err)
> > - goto err_close_xdp_sq;
> > + if (err) {
> > + if (c->xdp)
> > + goto err_close_xdp_sq;
> > + else
> > + goto err_close_sqs;
> > + }
> >
> > The fix does remove one problem, but the error path still cause the
> > kernel to crash. This time it seems related to correct disabling of
> > NAPI polling before disabling the queues.
> >
>
> Well a more proper fix will be to add a xdp check in close_xdp_sq error flow,
> rather than complicating the error handling branching decision.
>
> i.e:
> Keep:
> err = mlx5e_open_rq(c, &cparam->rq, &c->rq);
> if (err)
> goto err_close_xdp_sq;
> [...]
> err_close_xdp_sq:
> - mlx5e_close_sq(&c->xdp_sq);
> + if (priv->xdp_prog)
> + mlx5e_close_sq(&c->xdp_sq);
Agree, that would be better.
> One more thing, the error flow handling is missing mlx5e_close_cq(&c->xdp_sq.cq);
> which might be related to the other bug you reported below.
>
> > Now with another error:
> >
> > XXX: call mlx5e_close_sqs(c)
> > BUG: unable to handle kernel NULL pointer dereference at (null)
> > IP: [< (null)>] (null)
> > PGD 401e00067
> > PUD 40746e067 PMD 0
> > Oops: 0010 [#1] PREEMPT SMP
> > Modules linked in: mlx5_core coretemp kvm_intel kvm irqbypass intel_cstate mxm_wmi i2c_i801 i2c_smbus]
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.0-rc3-page_pool04+ #124
> > Hardware name: To Be Filled By O.E.M./Z97 Extreme4, BIOS P2.10 05/12/2015
> > task: ffffffff81c0c4c0 task.stack: ffffffff81c00000
> > RIP: 0010:[<0000000000000000>] [< (null)>] (null)
> > RSP: 0018:ffff88041fa03e70 EFLAGS: 00010286
> > RAX: 0000000000000000 RBX: ffff880401ecc000 RCX: 0000000000000005
> > RDX: 0000000000000000 RSI: ffff880401c38000 RDI: ffff880401ecc000
> > RBP: ffff88041fa03e88 R08: 0000000000000001 R09: ffff8803ea6a7230
> > R10: 0000000000000000 R11: 0000000000000040 R12: ffff880401c38000
> > R13: ffff880401ecf148 R14: 0000000000000040 R15: ffff880401ecc000
> > FS: 0000000000000000(0000) GS:ffff88041fa00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000000 CR3: 000000040c468000 CR4: 00000000001406f0
> > Stack:
> > ffffffffa02e62e0 0000000000000000 0000000000000001 ffff88041fa03ed0
> > ffffffffa02e84c2 0000ffff00000000 ffffffff00000040 ffff880401ecf148
> > 0000000000000040 0000000000000000 000000000000012c 0000000000000000
> > Call Trace:
> > <IRQ> [ 428.032595] [<ffffffffa02e62e0>] ? mlx5e_post_rx_wqes+0x80/0xc0 [mlx5_core]
> > [<ffffffffa02e84c2>] mlx5e_napi_poll+0xf2/0x530 [mlx5_core]
> > [<ffffffff8160e50c>] net_rx_action+0x1fc/0x350
> > [<ffffffff8172aff8>] __do_softirq+0xc8/0x2c6
> > [<ffffffff8106728e>] irq_exit+0xbe/0xd0
> > [<ffffffff8172ad44>] do_IRQ+0x54/0xd0
> > [<ffffffff8172937f>] common_interrupt+0x7f/0x7f
> > <EOI> [ 428.075157] [<ffffffff817285d0>] ? _raw_spin_unlock_irq+0x10/0x20
> > [<ffffffff81086db8>] ? finish_task_switch+0x78/0x200
> > [<ffffffff81722dfa>] __schedule+0x17a/0x670
> > [<ffffffff8172332d>] schedule+0x3d/0x90
> > [<ffffffff817236a5>] schedule_preempt_disabled+0x15/0x20
> > [<ffffffff810a560c>] cpu_startup_entry+0x12c/0x1c0
> > [<ffffffff8171c274>] rest_init+0x84/0x90
> > [<ffffffff81d95f14>] start_kernel+0x3fe/0x40b
> > [<ffffffff81d9528f>] x86_64_start_reservations+0x2a/0x2c
> > [<ffffffff81d953f9>] x86_64_start_kernel+0x168/0x176
> > Code: Bad RIP value.
> > RIP [< (null)>] (null)
> > RSP <ffff88041fa03e70>
> > CR2: 0000000000000000
> > ---[ end trace a871278f0d0523ac ]---
> >
> > Could you please look at fixing your driver?
> >
>
> Will handle it ASAP, Thank you Jesper.
Thanks for your quick response :-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-01 21:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-01 14:44 mlx5 bug in error path of mlx5e_open_channel() Jesper Dangaard Brouer
2016-11-01 15:48 ` Saeed Mahameed
2016-11-01 16:30 ` Jesper Dangaard Brouer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).