Re: [dpdk-dev] [PATCH] [v1]vhost: support inflight share memory protocol feature

["Yu, Jin" <jin.yu@intel.com>]

> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, July 5, 2019 4:47 PM
> To: Yu, Jin <jin.yu@intel.com>; dev@dpdk.org
> Cc: Liu, Changpeng <changpeng.liu@intel.com>; LinLi <lilin24@baidu.com>;
> XunNi <nixun@baidu.com>; YuZhang <zhangyu31@baidu.com>; Bie, Tiwei
> <tiwei.bie@intel.com>
> 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 <lilin24@baidu.com>
> > Signed-off-by: XunNi <nixun@baidu.com>
> > Signed-off-by: YuZhang <zhangyu31@baidu.com>
> > Signed-off-by: JinYu <jin.yu@intel.com>
> > ---
> > 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