Re: xennet: skb rides the rocket: 20 slots

[ANNIE LI <annie.li@oracle.com>]

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 "netchannel vs 
MAX_SKB_FRAGS". Maybe these two mechanism are all necessary?

Thanks
Annie
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f2d6b78..2f02da9 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -902,11 +902,13 @@ static int netbk_count_requests(struct xenvif *vif,
>          do {
>                  if (frags>= work_to_do) {
>                          netdev_dbg(vif->dev, "Need more frags\n");
> +                       net_warn_ratelimited("%s: Need more frags:%d, work_to_do:%d \n",vif->dev->name, frags, work_to_do);
>                          return -frags;
>                  }
>
>                  if (unlikely(frags>= MAX_SKB_FRAGS)) {
>                          netdev_dbg(vif->dev, "Too many frags\n");
> +                        net_warn_ratelimited("%s: Too many frags:%d, MAX_SKB_FRAGS:%d \n",vif->dev->name, frags, MAX_SKB_FRAGS);
>                          return -frags;
>                  }
>
> @@ -914,6 +916,7 @@ static int netbk_count_requests(struct xenvif *vif,
>                         sizeof(*txp));
>                  if (txp->size>  first->size) {
>                          netdev_dbg(vif->dev, "Frags galore\n");
> +                        net_warn_ratelimited("%s: Frags galore:%d, txp->size:%d  first->size:%d\n",vif->dev->name, frags, txp->size , first->size);
>                          return -frags;
>                  }
>
> @@ -923,6 +926,7 @@ static int netbk_count_requests(struct xenvif *vif,
>                  if (unlikely((txp->offset + txp->size)>  PAGE_SIZE)) {
>                          netdev_dbg(vif->dev, "txp->offset: %x, size: %u\n",
>                                   txp->offset, txp->size);
> +                        net_warn_ratelimited("%s: Hmm:%d, (txp->offset + txp->size):%d   PAGE_SIZE:%d\n",vif->dev->name, frags,  (txp->offset + txp->size)  ,PAGE_SIZE);
>                          return -frags;
>                  }
>          } while ((txp++)->flags&  XEN_NETTXF_more_data);