[dpdk-dev,5/6] vhost: add a flag to enable Tx zero copy
Commit Message
Just curious about the naming: vhost USER TX Zero copy. In fact, it's Vhost RX zero-copy
For virtio, it's Virtio TX zero-copy. So, I wonder why we call it as Vhost TX ZERO-COPY,
Any comments?
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yuanhan Liu
Sent: Tuesday, August 23, 2016 4:11 PM
To: dev@dpdk.org
Cc: Maxime Coquelin; Yuanhan Liu
Subject: [dpdk-dev] [PATCH 5/6] vhost: add a flag to enable Tx zero copy
Add a new flag ``RTE_VHOST_USER_TX_ZERO_COPY`` to explictily enable
Tx zero copy. If not given, Tx zero copy is disabled by default.
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
doc/guides/prog_guide/vhost_lib.rst | 7 ++++++-
lib/librte_vhost/rte_virtio_net.h | 1 +
lib/librte_vhost/socket.c | 5 +++++
lib/librte_vhost/vhost.c | 10 ++++++++++
lib/librte_vhost/vhost.h | 1 +
5 files changed, 23 insertions(+), 1 deletion(-)
Comments
On Tue, Sep 06, 2016 at 09:00:14AM +0000, Xu, Qian Q wrote:
> Just curious about the naming: vhost USER TX Zero copy. In fact, it's Vhost RX zero-copy
> For virtio, it's Virtio TX zero-copy. So, I wonder why we call it as Vhost TX ZERO-COPY,
> Any comments?
It's just that "Tx zero copy" looks more nature to me (yes, I took the
name from the virtio point of view).
Besides that, naming it to "vhost Rx zero copy" would be a little
weird, based on we have functions like "virtio_dev_rx" in the enqueue
path while here we just touch dequeue path.
OTOH, I seldome say "vhost-user Tx zero copy"; I normally mention it
as "Tx zero copy", without mentioning "vhost-user". For the flag
RTE_VHOST_USER_TX_ZERO_COPY, all vhost-user flags start with "RTE_VHOST_USER_"
prefix.
--yliu
2016-09-06 17:55, Yuanhan Liu:
> On Tue, Sep 06, 2016 at 09:00:14AM +0000, Xu, Qian Q wrote:
> > Just curious about the naming: vhost USER TX Zero copy. In fact, it's Vhost RX zero-copy
> > For virtio, it's Virtio TX zero-copy. So, I wonder why we call it as Vhost TX ZERO-COPY,
> > Any comments?
>
> It's just that "Tx zero copy" looks more nature to me (yes, I took the
> name from the virtio point of view).
>
> Besides that, naming it to "vhost Rx zero copy" would be a little
> weird, based on we have functions like "virtio_dev_rx" in the enqueue
> path while here we just touch dequeue path.
>
> OTOH, I seldome say "vhost-user Tx zero copy"; I normally mention it
> as "Tx zero copy", without mentioning "vhost-user". For the flag
> RTE_VHOST_USER_TX_ZERO_COPY, all vhost-user flags start with "RTE_VHOST_USER_"
> prefix.
I agree that the naming in vhost code is quite confusing.
It would be better to define a terminology and stop mixing virtio/vhost
directions as well as Rx/Tx and enqueue/dequeue.
Or at least, it should be documented.
On Wed, Sep 07, 2016 at 06:00:36PM +0200, Thomas Monjalon wrote:
> 2016-09-06 17:55, Yuanhan Liu:
> > On Tue, Sep 06, 2016 at 09:00:14AM +0000, Xu, Qian Q wrote:
> > > Just curious about the naming: vhost USER TX Zero copy. In fact, it's Vhost RX zero-copy
> > > For virtio, it's Virtio TX zero-copy. So, I wonder why we call it as Vhost TX ZERO-COPY,
> > > Any comments?
> >
> > It's just that "Tx zero copy" looks more nature to me (yes, I took the
> > name from the virtio point of view).
> >
> > Besides that, naming it to "vhost Rx zero copy" would be a little
> > weird, based on we have functions like "virtio_dev_rx" in the enqueue
> > path while here we just touch dequeue path.
> >
> > OTOH, I seldome say "vhost-user Tx zero copy"; I normally mention it
> > as "Tx zero copy", without mentioning "vhost-user". For the flag
> > RTE_VHOST_USER_TX_ZERO_COPY, all vhost-user flags start with "RTE_VHOST_USER_"
> > prefix.
>
> I agree that the naming in vhost code is quite confusing.
> It would be better to define a terminology and stop mixing virtio/vhost
> directions as well as Rx/Tx and enqueue/dequeue.
I think we could/should avoid using Rx/Tx in vhost, but we should keep
the enqueue/dequeue: that's how the two key vhost API named.
> Or at least, it should be documented.
Or, how about renaming it to RTE_VHOST_USER_DEQUEUE_ZERO_COPY, to align
with the function name rte_vhost_dequeue_burst?
--yliu
2016-09-08 15:21, Yuanhan Liu:
> On Wed, Sep 07, 2016 at 06:00:36PM +0200, Thomas Monjalon wrote:
> > 2016-09-06 17:55, Yuanhan Liu:
> > > On Tue, Sep 06, 2016 at 09:00:14AM +0000, Xu, Qian Q wrote:
> > > > Just curious about the naming: vhost USER TX Zero copy. In fact, it's Vhost RX zero-copy
> > > > For virtio, it's Virtio TX zero-copy. So, I wonder why we call it as Vhost TX ZERO-COPY,
> > > > Any comments?
> > >
> > > It's just that "Tx zero copy" looks more nature to me (yes, I took the
> > > name from the virtio point of view).
> > >
> > > Besides that, naming it to "vhost Rx zero copy" would be a little
> > > weird, based on we have functions like "virtio_dev_rx" in the enqueue
> > > path while here we just touch dequeue path.
> > >
> > > OTOH, I seldome say "vhost-user Tx zero copy"; I normally mention it
> > > as "Tx zero copy", without mentioning "vhost-user". For the flag
> > > RTE_VHOST_USER_TX_ZERO_COPY, all vhost-user flags start with "RTE_VHOST_USER_"
> > > prefix.
> >
> > I agree that the naming in vhost code is quite confusing.
> > It would be better to define a terminology and stop mixing virtio/vhost
> > directions as well as Rx/Tx and enqueue/dequeue.
>
> I think we could/should avoid using Rx/Tx in vhost, but we should keep
> the enqueue/dequeue: that's how the two key vhost API named.
>
> > Or at least, it should be documented.
>
> Or, how about renaming it to RTE_VHOST_USER_DEQUEUE_ZERO_COPY, to align
> with the function name rte_vhost_dequeue_burst?
Seems reasonable, yes.
@@ -79,7 +79,7 @@ The following is an overview of the Vhost API functions:
``/dev/path`` character device file will be created. For vhost-user server
mode, a Unix domain socket file ``path`` will be created.
- Currently two flags are supported (these are valid for vhost-user only):
+ Currently supported flags are (these are valid for vhost-user only):
- ``RTE_VHOST_USER_CLIENT``
@@ -97,6 +97,11 @@ The following is an overview of the Vhost API functions:
This reconnect option is enabled by default. However, it can be turned off
by setting this flag.
+ - ``RTE_VHOST_USER_TX_ZERO_COPY``
+
+ Tx zero copy will be enabled when this flag is set. It is disabled by
+ default.
+
* ``rte_vhost_driver_session_start()``
This function starts the vhost session loop to handle vhost messages. It
@@ -53,6 +53,7 @@
#define RTE_VHOST_USER_CLIENT (1ULL << 0)
#define RTE_VHOST_USER_NO_RECONNECT (1ULL << 1)
+#define RTE_VHOST_USER_TX_ZERO_COPY (1ULL << 2)
/* Enum for virtqueue management. */
enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
@@ -62,6 +62,7 @@ struct vhost_user_socket {
int connfd;
bool is_server;
bool reconnect;
+ bool tx_zero_copy;
};
struct vhost_user_connection {
@@ -203,6 +204,9 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket)
size = strnlen(vsocket->path, PATH_MAX);
vhost_set_ifname(vid, vsocket->path, size);
+ if (vsocket->tx_zero_copy)
+ vhost_enable_tx_zero_copy(vid);
+
RTE_LOG(INFO, VHOST_CONFIG, "new device, handle is %d\n", vid);
vsocket->connfd = fd;
@@ -499,6 +503,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
memset(vsocket, 0, sizeof(struct vhost_user_socket));
vsocket->path = strdup(path);
vsocket->connfd = -1;
+ vsocket->tx_zero_copy = flags & RTE_VHOST_USER_TX_ZERO_COPY;
if ((flags & RTE_VHOST_USER_CLIENT) != 0) {
vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT);
@@ -290,6 +290,16 @@ vhost_set_ifname(int vid, const char *if_name, unsigned int if_len)
dev->ifname[sizeof(dev->ifname) - 1] = '\0';
}
+void
+vhost_enable_tx_zero_copy(int vid)
+{
+ struct virtio_net *dev = get_device(vid);
+
+ if (dev == NULL)
+ return;
+
+ dev->tx_zero_copy = 1;
+}
int
rte_vhost_get_numa_node(int vid)
@@ -279,6 +279,7 @@ void vhost_destroy_device(int);
int alloc_vring_queue_pair(struct virtio_net *dev, uint32_t qp_idx);
void vhost_set_ifname(int, const char *if_name, unsigned int if_len);
+void vhost_enable_tx_zero_copy(int vid);
/*
* Backend-specific cleanup. Defined by vhost-cuse and vhost-user.