Re: [Xen-devel] [PATCH V2] xen/netfront: handle compound page fragments on transmit

[Eric Dumazet <eric.dumazet@xxxxxxxxx>]

On Tue, 2012-11-20 at 16:00 +0000, Ian Campbell wrote:
> An SKB paged fragment can consist of a compound page with order > 0.
> However the netchannel protocol deals only in PAGE_SIZE frames.
> 
> Handle this in xennet_make_frags by iterating over the frames which
> make up the page.
> 
> This is the netfront equivalent to 6a8ed462f16b for netback.
...

> -     frags += DIV_ROUND_UP(offset + len, PAGE_SIZE);
> -     if (unlikely(frags > MAX_SKB_FRAGS + 1)) {
> -             printk(KERN_ALERT "xennet: skb rides the rocket: %d frags\n",
> -                    frags);
> -             dump_stack();
> +     slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
> +             xennet_count_skb_frag_slots(skb);
> +     if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
> +             printk(KERN_ALERT "xennet: skb rides the rocket: %d slots\n",
> +                    slots);

I think this is wrong.

You should change netfront_tx_slot_available() to stop the queue before
this can happen.

Yes, you dont hit this on your tests, but a driver should not drop a
good packet.

>               goto drop;
>       }
>  
>       spin_lock_irqsave(&np->tx_lock, flags);
>  
>       if (unlikely(!netif_carrier_ok(dev) ||
> -                  (frags > 1 && !xennet_can_sg(dev)) ||
> +                  (slots > 1 && !xennet_can_sg(dev)) ||
>                    netif_needs_gso(skb, netif_skb_features(skb)))) {
>               spin_unlock_irqrestore(&np->tx_lock, flags);
>               goto drop;


diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..cb1e605 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -215,10 +215,13 @@ static void rx_refill_timeout(unsigned long data)
        napi_schedule(&np->napi);
 }
 
+/* Considering a 64Kb packet of 16 frags, each frag can be mapped
+ * to 3 order-0 parts on pathological cases
+ */
 static int netfront_tx_slot_available(struct netfront_info *np)
 {
        return (np->tx.req_prod_pvt - np->tx.rsp_cons) <
-               (TX_MAX_TARGET - MAX_SKB_FRAGS - 2);
+               (TX_MAX_TARGET - 3*MAX_SKB_FRAGS - 2);
 }
 
 static void xennet_maybe_wake_tx(struct net_device *dev)



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