RE: [PATCH net V2] Xen-netback: Fix issue caused by using gso_type wrongly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Annie Li [mailto:Annie.li@xxxxxxxxxx]
> Sent: 10 March 2014 14:59
> To: xen-devel@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx
> Cc: konrad.wilk@xxxxxxxxxx; Ian Campbell; Wei Liu; Paul Durrant;
> annie.li@xxxxxxxxxx
> Subject: [PATCH net V2] Xen-netback: Fix issue caused by using gso_type
> wrongly
> 
> From: Annie Li <annie.li@xxxxxxxxxx>
> 
> Current netback uses gso_type to check whether the skb contains
> gso offload, and this is wrong. Gso_size is the right one to
> check gso existence, and gso_type is only used to check gso type.
> 
> Some skbs contains nonzero gso_type and zero gso_size, current
> netback would treat these skbs as gso and create wrong response
> for this. This also causes ssh failure to domu from other server.
> 
> V2: use skb_is_gso function as Paul Durrant suggested
> 
> Signed-off-by: Annie Li <annie.li@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  drivers/net/xen-netback/netback.c |   39 +++++++++++++++++---------------
> ----
>  1 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> index e5284bc..438d0c0 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -240,7 +240,7 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
> struct sk_buff *skb,
>  	struct gnttab_copy *copy_gop;
>  	struct xenvif_rx_meta *meta;
>  	unsigned long bytes;
> -	int gso_type;
> +	int gso_type = XEN_NETIF_GSO_TYPE_NONE;
> 
>  	/* Data must not cross a page boundary. */
>  	BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
> @@ -299,12 +299,12 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
> struct sk_buff *skb,
>  		}
> 
>  		/* Leave a gap for the GSO descriptor. */
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> -		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> -		else
> -			gso_type = XEN_NETIF_GSO_TYPE_NONE;
> +		if (skb_is_gso(skb)) {
> +			if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> +				gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> +			else if (skb_shinfo(skb)->gso_type &
> SKB_GSO_TCPV6)
> +				gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> +		}
> 
>  		if (*head && ((1 << gso_type) & vif->gso_mask))
>  			vif->rx.req_cons++;
> @@ -338,19 +338,15 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  	int head = 1;
>  	int old_meta_prod;
>  	int gso_type;
> -	int gso_size;
> 
>  	old_meta_prod = npo->meta_prod;
> 
> -	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
> -		gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> -		gso_size = skb_shinfo(skb)->gso_size;
> -	} else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
> -		gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> -		gso_size = skb_shinfo(skb)->gso_size;
> -	} else {
> -		gso_type = XEN_NETIF_GSO_TYPE_NONE;
> -		gso_size = 0;
> +	gso_type = XEN_NETIF_GSO_TYPE_NONE;
> +	if (skb_is_gso(skb)) {
> +		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> +			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> +		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> +			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
>  	}
> 
>  	/* Set up a GSO prefix descriptor, if necessary */
> @@ -358,7 +354,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  		req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
>  		meta = npo->meta + npo->meta_prod++;
>  		meta->gso_type = gso_type;
> -		meta->gso_size = gso_size;
> +		meta->gso_size = skb_shinfo(skb)->gso_size;
>  		meta->size = 0;
>  		meta->id = req->id;
>  	}
> @@ -368,7 +364,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
> 
>  	if ((1 << gso_type) & vif->gso_mask) {
>  		meta->gso_type = gso_type;
> -		meta->gso_size = gso_size;
> +		meta->gso_size = skb_shinfo(skb)->gso_size;
>  	} else {
>  		meta->gso_type = XEN_NETIF_GSO_TYPE_NONE;
>  		meta->gso_size = 0;
> @@ -500,8 +496,9 @@ static void xenvif_rx_action(struct xenvif *vif)
>  			size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
>  			max_slots_needed += DIV_ROUND_UP(size,
> PAGE_SIZE);
>  		}
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
> -		    skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> +		if (skb_is_gso(skb) &&
> +		   (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
> +		    skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6))
>  			max_slots_needed++;
> 
>  		/* If the skb may not fit then bail out now */
> --
> 1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Discussion]     [TCP Instrumentation]     [Ethernet Bridging]     [Linux Wireless Networking]     [Linux WPAN Networking]     [Linux Host AP]     [Linux WPAN Networking]     [Linux Bluetooth Networking]     [Linux ATH6KL Networking]     [Linux Networking Users]     [Linux Coverity]     [VLAN]     [Git]     [IETF Annouce]     [Linux Assembly]     [Security]     [Bugtraq]     [Yosemite Information]     [MIPS Linux]     [ARM Linux Kernel]     [ARM Linux]     [Linux Virtualization]     [Linux IDE]     [Linux RAID]     [Linux SCSI]