Re: [Xen-devel] [PATCH V5 6/7] xen-netback: coalesce slots in TX path and fix regressions

[annie li <annie.li@xxxxxxxxxx>]

On 2013-4-15 13:06, Wei Liu wrote:
>   
>   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;
> -       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;
It is possible that vif is freed when packet size is less than 64K here 
but slots required >= max_skb_slots. Alough max_skb_slots can be 
configured, this kind of packets would be dropped in following if 
(!drop_err && slots >= XEN_NETIF_NR_SLOTS_MIN).
>                 }
>   
> -		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;
It is possible to drop packets like above(size < 64K && slot >= 
XEN_NETIF_NR_SLOTS_MIN). I do not know how frequently this kind of 
packets appear, maybe some SKBs with compound page(size < 64K && slot >= 
XEN_NETIF_NR_SLOTS_MIN) are dropped here?

Thanks
Annie


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