lkml.org 
[lkml]   [2011]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] net: add Faraday FTMAC100 10/100 Ethernet driver
From
Date
On Mon, 2011-01-17 at 18:29 +0100, Eric Dumazet wrote:
> Le lundi 17 janvier 2011 à 17:21 +0800, Po-Yu Chuang a écrit :
>
>
> > +static int ftmac100_rx_packet(struct ftmac100 *priv, int *processed)
> > +{
> > + struct net_device *netdev = priv->netdev;
> > + struct ftmac100_rxdes *rxdes;
> > + struct sk_buff *skb;
> > + int length;
> > + int copied = 0;
> > + int done = 0;
> > +
> > + rxdes = ftmac100_rx_locate_first_segment(priv);
> > + if (!rxdes)
> > + return 0;
> > +
> > + length = ftmac100_rxdes_frame_length(rxdes);
> > +
> > + netdev->stats.rx_packets++;
> > + netdev->stats.rx_bytes += length;
> > +
> > + if (unlikely(ftmac100_rx_packet_error(priv, rxdes))) {
> > + ftmac100_rx_drop_packet(priv);
> > + return 1;
> > + }
> > +
> > + /* start processing */
> > + skb = netdev_alloc_skb_ip_align(netdev, length);
> > + if (unlikely(!skb)) {
> > + if (net_ratelimit())
> > + netdev_err(netdev, "rx skb alloc failed\n");
> > +
> > + ftmac100_rx_drop_packet(priv);
> > + return 1;
> > + }
> > +
>
> Please dont increase rx_packets/rx_bytes before the
> netdev_alloc_skb_ip_align().
>
> In case of mem allocation failure, it would be better not pretending we
> handled a packet.
>
> drivers/net/r8169.c for example does the rx_packets/rx_bytes only if
> packet is delivered to upper stack.

That's news to me. I specifically advised Po-Yu Chuang to increment
these earlier because my understanding is that all packets/bytes should
be counted. And drivers which use hardware MAC stats will generally do
that, so I really don't think it makes sense to make other drivers
different deliberately.
Ben.

--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-01-17 20:01    [from the cache]
©2003-2011 Jasper Spaans