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

[Apeksha Gupta <apeksha.gupta@nxp.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, September 23, 2021 3:40 PM
> To: Apeksha Gupta <apeksha.gupta@nxp.com>;
> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru;
> ferruh.yigit@intel.com
> Cc: dev@dpdk.org; Sachin Saxena <sachin.saxena@nxp.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v3 1/5] net/enetfec: introduce NXP
> ENETFEC driver
> 
> Caution: EXT Email
> 
> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
> ww.nxp.com%2Fproducts%2Fprocessors-and-microcontrollers%2Farm-
> processors%2Fi-mx-applications-processors%2Fi-mx-8-processors%2Fi-mx-
> 8m-mini-arm-cortex-a53-cortex-m4-audio-voice-
> video%3Ai.MX8MMINI&amp;data=04%7C01%7Capeksha.gupta%40nxp.com
> %7C74fdc59c47574d828f7608d97e7a4df3%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C1%7C637679886024819097%7CUnknown%7CTWFpbGZsb3d8ey
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000&amp;sdata=TjuqIfC8yXwBa3DLCgA7djItfV0UmZ6lA3uLDQ7TFwY%3D
> &amp;reserved=0>
> > +
> > +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.
[Apeksha] Okay.
> 
> > +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?
[Apeksha] Yes you are correct. As per our knowledge, UIO framework is there for VM & PCI bus devices and not for vdev bus devices. 
That is why we are using standard UIO framework in DPDK to access with kernel 'fec-uio' driver.
In document description I will update the details more precisely.

> 
> btw, what is 'BD'?
[Apeksha] BD is Buffer Descriptor. we will update it in v4 patch series.

> 
> > 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?
[Apeksha] yes I do agree. We will update the figure in v4 patch series.

> 
> > +- 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Frele
> ases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F7.4-
> 2019.02%2Faarch64-linux-gnu%2Fgcc-linaro-7.4.1-2019.02-x86_64_aarch64-
> linux-
> gnu.tar.xz&amp;data=04%7C01%7Capeksha.gupta%40nxp.com%7C74fdc59c
> 47574d828f7608d97e7a4df3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C1%7C637679886024819097%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am
> p;sdata=Lun43hJqB%2Fw%2Bd07v9hOn3NrsRdlmeyoesiRrJLt81p8%3D&amp;
> reserved=0>`_.
> > +
> > +2. **Linux Kernel**
> > +
> > +  It can be obtained from `NXP's Github hosting
> <https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> ce.codeaurora.org%2Fexternal%2Fqoriq%2Fqoriq-
> components%2Flinux&amp;data=04%7C01%7Capeksha.gupta%40nxp.com%
> 7C74fdc59c47574d828f7608d97e7a4df3%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C1%7C637679886024819097%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000&amp;sdata=0EFI%2ByEZB1PvYGt%2BemrLA2poBMdgPFTti9S7CO0qP
> X4%3D&amp;reserved=0>`_.
> > +
> 
> There are multiple branches in that repo, is there more detail on which
> branch
> to use?
[Apeksha] lf-5.10.y branch, will add it in v4 patch series.

> 
> > +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
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcdim
> age.ubuntu.com%2Fubuntu-
> base%2Freleases%2F18.04%2Frelease%2Fubuntu-base-18.04.1-base-
> arm64.tar.gz&amp;data=04%7C01%7Capeksha.gupta%40nxp.com%7C74fdc5
> 9c47574d828f7608d97e7a4df3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C1%7C637679886024819097%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&
> amp;sdata=18AItEAX8Yp6bsyDdrGmVCu%2BQ9%2FJroX%2BS9CadeKsS%2Bw
> %3D&amp;reserved=0>`_.
> > +
> > +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?
[Apeksha] I agree this is a bug. 'link change status' support is planned to be implemented in future.
Meanwhile we will set link default status to UP so all other applications work seamlessly.

> 
> > +- Single Ethernet interface.
> 
> What does 'Single Ethernet interface' means, can you please elaborate?
[Apeksha] Your observation is correct, this is mistake from our side.
This is hardware feature i.e. only one Ethernet interface available on
i.MX 8M Mini. We realize that this is not a limitation. We will update it.

> 
> <...>
> 
> > +
> > +#define ENETFEC_NAME_PMD                net_enetfec
> > +#define ENETFEC_VDEV_GEM_ID_ARG         "intf"
> 
> Is this devarg used at all?
[Apeksha] No, I will update in v4 patch series.

> 
> <...>
> 
> > +/* Common log type name prefix */
> > +#define ENETFEC_LOGTYPE_PREFIX       "pmd.net.enetfec."
> > +
> 
> Is this macro used at all?
[Apeksha] No, I will update in v4 patch series.
> 
> <...>
> 
> > +# 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?
[Apeksha] There was some common MACROs that we are using & are part of dpaax (drivers/common/dpaax/compat.h).
As per suggestion we will remove this dependency in v4 patch series.