* [dpdk-dev] [PATCH 0/3] net/virtio: add Vhost-user protocol features
@ 2020-05-28 7:46 Maxime Coquelin
2020-05-28 7:46 ` [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support Maxime Coquelin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Maxime Coquelin @ 2020-05-28 7:46 UTC (permalink / raw)
To: dev, amorenoz, xiaolong.ye, shahafs, matan; +Cc: Maxime Coquelin
This series adds Vhost-user protocol features support
to Virtio-user PMD's Vhost-user backend.
The first patch introduces protocol features
negotiation, and the second one reply-ack feature.
The third patch adds support to the SET_STATUS
request, which is not merged upstream yet. Neither
in DPDK Vhost-user backend nor in Qemu.
So the first two patches can be applied as-is, while
for the last one, we have at least to wait the Qemu bits,
which includes the specification update, is merged
upstream.
Maxime Coquelin (3):
net/virtio: add vhost-user protocol features support
net/virtio: add reply-ack support to Virtio-user
net/virtio: add Virtio status support to Virtio-user
drivers/net/virtio/virtio_user/vhost.h | 18 ++++++
drivers/net/virtio/virtio_user/vhost_user.c | 35 ++++++++++-
.../net/virtio/virtio_user/virtio_user_dev.c | 58 ++++++++++++++++++-
.../net/virtio/virtio_user/virtio_user_dev.h | 4 ++
drivers/net/virtio/virtio_user_ethdev.c | 20 +++++++
5 files changed, 130 insertions(+), 5 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support 2020-05-28 7:46 [dpdk-dev] [PATCH 0/3] net/virtio: add Vhost-user protocol features Maxime Coquelin @ 2020-05-28 7:46 ` Maxime Coquelin 2020-06-17 11:57 ` Xia, Chenbo 2020-05-28 7:46 ` [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 3/3] net/virtio: add Virtio status " Maxime Coquelin 2 siblings, 1 reply; 6+ messages in thread From: Maxime Coquelin @ 2020-05-28 7:46 UTC (permalink / raw) To: dev, amorenoz, xiaolong.ye, shahafs, matan; +Cc: Maxime Coquelin This patch adds support for Vhost-user protocol features. It is required to support protocol features that were not in initial Vhost-user specification, such as reply-ack, MTU... Also, this patch prevents Virtio multiqueue feature negotiation if the slave does not support MQ protocol feature as stated in Vhost-user specification: "The multiple queues feature is supported only when the protocol feature ``VHOST_USER_PROTOCOL_F_MQ`` (bit 0) is set." Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost.h | 9 +++++ drivers/net/virtio/virtio_user/vhost_user.c | 3 ++ .../net/virtio/virtio_user/virtio_user_dev.c | 39 ++++++++++++++++++- .../net/virtio/virtio_user/virtio_user_dev.h | 3 ++ drivers/net/virtio/virtio_user_ethdev.c | 19 +++++++++ 5 files changed, 71 insertions(+), 2 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h index 1e784e58ef..9ace1a90c3 100644 --- a/drivers/net/virtio/virtio_user/vhost.h +++ b/drivers/net/virtio/virtio_user/vhost.h @@ -44,6 +44,15 @@ struct vhost_vring_addr { uint64_t log_guest_addr; }; +#ifndef VHOST_USER_F_PROTOCOL_FEATURES +#define VHOST_USER_F_PROTOCOL_FEATURES 30 +#endif + +/** Protocol features. */ +#ifndef VHOST_USER_PROTOCOL_F_MQ +#define VHOST_USER_PROTOCOL_F_MQ 0 +#endif + enum vhost_user_request { VHOST_USER_NONE = 0, VHOST_USER_GET_FEATURES = 1, diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index 74b82e56e4..b687665042 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -269,10 +269,12 @@ vhost_user_sock(struct virtio_user_dev *dev, switch (req) { case VHOST_USER_GET_FEATURES: + case VHOST_USER_GET_PROTOCOL_FEATURES: need_reply = 1; break; case VHOST_USER_SET_FEATURES: + case VHOST_USER_SET_PROTOCOL_FEATURES: case VHOST_USER_SET_LOG_BASE: msg.payload.u64 = *((__u64 *)arg); msg.size = sizeof(m.payload.u64); @@ -351,6 +353,7 @@ vhost_user_sock(struct virtio_user_dev *dev, switch (req) { case VHOST_USER_GET_FEATURES: + case VHOST_USER_GET_PROTOCOL_FEATURES: if (msg.size != sizeof(m.payload.u64)) { PMD_DRV_LOG(ERR, "Received bad msg size"); return -1; diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 7fb135f49a..3afb09df2d 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -151,8 +151,10 @@ virtio_user_start_device(struct virtio_user_dev *dev) if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0) goto error; - /* Step 1: set features */ + /* Step 1: negotiate protocol features & set features */ features = dev->features; + + /* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */ features &= ~(1ull << VIRTIO_NET_F_MAC); /* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */ @@ -417,13 +419,19 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) 1ULL << VIRTIO_NET_F_GUEST_TSO6 | \ 1ULL << VIRTIO_F_IN_ORDER | \ 1ULL << VIRTIO_F_VERSION_1 | \ - 1ULL << VIRTIO_F_RING_PACKED) + 1ULL << VIRTIO_F_RING_PACKED | \ + 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) + +#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ + (1ULL << VHOST_USER_PROTOCOL_F_MQ) int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, int cq, int queue_size, const char *mac, char **ifname, int server, int mrg_rxbuf, int in_order, int packed_vq) { + uint64_t protocol_features = 0; + pthread_mutex_init(&dev->mutex, NULL); strlcpy(dev->path, path, PATH_MAX); dev->started = 0; @@ -434,6 +442,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, dev->mac_specified = 0; dev->frontend_features = 0; dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; + dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; parse_mac(dev, mac); if (*ifname) { @@ -446,6 +455,10 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, return -1; } + if (!is_vhost_user_by_type(dev->path)) + dev->unsupported_features |= + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES); + if (!dev->is_server) { if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER, NULL) < 0) { @@ -460,6 +473,26 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, strerror(errno)); return -1; } + + + if (dev->device_features & + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { + if (dev->ops->send_request(dev, + VHOST_USER_GET_PROTOCOL_FEATURES, + &protocol_features)) + return -1; + } + + dev->protocol_features &= protocol_features; + + if (dev->ops->send_request(dev, + VHOST_USER_SET_PROTOCOL_FEATURES, + &dev->protocol_features)) + return -1; + + if (!(dev->protocol_features & + (1ULL << VHOST_USER_PROTOCOL_F_MQ))) + dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ); } else { /* We just pretend vhost-user can support all these features. * Note that this could be problematic that if some feature is @@ -469,6 +502,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES; } + + if (!mrg_rxbuf) dev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF); diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h index 3b6b6065a5..56e638f8a6 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h @@ -40,6 +40,9 @@ struct virtio_user_dev { uint64_t device_features; /* supported features by device */ uint64_t frontend_features; /* enabled frontend features */ uint64_t unsupported_features; /* unsupported features mask */ + uint64_t protocol_features; /* negotiated protocol features + * (Vhost-user only) + */ uint8_t status; uint16_t port_id; uint8_t mac_addr[RTE_ETHER_ADDR_LEN]; diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 798f191c32..ccb5a18e25 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -68,6 +68,7 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) int connectfd; struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id]; struct virtio_hw *hw = eth_dev->data->dev_private; + uint64_t protocol_features; connectfd = accept(dev->listenfd, NULL, NULL); if (connectfd < 0) @@ -81,6 +82,24 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) return -1; } + if (dev->device_features & + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { + if (dev->ops->send_request(dev, + VHOST_USER_GET_PROTOCOL_FEATURES, + &protocol_features)) + return -1; + + dev->protocol_features &= protocol_features; + + if (dev->ops->send_request(dev, + VHOST_USER_SET_PROTOCOL_FEATURES, + &dev->protocol_features)) + return -1; + } + + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) + dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ); + dev->device_features |= dev->frontend_features; /* umask vhost-user unsupported features */ -- 2.26.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support 2020-05-28 7:46 ` [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support Maxime Coquelin @ 2020-06-17 11:57 ` Xia, Chenbo 2020-06-17 12:22 ` Maxime Coquelin 0 siblings, 1 reply; 6+ messages in thread From: Xia, Chenbo @ 2020-06-17 11:57 UTC (permalink / raw) To: Maxime Coquelin, dev, amorenoz, Ye, Xiaolong, shahafs, matan Hi Maxime, > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin > Sent: Thursday, May 28, 2020 3:46 PM > To: dev@dpdk.org; amorenoz@redhat.com; Ye, Xiaolong > <xiaolong.ye@intel.com>; shahafs@mellanox.com; matan@mellanox.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features > support > > This patch adds support for Vhost-user protocol features. > It is required to support protocol features that were not in initial Vhost-user > specification, such as reply-ack, MTU... > > Also, this patch prevents Virtio multiqueue feature negotiation if the slave does > not support MQ protocol feature as stated in Vhost-user specification: > "The multiple queues feature is supported only when the protocol feature > ``VHOST_USER_PROTOCOL_F_MQ`` (bit 0) is set." > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/net/virtio/virtio_user/vhost.h | 9 +++++ > drivers/net/virtio/virtio_user/vhost_user.c | 3 ++ > .../net/virtio/virtio_user/virtio_user_dev.c | 39 ++++++++++++++++++- > .../net/virtio/virtio_user/virtio_user_dev.h | 3 ++ > drivers/net/virtio/virtio_user_ethdev.c | 19 +++++++++ > 5 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/vhost.h > b/drivers/net/virtio/virtio_user/vhost.h > index 1e784e58ef..9ace1a90c3 100644 > --- a/drivers/net/virtio/virtio_user/vhost.h > +++ b/drivers/net/virtio/virtio_user/vhost.h > @@ -44,6 +44,15 @@ struct vhost_vring_addr { > uint64_t log_guest_addr; > }; > > +#ifndef VHOST_USER_F_PROTOCOL_FEATURES > +#define VHOST_USER_F_PROTOCOL_FEATURES 30 #endif > + > +/** Protocol features. */ > +#ifndef VHOST_USER_PROTOCOL_F_MQ > +#define VHOST_USER_PROTOCOL_F_MQ 0 > +#endif > + > enum vhost_user_request { > VHOST_USER_NONE = 0, > VHOST_USER_GET_FEATURES = 1, > diff --git a/drivers/net/virtio/virtio_user/vhost_user.c > b/drivers/net/virtio/virtio_user/vhost_user.c > index 74b82e56e4..b687665042 100644 > --- a/drivers/net/virtio/virtio_user/vhost_user.c > +++ b/drivers/net/virtio/virtio_user/vhost_user.c > @@ -269,10 +269,12 @@ vhost_user_sock(struct virtio_user_dev *dev, > > switch (req) { > case VHOST_USER_GET_FEATURES: > + case VHOST_USER_GET_PROTOCOL_FEATURES: > need_reply = 1; > break; > > case VHOST_USER_SET_FEATURES: > + case VHOST_USER_SET_PROTOCOL_FEATURES: > case VHOST_USER_SET_LOG_BASE: > msg.payload.u64 = *((__u64 *)arg); > msg.size = sizeof(m.payload.u64); > @@ -351,6 +353,7 @@ vhost_user_sock(struct virtio_user_dev *dev, > > switch (req) { > case VHOST_USER_GET_FEATURES: > + case VHOST_USER_GET_PROTOCOL_FEATURES: > if (msg.size != sizeof(m.payload.u64)) { > PMD_DRV_LOG(ERR, "Received bad msg size"); > return -1; > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > b/drivers/net/virtio/virtio_user/virtio_user_dev.c > index 7fb135f49a..3afb09df2d 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > @@ -151,8 +151,10 @@ virtio_user_start_device(struct virtio_user_dev *dev) > if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0) > goto error; > > - /* Step 1: set features */ > + /* Step 1: negotiate protocol features & set features */ > features = dev->features; > + > + > /* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */ > features &= ~(1ull << VIRTIO_NET_F_MAC); > /* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know > */ @@ -417,13 +419,19 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) > 1ULL << VIRTIO_NET_F_GUEST_TSO6 | \ > 1ULL << VIRTIO_F_IN_ORDER | \ > 1ULL << VIRTIO_F_VERSION_1 | \ > - 1ULL << VIRTIO_F_RING_PACKED) > + 1ULL << VIRTIO_F_RING_PACKED | \ > + 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) > + > +#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ > + (1ULL << VHOST_USER_PROTOCOL_F_MQ) > > int > virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > int cq, int queue_size, const char *mac, char **ifname, > int server, int mrg_rxbuf, int in_order, int packed_vq) { > + uint64_t protocol_features = 0; > + > pthread_mutex_init(&dev->mutex, NULL); > strlcpy(dev->path, path, PATH_MAX); > dev->started = 0; > @@ -434,6 +442,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > dev->mac_specified = 0; > dev->frontend_features = 0; > dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; > + dev->protocol_features = > VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; > parse_mac(dev, mac); > > if (*ifname) { > @@ -446,6 +455,10 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > return -1; > } > > + if (!is_vhost_user_by_type(dev->path)) > + dev->unsupported_features |= > + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES); > + > if (!dev->is_server) { > if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER, > NULL) < 0) { > @@ -460,6 +473,26 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > strerror(errno)); > return -1; > } > + > + > + if (dev->device_features & > + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { > + if (dev->ops->send_request(dev, > + VHOST_USER_GET_PROTOCOL_FEATURES, > + &protocol_features)) > + return -1; > + } Should we put '}' after sending VHOST_USER_SET_PROTOCOL_FEATURES like in virtio_user_server_reconnect? > + > + dev->protocol_features &= protocol_features; > + > + if (dev->ops->send_request(dev, > + VHOST_USER_SET_PROTOCOL_FEATURES, > + &dev->protocol_features)) > + return -1; > + > + if (!(dev->protocol_features & > + (1ULL << > VHOST_USER_PROTOCOL_F_MQ))) > + dev->unsupported_features |= (1ull << > VIRTIO_NET_F_MQ); > } else { > /* We just pretend vhost-user can support all these features. > * Note that this could be problematic that if some feature is > @@ -469,6 +502,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES; > } > > + > + > if (!mrg_rxbuf) > dev->unsupported_features |= (1ull << > VIRTIO_NET_F_MRG_RXBUF); > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h > b/drivers/net/virtio/virtio_user/virtio_user_dev.h > index 3b6b6065a5..56e638f8a6 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h > @@ -40,6 +40,9 @@ struct virtio_user_dev { > uint64_t device_features; /* supported features by device */ > uint64_t frontend_features; /* enabled frontend features */ > uint64_t unsupported_features; /* unsupported features mask > */ > + uint64_t protocol_features; /* negotiated protocol features > + * (Vhost-user only) > + */ > uint8_t status; > uint16_t port_id; > uint8_t mac_addr[RTE_ETHER_ADDR_LEN]; > diff --git a/drivers/net/virtio/virtio_user_ethdev.c > b/drivers/net/virtio/virtio_user_ethdev.c > index 798f191c32..ccb5a18e25 100644 > --- a/drivers/net/virtio/virtio_user_ethdev.c > +++ b/drivers/net/virtio/virtio_user_ethdev.c > @@ -68,6 +68,7 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) > int connectfd; > struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id]; > struct virtio_hw *hw = eth_dev->data->dev_private; > + uint64_t protocol_features; > > connectfd = accept(dev->listenfd, NULL, NULL); > if (connectfd < 0) > @@ -81,6 +82,24 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) > return -1; > } > > + if (dev->device_features & > + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { > + if (dev->ops->send_request(dev, > + > VHOST_USER_GET_PROTOCOL_FEATURES, > + &protocol_features)) > + return -1; > + > + dev->protocol_features &= protocol_features; > + > + if (dev->ops->send_request(dev, > + > VHOST_USER_SET_PROTOCOL_FEATURES, > + &dev->protocol_features)) > + return -1; > + } > + > + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) > + dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ); Should we consider the case that vhost-user does not support VHOST_USER_F_PROTOCOL_FEATURES but support VIRTIO_NET_F_MQ? Because if the device negotiated feature does not include that, we should not use above logic to decide whether MQ is supported. If the case should be considered, the above two lines should be moved into last '{}' and same thing should be done in virtio_user_dev_init. Thanks! Chenbo > + > dev->device_features |= dev->frontend_features; > > /* umask vhost-user unsupported features */ > -- > 2.26.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support 2020-06-17 11:57 ` Xia, Chenbo @ 2020-06-17 12:22 ` Maxime Coquelin 0 siblings, 0 replies; 6+ messages in thread From: Maxime Coquelin @ 2020-06-17 12:22 UTC (permalink / raw) To: Xia, Chenbo, dev, amorenoz, Ye, Xiaolong, shahafs, matan On 6/17/20 1:57 PM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin >> Sent: Thursday, May 28, 2020 3:46 PM >> To: dev@dpdk.org; amorenoz@redhat.com; Ye, Xiaolong >> <xiaolong.ye@intel.com>; shahafs@mellanox.com; matan@mellanox.com >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> >> Subject: [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features >> support >> >> This patch adds support for Vhost-user protocol features. >> It is required to support protocol features that were not in initial Vhost-user >> specification, such as reply-ack, MTU... >> >> Also, this patch prevents Virtio multiqueue feature negotiation if the slave does >> not support MQ protocol feature as stated in Vhost-user specification: >> "The multiple queues feature is supported only when the protocol feature >> ``VHOST_USER_PROTOCOL_F_MQ`` (bit 0) is set." >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> drivers/net/virtio/virtio_user/vhost.h | 9 +++++ >> drivers/net/virtio/virtio_user/vhost_user.c | 3 ++ >> .../net/virtio/virtio_user/virtio_user_dev.c | 39 ++++++++++++++++++- >> .../net/virtio/virtio_user/virtio_user_dev.h | 3 ++ >> drivers/net/virtio/virtio_user_ethdev.c | 19 +++++++++ >> 5 files changed, 71 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/virtio/virtio_user/vhost.h >> b/drivers/net/virtio/virtio_user/vhost.h >> index 1e784e58ef..9ace1a90c3 100644 >> --- a/drivers/net/virtio/virtio_user/vhost.h >> +++ b/drivers/net/virtio/virtio_user/vhost.h >> @@ -44,6 +44,15 @@ struct vhost_vring_addr { >> uint64_t log_guest_addr; >> }; >> >> +#ifndef VHOST_USER_F_PROTOCOL_FEATURES >> +#define VHOST_USER_F_PROTOCOL_FEATURES 30 #endif >> + >> +/** Protocol features. */ >> +#ifndef VHOST_USER_PROTOCOL_F_MQ >> +#define VHOST_USER_PROTOCOL_F_MQ 0 >> +#endif >> + >> enum vhost_user_request { >> VHOST_USER_NONE = 0, >> VHOST_USER_GET_FEATURES = 1, >> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c >> b/drivers/net/virtio/virtio_user/vhost_user.c >> index 74b82e56e4..b687665042 100644 >> --- a/drivers/net/virtio/virtio_user/vhost_user.c >> +++ b/drivers/net/virtio/virtio_user/vhost_user.c >> @@ -269,10 +269,12 @@ vhost_user_sock(struct virtio_user_dev *dev, >> >> switch (req) { >> case VHOST_USER_GET_FEATURES: >> + case VHOST_USER_GET_PROTOCOL_FEATURES: >> need_reply = 1; >> break; >> >> case VHOST_USER_SET_FEATURES: >> + case VHOST_USER_SET_PROTOCOL_FEATURES: >> case VHOST_USER_SET_LOG_BASE: >> msg.payload.u64 = *((__u64 *)arg); >> msg.size = sizeof(m.payload.u64); >> @@ -351,6 +353,7 @@ vhost_user_sock(struct virtio_user_dev *dev, >> >> switch (req) { >> case VHOST_USER_GET_FEATURES: >> + case VHOST_USER_GET_PROTOCOL_FEATURES: >> if (msg.size != sizeof(m.payload.u64)) { >> PMD_DRV_LOG(ERR, "Received bad msg size"); >> return -1; >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> index 7fb135f49a..3afb09df2d 100644 >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> @@ -151,8 +151,10 @@ virtio_user_start_device(struct virtio_user_dev *dev) >> if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0) >> goto error; >> >> - /* Step 1: set features */ >> + /* Step 1: negotiate protocol features & set features */ >> features = dev->features; >> + >> + >> /* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */ >> features &= ~(1ull << VIRTIO_NET_F_MAC); >> /* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know >> */ @@ -417,13 +419,19 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >> 1ULL << VIRTIO_NET_F_GUEST_TSO6 | \ >> 1ULL << VIRTIO_F_IN_ORDER | \ >> 1ULL << VIRTIO_F_VERSION_1 | \ >> - 1ULL << VIRTIO_F_RING_PACKED) >> + 1ULL << VIRTIO_F_RING_PACKED | \ >> + 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) >> + >> +#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ >> + (1ULL << VHOST_USER_PROTOCOL_F_MQ) >> >> int >> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >> int cq, int queue_size, const char *mac, char **ifname, >> int server, int mrg_rxbuf, int in_order, int packed_vq) { >> + uint64_t protocol_features = 0; >> + >> pthread_mutex_init(&dev->mutex, NULL); >> strlcpy(dev->path, path, PATH_MAX); >> dev->started = 0; >> @@ -434,6 +442,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> dev->mac_specified = 0; >> dev->frontend_features = 0; >> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; >> + dev->protocol_features = >> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; >> parse_mac(dev, mac); >> >> if (*ifname) { >> @@ -446,6 +455,10 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> return -1; >> } >> >> + if (!is_vhost_user_by_type(dev->path)) >> + dev->unsupported_features |= >> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES); >> + >> if (!dev->is_server) { >> if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER, >> NULL) < 0) { >> @@ -460,6 +473,26 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> strerror(errno)); >> return -1; >> } >> + >> + >> + if (dev->device_features & >> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { >> + if (dev->ops->send_request(dev, >> + VHOST_USER_GET_PROTOCOL_FEATURES, >> + &protocol_features)) >> + return -1; >> + } > > Should we put '}' after sending VHOST_USER_SET_PROTOCOL_FEATURES like in virtio_user_server_reconnect? Good catch, we indeed don't want to send a Vhost-user request if it is not supported by the Vhost-user backend. >> + >> + dev->protocol_features &= protocol_features; >> + >> + if (dev->ops->send_request(dev, >> + VHOST_USER_SET_PROTOCOL_FEATURES, >> + &dev->protocol_features)) >> + return -1; >> + >> + if (!(dev->protocol_features & >> + (1ULL << >> VHOST_USER_PROTOCOL_F_MQ))) >> + dev->unsupported_features |= (1ull << >> VIRTIO_NET_F_MQ); >> } else { >> /* We just pretend vhost-user can support all these features. >> * Note that this could be problematic that if some feature is >> @@ -469,6 +502,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES; >> } >> >> + >> + >> if (!mrg_rxbuf) >> dev->unsupported_features |= (1ull << >> VIRTIO_NET_F_MRG_RXBUF); >> >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h >> b/drivers/net/virtio/virtio_user/virtio_user_dev.h >> index 3b6b6065a5..56e638f8a6 100644 >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h >> @@ -40,6 +40,9 @@ struct virtio_user_dev { >> uint64_t device_features; /* supported features by device */ >> uint64_t frontend_features; /* enabled frontend features */ >> uint64_t unsupported_features; /* unsupported features mask >> */ >> + uint64_t protocol_features; /* negotiated protocol features >> + * (Vhost-user only) >> + */ >> uint8_t status; >> uint16_t port_id; >> uint8_t mac_addr[RTE_ETHER_ADDR_LEN]; >> diff --git a/drivers/net/virtio/virtio_user_ethdev.c >> b/drivers/net/virtio/virtio_user_ethdev.c >> index 798f191c32..ccb5a18e25 100644 >> --- a/drivers/net/virtio/virtio_user_ethdev.c >> +++ b/drivers/net/virtio/virtio_user_ethdev.c >> @@ -68,6 +68,7 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) >> int connectfd; >> struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id]; >> struct virtio_hw *hw = eth_dev->data->dev_private; >> + uint64_t protocol_features; >> >> connectfd = accept(dev->listenfd, NULL, NULL); >> if (connectfd < 0) >> @@ -81,6 +82,24 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) >> return -1; >> } >> >> + if (dev->device_features & >> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { >> + if (dev->ops->send_request(dev, >> + >> VHOST_USER_GET_PROTOCOL_FEATURES, >> + &protocol_features)) >> + return -1; >> + >> + dev->protocol_features &= protocol_features; >> + >> + if (dev->ops->send_request(dev, >> + >> VHOST_USER_SET_PROTOCOL_FEATURES, >> + &dev->protocol_features)) >> + return -1; >> + } >> + >> + if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) >> + dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ); > > Should we consider the case that vhost-user does not support VHOST_USER_F_PROTOCOL_FEATURES but support > VIRTIO_NET_F_MQ? Because if the device negotiated feature does not include that, we should not use above logic to decide > whether MQ is supported. If the case should be considered, the above two lines should be moved into last '{}' and > same thing should be done in virtio_user_dev_init. The Vhost-user specification says: " Masters must not rely on the ``VHOST_USER_PROTOCOL_F_MQ`` protocol feature for devices with a fixed number of virtqueues. Only true multiqueue devices require this protocol feature. " Virtio-net device being a true multiqueue device it should require this. But for now Virtio-user PMD does not use VHOST_USER_GET_QUEUE_NUM, so it might not be mandatory. I can drop it for now, but we should consider adding support for sending VHOST_USER_GET_QUEUE_NUM, as the container may want to know how many queues are supported by the Vhost-user backend or the vDPA device. Thanks! Maxime > Thanks! > Chenbo > >> + >> dev->device_features |= dev->frontend_features; >> >> /* umask vhost-user unsupported features */ >> -- >> 2.26.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user 2020-05-28 7:46 [dpdk-dev] [PATCH 0/3] net/virtio: add Vhost-user protocol features Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support Maxime Coquelin @ 2020-05-28 7:46 ` Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 3/3] net/virtio: add Virtio status " Maxime Coquelin 2 siblings, 0 replies; 6+ messages in thread From: Maxime Coquelin @ 2020-05-28 7:46 UTC (permalink / raw) To: dev, amorenoz, xiaolong.ye, shahafs, matan; +Cc: Maxime Coquelin This patch adds support reply-ack vhost-user protocol feature, which is for now only used to ensure VHOST_USER_SET_MEM_TABLE requests are handled by the slave, but later will be used for VHOST_USER_SET_STATUS. Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost.h | 6 ++++- drivers/net/virtio/virtio_user/vhost_user.c | 24 ++++++++++++++++--- .../net/virtio/virtio_user/virtio_user_dev.c | 3 ++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h index 9ace1a90c3..260e1c3081 100644 --- a/drivers/net/virtio/virtio_user/vhost.h +++ b/drivers/net/virtio/virtio_user/vhost.h @@ -50,7 +50,11 @@ struct vhost_vring_addr { /** Protocol features. */ #ifndef VHOST_USER_PROTOCOL_F_MQ -#define VHOST_USER_PROTOCOL_F_MQ 0 +#define VHOST_USER_PROTOCOL_F_MQ 0 +#endif + +#ifndef VHOST_USER_PROTOCOL_F_REPLY_ACK +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 #endif enum vhost_user_request { diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index b687665042..f8d751c98e 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -32,6 +32,7 @@ struct vhost_user_msg { #define VHOST_USER_VERSION_MASK 0x3 #define VHOST_USER_REPLY_MASK (0x1 << 2) +#define VHOST_USER_NEED_REPLY_MASK (0x1 << 3) uint32_t flags; uint32_t size; /* the following payload size */ union { @@ -251,6 +252,7 @@ vhost_user_sock(struct virtio_user_dev *dev, struct vhost_user_msg msg; struct vhost_vring_file *file = 0; int need_reply = 0; + int has_reply_ack; int fds[VHOST_MEMORY_MAX_NREGIONS]; int fd_num = 0; int len; @@ -263,6 +265,9 @@ vhost_user_sock(struct virtio_user_dev *dev, if (dev->is_server && vhostfd < 0) return -1; + if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK)) + has_reply_ack = 1; + msg.request = req; msg.flags = VHOST_USER_VERSION; msg.size = 0; @@ -291,6 +296,9 @@ vhost_user_sock(struct virtio_user_dev *dev, msg.size = sizeof(m.payload.memory.nregions); msg.size += sizeof(m.payload.memory.padding); msg.size += fd_num * sizeof(struct vhost_memory_region); + + if (has_reply_ack) + msg.flags |= VHOST_USER_NEED_REPLY_MASK; break; case VHOST_USER_SET_LOG_FD: @@ -339,7 +347,7 @@ vhost_user_sock(struct virtio_user_dev *dev, return -1; } - if (need_reply) { + if (need_reply || msg.flags & VHOST_USER_NEED_REPLY_MASK) { if (vhost_user_read(vhostfd, &msg) < 0) { PMD_DRV_LOG(ERR, "Received msg failed: %s", strerror(errno)); @@ -369,8 +377,18 @@ vhost_user_sock(struct virtio_user_dev *dev, sizeof(struct vhost_vring_state)); break; default: - PMD_DRV_LOG(ERR, "Received unexpected msg type"); - return -1; + /* Reply-ack handling */ + if (msg.size != sizeof(m.payload.u64)) { + PMD_DRV_LOG(ERR, "Received bad msg size"); + return -1; + } + + if (msg.payload.u64 != 0) { + PMD_DRV_LOG(ERR, "Slave replied NACK"); + return -1; + } + + break; } } diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 3afb09df2d..ea22af5dc4 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -423,7 +423,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ - (1ULL << VHOST_USER_PROTOCOL_F_MQ) + (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ + 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, -- 2.26.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 3/3] net/virtio: add Virtio status support to Virtio-user 2020-05-28 7:46 [dpdk-dev] [PATCH 0/3] net/virtio: add Vhost-user protocol features Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user Maxime Coquelin @ 2020-05-28 7:46 ` Maxime Coquelin 2 siblings, 0 replies; 6+ messages in thread From: Maxime Coquelin @ 2020-05-28 7:46 UTC (permalink / raw) To: dev, amorenoz, xiaolong.ye, shahafs, matan; +Cc: Maxime Coquelin This patch adds support for VHOST_USER_SET_STATUS request. It is used to make the backend aware of Virtio devices status update. It is useful for the backend to know when the Virtio driver is done with the Virtio device configuration. Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost.h | 5 +++++ drivers/net/virtio/virtio_user/vhost_user.c | 8 ++++++++ .../net/virtio/virtio_user/virtio_user_dev.c | 20 ++++++++++++++++++- .../net/virtio/virtio_user/virtio_user_dev.h | 1 + drivers/net/virtio/virtio_user_ethdev.c | 1 + 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h index 260e1c3081..fc360564eb 100644 --- a/drivers/net/virtio/virtio_user/vhost.h +++ b/drivers/net/virtio/virtio_user/vhost.h @@ -57,6 +57,10 @@ struct vhost_vring_addr { #define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 #endif +#ifndef VHOST_USER_PROTOCOL_F_STATUS +#define VHOST_USER_PROTOCOL_F_STATUS 15 +#endif + enum vhost_user_request { VHOST_USER_NONE = 0, VHOST_USER_GET_FEATURES = 1, @@ -77,6 +81,7 @@ enum vhost_user_request { VHOST_USER_SET_PROTOCOL_FEATURES = 16, VHOST_USER_GET_QUEUE_NUM = 17, VHOST_USER_SET_VRING_ENABLE = 18, + VHOST_USER_SET_STATUS = 36, VHOST_USER_MAX }; diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index f8d751c98e..68232d75d7 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -278,6 +278,14 @@ vhost_user_sock(struct virtio_user_dev *dev, need_reply = 1; break; + case VHOST_USER_SET_STATUS: + if (!(dev->protocol_features & + (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) + return 0; + + if (has_reply_ack) + msg.flags |= VHOST_USER_NEED_REPLY_MASK; + /* Fallthrough */ case VHOST_USER_SET_FEATURES: case VHOST_USER_SET_PROTOCOL_FEATURES: case VHOST_USER_SET_LOG_BASE: diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index ea22af5dc4..cf29ea0b88 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ - 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) + 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ + 1ULL << VHOST_USER_PROTOCOL_F_STATUS) int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, @@ -782,3 +783,20 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx) __atomic_add_fetch(&vring->used->idx, 1, __ATOMIC_RELAXED); } } + +int +virtio_user_update_status(struct virtio_user_dev *dev, uint8_t status) +{ + int ret; + uint64_t arg = status; + + /* Vhost-user only for now */ + if (!is_vhost_user_by_type(dev->path)) + return 0; + + ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg); + if (ret) + return -1; + + return 0; +} diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h index 56e638f8a6..c49a896a88 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h @@ -71,4 +71,5 @@ void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx); void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx); uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs); +int virtio_user_update_status(struct virtio_user_dev *dev, uint8_t status); #endif diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index ccb5a18e25..bed161ba07 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -271,6 +271,7 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t status) else if (status == VIRTIO_CONFIG_STATUS_RESET) virtio_user_reset(hw); dev->status = status; + virtio_user_update_status(dev, status); } static uint8_t -- 2.26.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-17 12:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-28 7:46 [dpdk-dev] [PATCH 0/3] net/virtio: add Vhost-user protocol features Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 1/3] net/virtio: add vhost-user protocol features support Maxime Coquelin 2020-06-17 11:57 ` Xia, Chenbo 2020-06-17 12:22 ` Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 2/3] net/virtio: add reply-ack support to Virtio-user Maxime Coquelin 2020-05-28 7:46 ` [dpdk-dev] [PATCH 3/3] net/virtio: add Virtio status " Maxime Coquelin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).