From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 4713F201 for ; Mon, 25 Feb 2019 14:20:03 +0100 (CET) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20190225132003euoutp01a7aeec74fc61633c7a28064715849f60~GnfZZRmh61695516955euoutp019 for ; Mon, 25 Feb 2019 13:20:03 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20190225132003euoutp01a7aeec74fc61633c7a28064715849f60~GnfZZRmh61695516955euoutp019 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1551100803; bh=kEfIP/KAcLUnaB2oxdkZF2z9rqT2CWLnG1KD0u7J2qA=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=glaOFImhxIeqSeHDHev2IxRhvTifmwA1Pnc/7oWPMswy7Bm9o04Bc4wVvTKhZLsjQ P8bKFaqmxzf/V6yx/i0xS79XQagdT5rgtUEqEqxUByx+cPc2WFCE/e4ouWnpCSIeYW 7aWnmSj3lC5EUmlvZ+4De732LpFfehtcOpavTCnI= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20190225132002eucas1p17bb74c5c15254229080c2b93b1e3d895~GnfYzUEfg3120231202eucas1p1p; Mon, 25 Feb 2019 13:20:02 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 22.10.04294.28BE37C5; Mon, 25 Feb 2019 13:20:02 +0000 (GMT) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c~GnfX-d2aK2927929279eucas1p2_; Mon, 25 Feb 2019 13:20:01 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20190225132001eusmtrp28e6f14e70092df7609fce38774f8df1d~GnfXw0LQ51590615906eusmtrp2K; Mon, 25 Feb 2019 13:20:01 +0000 (GMT) X-AuditID: cbfec7f4-84fff700000010c6-6a-5c73eb82a677 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 2D.6D.04284.18BE37C5; Mon, 25 Feb 2019 13:20:01 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20190225132000eusmtip1756aba2277f7ec1b5504760a6302dd1d~GnfXOeHfi0085100851eusmtip1B; Mon, 25 Feb 2019 13:20:00 +0000 (GMT) To: Changpeng Liu , dev@dpdk.org Cc: dariusz.stojaczyk@intel.com, maxime.coquelin@redhat.com, tiwei.bie@intel.com, zhihong.wang@intel.com, Jason Wang From: Ilya Maximets Message-ID: <2c1b8abc-47a9-0d0b-5056-0092fc58e310@samsung.com> Date: Mon, 25 Feb 2019 16:19:56 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <1551081095-14286-1-git-send-email-changpeng.liu@intel.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTcRTH/d3Hdl1Nfk7LgwbCqj8K0wqrG6WVhU6i6J8s0rCpFx9tc+36 jsjM1CQfWLEULbXMN+U0NbHXCrXE+Qb7Q5Q0SEUltcwHmvMu8r/POd/zPQ84DCmrpB2ZcE0U p9MoVXKRhKpvWejckzTBB+xNGbBmuxd+idnppSrETs00EGxf6oKYfd4zS7Atd5sp9lXiKsHW ducQxxnFYmEJrXjaPEYopt/2ixSZdRXoHHVJcjSEU4XHcDo3zyuSsJn747RWfyBO32eiE1Gi SzqyZgC7Q+v0MyodSRgZLkOwMFxDCMEcgvnkZkswi6Arp4P6ZyleTLEIpQimq3st/p8I7jQO is1VdtgXRnt+rzvs8WFIM+WvF5E4E0FtVhFtFkTYBb5UfkJmlmJPKOv4s8YMQ+Gd0DbiYU5v wRfhY8aUpcQWPueOrve0xgponxdmkdgBkubKaYGdoWEynzTPAlwvhqJiAxLWPgUlGZW0wHYw 3lonFngbrL5+Qgh8E4aSx5BgTkOgN65YhGNQN2ESm5cj8S540eRmRsAn4FbnVgFtYGDSVljB BnLq9aSQlkJaikzosQOWPpSSAjvC16lZcTaS5204LG/DMXkbjsn7P7YQURXIgYvm1aEcv1/D xbrySjUfrQl1DY5UG9DaC7WvtM41oqblICPCDJJvlrYN8QEyWhnDx6uNCBhSbi91715LSUOU 8QmcLjJQF63ieCNyYii5g/S61bC/DIcqo7irHKfldP9UgrF2TEReYabxAqAKjvhU5frdG3xH EsuG5UmFe7ZREnDwZFGL93Z98fdywvCjWTx4u/9MNK+PqCh3qtk0gkXFMeGPfYksH3bAzS/4 ZUyQj7PqQvVZ/eUhUYSHt9WNqsCikdZDp8+rveICJf4J2uTUN1nvYx/ZPhy4ZugV+XZ9o6Pm tA/kFB+m3Leb1PHKvzXBTLc+AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrLIsWRmVeSWpSXmKPExsVy+t/xu7qNr4tjDB6s4LO4+PMru8X732sY Ld592s5kcaX9J7vFskufmSyOde5hsdja8J/JYvPFSUwOHB6/Fixl9Vi85yWTx/t9V9k8+ras YgxgidKzKcovLUlVyMgvLrFVija0MNIztLTQMzKx1DM0No+1MjJV0rezSUnNySxLLdK3S9DL +DT5FWvBdNOK6VfOsTYwNuh0MXJySAiYSCz61cbUxcjFISSwlFHiy9lbrBAJKYkfvy5A2cIS f651sYHYQgLvGSWu/ckCsYUFPCWeXPrGAmKLCFhKdJybwwIyiFmgh1Fi2pyN7BAN7hIPH0wB G8QmoCNxavURRhCbV8BOYsXZH0A2BweLgKrEice2IGFRgQiJj0/3MUGUCEqcnPkEbD6ngIfE 6e93wUYyC6hL/Jl3iRnCFpdo+rKSFcKWl9j+dg7zBEahWUjaZyFpmYWkZRaSlgWMLKsYRVJL i3PTc4sN9YoTc4tL89L1kvNzNzECo27bsZ+bdzBe2hh8iFGAg1GJh1fhUXGMEGtiWXFl7iFG CQ5mJRFek4tAId6UxMqq1KL8+KLSnNTiQ4ymQL9NZJYSTc4HJoS8knhDU0NzC0tDc2NzYzML JXHe8waVUUIC6YklqdmpqQWpRTB9TBycUg2MiX4HDquFdGREFcYYzDCp2aa1Zouh/QmdXse3 yV//xHtOqefhveqcqNb6v0SJf5r7ysu/Zs18HixpevpivYCQBE/0JAWFyBOdMxgyH/4+/Nji yqKmZ2n8szVnPN66PeRAUM/yOXniGUpSM7XlXQu1ctrFzr7hrGv5863wGoPcR+WG68c231yl xFKckWioxVxUnAgAFm1wW9ACAAA= X-CMS-MailID: 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c References: <1551081095-14286-1-git-send-email-changpeng.liu@intel.com> Subject: Re: [dpdk-dev] vhost: add virtio configuration space access socket messages 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: , X-List-Received-Date: Mon, 25 Feb 2019 13:20:04 -0000 On 25.02.2019 10:51, Changpeng Liu wrote: > This patch adds new vhost user messages GET_CONFIG and SET_CONFIG > used to get/set virtio device's PCI configuration space. Beside the fact that some additional description and reasoning required, I do not see the usage of this feature. You're defining the flag VHOST_USER_PROTOCOL_F_CONFIG, but it's never used. So, none of dpdk vhost backends (vdpa, vhost-user) will use this feature. You, probably, missed adding it to VHOST_USER_PROTOCOL_FEATURES or VDPA_SUPPORTED_PROTOCOL_FEATURES. >>From the other side, current implementation forces application to properly implement the get/set_config callbacks. Otherwise, receiving of the messages will result in RTE_VHOST_MSG_RESULT_ERR and subsequent vhost disconnection. This looks strange, because supported protocol features normally enabled by default. Am I misunderstood something ? One more thing. According to spec: "vhost-user slave uses zero length of payload to indicate an error to vhost-user master". However, current version of 'vhost_user_get_config' replies with size equal to 'sizeof(uint64_t)'. Best regards, Ilya Maximets. > Signed-off-by: Changpeng Liu > Reviewed-by: Darek Stojaczyk > --- > lib/librte_vhost/rte_vhost.h | 8 ++++++++ > lib/librte_vhost/vhost_user.c | 44 +++++++++++++++++++++++++++++++++++++++++++ > lib/librte_vhost/vhost_user.h | 16 ++++++++++++++++ > 3 files changed, 68 insertions(+) > > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h > index 2753670..ab710ce 100644 > --- a/lib/librte_vhost/rte_vhost.h > +++ b/lib/librte_vhost/rte_vhost.h > @@ -63,6 +63,10 @@ > #define VHOST_USER_PROTOCOL_F_PAGEFAULT 8 > #endif > > +#ifndef VHOST_USER_PROTOCOL_F_CONFIG > +#define VHOST_USER_PROTOCOL_F_CONFIG 9 > +#endif > + > #ifndef VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD > #define VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD 10 > #endif > @@ -173,6 +177,10 @@ struct vhost_device_ops { > > int (*vring_state_changed)(int vid, uint16_t queue_id, int enable); /**< triggered when a vring is enabled or disabled */ > > + int (*get_config)(int vid, uint8_t *config, uint32_t config_len); > + int (*set_config)(int vid, uint8_t *config, uint32_t offset, > + uint32_t len, uint32_t flags); > + > /** > * Features could be changed after the feature negotiation. > * For example, VHOST_F_LOG_ALL will be set/cleared at the > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index b086ad9..8e42734 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -73,6 +73,8 @@ > [VHOST_USER_NET_SET_MTU] = "VHOST_USER_NET_SET_MTU", > [VHOST_USER_SET_SLAVE_REQ_FD] = "VHOST_USER_SET_SLAVE_REQ_FD", > [VHOST_USER_IOTLB_MSG] = "VHOST_USER_IOTLB_MSG", > + [VHOST_USER_GET_CONFIG] = "VHOST_USER_GET_CONFIG", > + [VHOST_USER_SET_CONFIG] = "VHOST_USER_SET_CONFIG", > [VHOST_USER_CRYPTO_CREATE_SESS] = "VHOST_USER_CRYPTO_CREATE_SESS", > [VHOST_USER_CRYPTO_CLOSE_SESS] = "VHOST_USER_CRYPTO_CLOSE_SESS", > [VHOST_USER_POSTCOPY_ADVISE] = "VHOST_USER_POSTCOPY_ADVISE", > @@ -359,6 +361,46 @@ > return RTE_VHOST_MSG_RESULT_OK; > } > > +static int > +vhost_user_get_config(struct virtio_net **pdev, struct VhostUserMsg *msg, > + int main_fd __rte_unused) > +{ > + struct virtio_net *dev = *pdev; > + > + if (!dev->notify_ops->get_config) { > + msg->size = sizeof(uint64_t); > + return RTE_VHOST_MSG_RESULT_REPLY; > + } > + > + if (dev->notify_ops->get_config(dev->vid, > + msg->payload.config.region, > + msg->payload.config.size) != 0) { > + msg->size = sizeof(uint64_t); > + } > + > + return RTE_VHOST_MSG_RESULT_REPLY; > +} > + > +static int > +vhost_user_set_config(struct virtio_net **pdev, struct VhostUserMsg *msg, > + int main_fd __rte_unused) > +{ > + struct virtio_net *dev = *pdev; > + > + if (!dev->notify_ops->set_config) > + return RTE_VHOST_MSG_RESULT_ERR; > + > + if (dev->notify_ops->set_config(dev->vid, > + msg->payload.config.region, > + msg->payload.config.offset, > + msg->payload.config.size, > + msg->payload.config.flags) != 0) { > + return RTE_VHOST_MSG_RESULT_ERR; > + } > + > + return RTE_VHOST_MSG_RESULT_OK; > +} > + > /* > * Reallocate virtio_dev and vhost_virtqueue data structure to make them on the > * same numa node as the memory of vring descriptor. > @@ -1725,6 +1767,8 @@ typedef int (*vhost_message_handler_t)(struct virtio_net **pdev, > [VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu, > [VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd, > [VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg, > + [VHOST_USER_GET_CONFIG] = vhost_user_get_config, > + [VHOST_USER_SET_CONFIG] = vhost_user_set_config, > [VHOST_USER_POSTCOPY_ADVISE] = vhost_user_set_postcopy_advise, > [VHOST_USER_POSTCOPY_LISTEN] = vhost_user_set_postcopy_listen, > [VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end, > diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h > index 2a650fe..057cdec 100644 > --- a/lib/librte_vhost/vhost_user.h > +++ b/lib/librte_vhost/vhost_user.h > @@ -12,6 +12,11 @@ > > /* refer to hw/virtio/vhost-user.c */ > > +/* > + * Maximum size of virtio device config space > + */ > +#define VHOST_USER_MAX_CONFIG_SIZE 256 > + > #define VHOST_MEMORY_MAX_NREGIONS 8 > > #define VHOST_USER_PROTOCOL_FEATURES ((1ULL << VHOST_USER_PROTOCOL_F_MQ) | \ > @@ -49,6 +54,8 @@ > VHOST_USER_NET_SET_MTU = 20, > VHOST_USER_SET_SLAVE_REQ_FD = 21, > VHOST_USER_IOTLB_MSG = 22, > + VHOST_USER_GET_CONFIG = 24, > + VHOST_USER_SET_CONFIG = 25, > VHOST_USER_CRYPTO_CREATE_SESS = 26, > VHOST_USER_CRYPTO_CLOSE_SESS = 27, > VHOST_USER_POSTCOPY_ADVISE = 28, > @@ -60,6 +67,7 @@ > typedef enum VhostUserSlaveRequest { > VHOST_USER_SLAVE_NONE = 0, > VHOST_USER_SLAVE_IOTLB_MSG = 1, > + VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2, > VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3, > VHOST_USER_SLAVE_MAX > } VhostUserSlaveRequest; > @@ -112,6 +120,13 @@ > uint64_t offset; > } VhostUserVringArea; > > +typedef struct VhostUserConfig { > + uint32_t offset; > + uint32_t size; > + uint32_t flags; > + uint8_t region[VHOST_USER_MAX_CONFIG_SIZE]; > +} VhostUserConfig; > + > typedef struct VhostUserMsg { > union { > uint32_t master; /* a VhostUserRequest value */ > @@ -131,6 +146,7 @@ > struct vhost_vring_addr addr; > VhostUserMemory memory; > VhostUserLog log; > + VhostUserConfig config; > struct vhost_iotlb_msg iotlb; > VhostUserCryptoSessionParam crypto_session; > VhostUserVringArea area; >