From: Jacob Keller <jacob.e.keller@intel.com>
To: <intel-wired-lan@osuosl.org>
Subject: Re: [Intel-wired-lan] [net, v1] ice: Fix l2-fwd-offload toggle crash
Date: Mon, 17 Oct 2022 14:14:17 -0700 [thread overview]
Message-ID: <893a7b8b-3564-8515-1e05-fb6da203ed56@intel.com> (raw)
In-Reply-To: <68ac50c8-77cc-3db1-626a-80ebbad2dd4a@intel.com>
On 10/13/2022 9:51 AM, Benjamin Mikailenko wrote:
>
>
> On 10/13/2022 8:25 AM, Michal Schmidt wrote:
>> On Wed, Oct 12, 2022 at 6:03 PM Benjamin Mikailenko <benjamin.mikailenko@intel.com <mailto:benjamin.mikailenko@intel.com>> wrote:
>>
>> Running netperf traffic and toggling l2-fwd-offload in quick succession
>> caused the driver to crash.
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000020
>> [ 861.517803] #PF: supervisor read access in kernel mode
>> [ 861.517805] #PF: error_code(0x0000) - not-present page
>> [ 861.517808] PGD 0 P4D 0
>> [ 861.517811] Oops: 0000 [#1] PREEMPT SMP PTI
>> [ 861.517815] CPU: 60 PID: 16471 Comm: netperf Kdump: loaded Tainted: G S
>> [ 861.517818] Hardware name: Intel Corporation S2600WTT/S2600WTT, BIOS SE
>> [ 861.517820] RIP: 0010:ice_start_xmit+0xb0/0x1420 [ice]
>>
>> This crash would happen because during l2-fwd-offload configuration,
>> ice_init_macvlan or ice_deinit_macvlan would temporarily work on Tx rings.
>>
>>
>> What are ice_init_macvlan and ice_deinit_macvlan? Are they function names? I do not see such functions in the code.
>>
>
> Yes, but l2-fwd-offload for ICE is fixed to "off" in the kernel. So no need for those functions.
> This patch should be rejected.
>
>>
>> At the same time, ice_start_xmit would attempt to select the correct send
>> buffer from Tx rings but reach a NULL pointer.
>>
>> Fix this by checking if ring exists before proceeding xmit. If ring does
>> not exist, return NETDEV_TX_BUSY.
>>
>>
>> Isn't it still racy though?
>> Shouldn't rather whatever is fiddling with the rings make sure the Tx queues are stopped first with netif_tx_stop_queue or similar?
>> Michal
>>
>
> You're right here as well. I'll have a look into stopping queues.
>
> Thanks Michal, really appreciate the review!
> Ben
>
Ok, I'll drop this from queue. What about this reported bug? Is it
something that can't happen on the in-kernel driver?
Thanks,
Jake
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
next prev parent reply other threads:[~2022-10-17 21:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-12 15:55 [Intel-wired-lan] [net,v1] ice: Fix l2-fwd-offload toggle crash Benjamin Mikailenko
2022-10-13 15:25 ` [Intel-wired-lan] [net, v1] " Michal Schmidt
2022-10-13 16:51 ` Benjamin Mikailenko
2022-10-17 21:14 ` Jacob Keller [this message]
2022-10-17 21:13 ` Jacob Keller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=893a7b8b-3564-8515-1e05-fb6da203ed56@intel.com \
--to=jacob.e.keller@intel.com \
--cc=intel-wired-lan@osuosl.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox