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

[Long Li <longli@microsoft.com>]

> Subject: Re: [Patch v8 01/18] net/mana: add basic driver with build
> environment and doc
> 
> 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.

Sure, will send out updates.