From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from tama50.ecl.ntt.co.jp (tama50.ecl.ntt.co.jp [129.60.39.147]) by dpdk.org (Postfix) with ESMTP id 149C52BD5 for ; Tue, 13 Nov 2018 09:25:23 +0100 (CET) Received: from vc1.ecl.ntt.co.jp (vc1.ecl.ntt.co.jp [129.60.86.153]) by tama50.ecl.ntt.co.jp (8.13.8/8.13.8) with ESMTP id wAD8PMRk016344; Tue, 13 Nov 2018 17:25:22 +0900 Received: from vc1.ecl.ntt.co.jp (localhost [127.0.0.1]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 3047EEA7DE1; Tue, 13 Nov 2018 17:25:22 +0900 (JST) Received: from jcms-pop21.ecl.ntt.co.jp (jcms-pop21.ecl.ntt.co.jp [129.60.87.134]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 1B866EA7DDB; Tue, 13 Nov 2018 17:25:22 +0900 (JST) Received: from [IPv6:::1] (watercress.nslab.ecl.ntt.co.jp [129.60.13.73]) by jcms-pop21.ecl.ntt.co.jp (Postfix) with ESMTPSA id 163EC40033C; Tue, 13 Nov 2018 17:25:22 +0900 (JST) References: <201811090923.wA99MvqZ016850@ccmail04.silk.ntt-tx.co.jp> <201811130028.wAD0SYfP013800@ccmail04.silk.ntt-tx.co.jp> From: Yasufumi Ogawa Message-ID: <22b6ce74-37df-0fd7-2265-a2206a19b327@lab.ntt.co.jp> Date: Tue, 13 Nov 2018 17:22:55 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <201811130028.wAD0SYfP013800@ccmail04.silk.ntt-tx.co.jp> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-CC-Mail-RelayStamp: 1 To: Hideyuki Yamashita , Yasufumi Ogawa Cc: x-fn-spp@sl.ntt-tx.co.jp, ferruh.yigit@intel.com, spp@dpdk.org X-TM-AS-MML: disable 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 08:25:24 -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. 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? 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