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 4A406A0546; Thu, 16 Jul 2020 11:51:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8442E1BE95; Thu, 16 Jul 2020 11:51:36 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id 5C2414F9A for ; Thu, 16 Jul 2020 11:51:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594893094; 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=eIZVYJQvYFg8HhfZf2A+4y1KAKVIvGfdbR/FO9n7kFA=; b=BYBt9Af02HSqJkD6Cd3lkQmF/0bLmZPauQ2geGqsu2Y0E3isq08RW4E62SbFEeAOKO6tu9 gQ/UMxEcIiElNtrW8TYm8/KtxlFC5wEEqMyYkHtfjFVplD8UppgHeDdI+z+zuyo3rbYZpG LEJpHxHwBXEIps7nSQuSnLwoxLddeCY= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-110-vMq7qNkfMruenHGfMj1daA-1; Thu, 16 Jul 2020 05:51:31 -0400 X-MC-Unique: vMq7qNkfMruenHGfMj1daA-1 Received: by mail-wr1-f69.google.com with SMTP id d11so5327573wrw.12 for ; Thu, 16 Jul 2020 02:51:31 -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=eIZVYJQvYFg8HhfZf2A+4y1KAKVIvGfdbR/FO9n7kFA=; b=I9NceKwucrrvdd6eNaFCT6D1EevBbDbzXZc58D9DVfhBM4b4kRRKEfeJERPt/X1VO2 c3kib8C3awtI28rOkc5aSR3UIbXkuCGK8PquL2yjr4riAOqr6hK4BBWnAGcwShhV2S+k QM8/WM+SGZEv6tD0ZVtTzlIt2qUWRFhyRwmXf42hUYlkirZQ+3e9b/62nyQBJFa3xX9V dyGwCRXvqGRYoOKkH4XqA0aFCIEL4DZEosc3eqTv7iReelGdRTWzW3D1UUzSUQmJhalo OiSQRRTkk+im/2nlHxj30utEbDOyPHulMipqUTnS5KVy++h03+vZl0FGzhIy2+e5rGD6 Gphw== X-Gm-Message-State: AOAM532ITmv0rdiXVSo8HHApHmKJ00PNpSKq1ltYP6sWpSPmXD9eBtO1 YkLHhH4vC5pEHsNe54vZGmV6teFu2s6CpXAE8DWE8XaJaOnDHq43asg76OVlWu5AnpZAFIyXxxY yP3E= X-Received: by 2002:adf:8024:: with SMTP id 33mr4553865wrk.117.1594893090048; Thu, 16 Jul 2020 02:51:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyZUPhSfsHkWs1Of9jqFKgU8dGJrxdbUNZ9mVjKa6c/E04NfQbM4FiuT7WNDvop+okfcAROfA== X-Received: by 2002:adf:8024:: with SMTP id 33mr4553847wrk.117.1594893089805; Thu, 16 Jul 2020 02:51:29 -0700 (PDT) Received: from amorenoz.users.ipa.redhat.com ([92.176.159.3]) by smtp.gmail.com with ESMTPSA id c20sm10219445wrb.65.2020.07.16.02.51.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Jul 2020 02:51:29 -0700 (PDT) To: "Xia, Chenbo" , "dev@dpdk.org" Cc: "maxime.coquelin@redhat.com" , "Wang, Zhihong" References: <20200715171828.95887-1-amorenoz@redhat.com> <20200715171828.95887-4-amorenoz@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: Date: Thu, 16 Jul 2020 11:51:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=amorenoz@redhat.com 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 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio-user 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" On 7/16/20 10:58 AM, Xia, Chenbo wrote: > Hi Adrian, > >> -----Original Message----- >> From: Adrian Moreno >> Sent: Thursday, July 16, 2020 3:43 PM >> To: Xia, Chenbo ; dev@dpdk.org >> Cc: maxime.coquelin@redhat.com; Wang, Zhihong >> Subject: Re: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to Virtio- >> user >> >> >> >> On 7/16/20 5:15 AM, Xia, Chenbo wrote: >>> Hi Adrian, >>> >>>> -----Original Message----- >>>> From: Adrian Moreno >>>> Sent: Thursday, July 16, 2020 1:18 AM >>>> To: dev@dpdk.org >>>> Cc: maxime.coquelin@redhat.com; Wang, Zhihong >>>> ; amorenoz@redhat.com; Xia, Chenbo >>>> >>>> Subject: [PATCH 3/5] net/virtio: add VIRTIO_SET_STATUS support to >>>> Virtio-user >>>> >>>> From: Maxime Coquelin >>>> >>>> This patch adds support for VHOST_USER_SET_STATUS request. It is used >>>> to make the backend aware of Virtio devices status update. >>>> >>>> It is useful for the backend to know when the Virtio driver is done >>>> with the Virtio device configuration. >>>> >>>> Signed-off-by: Maxime Coquelin >>>> Signed-off-by: Adrian Moreno >>>> --- >>>> drivers/net/virtio/virtio_user/vhost.h | 6 +++++ >>>> drivers/net/virtio/virtio_user/vhost_user.c | 10 ++++++++ >>>> .../net/virtio/virtio_user/virtio_user_dev.c | 23 ++++++++++++++++++- >>>> .../net/virtio/virtio_user/virtio_user_dev.h | 1 + >>>> drivers/net/virtio/virtio_user_ethdev.c | 1 + >>>> 5 files changed, 40 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/virtio/virtio_user/vhost.h >>>> b/drivers/net/virtio/virtio_user/vhost.h >>>> index 260e1c308..8f49ef457 100644 >>>> --- a/drivers/net/virtio/virtio_user/vhost.h >>>> +++ b/drivers/net/virtio/virtio_user/vhost.h >>>> @@ -57,6 +57,10 @@ struct vhost_vring_addr { #define >>>> VHOST_USER_PROTOCOL_F_REPLY_ACK 3 #endif >>>> >>>> +#ifndef VHOST_USER_PROTOCOL_F_STATUS #define >>>> +VHOST_USER_PROTOCOL_F_STATUS 16 #endif >>>> + >>>> enum vhost_user_request { >>>> VHOST_USER_NONE = 0, >>>> VHOST_USER_GET_FEATURES = 1, >>>> @@ -77,6 +81,8 @@ enum vhost_user_request { >>>> VHOST_USER_SET_PROTOCOL_FEATURES = 16, >>>> VHOST_USER_GET_QUEUE_NUM = 17, >>>> VHOST_USER_SET_VRING_ENABLE = 18, >>>> + VHOST_USER_SET_STATUS = 39, >>>> + VHOST_USER_GET_STATUS = 40, >>>> VHOST_USER_MAX >>>> }; >>>> >>>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c >>>> b/drivers/net/virtio/virtio_user/vhost_user.c >>>> index 631d0e353..2332e01d1 100644 >>>> --- a/drivers/net/virtio/virtio_user/vhost_user.c >>>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c >>>> @@ -244,6 +244,8 @@ const char * const vhost_msg_strings[] = { >>>> [VHOST_USER_SET_VRING_ENABLE] = "VHOST_SET_VRING_ENABLE", >>>> [VHOST_USER_GET_PROTOCOL_FEATURES] = >>>> "VHOST_USER_GET_PROTOCOL_FEATURES", >>>> [VHOST_USER_SET_PROTOCOL_FEATURES] = >>>> "VHOST_USER_SET_PROTOCOL_FEATURES", >>>> + [VHOST_USER_SET_STATUS] = "VHOST_SET_STATUS", >>>> + [VHOST_USER_GET_STATUS] = "VHOST_GET_STATUS", >>>> }; >>>> >>>> static int >>>> @@ -280,6 +282,14 @@ vhost_user_sock(struct virtio_user_dev *dev, >>>> need_reply = 1; >>>> break; >>>> >>>> + case VHOST_USER_SET_STATUS: >>>> + if (!(dev->protocol_features & >>>> + (1ULL << >>>> VHOST_USER_PROTOCOL_F_STATUS))) >>>> + return 0; >>>> + >>>> + if (has_reply_ack) >>>> + msg.flags |= VHOST_USER_NEED_REPLY_MASK; >>>> + /* Fallthrough */ >>> >>> There's a coding style issue here: >>> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough >> comment. >>> Could you fix this? >>> >> "fallthrough" is not defined. I think this macro is defined in the linux kernel >> (where checkpatches.pl comes from?). We could define the same macro that >> would depend in compiler support for __falthrough__ attribute. >> >> In any case, this is not the only case: >> >> $ find -name *.c | xargs grep -i "/\* fallthrough \*/" | wc -l >> 62 >> >> So maybe this warning is new? >> Should I change all of them together in another patch? > > checkpatches.pl is in linux kernel. I think it prefers 'fallthrough' rather than > 'Fallthrough'. I think it's a linux style, right? > It refers to this: https://github.com/torvalds/linux/blob/91a9a90d040e8b9ff63d48ea71468e0f4db764ff/include/linux/compiler_attributes.h#L201 Since we don't have that pseudo-keyword, we should see if we can disable this in checkpatches. > Thanks! > Chenbo > >> >>>> case VHOST_USER_SET_FEATURES: >>>> case VHOST_USER_SET_PROTOCOL_FEATURES: >>>> case VHOST_USER_SET_LOG_BASE: >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> index 0a6991bcc..2c400a448 100644 >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> @@ -424,7 +424,8 @@ virtio_user_dev_setup(struct virtio_user_dev >>>> *dev) >>>> >>>> #define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ >>>> (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ >>>> - 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) >>>> + 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ >>>> + 1ULL << VHOST_USER_PROTOCOL_F_STATUS) >>>> >>>> int >>>> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int >>>> queues, @@ - >>>> 783,3 +784,23 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, >>>> uint16_t >>>> queue_idx) >>>> __atomic_add_fetch(&vring->used->idx, 1, >> __ATOMIC_RELAXED); >>>> } >>>> } >>>> + >>>> +int >>>> +virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t >>>> +status) { >>>> + int ret; >>>> + uint64_t arg = status; >>>> + >>>> + /* Vhost-user only for now */ >>>> + if (!is_vhost_user_by_type(dev->path)) >>>> + return 0; >>>> + >>>> + if (!(dev->protocol_features & (1ULL << >>>> VHOST_USER_PROTOCOL_F_STATUS))) >>>> + return 0; >>>> + >>>> + ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg); >>>> + if (ret) >>>> + return -1; >>> >>> Do you think we should add a log here to show if SET_STAUTS failed? >>> >> Good idea! Will do in the next version. Thanks >> >>> Thanks! >>> Chenbo >>> >>>> + >>>> + return 0; >>>> +} >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> index 10b274d19..a76d7cfaf 100644 >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> @@ -74,4 +74,5 @@ void virtio_user_handle_cq(struct virtio_user_dev >>>> *dev, uint16_t queue_idx); void virtio_user_handle_cq_packed(struct >>>> virtio_user_dev *dev, >>>> uint16_t queue_idx); >>>> uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t >>>> q_pairs); >>>> +int virtio_user_send_status_update(struct virtio_user_dev *dev, >>>> +uint8_t status); >>>> #endif >>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c >>>> b/drivers/net/virtio/virtio_user_ethdev.c >>>> index 5b06d8e89..e52f11341 100644 >>>> --- a/drivers/net/virtio/virtio_user_ethdev.c >>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c >>>> @@ -273,6 +273,7 @@ virtio_user_set_status(struct virtio_hw *hw, >>>> uint8_t >>>> status) >>>> else if (status == VIRTIO_CONFIG_STATUS_RESET) >>>> virtio_user_reset(hw); >>>> dev->status = status; >>>> + virtio_user_send_status_update(dev, status); >>>> } >>>> >>>> static uint8_t >>>> -- >>>> 2.26.2 >>> >> >> -- >> Adrián Moreno > -- Adrián Moreno