Re: [dpdk-dev] [PATCH v4 01/17] net/axgbe: add minimal dev init and uninit support

["Kumar, Ravi1" <Ravi1.Kumar@amd.com>]

>On 4/5/2018 7:39 AM, Ravi Kumar wrote:
>> add ethernet poll mode driver for AMD 10G devices embedded in AMD 
>> EPYC™ EMBEDDED 3000 family processors
>> 
>> Signed-off-by: Ravi Kumar <Ravi1.kumar@amd.com>
><...>
>

Hi Ferruh,

Please check my comments inline.

>> @@ -410,6 +410,12 @@ CONFIG_RTE_PMD_RING_MAX_TX_RINGS=16
>>  CONFIG_RTE_LIBRTE_PMD_SOFTNIC=y
>>  
>>  #
>> +# Compile AMD PMD
>> +#
>> +CONFIG_RTE_LIBRTE_AXGBE_DEBUG_INIT=n
>> +CONFIG_RTE_LIBRTE_AXGBE_PMD=y
>
>
>Please add alphabetically.
>
>RTE_LIBRTE_AXGBE_DEBUG_INIT is used for data path logs, otherwise it should be dynamic logging. So the name is wrong for the purpose, it is no more "init"
>debug log, you have dynamic log for init already.
>And in documentation this has been documented as "Toggle display of initialization related messages" which seems wrong as well.

[Ravi] Done. Corrected document as well.

>
><...>
>
>> @@ -12,6 +12,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += af_packet
>>  DIRS-$(CONFIG_RTE_LIBRTE_ARK_PMD) += ark
>>  DIRS-$(CONFIG_RTE_LIBRTE_AVF_PMD) += avf
>>  DIRS-$(CONFIG_RTE_LIBRTE_AVP_PMD) += avp
>> +DIRS-$(CONFIG_RTE_LIBRTE_AXGBE_PMD) += axgbe
>
>Please consider adding meson support too. Perhaps with a separate patch to this set.

[Ravi] Added meson build support in a separate patch.

>
><...>
>
>> +
>> +#
>> +# all source are stored in SRCS-y
>> +#
>> +SRCS-$(CONFIG_RTE_LIBRTE_AXGBE_PMD) += axgbe_ethdev.c
>
>Shared build causing build error, you need to add dependent libraries [1], and please test shared library builds:
>
>[1] something like:
> +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring  +LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs  +LDLIBS += -lrte_bus_pci
>
><...>
>

[Ravi] Supported shared builds. Thanks.

>> +RTE_PMD_REGISTER_PCI(net_axgbe, rte_axgbe_pmd); 
>> +RTE_PMD_REGISTER_PCI_TABLE(net_axgbe, pci_id_axgbe_map); 
>> +RTE_PMD_REGISTER_KMOD_DEP(net_axgbe, "* igb_uio | uio_pci_generic | 
>> +vfio-pci");
>
>Is vfio-pci supported?
>
>Documentation says:
>"
>AXGBE PMD works only with legacy interrupts. Load ``igb_uio`` module in legacy interrupt mode using module params.
>
>Bind the intended AMD device to igb_uio module "
><...>

[Ravi] PMD supports vfio-pci. Corrected old document.