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]
Date:	Wed, 21 May 2014 03:11:31 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>, cmetcalf@...era.com,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory Clement <gregory.clement@...e-electrons.com>,
	Simon Guinot <simon.guinot@...uanux.org>,
	Willy Tarreau <w@....eu>, Tawfik Bayouk <tawfik@...vell.com>,
	Lior Amsalem <alior@...vell.com>,
	Simon Guinot <sguinot@...ie.com>
Subject: Re: [PATCH 3/3] net: mvneta: Introduce a software TSO implementation

On Mon, 2014-05-05 at 11:47 -0300, Ezequiel Garcia wrote:
> Hi all,
> 
> On 10 Apr 07:58 PM, Ezequiel Garcia wrote:
> [..]
> > +
> > +	/* Calculate expected number of TX descriptors */
> > +	desc_count = skb_shinfo(skb)->gso_segs * 2 + skb_shinfo(skb)->nr_frags;
> > +	if ((txq->count + desc_count) >= txq->size)
> > +		return 0;
> > +
> 
> Is this calculus correct? Does it give the accurate number of needed
> descriptors or is it an approximation?
> 
> Tilera's tilegx driver does a much stricter descriptor count (see
> tso_count_edescs). This functions loops through the skb_frag_t fragments
> as it's done later in the data egress, hence strictly counting the
> needed descriptors.
> 
> However, as it's a much heavier routine than the one shown above,
> I'm wondering if we can get away without it.
> 
> Willy, Any ideas here?

There are (at least) three potential bugs you need to avoid:

1. You must not underestimate the number of descriptors required here,
otherwise the ring may overflow.  Hopefully that's obvious.
2. You must ensure that the worst case number of descriptors does
actually fit in the ring, and will probably need to set
net_dev->gso_max_segs accordingly.  Eric pointed that out already.
3. If you stop the queue because an skb doesn't fit, you should not wake
it before there is enough space.

A simple way to do this is:

- Set a maximum number of segments to support (for sfc, I reckoned that
100 was enough)
- Calculate the maximum number of descriptors that could be needed for
that number of segments
- Stop the queue when the free space in the ring is less than that
maximum
- Wake the queue when the free space reaches a threshold somewhere
between that maximum and completely free

It may make more sense to work backward from the ring size to maximum
number of segments.

Ben.

-- 
Ben Hutchings
Life would be so much easier if we could look at the source code.

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)

Powered by blists - more mailing lists