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 B0B4CA0588; Tue, 7 Apr 2020 14:55:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8C7CB1BEE1; Tue, 7 Apr 2020 14:54:56 +0200 (CEST) Received: from mail-il1-f194.google.com (mail-il1-f194.google.com [209.85.166.194]) by dpdk.org (Postfix) with ESMTP id 1527E2B86 for ; Tue, 7 Apr 2020 14:54:54 +0200 (CEST) Received: by mail-il1-f194.google.com with SMTP id j69so3010584ila.11 for ; Tue, 07 Apr 2020 05:54:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=SsxXzJp1NVc5fGeHAYDlEwQnZMbT//OH6YEEkoxU0D8=; b=PtsmYxBe4oNsbUNoNTUM6oNwTuIUvFtJRk8ve+Z+U6LeGti2Q1dtC+uNpxhmzokwtx VygECXBdNyFNM3astaDuqwBO268zJ77ZFlayP8Qo7tkjHwemgTTEhxgqfbmgWxUW0VOz u5rht3vju4XpS5bOP5sMYNCBn4HYG9ZrxqKdk5NtfmbJAsrRWwpx5+6Ss4fpR38x/0kf MUK3aaIbO0BPD6xUBtgBqjj0aHKSfAZ0f4GWMCU1dpmQRBRVh55YBz2wd8dKKnY0gJGR DCQtDKY6VAeTVImIACNlqmQtiSiRhVdKWJ4ncZqv8o716mcyWgHBA98SCyUOCcmk29U1 vp6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SsxXzJp1NVc5fGeHAYDlEwQnZMbT//OH6YEEkoxU0D8=; b=UkLbqTg+muhWonSWsS0TMe95b80lUx+fUSzaEP5ir0eDVbyV//g6qLDXttXRNv6CN5 GzBp+9v5fQpJWwbkRKI8WQwqAdYITSD05tkmoUkvYIW3ckGDELZl8hYxIUI38DtHrIko 6u0XjYhJ9qv+z2PLRSSA4YmP+nrDRyyiB+Z6FuuG5YIY+jn9Eh1eh2B6JG2VCwbaw0HC Ccay+3FaQt/Qg8rbLfFkpmd1YxTrxmeHLpPlvxdqt+oedco0agywvnscJ48m9CCDzE7I t3tHLjUE9c5tu6M0g/Jptj00b1bCJDSIS9fqQlYxZsV1ofIOY+R02uESHxUvMYh2BeUb 6nZA== X-Gm-Message-State: AGi0PubvRAPLkb9HhpJisdpRqsy8BbUbXEfoooUHYoz7uklIU94UQzao LN0TUyoC+GdaKxW5roPhX2jm2wknIScXAKhvR3w= X-Google-Smtp-Source: APiQypLTOrqWbVuKAy78R0lgpQn+TtFWhEXhVhke7DWeBo414dA2WC3N+6B8QGxbIkl+vVPhf/ntE3wSjxCPYvUbw9c= X-Received: by 2002:a92:a192:: with SMTP id b18mr1862481ill.294.1586264093256; Tue, 07 Apr 2020 05:54:53 -0700 (PDT) MIME-Version: 1.0 References: <1585464438-111285-1-git-send-email-orika@mellanox.com> <2907264.PWNTSA8uO9@thomas> In-Reply-To: <2907264.PWNTSA8uO9@thomas> From: Jerin Jacob Date: Tue, 7 Apr 2020 18:24:36 +0530 Message-ID: To: Thomas Monjalon Cc: Jerin Jacob Kollanukkaran , Ori Kam , "xiang.w.wang@intel.com" , Pavan Nikhilesh Bhagavatula , "dev@dpdk.org" , Shahaf Shuler , "hemant.agrawal@nxp.com" , Opher Reviv , Alex Rosenbaum , Dovrat Zifroni , Prasun Kapoor , "nipun.gupta@nxp.com" , "bruce.richardson@intel.com" , "yang.a.hong@intel.com" , "harry.chang@intel.com" , "gu.jian1@zte.com.cn" , "shanjiangh@chinatelecom.cn" , "zhangy.yun@chinatelecom.cn" , "lixingfu@huachentel.com" , "wushuai@inspur.com" , "yuyingxia@yxlink.com" , "fanchenggang@sunyainfo.com" , "davidfgao@tencent.com" , "liuzhong1@chinaunicom.cn" , "zhaoyong11@huawei.com" , "oc@yunify.com" , "jim@netgate.com" , "hongjun.ni@intel.com" , "j.bromhead@titan-ic.com" , "deri@ntop.org" , "fc@napatech.com" , "arthur.su@lionic.com" , Parav Pandit Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev core functions 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 Tue, Apr 7, 2020 at 5:57 PM Thomas Monjalon wrote: > > 07/04/2020 07:49, Jerin Jacob: > > > > > > > > If it abstracts it properly adding vdev and PCI is a simple change. > > > > See > > > > > > > > lib/librte_eventdev/rte_eventdev_pmd_vdev.h > > > > lib/librte_eventdev/rte_eventdev_pmd_pci.h > > > > > > > > I think, the common code should take from other matured subsystem instead if > > > > writing from scratch, > > > > > > > I agree with you about the rewrite, but this is why I don't want to add this code > > > until I know what this code should do and how it should be used. > > > I don't agree, that one subsystem is like other one by default, and that if something > > > is done for one subsystem it should be done for other. > > > Not always what was done before is the best. > > > > > > Some time back there was a long thread about ethdev and the rte device > > > where should it be released and by whom. > > > My basic thinking is that unless proven otherwise the code should be in the PMD > > > this is also why it is important for me to get this rte level API acked. > > > when starting to implement the code for the PMD it will be cleared what > > > is the shared code and how it is best to configure the system. > > > Also this is not external API so it can be changed at any time. > > > Saying that I don't think we should wait long before adding such code. > > > I think that when we will have our first PMD we know better if such > > > function is needed. > > > Also think about that maybe this PMD will be shared with the > > > net PMD so such function will also introduce more complexity. > > > > > > My thought process was I like this when I added the common code for eventdev. > > I have checked ethdev, cryptodev and followed a similar scheme > > wherever it applicable for eventdev. > > If we see the regexdev API, it is similar to ethdev. cryptodev and > > eventdev API. From the device > > API PoV, the framework needs to follow the same behavior to avoid > > having new behavior for regexdev, > > Especially in configure->queue_setup->start->rx_burst->tx_burst->stop->reconfigure->start > > sequence. > > > > > > Ethdev may be bloated by features, > > No, ethdev has more features and is evolving as a better API. I meant the same, where ethdev filled with "ethdev specific" features, better take another subsystem for devices specific thing. > > > My request is to take cryptodev and > > eventdev as a base > > change to accordingly. > > When writing a new API, we should learn from past lessons, > including eventdev and cryptodev, but also all the things evolving > in ethdev. It looks out of context. > > > That makes review process easy, As reviewer > > needs only think, The rationale behind, > > Why it regexdev common code chosen a different approach. Writing from > > scratch makes the reviewer's job > > difficult. > > > > We spend a lot of time reviewing and make mature cryptodev and > > eventdev common code, Please leverage that. > > Again, leveraging doesn't mean to implement all in one patch. > Instead of asking for adding more, please focus on what must be changed > in what is proposed in this series. Look at the configure function[1], Does it take care of reconfiguration? Common code takes care of allocating memory for point for queue objects. Where is that? http://patches.dpdk.org/patch/67311/ I will put in other way out, We know how other devices works in DPDK, if we are changing the means then please mention in the cover letter for the proposed design and why would think, the new solution is better. Regexdev specification(patch 1/4) is not different than other devices' fundamental view. In my understanding, we don't put any effort to understand, why other subsystems were chosen the specific implementation in the common code. For me, it worth spending my time on the review once basic stuff is there. int rte_regexdev_configure(uint8_t dev_id, const struct rte_regexdev_config *cfg) { if (dev_id >= RTE_MAX_REGEXDEV_DEVS) return -EINVAL; if (regex_devices[dev_id] == NULL) return -EINVAL; if (cfg == NULL) return -EINVAL; if (regex_devices[dev_id]->dev_ops->dev_configure == NULL) return -ENOTSUP; return regex_devices[dev_id]->dev_ops->dev_configure (regex_devices[dev_id], cfg); } > >