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
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 2/3] net/virtio: add reply-ack support to Virtio-user > > 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; Do we have counterpart in vhost-user to handle such case as VHOST_USER_SET_MEM_TABLE may have a need-reply mask? Do I miss something? Thanks, Chenbo > 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
Hi Chenbo, On 6/17/20 5:22 AM, 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 2/3] net/virtio: add reply-ack support to Virtio-user >> >> 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; > > Do we have counterpart in vhost-user to handle such case as VHOST_USER_SET_MEM_TABLE may have > a need-reply mask? Do I miss something? > That's actually already supported on Vhost-user backend side, and is handled for all the requests in a generic way. http://code.dpdk.org/dpdk/latest/source/lib/librte_vhost/vhost_user.c#L2792 Regards, Maxime