| Message ID | 20181123030016.4924-3-jasowang@redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | basic in order support for vhost_net | expand |
On Fri, Nov 23, 2018 at 11:00:15AM +0800, Jason Wang wrote: > This makes vhost_net to support in order feature. This is as simple as > use datacopy path when it was negotiated. An alternative is not to > advertise in order when zerocopy is enabled which tends to be > suboptimal consider zerocopy may suffer from e.g HOL issues. Well IIRC vhost_zerocopy_signal_used is used to actually reorder used ring to match available ring. So with a big comment explaining why it is so, we could just enable IN_ORDER there too. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vhost/net.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index d919284f103b..bdf5de5a7eb2 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -74,7 +74,8 @@ enum { > VHOST_NET_FEATURES = VHOST_FEATURES | > (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) | > (1ULL << VIRTIO_NET_F_MRG_RXBUF) | > - (1ULL << VIRTIO_F_IOMMU_PLATFORM) > + (1ULL << VIRTIO_F_IOMMU_PLATFORM) | > + (1ULL << VIRTIO_F_IN_ORDER) > }; > > enum { > @@ -971,7 +972,8 @@ static void handle_tx(struct vhost_net *net) > vhost_disable_notify(&net->dev, vq); > vhost_net_disable_vq(net, vq); > > - if (vhost_sock_zcopy(sock)) > + if (vhost_sock_zcopy(sock) && > + !vhost_has_feature(vq, VIRTIO_F_IN_ORDER)) > handle_tx_zerocopy(net, sock); > else > handle_tx_copy(net, sock); > -- > 2.17.1
On 2018/11/23 下午11:49, Michael S. Tsirkin wrote: > On Fri, Nov 23, 2018 at 11:00:15AM +0800, Jason Wang wrote: >> This makes vhost_net to support in order feature. This is as simple as >> use datacopy path when it was negotiated. An alternative is not to >> advertise in order when zerocopy is enabled which tends to be >> suboptimal consider zerocopy may suffer from e.g HOL issues. > Well IIRC vhost_zerocopy_signal_used is used to > actually reorder used ring to match available ring. > So with a big comment explaining why it is so, > we could just enable IN_ORDER there too. > The problem is we allow switching between zerocopy and datacopy. And what's more important, if we allow in order for zerocopy, a single packet delay may hang all the rest. Thanks
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index d919284f103b..bdf5de5a7eb2 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -74,7 +74,8 @@ enum { VHOST_NET_FEATURES = VHOST_FEATURES | (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) | (1ULL << VIRTIO_NET_F_MRG_RXBUF) | - (1ULL << VIRTIO_F_IOMMU_PLATFORM) + (1ULL << VIRTIO_F_IOMMU_PLATFORM) | + (1ULL << VIRTIO_F_IN_ORDER) }; enum { @@ -971,7 +972,8 @@ static void handle_tx(struct vhost_net *net) vhost_disable_notify(&net->dev, vq); vhost_net_disable_vq(net, vq); - if (vhost_sock_zcopy(sock)) + if (vhost_sock_zcopy(sock) && + !vhost_has_feature(vq, VIRTIO_F_IN_ORDER)) handle_tx_zerocopy(net, sock); else handle_tx_copy(net, sock);
This makes vhost_net to support in order feature. This is as simple as use datacopy path when it was negotiated. An alternative is not to advertise in order when zerocopy is enabled which tends to be suboptimal consider zerocopy may suffer from e.g HOL issues. Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/vhost/net.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)