On Fri, Dec 30, 2016 at 04:26:27PM -0500, Charles (Chas) Williams wrote:

On 12/29/2016 10:15 PM, Yuanhan Liu wrote:

quoted

On Thu, Dec 29, 2016 at 10:58:11AM -0500, Charles (Chas) Williams wrote:

quoted

On 12/29/2016 03:52 AM, Yuanhan Liu wrote:

quoted

On Wed, Dec 28, 2016 at 04:10:52PM -0500, Charles (Chas) Williams wrote:

quoted

Start a vhost server once during devinit instead of during device start
and stop.  Some vhost clients, QEMU, don't re-attaching to sockets when
the vhost server is stopped and later started.  Preserve existing behavior
for vhost clients.

I didn't quite get the idea what you are going to fix.

The issue I am trying to fix is QEMU interaction when DPDK's vhost is
acting as a server to QEMU vhost clients.  If you create a vhost server
device, it doesn't create the actual datagram socket until you call
.dev_start().  If you call .dev_stop() is also deletes those sockets.
For QEMU, this is a problem since QEMU doesn't know how to re-attach to
datagram sockets that have gone away.

Thanks! And I'd appreciate it if you could have written the commit log
this way firstly.

quoted

.dev_start()/.dev_stop() seems to roughly means link up and link down
so I understand why you might want to add/remove the datagram sockets.
However, in practice, this doesn't seem to make much sense for a DPDK
vhost server.

Agree.

quoted

This doesn't seem like the right way to indicate link
status to vhost clients.

It seems like it would just be easier to do this for both clients and
servers, but I don't know why it was done this way originally so I
choose to keep the client behavior.

I don't think there are any differences between DPDK acting as client or
server. To me, the right logic seems to be (for both DPDK as server and
client).

For register,
- register the vhost socket at probe stage (either at rte_pmd_vhost_probe
 or at eth_dev_vhost_create).
- start the vhost session right after the register when we haven't started
 it before.

For unregister,
- invoke rte_vhost_driver_unregister() at rte_pmd_vhost_remove().

OK. This will be much easier than what I submitted.

Good.

quoted

For dev_start/stop,
- set allow_queuing to 1/0 for start/stop, respectively.

Unfortunately, I don't think this will work.  new_device() doesn't happen
until a client connects.  allow_queueing seems to be following the status
of the "wire" as it where.  .dev_start()/.dev_stop() is the link of local
port connected to the wire (administratively up or down as it where).

.dev_start() can happen before new_device() and attempting to RX for a
client that doesn't exist doesn't seem like a good idea. 

Right.

Perhaps another
flag that follows dev_started, but for the queues?

I will comment it on your v3 patches.

	--yliu