| Message ID | 1308947605-4300-1-git-send-email-jesse@nicira.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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
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
> 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.
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
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; }
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(-)