Patchwork net: lpc_eth: no need to reserve 8 extra bytes in rx skb

login
register
mail settings
Submitter Eric Dumazet
Date April 3, 2012, 10:02 p.m.
Message ID <1333490531.18626.336.camel@edumazet-glaptop>
Download mbox | patch
Permalink /patch/150557/
State Accepted
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - April 3, 2012, 10:02 p.m.
Probably a leftover from ancient code...

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Roland Stigge <stigge@antcom.de>
---
This was mentioned in one of my review but ignored/lost.

 drivers/net/ethernet/nxp/lpc_eth.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stigge@antcom.de - April 3, 2012, 10:54 p.m.
Hi Eric,

On 04/04/12 00:02, Eric Dumazet wrote:
> Probably a leftover from ancient code...

...

> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -990,10 +990,10 @@ static int __lpc_handle_recv(struct net_device *ndev, int budget)
>  			ndev->stats.rx_errors++;
>  		} else {
>  			/* Packet is good */
> -			skb = dev_alloc_skb(len + 8);
> -			if (!skb)
> +			skb = dev_alloc_skb(len);

I remember this issue from the discussion, a note from Ben Hutchings
actually, where there was a further "skb_reserve(skb, 8);" in the "else"
case below. Looks like I only removed the skb_reserve().

Can't find this review from you - I hope there are no other issues left?
(Was I on CC?)

> +			if (!skb) {
>  				ndev->stats.rx_dropped++;
> -			else {
> +			} else {

Why add curly braces around a single statement?

Thanks,

Roland
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 3, 2012, 10:58 p.m.
From: Roland Stigge <stigge@antcom.de>
Date: Wed, 04 Apr 2012 00:54:19 +0200

> Why add curly braces around a single statement?

Because that's how we roll for else statements homie, see CodingStyle


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stigge@antcom.de - April 3, 2012, 11:42 p.m.
On 04/04/12 00:58, David Miller wrote:
>> Why add curly braces around a single statement?
> 
> Because that's how we roll for else statements homie, see CodingStyle

Oops :-) sorry you two!

Thanks,

Roland
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 4, 2012, 10:09 p.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 04 Apr 2012 00:02:11 +0200

> Probably a leftover from ancient code...
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Roland Stigge <stigge@antcom.de>

Applied to net-next, thanks Eric.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
index 6dfc26d..d3469d8 100644
--- a/drivers/net/ethernet/nxp/lpc_eth.c
+++ b/drivers/net/ethernet/nxp/lpc_eth.c
@@ -990,10 +990,10 @@  static int __lpc_handle_recv(struct net_device *ndev, int budget)
 			ndev->stats.rx_errors++;
 		} else {
 			/* Packet is good */
-			skb = dev_alloc_skb(len + 8);
-			if (!skb)
+			skb = dev_alloc_skb(len);
+			if (!skb) {
 				ndev->stats.rx_dropped++;
-			else {
+			} else {
 				prdbuf = skb_put(skb, len);
 
 				/* Copy packet from buffer */