From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 92E18A046B for ; Tue, 25 Jun 2019 09:50:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BA9BB1B9F0; Tue, 25 Jun 2019 09:50:26 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 13AEF1B9B2 for ; Tue, 25 Jun 2019 09:50:24 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5A4D8308FBB1; Tue, 25 Jun 2019 07:50:04 +0000 (UTC) Received: from [10.36.112.46] (ovpn-112-46.ams2.redhat.com [10.36.112.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C248860C43; Tue, 25 Jun 2019 07:50:00 +0000 (UTC) From: Maxime Coquelin To: Noa Ezra , "tiwei.bie@intel.com" , "zhihong.wang@intel.com" Cc: Matan Azrad , "dev@dpdk.org" References: <1560924898-221025-1-git-send-email-noae@mellanox.com> <5936b36c-7cd7-ae50-99ad-c8bfb1c4c853@redhat.com> Message-ID: <5f0ada2b-0e87-df2f-f082-18429eadf646@redhat.com> Date: Tue, 25 Jun 2019 09:49:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <5936b36c-7cd7-ae50-99ad-c8bfb1c4c853@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Tue, 25 Jun 2019 07:50:14 +0000 (UTC) Subject: Re: [dpdk-dev] [Suspected-Phishing][PATCH] net/vhost: add an API for get queue 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" And about short term, can't you just call rte_eth_vhost_get_queue_event() in loop at startup until you get -1 and build the states based on that? As states->seen is zero-initialized, you would get all queues that have been enabled before event handler is registered, right? Thanks, Maxime On 6/25/19 9:36 AM, Maxime Coquelin wrote: > > > On 6/25/19 9:00 AM, Noa Ezra wrote: >> >> >>> -----Original Message----- >>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >>> Sent: Monday, June 24, 2019 7:47 PM >>> To: Noa Ezra ; tiwei.bie@intel.com; >>> zhihong.wang@intel.com >>> Cc: Matan Azrad ; dev@dpdk.org >>> Subject: Re: [Suspected-Phishing][PATCH] net/vhost: add an API for get >>> queue status >>> >>> >>> >>> On 6/24/19 1:08 PM, Noa Ezra wrote: >>>> Hi, >>>> What do you say about this patch? >>> >>> I acknowledge we miss a way to get the queue state, but I don't like >>> introducing new APIs for PMD drivers. >>> >>> I looked at ethdev ops on Friday, but it seems no one currently >>> available can >>> do the job. >>> >>> I would suggest to create new ethdev ops like: >>> .get_tx_queue_state(int port_id, int queue_id) .get_rx_queue_state(int >>> port_id, int queue_id) >>> >>> That would return wether the queue is enabled or not. >>> >>> What do you think? >> >> I understand what you say about not having new APIs in the PMD, but I >> see that there are already APIs in the vhost pmd. > > Indeed, and I hope we can get rid off them one day. > >> rte_eth_vhost_get_queue_event allows to get the queue_state >> asynchronously, so it sounds reasonable to add an API that allows to >> get the queue_state synchronously. > > I think we can get rid off rte_eth_vhost_get_queue_event easily. > Maybe we can even just drop it if we implement the new ethdev ops. > >> Maybe in the future we can create new ethdev ops like you suggested >> and remove both APIs for getting queue_state. >> Currently I don’t have the resources for such implementation and we >> really need the ability to get queue_state. > > It is anyway too late for v19.08, as the proposal deadline is over 3 > weeks now. I will only accept fixes or small patches that have limited > impact. > > Here, the patch is adding a new API, which is significant. > And with the new API stability rules that is being put in place, it may > imply that we'll need to support it for more than 2 years. > > So for v19.11, if you think you don't have the bandwidth, maybe I can > try to do it. I really prefer that than having an API already deprecated > when merged in master. > > Thanks, > Maxime >> Thanks, >> Noa. >> >> >>> Thanks, >>> Maxime >>>> Thanks, >>>> Noa. >>>> >>>>> -----Original Message----- >>>>> From: Noa Ezra [mailto:noae@mellanox.com] >>>>> Sent: Wednesday, June 19, 2019 9:15 AM >>>>> To: maxime.coquelin@redhat.com >>>>> Cc: Matan Azrad ; dev@dpdk.org; Noa Ezra >>>>> >>>>> Subject: [Suspected-Phishing][PATCH] net/vhost: add an API for get >>>>> queue status >>>>> >>>>> Add an API that returns queue status for requested queue in the port. >>>>> The queue's status can be changed before the user has signed for the >>>>> queue state event interrupt. In this case the user can't know the >>>>> current queue's status. This API returns the current status. >>>>> >>>>> Signed-off-by: Noa Ezra >>>>> Reviewed-by: Matan Azrad >>>>> --- >>>>>    drivers/net/vhost/rte_eth_vhost.c           | 47 >>>>> +++++++++++++++++++++++++++++ >>>>>    drivers/net/vhost/rte_eth_vhost.h           | 18 +++++++++++ >>>>>    drivers/net/vhost/rte_pmd_vhost_version.map |  6 ++++ >>>>>    3 files changed, 71 insertions(+) >>>>> >>>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c >>>>> b/drivers/net/vhost/rte_eth_vhost.c >>>>> index 9a54020..cad1e5c 100644 >>>>> --- a/drivers/net/vhost/rte_eth_vhost.c >>>>> +++ b/drivers/net/vhost/rte_eth_vhost.c >>>>> @@ -855,6 +855,7 @@ struct vhost_xstats_name_off { >>>>>        /* won't be NULL */ >>>>>        state = vring_states[eth_dev->data->port_id]; >>>>>        rte_spinlock_lock(&state->lock); >>>>> + >>>>>        state->cur[vring] = enable; >>>>>        state->max_vring = RTE_MAX(vring, state->max_vring); >>>>>        rte_spinlock_unlock(&state->lock); >>>>> @@ -874,6 +875,52 @@ struct vhost_xstats_name_off {  }; >>>>> >>>>>    int >>>>> +rte_eth_vhost_get_queue_status(uint16_t port_id, bool rx, uint16_t >>>>> queue_id, >>>>> +        bool *queue_status) >>>>> +{ >>>>> +    struct rte_vhost_vring_state *state; >>>>> +    struct internal_list *list; >>>>> +    struct rte_eth_dev *eth_dev; >>>>> +    int found = 0; >>>>> +    uint16_t nb_q = 0; >>>>> + >>>>> +    if (port_id >= RTE_MAX_ETHPORTS) { >>>>> +        VHOST_LOG(ERR, "Invalid port id\n"); >>>>> +        return -1; >>>>> +    } >>>>> +    TAILQ_FOREACH(list, &internal_list, next) { >>>>> +        eth_dev = list->eth_dev; >>>>> +        if (eth_dev->data->port_id == port_id) { >>>>> +            nb_q = rx ? eth_dev->data->nb_rx_queues : >>>>> +                    eth_dev->data->nb_tx_queues; >>>>> +            found = 1; >>>>> +            break; >>>>> +        } >>>>> +    } >>>>> +    if (!found) { >>>>> +        VHOST_LOG(ERR, "No device found for port id %u\n", >>>>> port_id); >>>>> +        return -1; >>>>> +    } >>>>> +    if (queue_id >= nb_q) { >>>>> +        VHOST_LOG(ERR, "Invalid queue id\n"); >>>>> +        return -1; >>>>> +    } >>>>> + >>>>> +    state = vring_states[port_id]; >>>>> +    if (!state) { >>>>> +        VHOST_LOG(ERR, "Unused port\n"); >>>>> +        return -1; >>>>> +    } >>>>> + >>>>> +    rte_spinlock_lock(&state->lock); >>>>> +    *queue_status = rx ? state->cur[queue_id * 2 + 1] : >>>>> +            state->cur[queue_id * 2]; >>>>> +    rte_spinlock_unlock(&state->lock); >>>>> + >>>>> +    return 0; >>>>> +} >>>>> + >>>>> +int >>>>>    rte_eth_vhost_get_queue_event(uint16_t port_id, >>>>>            struct rte_eth_vhost_queue_event *event)  { diff --git >>>>> a/drivers/net/vhost/rte_eth_vhost.h >>>>> b/drivers/net/vhost/rte_eth_vhost.h >>>>> index 0e68b9f..1e65c69 100644 >>>>> --- a/drivers/net/vhost/rte_eth_vhost.h >>>>> +++ b/drivers/net/vhost/rte_eth_vhost.h >>>>> @@ -44,6 +44,24 @@ int rte_eth_vhost_get_queue_event(uint16_t >>> port_id, >>>>>            struct rte_eth_vhost_queue_event *event); >>>>> >>>>>    /** >>>>> + * Get queue status for specific queue in the port. >>>>> + * >>>>> + * @param[in] port_id >>>>> + *  Port id. >>>>> + * @param[in] rx >>>>> + *  True is rx, False if tx >>>>> + * @paran[in] queue_id >>>>> + *  Queue_id >>>>> + * @param[out] queue_status >>>>> + *  Pointer to a boolean, True is enable, False if disable. >>>>> + * @return >>>>> + *  - On success, zero, queue_status is updated. >>>>> + *  - On failure, a negative value, queue_status is not updated. >>>>> + */ >>>>> +int rte_eth_vhost_get_queue_status(uint16_t port_id, bool rx, >>>>> +uint16_t >>>>> queue_id, >>>>> +        bool *queue_status); >>>>> + >>>>> +/** >>>>>     * Get the 'vid' value associated with the specified port. >>>>>     * >>>>>     * @return >>>>> diff --git a/drivers/net/vhost/rte_pmd_vhost_version.map >>>>> b/drivers/net/vhost/rte_pmd_vhost_version.map >>>>> index 695db85..1eabfd2 100644 >>>>> --- a/drivers/net/vhost/rte_pmd_vhost_version.map >>>>> +++ b/drivers/net/vhost/rte_pmd_vhost_version.map >>>>> @@ -11,3 +11,9 @@ DPDK_16.11 { >>>>> >>>>>        rte_eth_vhost_get_vid_from_port_id; >>>>>    }; >>>>> + >>>>> +DPDK_19.08 { >>>>> +    global: >>>>> + >>>>> +    rte_eth_vhost_get_queue_status; >>>>> +}; >>>>> -- >>>>> 1.8.3.1 >>>>