netdev - Re: [E1000-devel] i40e: crash on NMI by continuous module reload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Fri, 27 Feb 2015 15:16:09 +0100
From:	Stefan Assmann <sassmann@...nic.de>
To:	nick <xerofoify@...il.com>, netdev <netdev@...r.kernel.org>
CC:	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>
Subject: Re: [E1000-devel] i40e: crash on NMI by continuous module reload

On 27.02.2015 15:02, nick wrote:

[...]

>>     i40e: Fix a bug where Rx would stop after some time
>> [...]
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index f7464e8..ff6d94d 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> [...]
>> @@ -9169,6 +9178,13 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	if (err)
>>  		dev_info(&pf->pdev->dev, "set phy mask fail, aq_err %d\n", err);
>>
>> +	msleep(75);
>> +	err = i40e_aq_set_link_restart_an(&pf->hw, true, NULL);
>> +	if (err) {
>> +		dev_info(&pf->pdev->dev, "link restart failed, aq_err=%d\n",
>> +			 pf->hw.aq.asq_last_status);
>> +	}
>> +
>>  	/* The main driver is (mostly) up and happy. We need to set this state
>>  	 * before setting up the misc vector or we get a race and the vector
>>  	 * ends up disabled forever.
>>
>> With this hunk removed the driver successfully unloaded/reloaded a
>> couple of hundred times. Would it be safe to just remove this hunk?
>> I haven't seen any negative effects by removing this yet.
>>
>>   Stefan
>>
> Stefan,
> I wouldn't remove them yet as this does look like a valid idea to check to see if the link is 
> restarting successfully. On the other hand can you try removing the msleep line as this one is
> most likely causing the issue due to sleeping for some long in a probe function is generally a
> bad idea.
> Thanks,
> Nick

Thanks Nick for the quick reply. I tested removing the msleep but that
didn't make a difference. You actually need to remove the complete hunk
to get a stable driver reload.

  Stefan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html