netdev - Re: [Xen-devel] [PATCH RFC] xen-netback: calculate the number of slots required for large MTU vifs

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: <20130711084627.GA23269@zion.uk.xensource.com>
Date:	Thu, 11 Jul 2013 09:46:27 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	Matt Wilson <msw@...zon.com>
CC:	Wei Liu <wei.liu2@...rix.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	<netdev@...r.kernel.org>, Annie Li <annie.li@...cle.com>,
	<xen-devel@...ts.xenproject.org>, Xi Xiong <xixiong@...zon.com>
Subject: Re: [Xen-devel] [PATCH RFC] xen-netback: calculate the number of
 slots required for large MTU vifs

On Wed, Jul 10, 2013 at 10:14:43PM -0700, Matt Wilson wrote:
[...]
> Yes, I wanted to get this out since we're talking about multiple
> changes in similar areas.
> 
> > The reason that your patches uses req_cons instead of pre-calculated
> > value is, that value returned by xen_netbk_skb_count_slots is actually
> > *wrong* -- that's what Annie tried to fix in her patch.
> 
> Yes and no. With this patch, the xen_netbk_count_skb_slots() is now
> correct. xen_netbk_count_skb_slots() was under-counting when slots
> were inefficiently consumed. With this patch they match.
> 

This is worth mentioning in the commit message IMHO.

> Since we've had trouble with the counting in the past, I felt like
> using the "real" number of slots here was safer and future-proof.
> 

Fair enough. Worth a comment in the code.

> > After fixing xen_netbk_skb_count_slots, we would need the above snippet
> > (or the snippet I proposed in my RFC patch) to prevent overruning the
> > ring.
> 
> Agreed.
> 
> > So a proper patch to this issue (not couting RX slots correctly causing
> > overrun of the ring) would be the above two aspects combined.
> 
> Yes, I think so.
> 
> > Comments?
> 
> I think that this patch addresses the problem more completely. Annie?
> 
> See also the thread from last August:
>    http://lists.xen.org/archives/html/xen-devel/2012-12/msg00274.html
> 

So now it seems that "turning head in start_new_rx_buffer to flag" part
should also be in the patch as it's actually a fix to match counting in
both functions. The side effect of this change is "improving efficiency
of the ring". This would also worth mentioning in the commit message
IMHO.


Wei.

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