bhyve: enable virtio-net mergeable rx buffers for tap(4)
ClosedPublic
Actions

Authored by vmaffione on Feb 2 2020, 10:00 PM.

Details

Reviewers

grehan
jhb
bryanv
markj
afedorov

Group Reviewers

bhyve

Commits

rS358438: MFC r358180
rS358180: bhyve: enable virtio-net mergeable rx buffers for tap(4)

Summary

This patch adds a new netbe_peek_recvlen() function to the net backend API. The new function allows the virtio-net receive code to know in advance how many virtio descriptors chains will be needed to receive the next packet. As a result, the implementation of the virtio-net mergeable rx buffers feature becomes efficient, so that we can enable it also with the tap(4) backend. For the tap(4) backend, a bounce buffer was introduced to implement the peeck_recvlen() callback, which means an additional packet copy on the receive datapath. In the future, it should be possible to remove the bounce buffer (and so the additional copy), by obtaining the length of the next packet from kevent data.

Test Plan

Tested VM-2-VM with tap(4) and vale(4).

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 29362

Event Timeline

vmaffione created this revision.Feb 2 2020, 10:00 PM

Herald added a reviewer: bhyve. · View Herald TranscriptFeb 2 2020, 10:00 PM

Herald added subscribers: bcran, rgrimes, imp. · View Herald Transcript

Harbormaster completed remote builds in B29113: Diff 67671.Feb 2 2020, 10:00 PM

afedorov added inline comments.Feb 7 2020, 9:24 AM

usr.sbin/bhyve/net_backends.c
310	I think we can replace this cycle with call buf_to_iov(priv->bbuf, priv->bbuflen, iov, iovcnt, 0) https://svnweb.freebsd.org/base/head/usr.sbin/bhyve/iov.c?view=markup#l122

vmaffione marked an inline comment as done.Feb 8 2020, 2:01 PM

vmaffione added inline comments.

usr.sbin/bhyve/net_backends.c
310	Good catch, thanks.

vmaffione updated this revision to Diff 67977.Feb 8 2020, 2:01 PM

vmaffione marked an inline comment as done.

Has anybody tested this on e1000?

Fix: Keep into account virtio-net header when collecting rx buffers.

Harbormaster completed remote builds in B29362: Diff 68280.Feb 13 2020, 8:55 PM

Ping. Any comments on this change?

Looks good.

This revision is now accepted and ready to land.Feb 18 2020, 11:45 AM

Sorry, I didn’t have time to test the patch. But as for the code, I have no questions.

In D23472#521553, @aleksandr.fedorov_itglobal.com wrote:

Sorry, I didn’t have time to test the patch. But as for the code, I have no questions.

It's fine with me if you want some time to test it.

In any case I tested 10 different combinations:

VM-2-VM + tap + virtio-net + mtu 1500
VM-2-VM + tap + virtio-net + mtu 9000
VM-2-VM + tap + e1000 + mtu 1500
VM-2-VM + tap + e1000 + mtu 9000
VM-2-VM + vale + virtio-net + mtu 1500
VM-2-VM + vale + virtio-net + mtu 9000
VM-2-VM + vale + e1000 + mtu 1500
VM-2-VM + vale + e1000 + mtu 9000
VM-2-host + tap + virtio-net + mtu 1500
VM-2-host + tap + virtio-net + mtu 9000

each one with ping (-D) and netperf. Everything looks good.

Ok, I'll just go ahead with the commit.

Closed by commit rS358180: bhyve: enable virtio-net mergeable rx buffers for tap(4) (authored by vmaffione). · Explain WhyFeb 20 2020, 9:07 PM

This revision was automatically updated to reflect the committed changes.

vmaffione added a commit: rS358180: bhyve: enable virtio-net mergeable rx buffers for tap(4).

I tested this patch with various OS’s(windows, Ubuntu, centos) and didn’t find any issues. So, I think it’s ready to go.

In D23472#522420, @aleksandr.fedorov_itglobal.com wrote:

I tested this patch with various OS’s(windows, Ubuntu, centos) and didn’t find any issues. So, I think it’s ready to go.

Thanks!

vmaffione added a commit: rS358438: MFC r358180.Feb 28 2020, 5:41 PM