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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20150519.143054.852529603260516108.davem@davemloft.net>
Date:	Tue, 19 May 2015 14:30:54 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	sergei.shtylyov@...entembedded.com
Cc:	robh+dt@...nel.org, pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, devicetree@...r.kernel.org,
	galak@...eaurora.org, netdev@...r.kernel.org,
	richardcochran@...il.com, linux-sh@...r.kernel.org,
	mitsuhiro.kimura.kc@...esas.com
Subject: Re: [PATCH v4 1/2] Renesas Ethernet AVB driver proper

From: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html