From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id 92A937D3E for ; Wed, 23 Aug 2017 17:09:21 +0200 (CEST) Received: by mail-wm0-f48.google.com with SMTP id l19so932243wmi.1 for ; Wed, 23 Aug 2017 08:09:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Fe2jvPoV84/3jBNMIqO+x0xQl618un0YB21r6yIf3ik=; b=my2SLQOv5yJqptX9NkbECEw4+Y4luuJGsaXwuUFh1pv4OlQiX7TcPDAX7aiAp2Yopi HT9tVi+HuCePaRkUkEp9xLOZ8MtfsMYiE9MJsK+EhsW33LVmAoC9YnEIO+1RS+BxLQLg PBWfWCdAgkWAiZvXhvnVXlip2BpQSezTXeIzAPGmRe7XHchxpFydc4as/++1ynWbuC1O Qh6k2Dsic5aWrIpisvFhPN1T4fwt8kaPKpzWBTytfUhqNgR8LJlnzN6fVV6NPSpv3Mv9 gzJOHanNqPaPOOViy4foqIKjOvQajiNI0T1AH0FgcUXv4NMGBGt7suroDtJPW0vm0NDf wE9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Fe2jvPoV84/3jBNMIqO+x0xQl618un0YB21r6yIf3ik=; b=ozeRNAmvZWJufuItrrKbY9mGreTXf18euLydkIzFNNzNBQAodXReir+GfSEcqoH/yf KIzm9HpI7RZcNS4Lv1t3V/T56foY7YQuP2unsZ0MUyoXgYjra1GB5VfirCxzA4p/3bnf LP914F4MlbiGafHE/JcRPeTLuuWx9ZHiVkk6qXXbZKuEAkYhjEApOPjewXC6+Br/k3pj V8ilt+B2V4x/KlMEluD+A9JQqTsXvrGyFqY2ro+ejf76t6MahDMF11Q1Ihz8KkIagOU+ 7zGGIaz4sR8drqpkdP3OYqlNekvlFqZ7yGKUz9JXyvaMhhGKbB3t2IXTiMfvmC3lvUgV 1Zhw== X-Gm-Message-State: AHYfb5gnG7+8hZx1v0oejD8d53u/oPpOo9lDBsbRk4MkOxvpA8GpkgRg Kk9272hjGal6auaT X-Received: by 10.28.125.69 with SMTP id y66mr1838051wmc.42.1503500960750; Wed, 23 Aug 2017 08:09:20 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 62sm2098960wrb.12.2017.08.23.08.09.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 23 Aug 2017 08:09:19 -0700 (PDT) Date: Wed, 23 Aug 2017 17:09:10 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Raslan Darawsheh Cc: thomas@monjalon.net, jingjing.wu@intel.com, dev@dpdk.org, salehals@mellanox.com Message-ID: <20170823150909.GA8124@bidouze.vm.6wind.com> References: <1503499024-12480-1-git-send-email-rasland@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1503499024-12480-1-git-send-email-rasland@mellanox.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] app/testpmd: app/testpmd: add device removal command 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: , X-List-Received-Date: Wed, 23 Aug 2017 15:09:21 -0000 Hello Raslan, On Wed, Aug 23, 2017 at 05:37:04PM +0300, Raslan Darawsheh wrote: > Added hotplug in testpmd, to be able to test hotplug function > in the PMD's. > > Signed-off-by: Raslan Darawsheh > --- > app/test-pmd/cmdline.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > app/test-pmd/testpmd.c | 18 ++++++++++++++++++ > app/test-pmd/testpmd.h | 1 + > 3 files changed, 63 insertions(+) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index cd8c358..b32a368 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -716,6 +716,9 @@ static void cmd_help_long_parsed(void *parsed_result, > "port config (port_id|all) l2-tunnel E-tag" > " (enable|disable)\n" > " Enable/disable the E-tag support.\n\n" > + > + " device remove (device)\n" > + " Remove a device" I think it should still be a part of the "port" command set (port attach|detach|stop|close, etc). This would probably be easier to understand for users. > ); > } > > @@ -1125,6 +1128,46 @@ cmdline_parse_inst_t cmd_operate_detach_port = { > }, > }; > > +/* *** Remove a specified device *** */ > +struct cmd_operate_device_remove_result { > + cmdline_fixed_string_t device; > + cmdline_fixed_string_t keyword; > + cmdline_fixed_string_t identifier; > +}; > + > +static void cmd_operate_device_remove_parsed(void *parsed_result, > + __attribute__((unused)) struct cmdline *cl, > + __attribute__((unused)) void *data) > +{ > + struct cmd_operate_device_remove_result *res = parsed_result; > + if (!strcmp(res->keyword, "remove")) > + device_remove((char *) res->identifier); > + else > + printf("Unknown parameter\n"); > +} > + > +cmdline_parse_token_string_t cmd_operate_device_remove_device = > + TOKEN_STRING_INITIALIZER(struct cmd_operate_device_remove_result, > + device, "device"); > +cmdline_parse_token_string_t cmd_operate_device_remove_keyword = > + TOKEN_STRING_INITIALIZER(struct cmd_operate_device_remove_result, > + keyword, "remove"); > +cmdline_parse_token_string_t cmd_operate_device_remove_identifier = > + TOKEN_STRING_INITIALIZER(struct cmd_operate_device_remove_result, > + identifier, NULL); > + > +cmdline_parse_inst_t cmd_operate_device_remove = { > + .f = cmd_operate_device_remove_parsed, > + .data = NULL, > + .help_str = "device remove : (device)", > + .tokens = { > + (void *)&cmd_operate_device_remove_device, > + (void *)&cmd_operate_device_remove_keyword, > + (void *)&cmd_operate_device_remove_identifier, > + NULL, > + }, > +}; > + Continuing on using the port ... format, then the port_id should allow to remove it instead of the device identifier. Using the device identifier will complexify your implementation. > /* *** configure speed for all ports *** */ > struct cmd_config_speed_all { > cmdline_fixed_string_t port; > @@ -14276,6 +14319,7 @@ cmdline_parse_ctx_t main_ctx[] = { > (cmdline_parse_inst_t *)&cmd_operate_specific_port, > (cmdline_parse_inst_t *)&cmd_operate_attach_port, > (cmdline_parse_inst_t *)&cmd_operate_detach_port, > + (cmdline_parse_inst_t *)&cmd_operate_device_remove, > (cmdline_parse_inst_t *)&cmd_config_speed_all, > (cmdline_parse_inst_t *)&cmd_config_speed_specific, > (cmdline_parse_inst_t *)&cmd_config_rx_tx, > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index 7d40139..a2e8526 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -1742,6 +1742,24 @@ detach_port(uint8_t port_id) > } > > void > +device_remove(char *device) { > + struct rte_devargs devargs; > + > + if (device == NULL) { > + printf("Invalid parameters are specified\n"); > + return; > + } I don't think those checks are necessary. This function should not be called with invalid parameters, and you will be the only one calling it. > + > + rte_eal_devargs_parse(device, &devargs); If you used a port_id, you would not have to parse the device string. You could do something like this: struct rte_eth_dev *eth_dev; struct rte_bus *bus; eth_dev = &rte_eth_devices[port_id]; bus = rte_bus_find_by_device(eth_dev->device); > + > + if (rte_eal_hotplug_remove(devargs.bus->name, devargs.name)) { + if (rte_eal_hotplug_remove(bus->name, eth_dev->device->name)) { As a side note, I don't see why we need to use those names in the function parameters. We could simply use the rte_device handle available. This handle should propose all necessary infos. It might be a future API change. > + printf("Fail to remove device\n"); > + return; > + } > + printf("Device removed successfully\n"); > +} > + > +void > pmd_test_exit(void) > { > portid_t pt_id; > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index c9d7739..0780f55 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -612,6 +612,7 @@ void stop_port(portid_t pid); > void close_port(portid_t pid); > void attach_port(char *identifier); > void detach_port(uint8_t port_id); > +void device_remove(char *device); void remove_port(uint8_t port_id); > int all_ports_stopped(void); > int port_is_started(portid_t port_id); > void pmd_test_exit(void); > -- > 2.7.4 > -- Gaëtan Rivet 6WIND