From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54376A0C43; Thu, 23 Sep 2021 15:24:43 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C478C41260; Thu, 23 Sep 2021 15:24:42 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 3B3E941257 for ; Thu, 23 Sep 2021 15:24:40 +0200 (CEST) Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.56]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4HFbTB0g3nzWNN0; Thu, 23 Sep 2021 21:23:26 +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.2308.8; Thu, 23 Sep 2021 21:24:36 +0800 Received: from [10.40.190.165] (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.2308.8; Thu, 23 Sep 2021 21:24:36 +0800 To: Jerin Jacob , Bruce Richardson CC: "Pai G, Sunil" , "Hu, Jiayu" , dpdk-dev , "Walsh, Conor" , "Laatz, Kevin" , Jerin Jacob , "Satananda Burla" , Radha Mohan Chintakuntla References: <8622d4b44e8e4b2e90a137a691f0c0a6@intel.com> From: fengchengwen Message-ID: <8b15cd2b-3a47-b471-3c41-a17c4f0849e3@huawei.com> Date: Thu, 23 Sep 2021 21:24:35 +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: 8bit 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] [PATCH v3 2/8] dmadev: add burst capacity API 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/9/23 1:29, Jerin Jacob wrote: > On Wed, Sep 22, 2021 at 10:06 PM Bruce Richardson > wrote: >> >> On Wed, Sep 22, 2021 at 08:56:50AM +0100, Bruce Richardson wrote: >>> On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote: >>>> On 2021/9/22 2:11, Jerin Jacob wrote: >>>>> On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil wrote: >>>>>> >>>>>> Hi Jerin, >>>>>> >>>>>> >>>>>> >>>>>>>>> To understand it better, Could you share more details on feedback >>>>>>>>> mechanism on your application enqueue >>>>>>>>> >>>>>>>>> app_enqueue_v1(.., nb_seg) >>>>>>>>> { >>>>>>>>> /* Not enough space, Let application handle it by >>>>>>>>> dropping or resubmitting */ >>>>>>>>> if (rte_dmadev_burst_capacity() < nb_seg) >>>>>>>>> return -ENOSPC; >>>>>>>>> >>>>>>>>> do rte_dma_op() in loop without checking error; >>>>>>>>> return 0; // Success >>>>>>>>> } >>>>>>>>> >>>>>>>>> vs >>>>>>>>> app_enqueue_v2(.., nb_seg) >>>>>>>>> { >>>>>>>>> int rc; >>>>>>>>> >>>>>>>>> rc |= rte_dma_op() in loop without checking error; >>>>>>>>> return rc; // return the actual status to application >>>>>>>>> if Not enough space, Let application handle it by dropping or >>>>>>>>> resubmitting */ } >>>>>>>>> >>>>>>>>> Is app_enqueue_v1() and app_enqueue_v2() logically the same from >>>>>>>>> application PoV. Right? >>>>>>>>> >>>>>>>>> If not, could you explain, the version you are planning to do for >>>>>>>>> app_enqueue() >>>>>>>> >>>>>>>> The exact version can be found here : >>>>>>>> >>>>>>> http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4 >>>>>>>> 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not >>>>>>>> same in our case because of the SW fallback we have for ring full scenario's. >>>>>>>> For a packet with 8 nb_segs, if the ring has only space for 4 , we >>>>>>>> would avoid this packet with app_enqueue_v1 while going ahead with an >>>>>>> enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies >>>>>>> for a packet which we would want to avoid. >>>>>>> >>>>>>> Thanks for RFC link. Usage is clear now, Since you are checking the space in >>>>>>> the completion handler, I am not sure, what is hard to get the remaining >>>>>>> space, Will following logic work to find the empty space. If not, please discuss >>>>>>> the issue with this approach. I am trying to avoid yet another fastpath API >>>>>>> and complication in driver as there is element checking space in the submit >>>>>>> queue and completion queue at least in our hardware. >>>>>>> >>>>>>> max_count = nb_desc; (power of 2) >>>>>>> mask = max_count - 1; >>>>>>> >>>>>>> for (i = 0; I < n; i++) { >>>>>>> submit_idx = rte_dma_copy(); >>>>>>> } >>>>>>> rc = rte_dma_completed(…, &completed_idx..); >>>>>>> space_in_queue = mask - ((submit_idx – completed_idx) & mask); >>>>>>> >>>>>> >>>>>> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :| >>>>>> i.e its pmd dependent. >>>>> >>>>> I did not pay much attention to Jiayu's reply as I did not know what is DSA. >>>>> Now I searched the DSA in ml, I could see the PMD patches. >>>>> >>>>> If it is PMD limitation then I am fine with the proposed API. >>>>> >>>>> @Richardson, Bruce @Laatz, Kevin @feng Since it is used fastpath, Can >>>>> we move to fastpath handler and >>>>> remove additional check in fastpath from common code like other APIs. >>>> >>>> +1 for move to fastpath. >>>> >>> >>> Will move in next revision. >> >> Follow-up question on this. If it's a fastpath function then we would not >> normally check for support from drivers. Therefore do we want to: >> 1. make it a mandatory function >> 2. add a feature capability flag >> >> Given that it's likely fairly easy for all drivers to implement, and it >> makes it easier for apps to avoid having to check a feature flag for, I'd >> tend towards option #1, but just would like consensus before I push any >> more patches. > > I think, if vhost using in that way then it needs to make it a > mandatory function. +1 > >> >> /Bruce > . >