Re: [Xen-devel] xennet: skb rides the rocket: 20 slots

[ANNIE LI <annie.li@xxxxxxxxxx>]

On 2013-1-9 23:08, Konrad Rzeszutek Wilk wrote:
> 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.
I am thinking to do re-fragment in netfront for these skbs like following,

Create a new skb, copy linear data and frag data from original skb into 
this one, and make every frags data size is PAGE_SIZE except for the 
last fragment. It is possible that the last fragment length is less than 
PAGE_SIZE, then free the original skb. The skb packet is large, and 
there will be lots of copys.
struct skbuff *xennet_refrag_skb(skb)
{
   create newskb
   copying data and doing fragmentation
   return newskb
}

.......

if (unlikely(slots>  MAX_SKB_FRAGS + 1)) {
        net_alert_ratelimited(
              "xennet: skb rides the rocket: %d slots\n", slots);
        skb = xennet_refrag_skb(skb);
}
.....

Thanks
Annie
> > "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.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel