netdev - Re: [PATCH 3/3] net: mvneta: Introduce a software TSO implementation

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: <20140522174754.GC1785@arch.cereza>
Date:	Thu, 22 May 2014 14:47:54 -0300
From:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
To:	Ben Hutchings <ben@...adent.org.uk>
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 21 May 03:11 AM, Ben Hutchings wrote:
> 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?
> > 
> 
> 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.

Yes, fully understood.

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

Yeah, I still need to take a deep look at the commit pointed out by Eric.

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

Hm, this seems a good suggestion. I'm wondering if this can be added to
the generic code. Although this is likely each driver's responsability.

BTW, have you seen the proposal for the generic TSO API?

http://permalink.gmane.org/gmane.linux.network/316852

Any comments about it would be much appreciated, as we'd really like
to see TSO implemented on our drivers.

Thanks,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
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