[PATCH v2 03/14] net/idpf: add support for device initialization

[Andrew Rybchenko]

On 9/5/22 13:58, Junfeng Guo wrote:
> Support device init and the following dev ops:
> 	- dev_configure
> 	- dev_start
> 	- dev_stop
> 	- dev_close

A bit strange set from my point of view since you can't start
without setup queues. Each patch should be testable.
It should be no dead code. It should be possible to stop after
the patch, build, run, for example, testpmd and probe,
configure, start, stop, close, unplug (depeneding on stage in
the patch series).

The patch is really big. It definitely worse to split
configre/close and start/stop (and reorder to have start
after queues setup supported).

> 
> Signed-off-by: Beilei Xing <beilei.xing at intel.com>
> Signed-off-by: Xiaoyun Li <xiaoyun.li at intel.com>
> Signed-off-by: Xiao Wang <xiao.w.wang at intel.com>
> Signed-off-by: Junfeng Guo <junfeng.guo at intel.com>
> ---
>   drivers/net/idpf/idpf_ethdev.c | 810 +++++++++++++++++++++++++++++++++
>   drivers/net/idpf/idpf_ethdev.h | 229 ++++++++++
>   drivers/net/idpf/idpf_vchnl.c  | 495 ++++++++++++++++++++
>   drivers/net/idpf/meson.build   |  18 +
>   drivers/net/idpf/version.map   |   3 +
>   drivers/net/meson.build        |   1 +
>   6 files changed, 1556 insertions(+)
>   create mode 100644 drivers/net/idpf/idpf_ethdev.c
>   create mode 100644 drivers/net/idpf/idpf_ethdev.h
>   create mode 100644 drivers/net/idpf/idpf_vchnl.c
>   create mode 100644 drivers/net/idpf/meson.build
>   create mode 100644 drivers/net/idpf/version.map
> 
> diff --git a/drivers/net/idpf/idpf_ethdev.c b/drivers/net/idpf/idpf_ethdev.c
> new file mode 100644
> index 0000000000..f0452de09e
> --- /dev/null
> +++ b/drivers/net/idpf/idpf_ethdev.c
> @@ -0,0 +1,810 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2022 Intel Corporation
> + */
> +
> +#include <rte_atomic.h>
> +#include <rte_eal.h>
> +#include <rte_ether.h>
> +#include <rte_malloc.h>
> +#include <rte_memzone.h>
> +#include <rte_dev.h>
> +
> +#include "idpf_ethdev.h"
> +
> +#define IDPF_VPORT		"vport"
> +
> +struct idpf_adapter_list adapter_list;
> +bool adapter_list_init;
> +
> +uint64_t idpf_timestamp_dynflag;
> +
> +static const char * const idpf_valid_args[] = {
> +	IDPF_VPORT,
> +	NULL
> +};
> +
> +static int idpf_dev_configure(struct rte_eth_dev *dev);
> +static int idpf_dev_start(struct rte_eth_dev *dev);
> +static int idpf_dev_stop(struct rte_eth_dev *dev);
> +static int idpf_dev_close(struct rte_eth_dev *dev);
> +
> +static const struct eth_dev_ops idpf_eth_dev_ops = {
> +	.dev_configure			= idpf_dev_configure,
> +	.dev_start			= idpf_dev_start,
> +	.dev_stop			= idpf_dev_stop,
> +	.dev_close			= idpf_dev_close,
> +};
> +
> +static int
> +idpf_init_vport_req_info(struct rte_eth_dev *dev)
> +{
> +	struct idpf_vport *vport = dev->data->dev_private;
> +	struct idpf_adapter *adapter = vport->adapter;
> +	struct virtchnl2_create_vport *vport_info;
> +	uint16_t idx = adapter->next_vport_idx;
> +
> +	if (!adapter->vport_req_info[idx]) {

DPDK coding style requires explicit comparison vs NULL [1].
It is not applicable to base driver, but PMD itself should
follow it. There is really huge number of places to fix.

It is applicable to comparision integers vs 0 as well.

[1] https://doc.dpdk.org/guides/contributing/coding_style.html#null-pointers

[snip]

> +static int
> +idpf_dev_configure(__rte_unused struct rte_eth_dev *dev)
> +{

I'm really confusing that the configure as empty.
It must validate configuration provided via
rte_eth_dev_configure() and reject if something is not
supported if ethdev does not the job.

> +	return 0;
> +}

[snip]

> diff --git a/drivers/net/idpf/meson.build b/drivers/net/idpf/meson.build
> new file mode 100644
> index 0000000000..7d776d3a15
> --- /dev/null
> +++ b/drivers/net/idpf/meson.build
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 Intel Corporation
> +
> +if is_windows
> +	build = false
> +	reason = 'not supported on Windows'
> +	subdir_done()
> +endif
> +
> +subdir('base')
> +objs = [base_objs]
> +
> +sources = files(
> +        'idpf_ethdev.c',
> +	'idpf_vchnl.c',

TAB vs spaces? Or is it just mail glitch?

> +)
> +
> +includes += include_directories('base')
> \ No newline at end of file
> diff --git a/drivers/net/idpf/version.map b/drivers/net/idpf/version.map
> new file mode 100644
> index 0000000000..b7da224860
> --- /dev/null
> +++ b/drivers/net/idpf/version.map
> @@ -0,0 +1,3 @@
> +DPDK_22 {
> +	local: *;
> +};
> \ No newline at end of file

It must be an empty line at the end of file.

> diff --git a/drivers/net/meson.build b/drivers/net/meson.build
> index e35652fe63..8faf8120c2 100644
> --- a/drivers/net/meson.build
> +++ b/drivers/net/meson.build
> @@ -28,6 +28,7 @@ drivers = [
>           'i40e',
>           'iavf',
>           'ice',
> +	'idpf',

TAB vs spaces? Or is it just mail glitch?

>           'igc',
>           'ionic',
>           'ipn3ke',