linux-kernel - Re: [GIT]: Networking

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A4CDCA4.3040505@nvidia.com>
Date:	Thu, 02 Jul 2009 12:13:24 -0400
From:	Ayaz Abdulla <aabdulla@...dia.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Ingo Molnar <mingo@...e.hu>, David Miller <davem@...emloft.net>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [GIT]: Networking



Eric Dumazet wrote:
> Eric Dumazet a écrit :
> 
>>Ingo Molnar a écrit :
>>
>>>>The following changes since commit 52989765629e7d182b4f146050ebba0abf2cb0b7:
>>>>  Linus Torvalds (1):
>>>>        Merge git://git.kernel.org/.../davem/net-2.6
>>>>
>>>>are available in the git repository at:
>>>>
>>>>  master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.git master
>>>
>>>Hm, something in this lot quickly wrecked networking here - see the 
>>>tx timeout dump below. It starts with:
>>>
>>>[  351.004596] WARNING: at net/sched/sch_generic.c:246 dev_watchdog+0x10b/0x19c()
>>>[  351.011815] Hardware name: System Product Name
>>>[  351.016220] NETDEV WATCHDOG: eth0 (forcedeth): transmit queue 0 timed out
>>>
>>>Config attached. Unfortunately i've got no time to do bisection 
>>>today.
>>
>>
>>
>>forcedeth might have a problem, in its netif_wake_queue() logic, but
>>I could not see why a recent patch could make this problem visible now.
>>
>>CPU0/1: AMD Athlon(tm) 64 X2 Dual Core Processor 3800+ stepping 02
>>is not a new cpu either :)
>>
>>forcedeth uses an internal tx_stop without appropriate barrier.
>>
>>Could you try following patch ?
>>
>>(random guess as I dont have much time right now)
> 
> 
> Oh well this patch was soooo stupid, sorry Ingo.
> 
> 
> We might have a race in napi_schedule(), leaving interrupts disabled forever.
> I cannot test this patch, I dont have the hardware...
> 
> Thanks
> 
> diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
> index 1094d29..3b4e076 100644
> --- a/drivers/net/forcedeth.c
> +++ b/drivers/net/forcedeth.c
> @@ -3514,11 +3514,13 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
>  	nv_msi_workaround(np);
>  
>  #ifdef CONFIG_FORCEDETH_NAPI
> -	napi_schedule(&np->napi);
> -
> -	/* Disable furthur irq's
> -	   (msix not enabled with napi) */
> -	writel(0, base + NvRegIrqMask);
> +	if (napi_schedule_prep(&np->napi)) {
> +		/*
> +		 * Disable further irq's (msix not enabled with napi)
> +		 */
> +		writel(0, base + NvRegIrqMask);
> +		__napi_schedule(&np->napi);
> +	}

Yes, good catch. There is a race condition here with napi poll.

I would prefer to do the following to keep the code simple and clean.

writel(0, base + NvRegIrqMask);
napi_schedule(&np->napi);

.....

>  
>  #else
>  	do
> @@ -3615,12 +3617,13 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
>  	nv_msi_workaround(np);
>  
>  #ifdef CONFIG_FORCEDETH_NAPI
> -	napi_schedule(&np->napi);
> -
> -	/* Disable furthur irq's
> -	   (msix not enabled with napi) */
> -	writel(0, base + NvRegIrqMask);
> -
> +	if (napi_schedule_prep(&np->napi)) {
> +		/*
> +		 * Disable further irq's (msix not enabled with napi)
> +		 */
> +		writel(0, base + NvRegIrqMask);
> +		__napi_schedule(&np->napi);
> +	}
>  #else
>  	do
>  	{
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/