netdev - RE: [PATCH 1/4] pch_gbe: Fix transmit queue management

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AE90C24D6B3A694183C094C60CF0A2F6026B744F@saturn3.aculab.com>
Date:	Mon, 2 Dec 2013 10:04:49 -0000
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"David Miller" <davem@...emloft.net>, <puzman@...il.com>
Cc:	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/4] pch_gbe: Fix transmit queue management

> From: David Miller
> 
> > According to Documentation/networking/driver.txt the ndo_start_xmit
> > method should not return NETDEV_TX_BUSY under normal circumstances.
> > Stop the transmit queue when tx_ring is full.
... 
> You should be instead preventing the transmit method from being invoked
> when it might be possible that a request cannot be satisfied.
> 
> This means that at the end of a transmit request, you must stop the
> queue if a packet with the maximum number of possible descriptors
> cannot be satisfied.  Then it is impossible for the transmit function
> to be invoked in a situation where it would need to fail for lack of
> available transmit descriptors.

I know you like ethernet drivers to work this way, but requiring enough
descriptors for a maximally fragmented packet requires a difficult
calculation and will cause the tx queue to be stopped unnecessarily.

IIRC a normal skb can have a maximum of 17 fragments, if these end up
being transmitted over USB using xhci they might need 34 descriptors
(because descriptors can't cross 64k boundaries).
However a typical 64k TCP packet just needs 4.
xhci has a further constraint that the ring end can only be at
specific alignments, in effect thus means that the descriptors for
a single packet cannot cross the end of a ring segment.
So if the available space in the xhci ring was used to stop the
ethernet tx queue (which it isn't at the moment) it would have to
be stopped very early.

Isn't there also a new skb structure that allows lists of fragments?
That might need even more descriptors for a worst case transmit.

I would have thought it would make sense for the ethernet driver
to stop the queue when there is a reasonable expectation that there
won't be enough descriptors for the next packet, but have a reasonable
code path when a packet with a large number of fragments won't fit
in the tx ring. This either requires the driver itself to hold the
skb, or to return NETDEV_TX_BUSY.
It might make sense to monitor recent traffic to find the 'expected
maximum number of fragments'.

	David

--
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