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 F14DEA0577; Tue, 7 Apr 2020 09:22:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D92052BE9; Tue, 7 Apr 2020 09:22:51 +0200 (CEST) Received: from mail-io1-f68.google.com (mail-io1-f68.google.com [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id 121642BE2 for ; Tue, 7 Apr 2020 09:22:51 +0200 (CEST) Received: by mail-io1-f68.google.com with SMTP id m4so2328229ioq.6 for ; Tue, 07 Apr 2020 00:22:51 -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=LzGKfJ8n3lVIRuFOw0WSDyFsFndr0gIY2FMiK4G5NNo=; b=eClaJaHj3mE5yLF9zmDUwWPaphzGnMfQ+Sd70olgFTMr7RzoEJXK0P2psG4cowhxRT NXY8BHTVawC3P8+qjoMCvroFSmkZuW/iHYt8xGQNJ0nw5osS/9jQ2l2FxMRa5GrtotG4 5voikhT0M1lKYczx/G29dq5L/j7p8hWK/p2RuhHxgQVnf1iTX9+1pGxQ7QYL8jy2e7A4 3SgeB4pN5VVoWk3OqfVOb7N+AyGY+qsr6ZWvvj3iMWY8DcMyayRYHPgNyDkjhtIDZkeP x16qLn7aUleYz+VzaiNO2Ov4Dio6iSS9gli6B+bvilVpDn17T5xOvvdTUeJ3LeLl4KUi C9EQ== 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=LzGKfJ8n3lVIRuFOw0WSDyFsFndr0gIY2FMiK4G5NNo=; b=iyPSexgzYuX0SvnLbRu0FuHoNOY/JLo3Y1lGZ2mugE3YFSZ4jax0RIN+ye1J7znxCe dCX6DOqKzDuqBPL3VK6HUADraZJPBLqoJwL5/zWDgNAfT5G15wjvdyDrX4IFCKVPIrZE oQty5IWMGDFlkiHaa/cUaSy/7I8Yf6EzheZodn3VWhkbpOfHdXWf2dbQVK+HJuF3YFwn wLOx2KArD2G0YdkfgPIeK5YCLHCpGBK23/wxFZGilCVbrYbwuWm3Gx6mSLptIBS3ZIB5 BlG51tqs3dwTTfVnMXkqHTSn8F8A5xFlWOoBMmJVdxp+0Xmt6MXiGqWvOaz7mDQbd2kn aqDw== X-Gm-Message-State: AGi0PuZUrIyxBOES4RMHkZZ5IrvTXn8Aq7JRcO8Jc98/ykFUncsvX+hn vjvKogQIqW/fMse1+KZPNuYjfNppeSeDp3V+LT4= X-Google-Smtp-Source: APiQypKtNuzkq3vA61df2LSqQso/2mpqsJklLZphBSvuVtZn9y4mNybcEM3axNsSOTbMkKfHvO/9YaA0CXrM9IJ29EU= X-Received: by 2002:a02:ac94:: with SMTP id x20mr637318jan.133.1586244170321; Tue, 07 Apr 2020 00:22:50 -0700 (PDT) MIME-Version: 1.0 References: <1585464438-111285-1-git-send-email-orika@mellanox.com> <2061995.157zFVKu8G@xps> In-Reply-To: From: Jerin Jacob Date: Tue, 7 Apr 2020 12:52:34 +0530 Message-ID: To: Ori Kam Cc: Thomas Monjalon , Jerin Jacob Kollanukkaran , "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 12:16 PM Ori Kam wrote: > > > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Tuesday, April 7, 2020 8:50 AM > > To: Ori Kam > > Cc: Thomas Monjalon ; Jerin Jacob Kollanukkaran > > ; 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 > > Subject: Re: [dpdk-dev] [EXT] [PATCH v1 3/4] regexdev: add regexdev core > > functions > > > > > > > > > > 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, My request is to take cryptodev and > > eventdev as a base > > change to accordingly. 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. > > I fully agree with you that we should leverage known code as much as we > can. But just like you said the idea is to know what the RegEx needs > and then see how it is done in other modules. > I don't know how the vdev pmd will look like (I can guess) > So I prefer to add this code in later stage where we will get better > Understanding on how best to implement it. OK. > > We have the configure, queue_setup, start, enqueue, dequeue and stop. > I will add support for reconfigure like I said in previous code. > So if I understand correctly from your point of view, this patch is just missing > the vdev functions right? If so lets keep them out for now, I promise you that if needed > I will add this function. OK. Just to clarify, what I meant is, when we add the vdev, it should simple as following[1] ie. rte_event_pmd_allocate() and rte_event_pmd_release() etc reused from PCI and VDEV. i.e a) lets have common routes like rte_event_pmd_allocate() for PMD device allocation irrespective of VDEV for PCI b) add PCI specific code c) add VDEV specific code like [1] [1] static inline struct rte_eventdev * rte_event_pmd_vdev_init(const char *name, size_t dev_private_size, int socket_id) { struct rte_eventdev *eventdev; /* Allocate device structure */ eventdev = rte_event_pmd_allocate(name, socket_id); if (eventdev == NULL) return NULL; /* Allocate private device structure */ if (rte_eal_process_type() == RTE_PROC_PRIMARY) { eventdev->data->dev_private = rte_zmalloc_socket("eventdev device private", dev_private_size, RTE_CACHE_LINE_SIZE, socket_id); if (eventdev->data->dev_private == NULL) rte_panic("Cannot allocate memzone for private device" " data"); } return eventdev; } static inline int rte_event_pmd_vdev_uninit(const char *name) { int ret; struct rte_eventdev *eventdev; if (name == NULL) return -EINVAL; eventdev = rte_event_pmd_get_named_dev(name); if (eventdev == NULL) return -ENODEV; ret = rte_event_dev_close(eventdev->data->dev_id); if (ret < 0) return ret; /* Free the event device */ rte_event_pmd_release(eventdev); return 0; } >