Re: [dpdk-dev] [PATCH v3 1/8] net/dpaa: add support for fmlib in dpdk

[Thomas Monjalon <thomas@monjalon.net>]

29/07/2020 08:39, Hemant Agrawal:
> Hi David
> 
> From: David Marchand <david.marchand@redhat.com>
> > 
> > Hello Hemant,
> > 
> > On Mon, Jul 20, 2020 at 6:50 AM Hemant Agrawal
> > <hemant.agrawal@nxp.com> wrote:
> > > >Why dropping this codebase in DPDK instead of pulling it as a
> > dependency?
> > > [Hemant] >I don't like seeing DPDK becoming a pile of code duplicated
> > from somewhere else.
> > > [Hemant] We are not using the library as it is and making some serious
> > changes in the generic library to suit the DPDK use-case.
> > > So, it is not possible for us to use the *original* code of fmlib, which is
> > public.
> > 
> > Looking at
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
> > b.com%2Fqoriq-open-
> > source%2Ffmlib&amp;data=02%7C01%7Chemant.agrawal%40nxp.com%7Ce
> > b9167f8bbd64a6234dd08d832fc01cb%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0%7C0%7C637315405224811211&amp;sdata=%2B65hKHyXFlWgXYq7yzZb
> > lEYlI%2FyKb8OINXRuGqX8dOM%3D&amp;reserved=0 which I think is the
> > original code for this library (?).
> > 
> > Some symbols from the original code were dropped in the dpdk copy of the
> > file.
> > Part of the API changed, with a param pointer now being passed.
> 
>  [Hemant] In DPAA1 is NXP's legacy architecture.
>  
> In the DPDK on ARM-DPAA1 (MAC is called FMAN) platforms,  we have been supporting static FMAN configuration utility (fmc) to pre-configure the queues, RSS and classification rules before starting the application. This config has several limitations, e.g. max DPAA_NUM_RX_QUEUES via env variable, etc. In last few years, several customers are using it in production. 
> 
> The legacy Fmlib supports dynamic FMAN configuration and it removes current limitation in DPAA PMD. In order to support both modes where we can support new dynamic configuration with fmlib and static fmc mode (with parsing facility to avoid env variables). We have done this modification in fmlib structures to support both fmc  parser and flow API on fmlib mode.
> This param change is part of that. 
> 
> 
> > The VSP symbols are in the fm_lib.c file from the original code while they
> > have been separated in a fm_vsp.c file in the dpdk copy.
> > Some logs / comments have changed.
> > 
> [Hemant] Yes, there is code structuring and removal of code, which we don't need for DPDK and ARM platforms. There are several small changes in VSP as well as PCD (Parse Classify Distribute) configuration. 
> 
> > For the sake of understanding:
> > - can you point at the problems with reusing this externally maintained
> > library?
> 
> [Hemant] The original library was designed to work with our legacy Userspace offering (USDPAA) and Bare metal offerings on PowerPC. We did tested it initially on ARM but now largely it  is only for supporting legacy customers. This library is in maintenance mode.

Given this quite old legacy library is in maintenance mode,
we can assume the number of fixes in future will be low.
That's why I think the DPDK fork could be reformatted to DPDK code style.


> NXP don’t want to add any new feature in this legacy standalone library  - as it is difficult to test them on PowerPC and they can break the legacy customers code. 
> On ARM, we are now only offering DPDK as the Userspace solution. So, we have done a branch out of the code for DPDK.
> Some changes you are already seeing and there are more in development to support DPDK style flow classification. 
> Since DPDK is only consumer for this new base code, there is no reason for creating external dependency for the DPAA PMD. 
> 
> > - this library is tightly linked to the fm kmod drivers I guess, how is maintained
> > the compatibility?
> 
> [Hemant] kernel module is already inbuilt in NXP Linux kernel for this platform. 

Is the kernel module upstream?