Hi benh,<br><br>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Please find my comments inline.<br><br><div class="gmail_quote">On Tue, Jun 24, 2008 at 4:50 AM, Benjamin Herrenschmidt &lt;<a href="mailto:benh@kernel.crashing.org">benh@kernel.crashing.org</a>&gt; wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d">On Mon, 2008-06-23 at 14:55 +0200, Stefan Roese wrote:<br>
&gt; From: Sathya Narayanan &lt;<a href="mailto:sathyan@teamf1.com">sathyan@teamf1.com</a>&gt;<br>
&gt;<br>
&gt; Short packets has to be discarded by the driver. So this patch addresses the<br>
&gt; issue of discarding the short packets of size lesser then ethernet header<br>
&gt; size.<br>
<br>
</div>You are freeing the skb, why ? Shouldn&#39;t we just keep the skb in the<br>
ring for further rx ?</blockquote><div><br>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 .<br>Since it is a invalid packet it has to be dropped and freed in driver.<br>
Actually if you see in code, the other invalid packets are also handelled similar.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<div><div></div><div class="Wj3C7c"><br>
&gt; Signed-off-by: Sathya Narayanan &lt;<a href="mailto:sathyan@teamf1.com">sathyan@teamf1.com</a>&gt;<br>
&gt; Signed-off-by: Stefan Roese &lt;<a href="mailto:sr@denx.de">sr@denx.de</a>&gt;<br>
&gt; ---<br>
&gt; &nbsp;drivers/net/ibm_newemac/core.c | &nbsp; &nbsp;7 +++++++<br>
&gt; &nbsp;1 files changed, 7 insertions(+), 0 deletions(-)<br>
&gt;<br>
&gt; diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c<br>
&gt; index 6dfc2c9..aa407b2 100644<br>
&gt; --- a/drivers/net/ibm_newemac/core.c<br>
&gt; +++ b/drivers/net/ibm_newemac/core.c<br>
&gt; @@ -1652,6 +1652,13 @@ static int emac_poll_rx(void *param, int budget)<br>
&gt;<br>
&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; skb_put(skb, len);<br>
&gt; &nbsp; &nbsp; &nbsp; push_packet:<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (skb-&gt;len &lt; ETH_HLEN) {<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; dev_kfree_skb(skb);<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; printk(KERN_WARNING &quot;%s: short packets dropped\n&quot;,<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;dev-&gt;ndev-&gt;name);<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; ++dev-&gt;estats.rx_dropped_stack;<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; goto next;<br>
&gt; + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }<br>
&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; skb-&gt;dev = dev-&gt;ndev;<br>
&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; skb-&gt;protocol = eth_type_trans(skb, dev-&gt;ndev);<br>
&gt; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; emac_rx_csum(dev, skb, ctrl);<br>
<br>
</div></div></blockquote></div><br>