[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [dpdk-dev] [20.11, PATCH v2] baseband/fpga_5gnr_fec: add companion PF config App
From:       "Chautru, Nicolas" <nicolas.chautru () intel ! com>
Date:       2020-07-31 15:17:10
Message-ID: BY5PR11MB4451650146CCE77F4E68174CF84E0 () BY5PR11MB4451 ! namprd11 ! prod ! outlook ! com
[Download RAW message or body]

Hi Maxime, 

> 
> Hi Nicolas,
> 
> On 7/16/20 10:20 PM, Nicolas Chautru wrote:
> > Adding companion application to configure HW Device from the PF.
> > Then the device can be accessed through BBDEV from VF (or PF).
> > 
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> > doc/guides/bbdevs/fpga_5gnr_fec.rst                |  80 +++--
> > .../baseband/fpga_5gnr_fec/pf_config_app/Makefile  |  36 +++
> > .../fpga_5gnr_fec/pf_config_app/config_app.c       | 333
> +++++++++++++++++++
> > .../pf_config_app/fpga_5gnr_cfg_app.c              | 351
> +++++++++++++++++++++
> > .../pf_config_app/fpga_5gnr_cfg_app.h              | 102 ++++++
> > .../pf_config_app/fpga_5gnr_cfg_parser.c           | 187 +++++++++++
> > .../pf_config_app/fpga_5gnr_config.cfg             |  18 ++
> > 7 files changed, 1087 insertions(+), 20 deletions(-)  create mode
> > 100644 drivers/baseband/fpga_5gnr_fec/pf_config_app/Makefile
> > create mode 100644
> > drivers/baseband/fpga_5gnr_fec/pf_config_app/config_app.c
> > create mode 100644
> > drivers/baseband/fpga_5gnr_fec/pf_config_app/fpga_5gnr_cfg_app.c
> > create mode 100644
> > drivers/baseband/fpga_5gnr_fec/pf_config_app/fpga_5gnr_cfg_app.h
> > create mode 100644
> > drivers/baseband/fpga_5gnr_fec/pf_config_app/fpga_5gnr_cfg_parser.c
> > create mode 100644
> > drivers/baseband/fpga_5gnr_fec/pf_config_app/fpga_5gnr_config.cfg
> 
> I think having the pf_config_app in the driver directory is not a good idea,
> this is not the place for applications.
> 
> Also, it is not integrated in the DPDK build system, so it cannot benefit from
> the CI. Having an external dependency that is not packaged in distributions
> will not help to have it integrated in the build system.
> 

Thanks for sharing.
Note that all these points were raised openly explicitly earlier as you know, ie part \
of both pros and cons.   Still happy to get feedback from others notably Thomas. It \
appears you had side conversations with him on this very topic. 

> I see some alternatives:
> 1. Move it in another directory in the main DPDK repo, but it is not a DPDK
> example, not a dev tool and not a build tool, so it would need a new
> directory.
> 2. Create a BBDEV tools repository on dpdk.org (It would require techboard
> approval).
> 3. Host it in a dedicated repository on Intel's github 4. Move it into some
> Intel FPGA tools repository

There are several others options which were indeed considered in case this option was \
not viable.  Still DPDK was considered best option so far to keep everything in one \
recognized place for BBDEV devices but happy to get further input from others. 

> I think option 3 would be the best to get it packaged into distributions as it
> has no build dependency with any DPDK library.
> You could maybe add inih library as a git sub-repository within this repo.
> Other advantage is you wouldn't depend on DPDK release cycles to get fixes
> merged.
> 
> Regarding the tool itself, I understand from the commit message that the
> tool has a dependency on the BBDEV PMD version, but the tool run on the
> host while the PMD driver is used in the guest/container. So having it in the
> driver directory will not really help, as host DPDK (if any) and guest DPDK may
> come from different parties.

Yes this was captured earlier, purely stored there as a companion application for a \
given version of the PMD (ie. different subdirectories for each PMD directory).
They do no run in the same container for deployment and are not built at the same \
time indeed, their interface is the HW really and one being needed to be run prior to \
the other one to be functional.  

> One question I have is whether this is the tool itself that has a dependency on
> the PMD, or just the config file?

Each PMD directory would have its own version of the companion PF config application.
Ie. the patch above is only for baseband/fpga_5gnr_fec ie. Intel Vista Creek with 5G \
LDPC user image.  There will be different companion applications upstreamed for each \
other PMD directories (current and future) as they rely on different HW devices with \
independent MMIO access.  Said otherwise both the config file (features exposed) and \
implementation (registers required for these features) are defined per HW device (+ \
user image for FPGA)  hence per PMD version. 

There 2 entities have no API between themselves, only indirectly through HW (no \
shared memory, VF2PF comms, etc..).  New features may have to be added concurrently \
though, hence splitting repos create room for version mismatch and complicate the \
ingredients line up.  That was part of the pros and cons described earlier and I can \
totally see arguments both ways, and that's what I have been trying to share openly \
in this ticket history. 


Basically I see nothing fundamentally new here in the discussion, but it is great to \
receive input and I am happy to hear further input from tech board or others towards \
a decision.  This started as an open discussion on this DPDK mailing list capturing \
explicitly both pros and cons of this approach which are arguable, and in case this \
is not deemed practical eventually then we can still totally come back internally to \
the drawing board with other options outside of DPDK. 

Thanks, 
Nic


[prev in list] [next in list] [prev in thread] [next in thread]