From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dariuszx.stojaczyk@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id BAFADDE0
 for <dev@dpdk.org>; Tue, 29 May 2018 15:38:41 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 29 May 2018 06:38:39 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.49,456,1520924400"; d="scan'208";a="60015542"
Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205])
 by orsmga001.jf.intel.com with ESMTP; 29 May 2018 06:38:38 -0700
Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by
 fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Tue, 29 May 2018 06:38:38 -0700
Received: from lcsmsx155.ger.corp.intel.com (10.186.165.233) by
 FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Tue, 29 May 2018 06:38:37 -0700
Received: from hasmsx105.ger.corp.intel.com ([169.254.1.132]) by
 LCSMSX155.ger.corp.intel.com ([169.254.12.62]) with mapi id 14.03.0319.002;
 Tue, 29 May 2018 16:38:34 +0300
From: "Stojaczyk, DariuszX" <dariuszx.stojaczyk@intel.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, Maxime Coquelin
 <maxime.coquelin@redhat.com>, "Bie, Tiwei" <tiwei.bie@intel.com>, "Tetsuya
 Mukawa" <mtetsuyah@gmail.com>, Thomas Monjalon <thomas@monjalon.net>,
 "yliu@fridaylinux.org" <yliu@fridaylinux.org>, "Harris, James R"
 <james.r.harris@intel.com>
Thread-Topic: [RFC v2] vhost: new rte_vhost API proposal
Thread-Index: AQHT7on2huGy3KtQDE+XGzZuey1czqRAEIsAgAaoa6A=
Date: Tue, 29 May 2018 13:38:33 +0000
Message-ID: <FBE7E039FA50BF47A673AD0BD3CD56A846140984@HASMSX105.ger.corp.intel.com>
References: <1525958573-184361-1-git-send-email-dariuszx.stojaczyk@intel.com>
 <1526648465-62579-1-git-send-email-dariuszx.stojaczyk@intel.com>
 <20180525100550.GD14757@stefanha-x1.localdomain>
In-Reply-To: <20180525100550.GD14757@stefanha-x1.localdomain>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
dlp-product: dlpe-windows
dlp-version: 11.0.200.100
dlp-reaction: no-action
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmM3MmZiMTAtNjAzZC00ODMwLWFiMzMtYzFhYTJkZDFmZDdkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJueVZzc3BVRjB3SXIrVFgwbCtuZ2NmT2U3UGlYRHhJcEFoNEc1cDJVQ1JyWDJWTllaWG13TWp4cDFjWEs1Z1JKIn0=
x-ctpclassification: CTP_NT
x-originating-ip: [10.103.103.123]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [RFC v2] vhost: new rte_vhost API proposal
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 29 May 2018 13:38:42 -0000



> -----Original Message-----
> From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> Sent: Friday, May 25, 2018 12:06 PM
> On Fri, May 18, 2018 at 03:01:05PM +0200, Dariusz Stojaczyk wrote:
> > +struct rte_vhost2_msg {
> > +	uint32_t id;
>=20
> Is this what the vhost-user specification calls the "request type"?  I
> suggest following the vhost-user spec terminology.
>=20
> > +	uint32_t flags;
> > +	uint32_t size; /**< The following payload size. */
> > +	void *payload;
> > +	int fds[RTE_VHOST2_MEMORY_MAX_NREGIONS];
>=20
> Is it necessary to expose file descriptor passing in the API?
> virtio-vhost-user doesn't have file descriptor passing, so it's best if t=
his
> can be hidden inside rte_vhost2.

So it's another argument for not exposing raw message handling to the user.
If there's some backend-specific vhost-user message in future that contains=
 an fd, it will need a set of new abstractions to work with virtio-vhost-us=
er anyway.
I guess I'll get back the original custom_msg idea from V1.

>=20
> > +};
> > +
> > +/** Single memory region. Both physically and virtually contiguous */
> > +struct rte_vhost2_mem_region {
> > +	uint64_t guest_phys_addr;
> > +	uint64_t guest_user_addr;
> > +	uint64_t host_user_addr;
> > +	uint64_t size;
> > +	void *mmap_addr;
> > +	uint64_t mmap_size;
> > +	int fd;
>=20
> virtio-vhost-user doesn't have an fd.  Why do API consumers need to
> know about the fd?

They don't. Ack. I'll strip this struct.

>=20
> > +/**
> > + * Device/queue related callbacks, all optional. Provided callback
> > + * parameters are guaranteed not to be NULL unless explicitly
> specified.
> > + */
>=20
> This is a good place to mention that all callbacks are asynchronous unles=
s
> specified otherwise.  Without that knowledge statements below like "If
> this is completed with a non-zero status" are confusing on a void
> function.

Ack.

>=20
> > +struct rte_vhost2_tgt_ops {
> > +	/**
> > +	 * New driver connected. If this is completed with a non-zero
> status,
> > +	 * rte_vhost2 will terminate the connection.
> > +	 */
> > +	void (*device_create)(struct rte_vhost2_dev *vdev);
> > +	/**
> > +	* Device is ready to operate. vdev data is now initialized. This
> callback
> > +	* may be called multiple times as e.g. memory mappings can
> change
> > +	* dynamically. All queues are guaranteed to be stopped by now.
> > +	*/
> > +	void (*device_init)(struct rte_vhost2_dev *vdev);
> > +	/**
> > +	* Features have changed in runtime. This is called at least once
> > +during
>=20
> s/in/at/

Ack.

>=20
> > +	/**
> > +	* Custom vhost-user message handler. This is called for
> > +	* backend-specific messages (net/crypto/scsi) that weren't
> recognized
> > +	* by the generic message parser. `msg` is available until
> > +	* \c rte_vhost2_tgt_cb_complete is called.
> > +	*/
> > +	void (*custom_msg)(struct rte_vhost2_dev *vdev, struct
> > +rte_vhost2_msg *msg);
>=20
> What happens if rte_vhost2_tgt_cb_complete() is called with a negative
> rc?  Does the specific errno value matter?

My current implementation only checks for rc !=3D 0 now. I'm still working =
this out.

>=20
> Where is the API for sending a vhost-user reply message?

I didn't push any. Now that you pointed out the fds in public API I think I=
'll rollback this custom_msg stuff to V1.

D.