From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 224A9C11F69 for ; Fri, 2 Jul 2021 13:31:18 +0000 (UTC) Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by mail.kernel.org (Postfix) with ESMTP id 96AB5613B4 for ; Fri, 2 Jul 2021 13:31:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 96AB5613B4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 776B341363; Fri, 2 Jul 2021 15:31:16 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 1B14341353 for ; Fri, 2 Jul 2021 15:31:14 +0200 (CEST) Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.56]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4GGbTV2Vkmz74J7; Fri, 2 Jul 2021 21:26:54 +0800 (CST) Received: from dggpeml500024.china.huawei.com (7.185.36.10) by dggemv711-chm.china.huawei.com (10.1.198.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Fri, 2 Jul 2021 21:31:10 +0800 Received: from [127.0.0.1] (10.40.190.165) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Fri, 2 Jul 2021 21:31:10 +0800 To: Bruce Richardson CC: Jerin Jacob , Jerin Jacob , =?UTF-8?Q?Morten_Br=c3=b8rup?= , Nipun Gupta , Thomas Monjalon , Ferruh Yigit , dpdk-dev , Hemant Agrawal , Maxime Coquelin , Honnappa Nagarahalli , David Marchand , Satananda Burla , "Prasun Kapoor" References: <25d29598-c26d-8497-2867-9b650c79df49@huawei.com> <3db2eda0-4490-2b8f-c65d-636bcf794494@huawei.com> From: fengchengwen Message-ID: Date: Fri, 2 Jul 2021 21:31:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.40.190.165] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] dmadev discussion summary X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 2021/6/28 18:00, Bruce Richardson wrote: >> 4) The driver's ops design (here we only list key points): >> [dev_info_get]: mainly return the number of HW-queues >> [dev_configure]: nothing important >> [queue_setup]: create one virt-queue, has following main parameters: >> HW-queue-index: the HW-queue index used >> nb_desc: the number of HW descriptors >> opaque: driver's specific info >> Note1: this API return virt-queue index which will used in later API. >> If user want create multiple virt-queue one the same HW-queue, >> they could achieved by call queue_setup with the same >> HW-queue-index. >> Note2: I think it's hard to define queue_setup config paramter, and >> also this is control API, so I think it's OK to use opaque >> pointer to implement it. > I'm not sure opaque pointer will work in practice, so I think we should try > and standardize the parameters as much as possible. Since it's a control > plane API, using a struct with a superset of parameters may be workable. > Let's start with a minimum set and build up from there. I tried to standardize a few parameters, which you can see on the new patch >> uint16_t rte_dmadev_completed(dev, vq_id, dma_cookie_t *cookie, >> uint16_t nb_cpls, bool *has_error) >> -- nb_cpls: indicate max process operations number >> -- has_error: indicate if there is an error >> -- return value: the number of successful completed operations. >> -- example: >> 1) If there are already 32 completed ops, and 4th is error, and >> nb_cpls is 32, then the ret will be 3(because 1/2/3th is OK), and >> has_error will be true. >> 2) If there are already 32 completed ops, and all successful >> completed, then the ret will be min(32, nb_cpls), and has_error >> will be false. >> 3) If there are already 32 completed ops, and all failed completed, >> then the ret will be 0, and has_error will be true. > +1 for this > >> uint16_t rte_dmadev_completed_status(dev_id, vq_id, dma_cookie_t *cookie, >> uint16_t nb_status, uint32_t *status) >> -- return value: the number of failed completed operations. >> And here I agree with Morten: we should design API which adapts to DPDK >> service scenarios. So we don't support some sound-cards DMA, and 2D memory >> copy which mainly used in video scenarios. > > Can I suggest a few adjustments here to the semantics of this API. In > future we may have operations which return a status value, e.g. our > hardware can support ops like compare equal/not-equal, which means that > this API would be meaningful even in case of success. Therefore, I suggest > that the return value be changed to allow success also to be returned in > the array, and the return value is not the number of failed ops, but the > number of ops for which status is being returned. > > Also for consideration: when trying to implement this in a prototype in our > driver, it would be easier if we relax the restriction on the "completed" > API so that we can flag has_error when an error is detected rather than > guaranteeing to return all elements right up to the error. For example, if > we have a burst of packets and one is problematic, it may be easier to flag > the error at the start of the burst and then have a few successful entries > at the start of the completed_status array. [Separate from this] We should > also have a "has_error" or "more_errors" flag on this API too, to indicate > when the user can switch back to using the regular "completed" API. This > means that apps switch from one API to the other when "has_error" is true, > and only switch back when it becomes false again. > We've discussed this before, and I prefer a relatively straightforward API, so in the new version I'll explicitly name it as rte_dmadev_completed_fails. We can continue this on the new patch, and I think that's probably the biggest difference.