On 10/20/2017 1:26 AM, Jingjing Wu wrote:
> Signed-off-by: Jingjing Wu <jingjing...@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.
<...>
