From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by dpdk.org (Postfix) with ESMTP id 08B8EC168
 for <dev@dpdk.org>; Wed, 17 Feb 2016 20:39:08 +0100 (CET)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga104.fm.intel.com with ESMTP; 17 Feb 2016 11:39:07 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.22,462,1449561600"; d="scan'208";a="905401955"
Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159])
 by fmsmga001.fm.intel.com with ESMTP; 17 Feb 2016 11:39:07 -0800
Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by
 IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS)
 id 14.3.248.2; Wed, 17 Feb 2016 19:39:06 +0000
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.237]) by
 irsmsx155.ger.corp.intel.com ([169.254.14.217]) with mapi id 14.03.0248.002;
 Wed, 17 Feb 2016 19:39:06 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH v2 3/3] examples/ethtool: add control
 interface	support to the application
Thread-Index: AQHRZZvW35CEAmIgD0yMcg4ojc8QK58wpPIg
Date: Wed, 17 Feb 2016 19:39:05 +0000
Message-ID: <2601191342CEEE43887BDE71AB97725836B07BDD@irsmsx105.ger.corp.intel.com>
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>
In-Reply-To: <1455284735-9606-4-git-send-email-ferruh.yigit@intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTI4MTczNmYtZWFlOC00MzNlLTkzY2UtYTQxYzc5Nzk1OWQyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImY3bXBZRitUNGJ6aVN3VG1RVjN3dCtHXC9oRHhKQ1J3ZFRtV2FnTzJ6eXJBPSJ9
x-ctpclassification: CTP_IC
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 17 Feb 2016 19:39:09 -0000

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 interfac=
e support to the application
>=20
> Control interface APIs added into the sample application.
>=20
> To have the support corresponding kernel module (KCP) needs to be inserte=
d.
> If kernel module is not there, application will run as it is without
> kernel control path support.
>=20
> 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.
>=20
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  doc/guides/sample_app_ug/ethtool.rst | 41 ++++++++++++++++++++++++++++++=
++++++
>  examples/ethtool/ethtool-app/main.c  | 10 +++++++--
>  2 files changed, 49 insertions(+), 2 deletions(-)
>=20
> 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.

>=20
> +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=3D4099<UP,BROADCAST,MULTICAST>  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=3D4099<UP,BROADCAST,MULTICAST>  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
>  -----------------
>=20
> diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethto=
ol-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 <rte_memory.h>
>  #include <rte_mempool.h>
>  #include <rte_mbuf.h>
> +#include <rte_ctrl_if.h>
>=20
>  #include "ethapp.h"
>=20
> @@ -54,7 +55,6 @@
>  #define PKTPOOL_EXTRA_SIZE 512
>  #define PKTPOOL_CACHE 32
>=20
> -
>  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 *p=
tr_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 th=
e same port =20
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 calli=
ng  rte_eth_control_interface_process_msg().
     As it can't distinguish message for what particular port it is going t=
o process.
    Need to address it somehow - either add function that would return curr=
ent message port_id,
   or introduce a sync callback function and add is a parameter for  rte_et=
h_control_interface_process_msg() ,
  or probably something else.

Konstantin
 =20