From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail04.ics.ntt-tx.co.jp (mail05.ics.ntt-tx.co.jp [210.232.35.69]) by dpdk.org (Postfix) with ESMTP id E940B201 for ; Tue, 13 Nov 2018 01:29:31 +0100 (CET) Received: from gwchk03.silk.ntt-tx.co.jp (gwchk03.silk.ntt-tx.co.jp [10.107.0.111]) by mail04.ics.ntt-tx.co.jp (unknown) with ESMTP id wAD0TUh6016343; Tue, 13 Nov 2018 09:29:30 +0900 Received: (from root@localhost) by gwchk03.silk.ntt-tx.co.jp (unknown) id wAD0TUi0021607; Tue, 13 Nov 2018 09:29:30 +0900 Received: from gwchk.silk.ntt-tx.co.jp [10.107.0.110] by gwchk03.silk.ntt-tx.co.jp with ESMTP id KAA21606; Tue, 13 Nov 2018 09:29:30 +0900 Received: from imss06.silk.ntt-tx.co.jp (localhost [127.0.0.1]) by ccmail04.silk.ntt-tx.co.jp (unknown) with ESMTP id wAD0TT1T014428; Tue, 13 Nov 2018 09:29:29 +0900 Received: from imss06.silk.ntt-tx.co.jp (localhost [127.0.0.1]) by imss06.silk.ntt-tx.co.jp (unknown) with ESMTP id wAD0TTSN010612; Tue, 13 Nov 2018 09:29:29 +0900 Received: from ccmail04 (smtp03.silk.ntt-tx.co.jp [10.107.0.135]) by imss06.silk.ntt-tx.co.jp (unknown) with SMTP id wAD0TTZK010609; Tue, 13 Nov 2018 09:29:29 +0900 Date: Tue, 13 Nov 2018 09:27:39 +0900 From: Hideyuki Yamashita In-Reply-To: References: <201811090923.wA99MvqZ016850@ccmail04.silk.ntt-tx.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Becky! ver. 2.74 [ja] X-CCMail7: CC-Mail-V7.0.2-Client-Relayed Message-Id: <201811130028.wAD0SYfP013800@ccmail04.silk.ntt-tx.co.jp> X-TM-AS-MML: No X-CC-Mail-RelayStamp: CC-Mail-V5.14-Server To: Yasufumi Ogawa Cc: Yasufumi Ogawa , x-fn-spp@sl.ntt-tx.co.jp, ferruh.yigit@intel.com, spp@dpdk.org Subject: Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs X-BeenThere: spp@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Soft Patch Panel List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Nov 2018 00:29:32 -0000 Hello Yasufumi-san, Thanks for your comments. Please see inline. > > Hello Yasufumi-san, > > > > Thanks so much for your comments. > > > > About your first point, I think the follwing naming may be fit with DPDK > > and SPP naming conventions, what do you think? > > > > attach -> spp_eth_dev_attach > > detach -> spp_eth_dev_detach > I think you should not use prefix `spp` without if it defines a behaviour of SPP itself. This function is more essential to attach device. > > For `spp_eth_dev_attach`, You might think similar name from original `rte_eth_dev_attach`, but I think we do not keep the name of deprecated APIs. More simply, how about `dev_attach_by_devargs` instead of? I have no problem about naming `dev_attach_by_devargs`. > For `spp_eth_dev_detach`, I think `dev_detach_by_port_id` is more prefer. Other than the name of the function, I am curious why it takes second argument `name` because it is not used in the function. I have no problem about namign `dev_detach_by_port_id`. As you say, second argument is NOT used within the function and I will remove it from revised version. > By the way, I am not clear what is the difference between your functions and `rte_eth_dev_attach` defined in `dpdk/lib/librte_ethdev/rte_ethdev.c` exactly. Could you explain about the difference shortly for helping my understanding? Basically both of those functions uses `rte_eal_hotplug_add` to hotplug device. Difference is described as following: `rte_eth_dev_attach` uses global variable named eth_dev_last_created_port when retrieving newly created dpdk port while my new function uses `rte_eth_dev_get_port_by_name` becauase this function is defined outside of dpdk library and can not retrieve global variable within dpdk library. BR, Hideyuki Yamashita NTT TechnoCross > Thanks > > About your second point, I appologize about my mistake. > > > > Once I get ack for above new naming of two interfaces from you, > > I will revise my patch set including cover-letter and will send > > those to mailing list. > > > > Thanks for your co-opearation! > > > > BR, > > Hideyuki Yamashita > > NTT TechnoCorss > > > >> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote: > >>> From: Hideyuki Yamashita > >>> > >>> >From DPDK-18.08, the follwing APIs become deprecated and > >>> will be deleted in DPDK18.08. > >>> - rte_eth_dev_attach() > >>> - rte_eth_dev_detach() > >>> > >>> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended. > >>> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended. > >>> > >>> To follow the above changes, this patch set provides replacement of > >>> those APIs. > >> Hideyuki, > >> > >> Thank you for suggesting to update to v18.08! > >> > >> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works. > >> > >> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines. > >> > >> Thanks > >>> Signed-off-by: Hideyuki Yamashita > >>> Signed-off-by: Naoki Takada > >>> > >>> Hideyuki Yamashita (6): > >>> shared: addition of attach() > >>> spp_nfv: replacement of rte_eth_dev_attach() > >>> spp_vf:replacement of rte_eth_dev_attach() > >>> shared: addition of detach() > >>> spp_nfv: replacement of rte_eth_dev_detach() > >>> spp_vm: replacement of rte_eth_dev_detach(). > >>> > >>> src/nfv/nfv.c | 12 ++++----- > >>> src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++ > >>> src/shared/common.h | 32 +++++++++++++++++++++++ > >>> src/vf/spp_vf.c | 4 +-- > >>> src/vm/main.c | 2 +- > >>> 5 files changed, 105 insertions(+), 9 deletions(-) > >>> > >> > >>