Date: Mon, 27 May 2019 21:24:04 +0000 From: "v.maffione_gmail.com (Vincenzo Maffione)" <phabric-noreply@FreeBSD.org> To: Phabricator <phabric-noreply@FreeBSD.org> Cc: freebsd-virtualization@freebsd.org Subject: [Differential] D20276: [bhyve][virtio-net] Allow guest VM's to set JUMBO MTU in case of using the VALE switch. Message-ID: <b1e32866b0cc2f7335ae50272a310e45@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-pueoqfdkfh54jyuh6emz-req@reviews.freebsd.org> References: <differential-rev-PHID-DREV-pueoqfdkfh54jyuh6emz-req@reviews.freebsd.org>
index | next in thread | previous in thread | raw e-mail
v.maffione_gmail.com added a comment.
Yes, but I think the story is more complex than that. Guests can publish single-descriptor chains or multi-descriptor chains, depending on feature negotiation and driver implementation. With mergeable buffers enabled, or TSO features disabled, guests will normally submit single-descriptor chains (because it makes sense), but this is not mandatory. With TSO enabled and mergeable buffers disabled, guests will normally pass in multi-descriptor chains, each one describing 64K or 32K buffers.
It makes sense to add more vq methods to handle the mergeable rx bufs case, but think they should be moved to usr.sbin/bhyve/virtio.c, so that they can be reused.
INLINE COMMENTS
> pci_virtio_net.c:525
> +
> + if (ndesc < minavail)
> + return (0);
why this check? if there are not enough descriptors to cover 'len' bytes the function will return 0 anyway.
> pci_virtio_net.c:526
> + if (ndesc < minavail)
> + return (0);
> +
You are mixing declarations and code here. Is it allowed?
> pci_virtio_net.c:670
>
> - /* Interrupt if needed, including for NOTIFY_ON_EMPTY. */
> + vq_inc_used_idx_and_last_avail(vq, used);
> vq_endchains(vq, 1);
I think you should call vq_inc_used_idx_and_last_avail() after each packet, otherwise you are introducing artificial latency. As a side effect, this allows you to remove the "start" argument ("used" local variable) and the used local variable.
> pci_virtio_net.c:673
> +
> + return;
> }
why this return?
CHANGES SINCE LAST ACTION
https://reviews.freebsd.org/D20276/new/
REVISION DETAIL
https://reviews.freebsd.org/D20276
EMAIL PREFERENCES
https://reviews.freebsd.org/settings/panel/emailpreferences/
To: aleksandr.fedorov_itglobal.com, #bhyve, jhb, rgrimes, krion, v.maffione_gmail.com
Cc: mizhka_gmail.com, novel, olevole_olevole.ru, freebsd-virtualization-list, evgueni.gavrilov_itglobal.com, bcran
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b1e32866b0cc2f7335ae50272a310e45>