netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] xen-netback: fix gso_prefix check
@ 2013-12-12 14:20 Paul Durrant
  2013-12-12 14:42 ` Ian Campbell
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Durrant @ 2013-12-12 14:20 UTC (permalink / raw)
  To: xen-devel, netdev; +Cc: Paul Durrant, Wei Liu, Ian Campbell, David Vrabel

There is a mistake in checking the gso_prefix mask when passing large
packets to a guest. The wrong shift is applied to the bit - the raw skb
gso type is used rather then the translated one. This leads to large packets
being handed to the guest without the GSO metadata. This patch fixes the
check.

The mistake manifested as errors whilst running Microsoft HCK large packet
offload tests between a pair of Windows 8 VMs. I have verified this patch
fixes those errors.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/netback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 56c3e5a..999e290 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -452,7 +452,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
 	}
 
 	/* Set up a GSO prefix descriptor, if necessary */
-	if ((1 << skb_shinfo(skb)->gso_type) & vif->gso_prefix_mask) {
+	if ((1 << gso_type) & vif->gso_prefix_mask) {
 		req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
 		meta = npo->meta + npo->meta_prod++;
 		meta->gso_type = gso_type;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] xen-netback: fix gso_prefix check
  2013-12-12 14:20 [PATCH net] xen-netback: fix gso_prefix check Paul Durrant
@ 2013-12-12 14:42 ` Ian Campbell
  2013-12-12 14:44   ` Paul Durrant
  2013-12-12 20:48   ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Ian Campbell @ 2013-12-12 14:42 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, netdev, Wei Liu, David Vrabel

On Thu, 2013-12-12 at 14:20 +0000, Paul Durrant wrote:
> There is a mistake in checking the gso_prefix mask when passing large
> packets to a guest. The wrong shift is applied to the bit - the raw skb
> gso type is used rather then the translated one. This leads to large packets
> being handed to the guest without the GSO metadata. This patch fixes the
> check.
> 
> The mistake manifested as errors whilst running Microsoft HCK large packet
> offload tests between a pair of Windows 8 VMs. I have verified this patch
> fixes those errors.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/net/xen-netback/netback.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 56c3e5a..999e290 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -452,7 +452,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  	}
>  
>  	/* Set up a GSO prefix descriptor, if necessary */
> -	if ((1 << skb_shinfo(skb)->gso_type) & vif->gso_prefix_mask) {
> +	if ((1 << gso_type) & vif->gso_prefix_mask) {

gso_type here is XEN_NETIF_GSO_TYPE_TCPV4, vif->gso_prefix_mask is
initialised with GSO_BIT(TCPV4) et al. Where
        #define GSO_BIT(type) \
                (1 << XEN_NETIF_GSO_TYPE_ ## type)
        
So, 
Acked-by: Ian Campbell <ian.campbell@citrix.com>

But it might be nice to use BIT() from bitops.h for both GSO_BIT and the
check, since it took me quite a bit of wandering around to figure out
how these ended up the same.

Ian.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH net] xen-netback: fix gso_prefix check
  2013-12-12 14:42 ` Ian Campbell
@ 2013-12-12 14:44   ` Paul Durrant
  2013-12-12 20:48   ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: Paul Durrant @ 2013-12-12 14:44 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Wei Liu,
	David Vrabel

> -----Original Message-----
> From: Ian Campbell
> Sent: 12 December 2013 14:42
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; David Vrabel
> Subject: Re: [PATCH net] xen-netback: fix gso_prefix check
> 
> On Thu, 2013-12-12 at 14:20 +0000, Paul Durrant wrote:
> > There is a mistake in checking the gso_prefix mask when passing large
> > packets to a guest. The wrong shift is applied to the bit - the raw skb
> > gso type is used rather then the translated one. This leads to large packets
> > being handed to the guest without the GSO metadata. This patch fixes the
> > check.
> >
> > The mistake manifested as errors whilst running Microsoft HCK large packet
> > offload tests between a pair of Windows 8 VMs. I have verified this patch
> > fixes those errors.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > ---
> >  drivers/net/xen-netback/netback.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> > index 56c3e5a..999e290 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -452,7 +452,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
> >  	}
> >
> >  	/* Set up a GSO prefix descriptor, if necessary */
> > -	if ((1 << skb_shinfo(skb)->gso_type) & vif->gso_prefix_mask) {
> > +	if ((1 << gso_type) & vif->gso_prefix_mask) {
> 
> gso_type here is XEN_NETIF_GSO_TYPE_TCPV4, vif->gso_prefix_mask is
> initialised with GSO_BIT(TCPV4) et al. Where
>         #define GSO_BIT(type) \
>                 (1 << XEN_NETIF_GSO_TYPE_ ## type)
> 
> So,
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> But it might be nice to use BIT() from bitops.h for both GSO_BIT and the
> check, since it took me quite a bit of wandering around to figure out
> how these ended up the same.
> 

Ok. I'll come up with a separate patch for that.

  Paul

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] xen-netback: fix gso_prefix check
  2013-12-12 14:42 ` Ian Campbell
  2013-12-12 14:44   ` Paul Durrant
@ 2013-12-12 20:48   ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2013-12-12 20:48 UTC (permalink / raw)
  To: Ian.Campbell; +Cc: paul.durrant, xen-devel, netdev, wei.liu2, david.vrabel

From: Ian Campbell <Ian.Campbell@citrix.com>
Date: Thu, 12 Dec 2013 14:42:22 +0000

> On Thu, 2013-12-12 at 14:20 +0000, Paul Durrant wrote:
>> There is a mistake in checking the gso_prefix mask when passing large
>> packets to a guest. The wrong shift is applied to the bit - the raw skb
>> gso type is used rather then the translated one. This leads to large packets
>> being handed to the guest without the GSO metadata. This patch fixes the
>> check.
>> 
>> The mistake manifested as errors whilst running Microsoft HCK large packet
>> offload tests between a pair of Windows 8 VMs. I have verified this patch
>> fixes those errors.
>> 
>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: David Vrabel <david.vrabel@citrix.com>
>> ---
>>  drivers/net/xen-netback/netback.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
>> index 56c3e5a..999e290 100644
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -452,7 +452,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>>  	}
>>  
>>  	/* Set up a GSO prefix descriptor, if necessary */
>> -	if ((1 << skb_shinfo(skb)->gso_type) & vif->gso_prefix_mask) {
>> +	if ((1 << gso_type) & vif->gso_prefix_mask) {
> 
> gso_type here is XEN_NETIF_GSO_TYPE_TCPV4, vif->gso_prefix_mask is
> initialised with GSO_BIT(TCPV4) et al. Where
>         #define GSO_BIT(type) \
>                 (1 << XEN_NETIF_GSO_TYPE_ ## type)
>         
> So, 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-12-12 20:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-12 14:20 [PATCH net] xen-netback: fix gso_prefix check Paul Durrant
2013-12-12 14:42 ` Ian Campbell
2013-12-12 14:44   ` Paul Durrant
2013-12-12 20:48   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).