Re: [PATCH RFC] xen/netback: Count ring slots properly when larger MTU sizes are used

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2012-08-22 at 14:30 +0100, Palagummi, Siva wrote:
> > >
> > >
> > > > This:
> > > > 		/* Filled the batch queue? */
> > > > 		if (count + MAX_SKB_FRAGS >= XEN_NETIF_RX_RING_SIZE)
> > > > 			break;
> > > > seems a bit iffy to me too. I wonder if MAX_SKB_FRAGS should be
> > > > max_required_rx_slots(vif)? Or maybe the preflight checks from
> > > > xenvif_start_xmit save us from this fate?
> > > >
> > > > Ian.
> > >
> > > You are right Ian. The intention of this check seems to be to ensure
> > > that enough slots are still left prior to picking up next skb. But
> > > instead of invoking max_required_rx_slots with already received skb,
> > > we may have to do skb_peek and invoke max_required_rx_slots on skb
> > > that we are about to dequeue. Is there any better way?
> > 
> > max_required_rx_slots doesn't take an skb as an argument, just a vif.
> > It
> > returns the worst case number of slots for any skb on that vif.
> > 
> > Ian.
> 
> That’s true. What I meant is to peek to next skb and get vif from that
> structure to invoke max_required_rx_slots. Don't you think we need to
> do like that?

Do you mean something other than max_required_rx_slots? Because the
prototype of that function is
        static int max_required_rx_slots(struct xenvif *vif)
i.e. it doesn't need an skb.

I think it is acceptable to check for the worst case number of slots.
That's what we do e.g. in xen_netbk_rx_ring_full.

Using skb_peek might work too though, assuming all the locking etc is ok
-- this is a private queue so I think it is probably ok. Rather than
calculating the number of slots in xen_netbk_rx_action you probably want
to remember the value from the call to xen_netbk_count_skb_slots in
start_xmit. Perhaps by stashing it in skb->cb? (see NETFRONT_SKB_CB for
an example of how to do this)

Ian.


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