Re: [Qemu-devel] [RFC 13/29] vhost+postcopy: Transmit 'listen' to client

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 07/26/2017 07:42 PM, Dr. David Alan Gilbert wrote:
> * Maxime Coquelin (maxime.coquelin@redhat.com) wrote:
>>
>>
>> On 06/28/2017 09:00 PM, Dr. David Alan Gilbert (git) wrote:
>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>> index b98fbe4834..1f70f5760f 100644
>>> --- a/hw/virtio/vhost-user.c
>>> +++ b/hw/virtio/vhost-user.c
>>> @@ -67,6 +67,7 @@ typedef enum VhostUserRequest {
>>>        VHOST_USER_SET_SLAVE_REQ_FD = 21,
>>>        VHOST_USER_IOTLB_MSG = 22,
>>>        VHOST_USER_POSTCOPY_ADVISE  = 23,
>>> +    VHOST_USER_POSTCOPY_LISTEN  = 24,
>>>        VHOST_USER_MAX
>>>    } VhostUserRequest;
>>> @@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
>>>        return 0;
>>>    }
>>> +/*
>>> + * Called at the switch to postcopy on reception of the 'listen' command.
>>> + */
>>> +static int vhost_user_postcopy_listen(struct vhost_dev *dev, Error **errp)
>>> +{
>>> +    VhostUserMsg msg = {
>>> +        .request = VHOST_USER_POSTCOPY_LISTEN,
>>> +        .flags = VHOST_USER_VERSION,
>>> +    };
>>
>> I think it should use REPLY_ACK feature when available for two reasons:
>> 1. The backend could reply nack if nregions is already set.
>> 2. When leaving vhost_user_postcopy_listen(), the message might likely
>>     not have been handled yet by the backend.
> 
> OK, I can do that.  What are the rules on features like that?   Can I
> just assume that if you've got POSTCOPY then you'll ack for new messages
> we add?

You can have a look at vhost_user_net_set_mtu() for example.
Before sending the message, just set VHOST_USER_NEED_REPLY_MASK bit in
message's flags if VHOST_USER_PROTOCOL_F_REPLY_ACK is advertised by the
backend.

After having sent the message, instead of returning directly, wait for
the ack/nack by calling process_message_reply() if
VHOST_USER_PROTOCOL_F_REPLY_ACK is advertised.

> The current contrib/libvhost-user code doesn't seem to do acks yet, but
> I can add it here quite easily.

Yes, agree it is not implemented in libvhost-user, but we will be able
to test it with DPDK prototype.

Thanks,
Maxime

> Dave
> 
>>> +    trace_vhost_user_postcopy_listen();
>>> +    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
>>> +        error_setg(errp, "Failed to send postcopy_listen to vhost");
>>> +        return -1;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>    static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
>>>                                            void *opaque)
>>>    {
>>> @@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
>>>        case POSTCOPY_NOTIFY_INBOUND_ADVISE:
>>>            return vhost_user_postcopy_advise(dev, pnd->errp);
>>> +    case POSTCOPY_NOTIFY_INBOUND_LISTEN:
>>> +        return vhost_user_postcopy_listen(dev, pnd->errp);
>>> +
>>>        default:
>>>            /* We ignore notifications we don't know */
>>>            break;
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>