Re: [PATCH v3] Ethernet driver for the WIZnet W5300 chip

[Eric Dumazet]

Le samedi 24 mars 2012 Ã 16:02 +0500, Mike Sinkovsky a Ãcrit :
> Based on original driver from chip manufacturer, but nearly full rewite.
> 
> Tested and used in production with Blackfin BF531 embedded processor.
> 
> Signed-off-by: Mike Sinkovsky <msink@xxxxxxxxxxxxx>
> ---

> +
> +static int w5300_start_tx(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct w5300_private *priv = netdev_priv(ndev);
> +
> +	if (unlikely(w5300_read32(priv, W5300_S0_TX_FSR) < skb->len)) {
> +		ndev->stats.tx_dropped++;
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	w5300_write_frame(priv, skb->data, skb->len);
> +	ndev->stats.tx_packets++;
> +	ndev->stats.tx_bytes += skb->len;
> +	dev_kfree_skb(skb);
> +
> +	return NETDEV_TX_OK;
> +}
> +

As there is no tx interrupt at the end of transmit to eventually XON the
queue, you cant XOFF it when there it no room in transmit queue.

Of course, this means qdisc will never have a backlog, since we never
stop the dequeue process.

Returning NETDEV_TX_BUSY essentialy is going to burn cpu cycles,
spinning there is enough room in NIC.

So you must drop the frame, and return NETDEV_TX_OK instead.


static int w5300_start_tx(struct sk_buff *skb, struct net_device *ndev)
{
       struct w5300_private *priv = netdev_priv(ndev);

       if (unlikely(w5300_read32(priv, W5300_S0_TX_FSR) < skb->len)) {
               ndev->stats.tx_dropped++;
               dev_kfree_skb(skb);
               return NETDEV_TX_OK;
       }



Rule to use NETDEV_TX_BUSY is : 

Before returning NETDEV_TX_BUSY, qdisc must have been stopped by
netif_stop_queue(ndev) (and tx completion might call netif_wake_queue()
later)




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