Re: [dpdk-dev] [PATCH] ethdev: moved bypass functions to ixgbe pmd

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/26/2017 3:30 PM, Radu Nicolau wrote:
> Moved all bypass functions to ixgbe pmd and removed function
> pointers from the eth_dev_ops struct.

Hi Radu,

Thanks for taking care of this, this was waiting for a while.

> 
> Also cleared some checkpatch errors.
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

<...>

> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0afac68..66f9e63 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -523,7 +523,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"   Flush (default) or don't flush RX streams before"
>  			" forwarding. Mainly used with PCAP drivers.\n\n"
>  
> -			#ifdef RTE_NIC_BYPASS
> +#if defined(RTE_NIC_BYPASS) && defined(RTE_LIBRTE_IXGBE_PMD)

What we did for other PMD specific API is:
Enable commands always, do not disable them with compile time options,
this is mostly to prevent different test paths.

And move PMD specific #ifdefs into function, if the PMDs that support
his feature are not enabled return not supported error.

And if function called for port_id that doesn't support the function,
again return not supported.

>  			"set bypass mode (normal|bypass|isolate) (port_id)\n"
>  			"   Set the bypass mode for the lowest port on bypass enabled"
>  			" NIC.\n\n"
> @@ -3904,7 +3904,7 @@ cmdline_parse_inst_t cmd_set_link_check = {
>  	},
>  };
>  

<...>

> diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c b/drivers/net/ixgbe/rte_pmd_ixgbe.c
> index e8fc9a6..2b44a33 100644
> --- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
> +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
> @@ -908,3 +908,110 @@ rte_pmd_ixgbe_set_tc_bw_alloc(uint8_t port,
>  
>  	return 0;
>  }
> +
> +#ifdef RTE_NIC_BYPASS
> +int
> +rte_pmd_ixgbe_bypass_init(uint8_t port_id)
> +{
> +	struct rte_eth_dev *dev;

Should we add a check to all APIs if the port_id is ixgbe port_id? In
case user provided a port_id of different NIC?

> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	dev = &rte_eth_devices[port_id];
> +	ixgbe_bypass_init(dev);
> +	return 0;
> +}

<...>