From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id CC2E3C342 for ; Thu, 18 Feb 2016 11:11:43 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP; 18 Feb 2016 02:11:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,464,1449561600"; d="scan'208";a="50066525" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by fmsmga004.fm.intel.com with ESMTP; 18 Feb 2016 02:11:22 -0800 Received: from sivlogin002.ir.intel.com (sivlogin002.ir.intel.com [10.237.217.37]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id u1IABMss011865; Thu, 18 Feb 2016 10:11:22 GMT Received: from sivlogin002.ir.intel.com (localhost [127.0.0.1]) by sivlogin002.ir.intel.com with ESMTP id u1IABMU6015960; Thu, 18 Feb 2016 10:11:22 GMT Received: (from fyigit@localhost) by sivlogin002.ir.intel.com with œ id u1IABM41015956; Thu, 18 Feb 2016 10:11:22 GMT X-Authentication-Warning: sivlogin002.ir.intel.com: fyigit set sender to ferruh.yigit@intel.com using -f Date: Thu, 18 Feb 2016 10:11:21 +0000 From: "Yigit, Ferruh" To: "Ananyev, Konstantin" Message-ID: <20160218101121.GA15025@sivlogin002.ir.intel.com> Mail-Followup-To: "Ananyev, Konstantin" , "dev@dpdk.org" References: <1453911849-16562-1-git-send-email-ferruh.yigit@intel.com> <1455284735-9606-1-git-send-email-ferruh.yigit@intel.com> <1455284735-9606-4-git-send-email-ferruh.yigit@intel.com> <2601191342CEEE43887BDE71AB97725836B07BDD@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB97725836B07BDD@irsmsx105.ger.corp.intel.com> User-Agent: Mutt/1.5.17 (2007-11-01) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 3/3] examples/ethtool: add control interface support to the application X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Feb 2016 10:11:44 -0000 On Wed, Feb 17, 2016 at 07:39:05PM +0000, Ananyev, Konstantin wrote: > Hi Ferruh, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit > > Sent: Friday, February 12, 2016 1:46 PM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 3/3] examples/ethtool: add control interface support to the application > > > > Control interface APIs added into the sample application. > > > > To have the support corresponding kernel module (KCP) needs to be inserted. > > If kernel module is not there, application will run as it is without > > kernel control path support. > > > > When KCP module inserted, running application creates a virtual Linux > > network interface (dpdk$) per DPDK port. This interface can be used by > > traditional Linux tools. > > > > Signed-off-by: Ferruh Yigit > > --- > > doc/guides/sample_app_ug/ethtool.rst | 41 ++++++++++++++++++++++++++++++++++++ > > examples/ethtool/ethtool-app/main.c | 10 +++++++-- > > 2 files changed, 49 insertions(+), 2 deletions(-) > > > > diff --git a/doc/guides/sample_app_ug/ethtool.rst b/doc/guides/sample_app_ug/ethtool.rst > > index 4d1697e..2174288 100644 > > --- a/doc/guides/sample_app_ug/ethtool.rst > > +++ b/doc/guides/sample_app_ug/ethtool.rst > > @@ -131,6 +131,47 @@ application`_. Individual call-back functions handle the detail > > associated with each command, which make use of the functions > > defined in the `Ethtool interface`_ to the DPDK functions. > > There is ~100% code duplication between > lib/librte_ctrl_if/rte_ethtool.c and examples/ethtool/lib/rte_ethtool.c > That need to be addressed somehow. > Yes you are right, there is dublication. In next revision, I will move examples/ethtool/lib to lib/ folder and update according both ctrl_if library and ethtool sample app use it > > > > +Control Interface > > +~~~~~~~~~~~~~~~~~ > > + > > +If Kernel Control Path (KCP) kernel module (rte_kcp.ko) inserted, > > +virtual interfaces created for each DPDK port for control purposes. > > + > > +Created interfaces are named as dpdk#, like: > > + > > +.. code-block:: console > > + > > + # ifconfig dpdk0; ifconfig dpdk1 > > + dpdk0: flags=4099 mtu 1500 > > + ether 90:e2:ba:0e:49:b9 txqueuelen 1000 (Ethernet) > > + RX packets 0 bytes 0 (0.0 B) > > + RX errors 0 dropped 0 overruns 0 frame 0 > > + TX packets 0 bytes 0 (0.0 B) > > + TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0 > > + > > + dpdk1: flags=4099 mtu 1500 > > + ether 00:1b:21:76:fa:21 txqueuelen 1000 (Ethernet) > > + RX packets 0 bytes 0 (0.0 B) > > + RX errors 0 dropped 0 overruns 0 frame 0 > > + TX packets 0 bytes 0 (0.0 B) > > + TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0 > > + > > +Regular Linux commands can be issued on interfaces: > > + > > +.. code-block:: console > > + > > + # ethtool -i dpdk0 > > + driver: rte_ixgbe_pmd > > + version: RTE 2.3.0-rc0 > > + firmware-version: > > + expansion-rom-version: > > + bus-info: 0000:08:00.1 > > + supports-statistics: yes > > + supports-test: no > > + supports-eeprom-access: yes > > + supports-register-dump: yes > > + supports-priv-flags: no > > + > > Ethtool interface > > ----------------- > > > > diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethtool-app/main.c > > index e21abcd..68b13ad 100644 > > --- a/examples/ethtool/ethtool-app/main.c > > +++ b/examples/ethtool/ethtool-app/main.c > > @@ -1,7 +1,7 @@ > > /*- > > * BSD LICENSE > > * > > - * Copyright(c) 2015 Intel Corporation. All rights reserved. > > + * Copyright(c) 2016 Intel Corporation. All rights reserved. > > * All rights reserved. > > * > > * Redistribution and use in source and binary forms, with or without > > @@ -44,6 +44,7 @@ > > #include > > #include > > #include > > +#include > > > > #include "ethapp.h" > > > > @@ -54,7 +55,6 @@ > > #define PKTPOOL_EXTRA_SIZE 512 > > #define PKTPOOL_CACHE 32 > > > > - > > struct txq_port { > > uint16_t cnt_unsent; > > struct rte_mbuf *buf_frames[MAX_BURST_LENGTH]; > > @@ -254,6 +254,8 @@ static int slave_main(__attribute__((unused)) void *ptr_data) > > } > > rte_spinlock_unlock(&ptr_port->lock); > > } /* end for( idx_port ) */ > > + rte_eth_control_interface_process_msg( > > + RTE_ETHTOOL_CTRL_IF_PROCESS_MSG, 0); > > > As I can see, few problems here: > 1. Race condition was introduced between slave_main() and ethapp_main() - > both can try to do dev_start()/dev_stop() or other intrusive things over the same port > simultaneously. > 2. Better to avoid calling rte_eth_control_interface_process_msg() from RT code path > that doing RX/TX packets - it is too slow for that. > 3. Right now - if you'll have to postpone any RX/TX on any ports when calling rte_eth_control_interface_process_msg(). > As it can't distinguish message for what particular port it is going to process. > Need to address it somehow - either add function that would return current message port_id, > or introduce a sync callback function and add is a parameter for rte_eth_control_interface_process_msg() , > or probably something else. > I will address these in next version of the patch. Thanks for the comments, ferruh