> From: Maxime Coquelin [mailto:[email protected]] > Sent: Friday, July 5, 2019 4:47 PM > To: Yu, Jin <[email protected]>; [email protected] > Cc: Liu, Changpeng <[email protected]>; LinLi <[email protected]>; > XunNi <[email protected]>; YuZhang <[email protected]>; Bie, Tiwei > <[email protected]> > Subject: Re: [dpdk-dev] [PATCH] [v1]vhost: support inflight share memory > protocol feature > > Hi Jin, > > On 6/26/19 6:38 PM, JinYu wrote: > > This patch introduces two new messages VHOST_USER_GET_INFLIGHT_FD > and > > VHOST_USER_SET_INFLIGHT_FD to support transferring a shared buffer > > between qemu and backend. > > > > Firstly, qemu uses VHOST_USER_GET_INFLIGHT_FD to get the shared buffer > > from backend. Then qemu should send it back through > > VHOST_USER_SET_INFLIGHT_FD each time we start vhost-user. > > > > This shared buffer is used to process inflight I/O when backend > > reconnect. > > > > Signed-off-by: LinLi <[email protected]> > > Signed-off-by: XunNi <[email protected]> > > Signed-off-by: YuZhang <[email protected]> > > Signed-off-by: JinYu <[email protected]> > > --- > > V1 - specify the APIs are split-ring only > > --- > > lib/librte_vhost/rte_vhost.h | 99 +++++++++ > > lib/librte_vhost/rte_vhost_version.map | 4 + > > lib/librte_vhost/vhost.c | 135 ++++++++++++ > > lib/librte_vhost/vhost.h | 12 + > > lib/librte_vhost/vhost_user.c | 292 +++++++++++++++++++++++++ > > lib/librte_vhost/vhost_user.h | 13 +- > > 6 files changed, 554 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_vhost/rte_vhost.h > > b/lib/librte_vhost/rte_vhost.h index 0226b3eff..c9959c15a 100644 > > --- a/lib/librte_vhost/rte_vhost.h > > +++ b/lib/librte_vhost/rte_vhost.h > > @@ -71,6 +71,10 @@ extern "C" { > > #define VHOST_USER_PROTOCOL_F_HOST_NOTIFIER 11 > > #endif > > > > +#ifndef VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD > > +#define VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD 12 #endif > > + > > /** Indicate whether protocol features negotiation is supported. */ > > #ifndef VHOST_USER_F_PROTOCOL_FEATURES > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > @@ -98,12 +102,41 @@ struct rte_vhost_memory { > > struct rte_vhost_mem_region regions[]; > > }; > > > > +struct inflight_desc_split { > > + uint8_t inflight; > > + uint8_t padding[5]; > > + uint16_t next; > > + uint64_t counter; > > +}; > > + > > +struct inflight_info_split { > > + uint64_t features; > > + uint16_t version; > > + uint16_t desc_num; > > + uint16_t last_inflight_io; > > + uint16_t used_idx; > > + struct inflight_desc_split desc[0]; > > +}; > > + > > +struct resubmit_desc { > > + uint16_t index; > > + uint64_t counter; > > +}; > > + > > +struct resubmit_info { > > + struct resubmit_desc *resubmit_list; > > + uint16_t resubmit_num; > > +}; > > + > > struct rte_vhost_vring { > > struct vring_desc *desc; > > struct vring_avail *avail; > > struct vring_used *used; > > uint64_t log_guest_addr; > > > > + struct inflight_info_split *inflight; > > + struct resubmit_info *resubmit_inflight; > > + > > This is breaking the ABI, and discussing with Tiwei, we think it could be > avoided by defining its own structure of inflight and inflight resubmit > pointers. More below: So you means that define a new struct such as struct rte_vhost_inflight_ring? > > > /** Deprecated, use rte_vhost_vring_call() instead. */ > > int callfd; > > > > @@ -603,6 +636,22 @@ uint16_t rte_vhost_dequeue_burst(int vid, > uint16_t queue_id, > > */ > > int rte_vhost_get_mem_table(int vid, struct rte_vhost_memory **mem); > > > > +/** > > + * Get guest vring info, including the vring address, vring size, > > inflight, etc. > > + * > > + * @param vid > > + * vhost device ID > > + * @param vring_idx > > + * vring index > > + * @param vring > > + * the structure to hold the requested vring info > > + * @return > > + * 0 on success, -1 on failure > > + */ > > +int __rte_experimental > > +rte_vhost_get_vhost_vring_with_inflight_split(int vid, uint16_t vring_idx, > > + struct rte_vhost_vring *vring); > > Here a new API is introduced to get vting info with inflight. > It could be just changed to get only the inflight info. It means the caller > would > first call rte_vhost_get_vhost_vring() and then the new function, which I > guess may be renamed to: > > rte_vhost_get_vhost_vring_inflight_split Got It. > > Other than that, the patch looks good to me. > > (Next time, please run get_maintainer.sh script so that all the maintainers > are in Cc:). > > Thanks, > Maxime Thanks Maxime, also I will submit an example code of vhost_user_blk which support the inflight feature. It can show how these APIs work.
Thanks Jin

