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

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

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