[dpdk-dev] [PATCH v4 2/5] eal: add Port Representor command-line option

[Ferruh Yigit]

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 at intel.com>
> Signed-off-by: Mohammad Abdul Awal <mohammad.abdul.awal at intel.com>
> Signed-off-by: Remy Horton <remy.horton at 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?