Re: xennet: skb rides the rocket: 20 slots

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Jan 09, 2013 at 03:10:56PM +0800, ANNIE LI wrote:
> 
> 
> On 2013-1-9 4:55, Sander Eikelenboom wrote:
> >>                  if (unlikely(frags>= MAX_SKB_FRAGS)) {
> >>                          netdev_dbg(vif->dev, "Too many frags\n");
> >>                          return -frags;
> >>                  }
> >I have added some rate limited warns in this function. However none seems to be triggered while the pv-guest reports the "skb rides the rocket" ..
> 
> Oh,  yes, "skb rides the rocket" is a protect mechanism in netfront,
> and it is not caused by netback checking code, but they all concern
> about the same thing(frags >= MAX_SKB_FRAGS ). I thought those
> packets were dropped by backend check, sorry for the confusion.
> 
> In netfront, following code would check whether required slots
> exceed MAX_SKB_FRAGS, and drop skbs which does not meet this
> requirement directly.
> 
>         if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
>                 net_alert_ratelimited(
>                         "xennet: skb rides the rocket: %d slots\n", slots);
>                 goto drop;
>         }
> 
> In netback, following code also compared frags with MAX_SKB_FRAGS,
> and create error response for netfront which does not meet this
> requirment. In this case, netfront will also drop corresponding
> skbs.
> 
>                 if (unlikely(frags >= MAX_SKB_FRAGS)) {
>                         netdev_dbg(vif->dev, "Too many frags\n");
>                         return -frags;
>                 }
> 
> So it is correct that netback log was not print out because those
> packets are drops directly by frontend check, not by backend check.
> Without the frontend check, it is likely that netback check would
> block these skbs and create error response for netfront.
> 
> So two ways are available: workaround in netfront for those packets,
> doing re-fragment copying, but not sure how copying hurt
> performance. Another is to implement in netback, as discussed in

There is already some copying done (the copying of the socket data
from userspace to the kernel) - so the extra copy might not be that
bad as the data can be in the cache. This would probably be a way
to deal with old backends that cannot deal with this new feature-flag.

> "netchannel vs MAX_SKB_FRAGS". Maybe these two mechanism are all
> necessary?

Lets see first if this is indeed the problem. Perhaps a simple debug
patch that just does:

	s/MAX_SKB_FRAGS/DEBUG_MAX_FRAGS/
	#define DEBUG_MAX_FRAGS 21

in both netback and netfront to set the maximum number of frags we can
handle to 21? If that works with Sander test - then yes, it looks like
we really need to get this 'feature-max-skb-frags' done.