26/06/2018 10:22, Tiwei Bie:
> On Mon, Jun 25, 2018 at 08:17:08PM +0800, Stojaczyk, DariuszX wrote:
> > From: Tiwei Bie
> > >
> > > Hi Dariusz,
> >
> > Hi Tiwei,
> >
> > > Thank you for putting efforts in making the DPDK
> > > vhost more generic!
> > >
> > > From my understanding, your proposal is that:
> > >
> > > 1) Introduce rte_vhost2 to provide the APIs which
> > > allow users to implement vhost backends like
> > > SCSI, net, crypto, ..
> >
> > That's right.
> >
> > > 2) Refactor the existing rte_vhost to use rte_vhost2.
> > > The rte_vhost will still provide below existing
> > > sets of APIs:
> > > 1. The APIs which allow users to implement
> > > external vhost backends (these APIs were
> > > designed for SPDK previously)
> > > 2. The APIs provided by the net backend
> > > 3. The APIs provided by the crypto backend
> > > And above APIs in rte_vhost won't be changed.
> >
> > That's correct. Rte_vhost would register its own rte_vhost2_tgt_ops
> > underneath and will call existing vhost_device_ops for e.g. starting the
> > device once all queues are started.
>
> Currently I have below concerns and questions:
>
> - The rte_vhost's problem is still there. Even though
> rte_vhost2 is introduced, the net and crypto backends
> in rte_vhost won't benefit from the new callbacks.
>
> The existing rte_vhost in DPDK not only provides the
> APIs for DPDK applications to implement the external
> backends. But also provides high performance net and
> crypto backends implementation (maybe more in the
> future). So it's important that besides the DPDK
> applications which implement their external backends,
> the DPDK applications which use the builtin backends
> will also benefit from the new callbacks.
>
> So we should have a clear plan on how will the legacy
> callbacks in rte_vhost be dealt with in the next step.
>
> Besides, the new library's name is a bit misleading.
> It makes the existing rte_vhost library sound like an
> obsolete library. But actually the existing rte_vhost
> isn't an obsolete library. It will still provide the
> net and crypto backends. So if we want to introduce
> this new library, we should give it a better name.
>
> - It's possible to solve rte_vhost's problem you met
> by refactoring the existing vhost library directly
> instead of re-implementing a new vhost library from
> scratch and keeping the old one's problem as is.
+1
> In this way, it will solve the problem you met and
> also solve the problem for rte_vhost. Why not go
> this way? Something like:
>
> Below is the existing callbacks set in rte_vhost.h:
>
> /**
> * Device and vring operations.
> */
> struct vhost_device_ops {
> ......
> };
>
> It's a legacy implementation, and doesn't really
> follow the DPDK API design (e.g. no rte_ prefix).
> We can design and implement a new message handling
> and a new set of callbacks for rte_vhost to solve
> the problem you met without changing the old one.
> Something like:
>
> struct rte_vhost_device_ops {
> ......
> }
>
> int
> vhost_user_msg_handler(struct vhost_dev *vdev, struct vhost_user_msg *msg)
> {
> ......
>
> if (!vdev->is_using_new_device_ops) {
> // Call the existing message handler
> return vhost_user_msg_handler_legacy(vdev, msg);
> }
>
> // Implement the new logic here
> ......
> }
>
> A vhost application is allowed to register only struct
> rte_vhost_device_ops or struct vhost_device_ops (which
> should be deprecated in the future). The two ops cannot
> be registered at the same time.
>
> The existing applications could use the old ops. And
> if an application registers struct rte_vhost_device_ops,
> the new callbacks and message handler will be used.
