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 D33984CAB for ; Wed, 14 Nov 2018 01:41:57 +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 wAE0fu4L008819; Wed, 14 Nov 2018 09:41:56 +0900 Received: (from root@localhost) by gwchk03.silk.ntt-tx.co.jp (unknown) id wAE0fu4S023006; Wed, 14 Nov 2018 09:41:56 +0900 Received: from gwchk.silk.ntt-tx.co.jp [10.107.0.110] by gwchk03.silk.ntt-tx.co.jp with ESMTP id KAA23005; Wed, 14 Nov 2018 09:41:56 +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 wAE0ftru013566; Wed, 14 Nov 2018 09:41:55 +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 wAE0ftGO000590; Wed, 14 Nov 2018 09:41:55 +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 wAE0ftvE000587; Wed, 14 Nov 2018 09:41:55 +0900 Date: Wed, 14 Nov 2018 09:40:28 +0900 From: Hideyuki Yamashita In-Reply-To: <22b6ce74-37df-0fd7-2265-a2206a19b327@lab.ntt.co.jp> References: <201811130028.wAD0SYfP013800@ccmail04.silk.ntt-tx.co.jp> <22b6ce74-37df-0fd7-2265-a2206a19b327@lab.ntt.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: <201811140041.wAE0fOtM013055@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: Wed, 14 Nov 2018 00:41:59 -0000 Hello Yasufumi-san, Please see inline. > > 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. > Thank you for considering updates. > > > > 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. > I've got understand your point. Could you confirm why you avoid to use global variable? rte_eth_dev_attach is function inside dpdk library so it is somehow natural to use global variable when retrieve newly created dpdk port. As I said earlier, this time new function is located in application side. It is unnatural to use global variable of library from application rather I prefer to use existing function for retrieving allocated dpdk from appliction(rte_eth_dev_get_port_by_name). I hope above explanation answers to your question. BTW, I am preparing revising patch set including cover letter. BR, Hideyuki Yamashita NTT TechnoCross > Thanks > > 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(-) > >>>>> > >>>> > >>>> > > > > > > > > > > -- Yasufumi Ogawa > NTT Network Service Systems Labs