From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Coquelin Subject: Re: [PATCH v2 1/6] vhost: export vhost feature definitions Date: Tue, 6 Mar 2018 15:03:00 +0100 Message-ID: References: <1517614137-62926-1-git-send-email-zhihong.wang@intel.com> <20180213092106.57996-1-zhihong.wang@intel.com> <20180213092106.57996-2-zhihong.wang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Bie, Tiwei" , "yliu@fridaylinux.org" , "Liang, Cunming" , "Wang, Xiao W" , "Daly, Dan" To: "Tan, Jianfeng" , "Wang, Zhihong" , "dev@dpdk.org" Return-path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id BE96E4CA1 for ; Tue, 6 Mar 2018 15:03:05 +0100 (CET) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 03/06/2018 10:37 AM, Tan, Jianfeng wrote: > > >> -----Original Message----- >> From: Wang, Zhihong >> Sent: Tuesday, February 13, 2018 5:21 PM >> To: dev@dpdk.org >> Cc: Tan, Jianfeng; Bie, Tiwei; maxime.coquelin@redhat.com; >> yliu@fridaylinux.org; Liang, Cunming; Wang, Xiao W; Daly, Dan; Wang, >> Zhihong >> Subject: [PATCH v2 1/6] vhost: export vhost feature definitions >> >> This patch exports vhost-user protocol features to support device driver >> development. >> >> Signed-off-by: Zhihong Wang >> --- >> lib/librte_vhost/rte_vhost.h | 8 ++++++++ >> lib/librte_vhost/vhost.h | 4 +--- >> lib/librte_vhost/vhost_user.c | 9 +++++---- >> lib/librte_vhost/vhost_user.h | 20 +++++++------------- >> 4 files changed, 21 insertions(+), 20 deletions(-) >> >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h >> index d33206997..b05162366 100644 >> --- a/lib/librte_vhost/rte_vhost.h >> +++ b/lib/librte_vhost/rte_vhost.h >> @@ -29,6 +29,14 @@ extern "C" { >> #define RTE_VHOST_USER_DEQUEUE_ZERO_COPY (1ULL << 2) >> #define RTE_VHOST_USER_IOMMU_SUPPORT (1ULL << 3) >> >> +#define RTE_VHOST_USER_PROTOCOL_F_MQ 0 > > Instead of adding a "RTE_" prefix. I prefer to define it like this: > #ifndef VHOST_USER_PROTOCOL_F_MQ > #define VHOST_USER_PROTOCOL_F_MQ 0 > #endif > > Similar to other macros. I agree, it is better to keep same naming as in the spec IMHO. >> +#define RTE_VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 >> +#define RTE_VHOST_USER_PROTOCOL_F_RARP 2 >> +#define RTE_VHOST_USER_PROTOCOL_F_REPLY_ACK 3 >> +#define RTE_VHOST_USER_PROTOCOL_F_NET_MTU 4 >> +#define RTE_VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 >> +#define RTE_VHOST_USER_F_PROTOCOL_FEATURES 30 Please put the above declaration separately, it could be misleading, making to think it is a vhost-user protocol feature whereas it is a Virtio feature. >> + >> /** >> * Information relating to memory regions including offsets to >> * addresses in QEMUs memory file. >> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h >> index 58aec2e0d..a0b0520e2 100644 >> --- a/lib/librte_vhost/vhost.h >> +++ b/lib/librte_vhost/vhost.h >> @@ -174,8 +174,6 @@ struct vhost_msg { >> #define VIRTIO_F_VERSION_1 32 >> #endif >> >> -#define VHOST_USER_F_PROTOCOL_FEATURES 30 >> - >> /* Features supported by this builtin vhost-user net driver. */ >> #define VIRTIO_NET_SUPPORTED_FEATURES ((1ULL << >> VIRTIO_NET_F_MRG_RXBUF) | \ >> (1ULL << VIRTIO_F_ANY_LAYOUT) | \ >> @@ -185,7 +183,7 @@ struct vhost_msg { >> (1ULL << VIRTIO_NET_F_MQ) | \ >> (1ULL << VIRTIO_F_VERSION_1) | \ >> (1ULL << VHOST_F_LOG_ALL) | \ >> - (1ULL << >> VHOST_USER_F_PROTOCOL_FEATURES) | \ >> + (1ULL << >> RTE_VHOST_USER_F_PROTOCOL_FEATURES) | \ >> (1ULL << VIRTIO_NET_F_GSO) | \ >> (1ULL << VIRTIO_NET_F_HOST_TSO4) | \ >> (1ULL << VIRTIO_NET_F_HOST_TSO6) | \ >> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >> index 5c5361066..c93e48e4d 100644 >> --- a/lib/librte_vhost/vhost_user.c >> +++ b/lib/librte_vhost/vhost_user.c >> @@ -527,7 +527,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, >> VhostUserMsg *msg) >> vring_invalidate(dev, vq); >> >> if (vq->enabled && (dev->features & >> - (1ULL << >> VHOST_USER_F_PROTOCOL_FEATURES))) { >> + (1ULL << >> RTE_VHOST_USER_F_PROTOCOL_FEATURES))) { >> dev = translate_ring_addresses(dev, msg- >>> payload.addr.index); >> if (!dev) >> return -1; >> @@ -897,11 +897,11 @@ vhost_user_set_vring_kick(struct virtio_net >> **pdev, struct VhostUserMsg *pmsg) >> vq = dev->virtqueue[file.index]; >> >> /* >> - * When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated, >> + * When RTE_VHOST_USER_F_PROTOCOL_FEATURES is not >> negotiated, >> * the ring starts already enabled. Otherwise, it is enabled via >> * the SET_VRING_ENABLE message. >> */ >> - if (!(dev->features & (1ULL << >> VHOST_USER_F_PROTOCOL_FEATURES))) >> + if (!(dev->features & (1ULL << >> RTE_VHOST_USER_F_PROTOCOL_FEATURES))) >> vq->enabled = 1; >> >> if (vq->kickfd >= 0) >> @@ -1012,7 +1012,8 @@ vhost_user_get_protocol_features(struct >> virtio_net *dev, >> * Qemu versions (from v2.7.0 to v2.9.0). >> */ >> if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) >> - protocol_features &= ~(1ULL << >> VHOST_USER_PROTOCOL_F_REPLY_ACK); >> + protocol_features &= >> + ~(1ULL << >> RTE_VHOST_USER_PROTOCOL_F_REPLY_ACK); >> >> msg->payload.u64 = protocol_features; >> msg->size = sizeof(msg->payload.u64); >> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h >> index 0fafbe6e0..066e772dd 100644 >> --- a/lib/librte_vhost/vhost_user.h >> +++ b/lib/librte_vhost/vhost_user.h >> @@ -14,19 +14,13 @@ >> >> #define VHOST_MEMORY_MAX_NREGIONS 8 >> >> -#define VHOST_USER_PROTOCOL_F_MQ 0 >> -#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 >> -#define VHOST_USER_PROTOCOL_F_RARP 2 >> -#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 >> -#define VHOST_USER_PROTOCOL_F_NET_MTU 4 >> -#define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 >> - >> -#define VHOST_USER_PROTOCOL_FEATURES ((1ULL << >> VHOST_USER_PROTOCOL_F_MQ) | \ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_RARP) | \ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_REPLY_ACK) | \ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_NET_MTU) | \ >> - (1ULL << >> VHOST_USER_PROTOCOL_F_SLAVE_REQ)) >> +#define VHOST_USER_PROTOCOL_FEATURES \ >> + ((1ULL << RTE_VHOST_USER_PROTOCOL_F_MQ) | \ >> + (1ULL << >> RTE_VHOST_USER_PROTOCOL_F_LOG_SHMFD) |\ >> + (1ULL << RTE_VHOST_USER_PROTOCOL_F_RARP) | \ >> + (1ULL << >> RTE_VHOST_USER_PROTOCOL_F_REPLY_ACK) | \ >> + (1ULL << >> RTE_VHOST_USER_PROTOCOL_F_NET_MTU) | \ >> + (1ULL << >> RTE_VHOST_USER_PROTOCOL_F_SLAVE_REQ)) >> >> typedef enum VhostUserRequest { >> VHOST_USER_NONE = 0, >> -- >> 2.13.6 >