[dpdk-dev] [RFC 2/9] net/avf: initilization of avf PMD

[Ferruh Yigit]

On 10/20/2017 1:26 AM, Jingjing Wu wrote:
> Signed-off-by: Jingjing Wu <jingjing.wu at intel.com>

<...>

>  #
> +# Compile burst-oriented AVF PMD driver
> +#
> +CONFIG_RTE_LIBRTE_AVF_PMD=y

Lets start PMD disabled and enable it after it become functional.

If you need to run a git bisect in the future on this commit, and you have a AVF
supported device. Device will be probed but since this patch is missing
avf_eth_dev_ops, I am not sure how app behaves, it may fail or crash, and you
can't complete git bisect run because of this patch.

<...>

> @@ -69,6 +69,8 @@ DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e
>  DEPDIRS-i40e = $(core-libs) librte_hash
>  DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe
>  DEPDIRS-ixgbe = $(core-libs) librte_hash
> +DIRS-$(CONFIG_RTE_LIBRTE_AVF_PMD) += avf

Can you please add this in alphabetically sorted manner?

> +DEPDIRS-avf = $(core-libs)

This is changed in prev release, DEPDIRS removed and library dependency part
moved to individual driver files as LDLIBS variable.

<...>

> +#
> +# Add extra flags for base driver files (also known as shared code)
> +# to disable warnings
> +#
> +ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
> +CFLAGS_BASE_DRIVER = -wd593 -wd188
> +else ifeq ($(CONFIG_RTE_TOOLCHAIN_CLANG),y)
> +CFLAGS_BASE_DRIVER += -Wno-sign-compare
> +CFLAGS_BASE_DRIVER += -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-unused-parameter
> +CFLAGS_BASE_DRIVER += -Wno-strict-aliasing
> +CFLAGS_BASE_DRIVER += -Wno-format
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast
> +CFLAGS_BASE_DRIVER += -Wno-format-nonliteral
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable

Lots of these common for clang and gcc, can it be possible to remove duplication?

> +else
> +CFLAGS_BASE_DRIVER  = -Wno-sign-compare
> +CFLAGS_BASE_DRIVER += -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-unused-parameter
> +CFLAGS_BASE_DRIVER += -Wno-strict-aliasing
> +CFLAGS_BASE_DRIVER += -Wno-format
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast
> +CFLAGS_BASE_DRIVER += -Wno-format-nonliteral
> +CFLAGS_BASE_DRIVER += -Wno-format-security
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable

Are these options to remove warnings specific to this driver? Looks like
copy-paste from old driver.

I believe we should reduce number of disabled compiler warning as much as
possible, what do you think removing them all and add back if it is needed?
This may cause a few compile fix patches but if they are send before integration
deadline, they can be squashed in next-net.

<...>

> +static int
> +avf_init_vf(struct rte_eth_dev *dev)
> +{
> +	int i, err, bufsz;
> +	struct avf_adapter *adapter =
> +		AVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> +	struct avf_hw *hw = AVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct avf_info *vf = AVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> +	uint16_t interval =
> +		avf_calc_itr_interval(AVF_QUEUE_ITR_INTERVAL_MAX);
> +
> +	err = avf_set_mac_type(hw);
> +	if (err) {
> +		PMD_INIT_LOG(ERR, "set_mac_type failed: %d", err);

You may want to dynamically register log type and level (rte_log_register,
rte_log_set_level) for avf_logtype_init & avf_logtype_driver before start using
loggig functions.

<...>

> +/*
> + * virtual function driver struct
> + */
> +static struct rte_pci_driver rte_avf_pmd = {
> +	.id_table = pci_id_avf_map,
> +	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,

RTE_PCI_DRV_IOVA_AS_VA ? As it has been added to i40e vf driver.

<...>

> diff --git a/drivers/net/avf/rte_pmd_avf_version.map b/drivers/net/avf/rte_pmd_avf_version.map
> new file mode 100644
> index 0000000..a70bd19
> --- /dev/null
> +++ b/drivers/net/avf/rte_pmd_avf_version.map
> @@ -0,0 +1,4 @@
> +DPDK_17.11 {

Needs to be changed for new release.

<...>