Re: [dpdk-dev] [PATCH v3 1/5] net/enetfec: introduce NXP ENETFEC driver

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

On 9/9/2021 9:43 PM, Apeksha Gupta wrote:
> ENETFEC (Fast Ethernet Controller) is a network poll mode driver
> for NXP SoC i.MX 8M Mini.
> 

Hi Apeksha,

Before going into details, I have some high level comments to start with, please
find comments below.

> This patch adds skeleton for enetfec driver with probe function.
> 
> Signed-off-by: Sachin Saxena <sachin.saxena@nxp.com>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>

<...>

> +++ b/doc/guides/nics/enetfec.rst
> @@ -0,0 +1,122 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +   Copyright 2021 NXP
> +
> +ENETFEC Poll Mode Driver
> +========================
> +
> +The ENETFEC NIC PMD (**librte_net_enetfec**) provides poll mode driver
> +support for the inbuilt NIC found in the ** NXP i.MX 8M Mini** SoC.
> +
> +More information can be found at NXP Official Website
> +<https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/i-mx-applications-processors/i-mx-8-processors/i-mx-8m-mini-arm-cortex-a53-cortex-m4-audio-voice-video:i.MX8MMINI>
> +
> +ENETFEC
> +-------
> +
> +This section provides an overview of the NXP ENETFEC and how it is
> +integrated into the DPDK.
> +
> +Contents summary
> +
> +- ENETFEC overview
> +- ENETFEC features
> +- Supported ENETFEC SoCs
> +- Prerequisites
> +- Driver compilation and testing
> +- Limitations
> +
> +ENETFEC Overview
> +~~~~~~~~~~~~~~~~
> +The i.MX 8M Mini Media Applications Processor is built to achieve both high
> +performance and low power consumption. ENETFEC is a hardware programmable
> +packet forwarding engine to provide high performance Ethernet interface.

It has 1Gbps interface, right? It can be good to give more details on the
Ethernet interface.

> +The diagram below shows a system level overview of ENETFEC:
> +
> +   ====================================================+===============
> +   US   +-----------------------------------------+    | Kernel Space
> +	|					  |    |
> +	|		ENETFEC Driver		  |    |
> +	+-----------------------------------------+    |
> +			  ^   |			       |
> +   ENETFEC	      RXQ |   | TXQ		       |
> +   PMD			  |   |			       |
> +			  |   v			       |   +----------+
> +		     +-------------+		       |   | fec-uio  |
> +		     | net_enetfec |		       |   +----------+
> +		     +-------------+		       |
> +			  ^   |			       |
> +		      TXQ |   | RXQ		       |
> +			  |   |			       |
> +			  |   v			       |
> +    ===================================================+===============
> +	 +----------------------------------------+
> +	 |					  |	  HW
> +	 |	     i.MX 8M MINI EVK		  |
> +	 |		 +-----+		  |
> +	 |		 | MAC |		  |
> +	 +---------------+-----+------------------+
> +			 | PHY |
> +			 +-----+
> +
> +ENETFEC Ethernet driver is traditional DPDK PMD driver running in the userspace.
> +The MAC and PHY are the hardware blocks. 'fec-uio' is the UIO driver, ENETFEC PMD
> +uses UIO interface to interact with kernel for PHY initialisation and for mapping
> +the allocated memory of register & BD in kernel with DPDK which gives access to
> +non-cacheable memory for BD. 

Why a specific uio driver, 'fec-uio', is required? I think this is the major
issue to clarify to proceed.

In DPDK we have full framework to support uio, to do the all memory mapping,
interrupt configuration etc..., common to all drivers.
But in this case driver is implemented as virtual driver and it handles its own
uoi handling itself. Why the driver can't use existing support and implemented
as physical driver?

btw, what is 'BD'?

> net_enetfec is logical Ethernet interface, created by
> +ENETFEC driver.
> +

It is confusing to draw 'net_enetfec' as middle man between HW and driver, isn't
it same entity as driver? Why the Rx/Tx notation is reverse before and after
net_enetfec, as HW --TX--> net_enetfec --RX--> driver?

> +- ENETFEC driver registers the device in virtual device driver.
> +- RTE framework scans and will invoke the probe function of ENETFEC driver.
> +- The probe function will set the basic device registers and also setups BD rings.
> +- On packet Rx the respective BD Ring status bit is set which is then used for
> +  packet processing.
> +- Then Tx is done first followed by Rx via logical interfaces.
> +
> +ENETFEC Features
> +~~~~~~~~~~~~~~~~~
> +
> +- Linux
> +- ARMv8
> +
> +Supported ENETFEC SoCs
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +- i.MX 8M Mini
> +
> +Prerequisites
> +~~~~~~~~~~~~~
> +
> +There are three main pre-requisites for executing ENETFEC PMD on a i.MX 8M Mini
> +compatible board:
> +
> +1. **ARM 64 Tool Chain**
> +
> +   For example, the `*aarch64* Linaro Toolchain <https://releases.linaro.org/components/toolchain/binaries/7.4-2019.02/aarch64-linux-gnu/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz>`_.
> +
> +2. **Linux Kernel**
> +
> +  It can be obtained from `NXP's Github hosting <https://source.codeaurora.org/external/qoriq/qoriq-components/linux>`_.
> +

There are multiple branches in that repo, is there more detail on which branch
to use?

> +3. **Rootfile system**
> +
> +   Any *aarch64* supporting filesystem can be used. For example,
> +   Ubuntu 18.04 LTS (Bionic) or 20.04 LTS(Focal) userland which can be obtained
> +   from `here <http://cdimage.ubuntu.com/ubuntu-base/releases/18.04/release/ubuntu-base-18.04.1-base-arm64.tar.gz>`_.
> +
> +4. The Ethernet device will be registered as virtual device, so ENETFEC has dependency on
> +   **rte_bus_vdev** library and it is mandatory to use `--vdev` with value `net_enetfec` to
> +   run DPDK application.
> +
> +Driver compilation and testing
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Follow instructions available in the document
> +:ref:`compiling and testing a PMD for a NIC <pmd_build_and_test>`
> +to launch **dpdk-testpmd**
> +
> +Limitations
> +~~~~~~~~~~~
> +
> +- Multi queue is not supported.
> +- Link status is down always.

This seems a big limitation, it can prevents some application to use the NIC.
What prevents the having proper link status?

> +- Single Ethernet interface.

What does 'Single Ethernet interface' means, can you please elaborate?

<...>

> +
> +#define ENETFEC_NAME_PMD                net_enetfec
> +#define ENETFEC_VDEV_GEM_ID_ARG         "intf"

Is this devarg used at all?

<...>

> +/* Common log type name prefix */
> +#define ENETFEC_LOGTYPE_PREFIX	"pmd.net.enetfec."
> +

Is this macro used at all?

<...>

> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2021 NXP
> +
> +if not is_linux
> +	build = false
> +	reason = 'only supported on linux'
> +endif
> +
> +deps += ['common_dpaax']
> +

Why there is a dependency to dpaax?