[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.