Patchwork vmxnet3: Enable GRO support.

login
register
mail settings
Submitter Jesse Gross
Date June 24, 2011, 8:33 p.m.
Message ID <1308947605-4300-1-git-send-email-jesse@nicira.com>
Download mbox | patch
Permalink /patch/101893/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jesse Gross - June 24, 2011, 8:33 p.m.
When receiving packets from another guest on the same hypervisor, it's
generally possible to receive large packets because no segmentation is
necessary and these packets are handled by LRO.  However, when doing
routing or bridging we must disable LRO and lose this benefit.  In
these cases GRO can still be used and it is very effective because the
packets which are segmented in the hypervisor are received very close
together and can easily be merged.

CC: Shreyas Bhatewara <sbhatewara@vmware.com>
CC: VMware PV-Drivers <pv-drivers@vmware.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
This applies on top of my previous vmxnet3 patch
"vmxnet3: Convert to new vlan model."
---
 drivers/net/vmxnet3/vmxnet3_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Scott J. Goldman - June 24, 2011, 10:02 p.m.
Hi Jesse.

 
> When receiving packets from another guest on the same hypervisor, it's
> generally possible to receive large packets because no segmentation is
> necessary and these packets are handled by LRO.  However, when doing
> routing or bridging we must disable LRO and lose this benefit.  In
> these cases GRO can still be used and it is very effective because the
> packets which are segmented in the hypervisor are received very close
> together and can easily be merged.


> -			netif_receive_skb(skb);
> +			napi_gro_receive(&rq->napi, skb);

So... this doesn't discriminate between if LRO is off or on.  The last time I tried using GRO on top of our hardware LRO, there was actually some minor performance penalty. Do you have any benchmarks showing that this is ok? If not, do you think it might make sense to just do gro only if(unlikely(lro is off))?

Thanks,
-sjg
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesse Gross - June 24, 2011, 10:47 p.m.
On Fri, Jun 24, 2011 at 3:02 PM, Scott Goldman <scottjg@vmware.com> wrote:
>> -                     netif_receive_skb(skb);
>> +                     napi_gro_receive(&rq->napi, skb);
>
> So... this doesn't discriminate between if LRO is off or on.  The last time I tried using GRO on top of our hardware LRO, there was actually some minor performance penalty. Do you have any benchmarks showing that this is ok? If not, do you think it might make sense to just do gro only if(unlikely(lro is off))?

I ran some benchmarks and do see a slight performance drop with GRO
when LRO is also on, so it seems reasonable to avoid it in that
situation.  I can resubmit with that change.

As an aside, in many cases the hypervisor actually has all of the
information that is necessary to keep LRO on but does not provide it
to the guest.  For example, in the VM-to-VM case the MSS is provided
by the sender as part of the TSO descriptor and if given to the
receiver we could generate a GSO frame and avoid the need to do GRO in
the first place.  Do you know if it is possible to do this?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott J. Goldman - June 24, 2011, 11:23 p.m.
> I ran some benchmarks and do see a slight performance drop with GRO

> when LRO is also on, so it seems reasonable to avoid it in that

> situation.  I can resubmit with that change.


Cool, thanks.

> As an aside, in many cases the hypervisor actually has all of the

> information that is necessary to keep LRO on but does not provide it

> to the guest.  For example, in the VM-to-VM case the MSS is provided

> by the sender as part of the TSO descriptor and if given to the

> receiver we could generate a GSO frame and avoid the need to do GRO in

> the first place.  Do you know if it is possible to do this?


I think that sounds like a pretty good idea, but if I understand correctly, that change needs to go in the hypervisor, not just the driver. The device emulation backend needs to populate that MSS somewhere in the receive descriptor. I will file an internal bug about it, but just to set expectations, ESX 5.0 is about to be released, so realistically at the earliest, this change may not be publically available for another year.
	
-sjg

P.S. Ronghua, the vmxnet3 mastermind, works at Nicira now. If you see him, tell him I said hi.
Jesse Gross - June 25, 2011, 12:25 a.m.
On Fri, Jun 24, 2011 at 4:23 PM, Scott Goldman <scottjg@vmware.com> wrote:
>> As an aside, in many cases the hypervisor actually has all of the
>> information that is necessary to keep LRO on but does not provide it
>> to the guest.  For example, in the VM-to-VM case the MSS is provided
>> by the sender as part of the TSO descriptor and if given to the
>> receiver we could generate a GSO frame and avoid the need to do GRO in
>> the first place.  Do you know if it is possible to do this?
>
> I think that sounds like a pretty good idea, but if I understand correctly, that change needs to go in the hypervisor, not just the driver. The device emulation backend needs to populate that MSS somewhere in the receive descriptor. I will file an internal bug about it, but just to set expectations, ESX 5.0 is about to be released, so realistically at the earliest, this change may not be publically available for another year.

I had hoped that maybe the information was already being populated by
the hypervisor but not used by the driver.  However, from what I have
gathered that's not the case.

> P.S. Ronghua, the vmxnet3 mastermind, works at Nicira now. If you see him, tell him I said hi.

Sure, I'll say hi next time I see him.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index c84b1dd..5353429 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1234,7 +1234,7 @@  vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
 			if (unlikely(rcd->ts))
 				__vlan_hwaccel_put_tag(skb, rcd->tci);
 
-			netif_receive_skb(skb);
+			napi_gro_receive(&rq->napi, skb);
 
 			ctx->skb = NULL;
 		}