From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v4 2/5] eal: add Port Representor
 command-line option
Date: Tue, 9 Jan 2018 22:07:43 +0000
Message-ID: <f05d97e2-e9a9-6a28-b037-1371de17ea65@intel.com>
References: <20180108143720.7994-1-remy.horton@intel.com>
 <20180108143720.7994-3-remy.horton@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: John McNamara <john.mcnamara@intel.com>, Wenzhuo Lu
 <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
 Declan Doherty <declan.doherty@intel.com>,
 Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
To: Remy Horton <remy.horton@intel.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id F129D2BC8
 for <dev@dpdk.org>; Tue,  9 Jan 2018 23:07:47 +0100 (CET)
In-Reply-To: <20180108143720.7994-3-remy.horton@intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 1/8/2018 2:37 PM, Remy Horton wrote:
> Port Representors provide a logical presentation in DPDK of VF (virtual
> function) ports for the purposes of control and monitoring. Each port
> representor device represents a single VF and is associated with it's
> parent physical function (PF) PMD which provides the back-end hooks for
> the representor device ops and defines the control domain to which that
> port belongs. This allows to use existing DPDK APIs to monitor and control
> the port without the need to create and maintain VF specific APIs.
> 
> By default the Port Representor infrastructure is not enabled. This
> patch implements the --enable-representor EAL command-line parameter
> that activates representation functionality.>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
> Signed-off-by: Remy Horton <remy.horton@intel.com>

<...>

> @@ -78,6 +78,7 @@ const struct option
>  eal_long_options[] = {
>  	{OPT_BASE_VIRTADDR,     1, NULL, OPT_BASE_VIRTADDR_NUM    },
>  	{OPT_CREATE_UIO_DEV,    0, NULL, OPT_CREATE_UIO_DEV_NUM   },
> +	{OPT_ENABLE_REPRESENTOR, 0, NULL, OPT_ENABLE_REPRESENTOR_NUM   },

There must be some kind of documentation to document new eal option.

<...>

> @@ -83,6 +83,8 @@ enum {
>  	OPT_VFIO_INTR_NUM,
>  #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
>  	OPT_VMWARE_TSC_MAP_NUM,
> +#define OPT_ENABLE_REPRESENTOR    "enable-representor"
> +	OPT_ENABLE_REPRESENTOR_NUM,

In this context the meaning is clear. But when a newcomer checking the source
code from scratch I believe it may not be very clear what "enable-representor"
does, what is represented etc. What do you think?

Overall there are mixed usage, but I would vote for using port-representor
everywhere just representor used, will it be too long? Also I have seen this
shorten as "prep" which where looks like short of "prepare", what do you think
about another keyword?

<...>

> +
> +int rte_representor_enabled(void)
> +{
> +	return internal_config.enable_representor;
> +}

Isn't this need to be in .map file. I didn't test but will it work when build
for shared library?