[FIX] ARM: IXP4xx Ethernet NAPI fix

[FIX] ARM: IXP4xx Ethernet NAPI fix

[Krzysztof Halasa]

This patch removes some weirdness from IXP4xx Ethernet driver.
Ideally it should go in 2.6.26 though it's not that critical. Thanks.

Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>

diff --git a/drivers/net/arm/ixp4xx_eth.c b/drivers/net/arm/ixp4xx_eth.c
index c617b64..9b777d9 100644
--- a/drivers/net/arm/ixp4xx_eth.c
+++ b/drivers/net/arm/ixp4xx_eth.c
@@ -522,7 +522,6 @@ static int eth_poll(struct napi_struct *napi, int budget)
 #endif
 
 		if ((n = queue_get_desc(rxq, port, 0)) < 0) {
-			received = 0; /* No packet received */
 #if DEBUG_RX
 			printk(KERN_DEBUG "%s: eth_poll netif_rx_complete\n",
 			       dev->name);
@@ -543,7 +542,7 @@ static int eth_poll(struct napi_struct *napi, int budget)
 			printk(KERN_DEBUG "%s: eth_poll all done\n",
 			       dev->name);
 #endif
-			return 0; /* all work done */
+			return received; /* all work done */
 		}
 
 		desc = rx_desc_ptr(port, n);

Re: [FIX] ARM: IXP4xx Ethernet NAPI fix

[Jeff Garzik]

Krzysztof Halasa wrote:
> This patch removes some weirdness from IXP4xx Ethernet driver.
> Ideally it should go in 2.6.26 though it's not that critical. Thanks.
> 
> Signed-off-by: Krzysztof Hałasa <khc@pm.waw.pl>
> 
> diff --git a/drivers/net/arm/ixp4xx_eth.c b/drivers/net/arm/ixp4xx_eth.c
> index c617b64..9b777d9 100644
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
> @@ -522,7 +522,6 @@ static int eth_poll(struct napi_struct *napi, int budget)
>  #endif
>  
>  		if ((n = queue_get_desc(rxq, port, 0)) < 0) {
> -			received = 0; /* No packet received */
>  #if DEBUG_RX
>  			printk(KERN_DEBUG "%s: eth_poll netif_rx_complete\n",
>  			       dev->name);
> @@ -543,7 +542,7 @@ static int eth_poll(struct napi_struct *napi, int budget)
>  			printk(KERN_DEBUG "%s: eth_poll all done\n",
>  			       dev->name);
>  #endif
> -			return 0; /* all work done */
> +			return received; /* all work done */

applied

Re: [FIX] ARM: IXP4xx Ethernet NAPI fix

[Andrew Morton]

On Wed, 09 Jul 2008 13:10:32 +0200 Krzysztof Halasa <khc@pm.waw.pl> wrote:

> This patch removes some weirdness from IXP4xx Ethernet driver.
> Ideally it should go in 2.6.26 though it's not that critical. Thanks.
> 
> Signed-off-by: Krzysztof Ha__asa <khc@pm.waw.pl>
> 
> diff --git a/drivers/net/arm/ixp4xx_eth.c b/drivers/net/arm/ixp4xx_eth.c
> index c617b64..9b777d9 100644
> --- a/drivers/net/arm/ixp4xx_eth.c
> +++ b/drivers/net/arm/ixp4xx_eth.c
> @@ -522,7 +522,6 @@ static int eth_poll(struct napi_struct *napi, int budget)
>  #endif
>  
>  		if ((n = queue_get_desc(rxq, port, 0)) < 0) {
> -			received = 0; /* No packet received */
>  #if DEBUG_RX
>  			printk(KERN_DEBUG "%s: eth_poll netif_rx_complete\n",
>  			       dev->name);
> @@ -543,7 +542,7 @@ static int eth_poll(struct napi_struct *napi, int budget)
>  			printk(KERN_DEBUG "%s: eth_poll all done\n",
>  			       dev->name);
>  #endif
> -			return 0; /* all work done */
> +			return received; /* all work done */
>  		}
>  
>  		desc = rx_desc_ptr(port, n);

This is a functional change, and I do not believe that "fixes some
weirdness" is an adequate description of it.

Please: what was wrong with the old code?  Were the effects user-visible? 
How does the new code fix those problems?  Stuff like that.

You hint that this might be a 2.6.26-worthy change, but we should be able
to work this out ourselves from your description of it.  But the description
was no good :(

Re: [FIX] ARM: IXP4xx Ethernet NAPI fix

[Krzysztof Halasa]

Andrew Morton <akpm@linux-foundation.org> writes:

>> +++ b/drivers/net/arm/ixp4xx_eth.c
>> @@ -522,7 +522,6 @@ static int eth_poll(struct napi_struct *napi, int budget)
>>  #endif
>>  
>>  		if ((n = queue_get_desc(rxq, port, 0)) < 0) {
>> -			received = 0; /* No packet received */
>>  #if DEBUG_RX
>>  			printk(KERN_DEBUG "%s: eth_poll netif_rx_complete\n",
>>  			       dev->name);
>> @@ -543,7 +542,7 @@ static int eth_poll(struct napi_struct *napi, int budget)
>>  			printk(KERN_DEBUG "%s: eth_poll all done\n",
>>  			       dev->name);
>>  #endif
>> -			return 0; /* all work done */
>> +			return received; /* all work done */
>>  		}
>>  
>>  		desc = rx_desc_ptr(port, n);
>
> This is a functional change, and I do not believe that "fixes some
> weirdness" is an adequate description of it.

Well, tried my best :-)

> Please: what was wrong with the old code?

It shouldn't do anything like that, the function has to return the
number of received packets. I.e., it was not up to the NAPI specs. Now
it does what NAPI wants, hopefully. I have no idea why did I put that
in :-(

>  Were the effects user-visible?

I don't know. Actually, for me it works the same before and after the
change.
-- 
Krzysztof Halasa