From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6415AA0519; Mon, 22 Jun 2020 12:18:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EEE9D1D643; Mon, 22 Jun 2020 12:18:16 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 234ED1D61D for ; Mon, 22 Jun 2020 12:18:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1592821094; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=xN7CFoUTROyDRVALCxJX7MebXotlrhkCNlp6NIkxIVY=; b=ecBTFPOrthrkHSOb4SnWkxe2Fi2EY/EvrLu+BmCyemZFuv21rirzVHDW61J2ilnTo3IL7F FdhRLEw+Btfxp6zIoZLXfO9yQzmvZhgsXJexkQz1evldHNAZZPpUNnCdlxBSpCyiT0oOlG jaifqlCq4SfMwSaxpoIbO7HjplxET7Y= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-475-eYbNmAftOyWD9c9RGT1OiA-1; Mon, 22 Jun 2020 06:18:11 -0400 X-MC-Unique: eYbNmAftOyWD9c9RGT1OiA-1 Received: by mail-wm1-f72.google.com with SMTP id g187so2877349wme.0 for ; Mon, 22 Jun 2020 03:18:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=xN7CFoUTROyDRVALCxJX7MebXotlrhkCNlp6NIkxIVY=; b=n+NNHR7zUFUEW91oGMZaptw0iIINUBPW5p9Vyjgx8pPHvhAiggLg/Z0JUgds5TxgyI tP1oxPp+56Ko14MIh8ZHRb/W2CW4tjZkYjj45ob+MROz9vOvRX1/MJb0Ttr0dxz6Dj3u CQm5Cd+/CVYADT6Dw/TUnWimI4m+12UtglKdImel+FswzgreT7A6gBV8B5PjFKDBvrY/ 6V/g8VPjqeeMiz6v1js9XxSY0R86pWtole0Zye1QUuVSxfcBaG5ePy7ZByqcfhQWPD85 /hVNbIDs+xe9ekm3Ap99ZhAwUrVxoBRnsoNKQWVFKupXMHTLMZUDY1c0dUe1hoYuUhj7 Fymg== X-Gm-Message-State: AOAM532lR3D6B6b0dCbP0Hh4e6qRxeRW4906YBt0y88R4Ssk8jBxIlS1 VTqYkK2gihCg6TsRmI/C31eEbvEXarvqVBtoi6gcW957Sjvh/4yLnr0BRzJNErM+clV7IRuwXrd 7pbA= X-Received: by 2002:a5d:4244:: with SMTP id s4mr16052502wrr.5.1592821089987; Mon, 22 Jun 2020 03:18:09 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyUrtbc5WrknV347D/cKmhpfzg2KvAPh3w79Mq0fspLw0/psaUvn08TCG7o8Am2Z0yfirfbwg== X-Received: by 2002:a5d:4244:: with SMTP id s4mr16052469wrr.5.1592821089725; Mon, 22 Jun 2020 03:18:09 -0700 (PDT) Received: from amorenoz.users.ipa.redhat.com ([92.176.159.3]) by smtp.gmail.com with ESMTPSA id 30sm13167824wrm.74.2020.06.22.03.18.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 Jun 2020 03:18:09 -0700 (PDT) To: "Xia, Chenbo" , Maxime Coquelin , "Ye, Xiaolong" , "shahafs@mellanox.com" , "matan@mellanox.com" , "Wang, Xiao W" , "viacheslavo@mellanox.com" , "dev@dpdk.org" Cc: "jasowang@redhat.com" , "lulu@redhat.com" References: <20200514080218.1435344-1-maxime.coquelin@redhat.com> <20200514080218.1435344-7-maxime.coquelin@redhat.com> From: Adrian Moreno Autocrypt: addr=amorenoz@redhat.com; prefer-encrypt=mutual; keydata= mQENBF1syNUBCADQ9dk3fDMxOZ/+OQpmbanpodYxEv8IRtDz8PXw8YX7UyGfozOpLjQ8Fftj ZxuubYNbt2QVbSgviFilFdNWu2eTnN/JaGtfhmTOLPVoakkPHZF8lbgImMoch7L0fH8wN2IM KPxQyPNlX+K9FD5brHsV1lfe1TwAxmhcvLW8yNrVq+9eDIDykxc7tH4exIqXgZroahGxMHKy c8Ti2kJka/t6pDfRaY0J+6J7I1nrn6GXXSMNA45EH8+0N/QlcXhP3rfftnoPeVmpjswzvJqY FNjf/Q5VPLx7RX0Qx+y8mMB2JcChV5Bl7D7x5EUbItj6+Sy7QfOgCtPegk9HSrBCNYaLABEB AAG0I0FkcmlhbiBNb3Jlbm8gPGFtb3Jlbm96QHJlZGhhdC5jb20+iQFUBBMBCAA+FiEEogUD gihhmbOPHy26d5C5fbYeFsUFAl1syNUCGwMFCQHhM4AFCwkIBwIGFQoJCAsCBBYCAwECHgEC F4AACgkQd5C5fbYeFsX7qwgArGHSkX+ILNcujkVzjTG4OtkpJMPFlkn/1PxSEKD0jLuzx14B COzpg/Mqj3Re/QBuOas+ci9bsUA0/2nORtmmEBvzDOJpR5FH1jaGCx8USlY4WM6QqEDNZgTw hsy9KhjFzFjMk+oo3HyItXA+Uq9yrRBTjNBGTXxezMRcMuUZ4MIAfY0IRBglL2BufiuL43jD BvTENNFLoQ/wFV7qkFWSkv+8IjTsxr7M6XUo1QLd29Hn0dvwssN579HL1+BP46i2REpzeBEG L75iVChi+YnIQQNMJ9NYarVabZx4Y1Gn8+7B/1SNArDV+IDgnYgt7E58otoV2Ap310dmtuvE VbxGpbkBDQRdbMjVAQgAqyp9oA7WDu7/Y9T4Ommt69iZx8os7shUIfdgPEy5xrcPn6qGwN1/ HQ4j8nWfBG9uuX1X0RXUZIUEtYTxtED4yaCQMTqDUf9cBAwAA2mYxBfoiNYx8YqxM+sT0/J4 2qmDd+y+20UR4yzHE8AmIbspTzDFIJDAi+jKSR8F355z0sfW7CIMDC4ZWrPsskjEy1YN/U10 r6tRRH1kNyrCSbTG0d9MtcQO58h7DLXuzUhErB+BtG52A04t5cweIJTJC+koV5XPeilzlHnm RFoj0ncruGa9Odns21BNt3cy9wLfK+aUnWuAB1uc6bJGQPiAwjkilz7g7MBRUuIQ2Zt7HGLc SwARAQABiQE8BBgBCAAmFiEEogUDgihhmbOPHy26d5C5fbYeFsUFAl1syNUCGwwFCQHhM4AA CgkQd5C5fbYeFsUlSwf8CH+u/IXaE7WeWxwFkMaORfW8cM4q0xrL3M6yRGuQNW+kMjnrvK9U J9G+L1/5uTRbDQ/4LdoKqize8LjehA+iF6ba4t9Npikh8fLKWgaJfQ/hPhH4C3O5gWPOLTW6 ylGxiuER4CdFwQIoAMMslhFA7G+teeOKBq36E+1+zrybI6Xy1UBSlpDK9j4CtTnMQejjuSQb Qhle+l8VroaUHq869wjAhRHHhqmtJKggI+OvzgQpDIwfHIDypb1BuKydi2W6cVYEALUYyCLS dTBDhzj8zR5tPCsga8J7+TclQzkWOiI2C6ZtiWrMsL/Uym3uXk5nsoc7lSj7yLd/MrBRhYfP JQ== Message-ID: <49d8b72c-8e7e-c4f9-2a17-ceed01e9464e@redhat.com> Date: Mon, 22 Jun 2020 12:18:07 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 6/9] vhost: add support for virtio status X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Chenbo On 6/16/20 6:29 AM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: dev On Behalf Of Maxime Coquelin >> Sent: Thursday, May 14, 2020 4:02 PM >> To: Ye, Xiaolong ; shahafs@mellanox.com; >> matan@mellanox.com; amorenoz@redhat.com; Wang, Xiao W >> ; viacheslavo@mellanox.com; dev@dpdk.org >> Cc: jasowang@redhat.com; lulu@redhat.com; Maxime Coquelin >> >> Subject: [dpdk-dev] [PATCH 6/9] vhost: add support for virtio status >> >> This patch adds support to the new Virtio device status Vhost-user protocol >> feature. >> >> Getting such information in the backend helps to know when the driver is done >> with the device configuration and so makes the initialization phase more robust. >> >> Signed-off-by: Maxime Coquelin >> --- >> lib/librte_vhost/rte_vhost.h | 4 ++++ >> lib/librte_vhost/vhost.h | 9 ++++++++ >> lib/librte_vhost/vhost_user.c | 40 +++++++++++++++++++++++++++++++++++ >> lib/librte_vhost/vhost_user.h | 6 ++++-- >> 4 files changed, 57 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h index >> 5c72fba797..b4b7aa1928 100644 >> --- a/lib/librte_vhost/rte_vhost.h >> +++ b/lib/librte_vhost/rte_vhost.h >> @@ -85,6 +85,10 @@ extern "C" { >> #define VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD 12 #endif >> >> +#ifndef VHOST_USER_PROTOCOL_F_STATUS >> +#define VHOST_USER_PROTOCOL_F_STATUS 15 #endif >> + >> /** Indicate whether protocol features negotiation is supported. */ #ifndef >> VHOST_USER_F_PROTOCOL_FEATURES >> #define VHOST_USER_F_PROTOCOL_FEATURES 30 >> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index >> df98d15de6..9a9c0a98f5 100644 >> --- a/lib/librte_vhost/vhost.h >> +++ b/lib/librte_vhost/vhost.h >> @@ -202,6 +202,14 @@ struct vhost_virtqueue { >> TAILQ_HEAD(, vhost_iotlb_entry) iotlb_pending_list; } >> __rte_cache_aligned; >> >> +/* Virtio device status as per Virtio specification */ >> +#define VIRTIO_DEVICE_STATUS_ACK 0x01 >> +#define VIRTIO_DEVICE_STATUS_DRIVER 0x02 >> +#define VIRTIO_DEVICE_STATUS_DRIVER_OK 0x04 >> +#define VIRTIO_DEVICE_STATUS_FEATURES_OK 0x08 >> +#define VIRTIO_DEVICE_STATUS_DEV_NEED_RESET 0x40 >> +#define VIRTIO_DEVICE_STATUS_FAILED 0x80 >> + >> /* Old kernels have no such macros defined */ #ifndef >> VIRTIO_NET_F_GUEST_ANNOUNCE >> #define VIRTIO_NET_F_GUEST_ANNOUNCE 21 @@ -364,6 +372,7 @@ struct >> virtio_net { >> uint64_t log_addr; >> struct rte_ether_addr mac; >> uint16_t mtu; >> + uint8_t status; >> >> struct vhost_device_ops const *notify_ops; >> >> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index >> 4a847f368c..e5a44be58d 100644 >> --- a/lib/librte_vhost/vhost_user.c >> +++ b/lib/librte_vhost/vhost_user.c >> @@ -87,6 +87,7 @@ static const char *vhost_message_str[VHOST_USER_MAX] >> = { >> [VHOST_USER_POSTCOPY_END] = "VHOST_USER_POSTCOPY_END", >> [VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD", >> [VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD", >> + [VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS", >> }; >> >> static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg); @@ -1328,6 >> +1329,11 @@ virtio_is_ready(struct virtio_net *dev) >> return 0; >> } >> >> + /* If supported, ensure the frontend is really done with config */ >> + if (dev->protocol_features & (1ULL << >> VHOST_USER_PROTOCOL_F_STATUS)) >> + if (!(dev->status & VIRTIO_DEVICE_STATUS_DRIVER_OK)) >> + return 0; >> + >> dev->flags |= VIRTIO_DEV_READY; >> >> VHOST_LOG_CONFIG(INFO, >> @@ -2425,6 +2431,39 @@ vhost_user_postcopy_end(struct virtio_net **pdev, >> struct VhostUserMsg *msg, >> return RTE_VHOST_MSG_RESULT_REPLY; >> } >> >> +static int >> +vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg, >> + int main_fd __rte_unused) >> +{ >> + struct virtio_net *dev = *pdev; >> + >> + /* As per Virtio specification, the device status is 8bits long */ >> + if (msg->payload.u64 > UINT8_MAX) { >> + VHOST_LOG_CONFIG(ERR, "Invalid VHOST_USER_SET_STATUS >> payload 0x%" PRIx64 "\n", >> + msg->payload.u64); >> + return RTE_VHOST_MSG_RESULT_ERR; >> + } >> + >> + dev->status = msg->payload.u64; >> + >> + VHOST_LOG_CONFIG(INFO, "New device status(0x%08x):\n" >> + "\t-ACKNOWLEDGE: %u\n" >> + "\t-DRIVER: %u\n" >> + "\t-FEATURES_OK: %u\n" >> + "\t-DRIVER_OK: %u\n" >> + "\t-DEVICE_NEED_RESET: %u\n" >> + "\t-FAILED: %u\n", >> + dev->status, >> + !!(dev->status & VIRTIO_DEVICE_STATUS_ACK), >> + !!(dev->status & VIRTIO_DEVICE_STATUS_DRIVER), >> + !!(dev->status & >> VIRTIO_DEVICE_STATUS_FEATURES_OK), >> + !!(dev->status & VIRTIO_DEVICE_STATUS_DRIVER_OK), >> + !!(dev->status & >> VIRTIO_DEVICE_STATUS_DEV_NEED_RESET), >> + !!(dev->status & VIRTIO_DEVICE_STATUS_FAILED)); >> + >> + return RTE_VHOST_MSG_RESULT_OK; > > I see in your patch for virtio-user SET_STATUS support (http://patchwork.dpdk.org/patch/70677/), the > VHOST_USER_SET_STATUS msg may request a reply, but this func does not handle this case. If we don't > handle here, vhost_user_msg_handler will return RTE_VHOST_MSG_RESULT_ERR later. So should we > handle the case here? > Why should the reply be handled in this function? After this function is called, vhost_user_msg_handler() should handle the replies in: if (msg.flags & VHOST_USER_NEED_REPLY) { msg.payload.u64 = ret == RTE_VHOST_MSG_RESULT_ERR; msg.size = sizeof(msg.payload.u64); msg.fd_num = 0; send_vhost_reply(fd, &msg); } else if (ret == RTE_VHOST_MSG_RESULT_ERR) { VHOST_LOG_CONFIG(ERR, "vhost message handling failed.\n"); return -1; } Am I missing something? > Thanks, > Chenbo > >> +} >> + >> typedef int (*vhost_message_handler_t)(struct virtio_net **pdev, >> struct VhostUserMsg *msg, >> int main_fd); >> @@ -2457,6 +2496,7 @@ static vhost_message_handler_t >> vhost_message_handlers[VHOST_USER_MAX] = { >> [VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end, >> [VHOST_USER_GET_INFLIGHT_FD] = vhost_user_get_inflight_fd, >> [VHOST_USER_SET_INFLIGHT_FD] = vhost_user_set_inflight_fd, >> + [VHOST_USER_SET_STATUS] = vhost_user_set_status, >> }; >> >> /* return bytes# of read on success or negative val on failure. */ diff --git >> a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h index >> 1f65efa4a9..74fd361e3a 100644 >> --- a/lib/librte_vhost/vhost_user.h >> +++ b/lib/librte_vhost/vhost_user.h >> @@ -23,7 +23,8 @@ >> (1ULL << >> VHOST_USER_PROTOCOL_F_CRYPTO_SESSION) | \ >> (1ULL << >> VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD) | \ >> (1ULL << >> VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) | \ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_PAGEFAULT)) >> + (1ULL << >> VHOST_USER_PROTOCOL_F_PAGEFAULT) | \ >> + (1ULL << >> VHOST_USER_PROTOCOL_F_STATUS)) >> >> typedef enum VhostUserRequest { >> VHOST_USER_NONE = 0, >> @@ -56,7 +57,8 @@ typedef enum VhostUserRequest { >> VHOST_USER_POSTCOPY_END = 30, >> VHOST_USER_GET_INFLIGHT_FD = 31, >> VHOST_USER_SET_INFLIGHT_FD = 32, >> - VHOST_USER_MAX = 33 >> + VHOST_USER_SET_STATUS = 36, >> + VHOST_USER_MAX = 37 >> } VhostUserRequest; >> >> typedef enum VhostUserSlaveRequest { >> -- >> 2.25.4 > -- Adrián Moreno