Re: [PATCH v4 1/2] Renesas Ethernet AVB driver proper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




From: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
Date: Sun, 17 May 2015 23:52:42 +0300

> +/* Packet transmit function for Ethernet AVB */
> +static netdev_tx_t ravb_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
 ...
> +	spin_lock_irqsave(&priv->lock, flags);
> +	if (priv->cur_tx[q] - priv->dirty_tx[q] > priv->num_tx_ring[q]) {
> +		if (!ravb_tx_free(ndev, q)) {
> +			netif_warn(priv, tx_queued, ndev, "TX FD exhausted.\n");
> +			netif_stop_queue(ndev);
> +			spin_unlock_irqrestore(&priv->lock, flags);
> +			return NETDEV_TX_BUSY;
> +		}

This is not an acceptable way to manage your transmit queue state.

When your transmit queue fills up and you cannot accept the next
arbitrary packet into your TX queue, you must stop the queue
before returning from this function.

Therefore, only error conditions should cause NETDEV_TX_BUSY to
ever be returned from this routine.

I know what argument you are going to make to me in response to this
email.  You are going to tell me that you have these two queues, one
for tstamp packets and one for the rest, and therefore that model of
TX queue management can't be made to work.

That's not acceptable.

Instead, you should classify packets to the different queues using
->ndo_select_queue() and therefore have multiple TX queues, which you
can management the "fullness" of independantly.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux