Re: [Xen-devel] [PATCH net-next V7 3/4] xen-netback: coalesce slots in TX path and fix regressions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.04.13 at 14:20, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> @@ -898,47 +928,78 @@ static void netbk_fatal_tx_err(struct xenvif *vif)
>  
>  static int netbk_count_requests(struct xenvif *vif,
>                               struct xen_netif_tx_request *first,
> +                             RING_IDX first_idx,
>                               struct xen_netif_tx_request *txp,
>                               int work_to_do)
>  {
>       RING_IDX cons = vif->tx.req_cons;

It looks bogus and confusing to me to pass first_idx into this
function when this really is the same as cons.

Jan

> -     int frags = 0;
> +     int slots = 0;
> +     int drop_err = 0;
>  
>       if (!(first->flags & XEN_NETTXF_more_data))
>               return 0;
>  
>       do {
> -             if (frags >= work_to_do) {
> -                     netdev_err(vif->dev, "Need more frags\n");
> +             if (slots >= work_to_do) {
> +                     netdev_err(vif->dev,
> +                                "Asked for %d slots but exceeds this 
> limit\n",
> +                                work_to_do);
>                       netbk_fatal_tx_err(vif);
>                       return -ENODATA;
>               }
>  
> -             if (unlikely(frags >= MAX_SKB_FRAGS)) {
> -                     netdev_err(vif->dev, "Too many frags\n");
> +             /* This guest is really using too many slots and
> +              * considered malicious.
> +              */
> +             if (unlikely(slots >= max_skb_slots)) {
> +                     netdev_err(vif->dev,
> +                                "Malicious frontend using %d slots, 
> threshold %u\n",
> +                                slots, max_skb_slots);
>                       netbk_fatal_tx_err(vif);
>                       return -E2BIG;
>               }
>  
> -             memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + frags),
> +             /* Xen network protocol had implicit dependency on
> +              * MAX_SKB_FRAGS. XEN_NETIF_NR_SLOTS_MIN is set to the
> +              * historical MAX_SKB_FRAGS value 18 to honor the same
> +              * behavior as before. Any packet using more than 18
> +              * slots but less than max_skb_slots slots is dropped
> +              */
> +             if (!drop_err && slots >= XEN_NETIF_NR_SLOTS_MIN) {
> +                     if (net_ratelimit())
> +                             netdev_dbg(vif->dev,
> +                                        "Too many slots (%d) exceeding limit 
> (%d), dropping packet\n",
> +                                        slots, XEN_NETIF_NR_SLOTS_MIN);
> +                     drop_err = -E2BIG;
> +             }
> +
> +             memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + slots),
>                      sizeof(*txp));
>               if (txp->size > first->size) {
> -                     netdev_err(vif->dev, "Frag is bigger than frame.\n");
> +                     netdev_err(vif->dev,
> +                                "Invalid tx request, slot size %u > 
> remaining size %u\n",
> +                                txp->size, first->size);
>                       netbk_fatal_tx_err(vif);
>                       return -EIO;
>               }
>  
>               first->size -= txp->size;
> -             frags++;
> +             slots++;
>  
>               if (unlikely((txp->offset + txp->size) > PAGE_SIZE)) {
> -                     netdev_err(vif->dev, "txp->offset: %x, size: %u\n",
> +                     netdev_err(vif->dev, "Cross page boundary, txp->offset: 
> %x, size: %u\n",
>                                txp->offset, txp->size);
>                       netbk_fatal_tx_err(vif);
>                       return -EINVAL;
>               }
>       } while ((txp++)->flags & XEN_NETTXF_more_data);
> -     return frags;
> +
> +     if (drop_err) {
> +             netbk_tx_err(vif, first, first_idx + slots);
> +             return drop_err;
> +     }
> +
> +     return slots;
>  }
>  
>  static struct page *xen_netbk_alloc_page(struct xen_netbk *netbk,



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