Hi benh,

            Please find my comments inline.

On Tue, Jun 24, 2008 at 4:50 AM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:

On Mon, 2008-06-23 at 14:55 +0200, Stefan Roese wrote:

quoted

From: Sathya Narayanan <redacted>

Short packets has to be discarded by the driver. So this patch addresses

the

quoted

issue of discarding the short packets of size lesser then ethernet header
size.

You are freeing the skb, why ? Shouldn't we just keep the skb in the
ring for further rx ?

Actually , short packets are not allowed to flow through the higher layers,
If any of the layer tried to use the extra room available may hit wit crash
.
Since it is a invalid packet it has to be dropped and freed in driver.
Actually if you see in code, the other invalid packets are also handelled
similar.

quoted

Signed-off-by: Sathya Narayanan <redacted>
Signed-off-by: Stefan Roese <sr@denx.de>
---
 drivers/net/ibm_newemac/core.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ibm_newemac/core.c

b/drivers/net/ibm_newemac/core.c

quoted

index 6dfc2c9..aa407b2 100644

--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c

@@ -1652,6 +1652,13 @@ static int emac_poll_rx(void *param, int budget)

              skb_put(skb, len);
      push_packet:
+             if (skb->len < ETH_HLEN) {
+                     dev_kfree_skb(skb);
+                     printk(KERN_WARNING "%s: short packets dropped\n",
+                            dev->ndev->name);
+                     ++dev->estats.rx_dropped_stack;
+                     goto next;
+             }
              skb->dev = dev->ndev;
              skb->protocol = eth_type_trans(skb, dev->ndev);
              emac_rx_csum(dev, skb, ctrl);