Re: [Patch v8 01/18] net/mana: add basic driver with build environment and doc

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

On 9/8/2022 10:56 PM, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
> 
> MANA is a PCI device. It uses IB verbs to access hardware through the
> kernel RDMA layer. This patch introduces build environment and basic
> device probe functions.
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> Change log:
> v2:
> Fix typos.
> Make the driver build only on x86-64 and Linux.
> Remove unused header files.
> Change port definition to uint16_t or uint8_t (for IB).
> Use getline() in place of fgets() to read and truncate a line.
> v3:
> Add meson build check for required functions from RDMA direct verb header file
> v4:
> Remove extra "\n" in logging code.
> Use "r" in place of "rb" in fopen() to read text files.
> v7:
> Remove RTE_ETH_TX_OFFLOAD_TCP_TSO from offload cap.
> v8:
> Add clarification on driver args usage to nics guide.
> Fix coding sytle on function definitions.
> Use different variable names in MANA_MKSTR.
> Use MANA_ prefix for all macros.
> Use RTE_PMD_REGISTER_PCI in place of rte_pci_register.
> Add .vendor_id = 0 to the end of PCI table.
> Remove RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS from dev_flags.
> 

<...>

> +Prerequisites
> +-------------
> +
> +This driver relies on external libraries and kernel drivers for resources
> +allocations and initialization. The following dependencies are not part of
> +DPDK and must be installed separately:
> +
> +- **libibverbs** (provided by rdma-core package)
> +
> +  User space verbs framework used by librte_net_mana. This library provides
> +  a generic interface between the kernel and low-level user space drivers
> +  such as libmana.
> +
> +  It allows slow and privileged operations (context initialization, hardware
> +  resources allocations) to be managed by the kernel and fast operations to
> +  never leave user space.
> +
> +- **libmana** (provided by rdma-core package)
> +
> +  Low-level user space driver library for Microsoft Azure Network Adapter
> +  devices, it is automatically loaded by libibverbs. The minimal version of
> +  rdma-core with libmana is v43.
> +
> +- **Kernel modules**
> +
> +  They provide the kernel-side verbs API and low level device drivers that
> +  manage actual hardware initialization and resources sharing with user
> +  space processes.
> +
> +  Unlike most other PMDs, these modules must remain loaded and bound to
> +  their devices:
> +
> +  - mana: Ethernet device driver that provides kernel network interfaces.
> +  - mana_ib: InifiniBand device driver.
> +  - ib_uverbs: user space driver for verbs (entry point for libibverbs).
> +

Can you please add minimum required versions of kernel and libibverbs 
(if it applies)?

<...>

> +
> +static struct rte_pci_driver mana_pci_driver = {
> +	.driver = {
> +		.name = "net_mana",
> +	},

No need to set .driver.name, 'RTE_PMD_REGISTER_PCI' macro below should 
be setting it already, can you please check?

<...>

> +#define MANA_DEV_TX_OFFLOAD_SUPPORT ( \
> +		RTE_ETH_TX_OFFLOAD_MULTI_SEGS | \
> +		RTE_ETH_TX_OFFLOAD_IPV4_CKSUM | \
> +		RTE_ETH_TX_OFFLOAD_TCP_CKSUM | \
> +		RTE_ETH_TX_OFFLOAD_UDP_CKSUM)
> +

Can you please add code when they are used, instead of batch adding the 
header, this helps to keep all patches as logical entities.

This comment is valid for multiple code below.