[v2,10/13] baseband/fpga_5gnr_fec: add configure function

  Add configure function to configure the PF from within
the bbdev-test itself without external application
configuration the device.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 app/test-bbdev/test_bbdev_perf.c                   |  57 ++++++
 doc/guides/bbdevs/fpga_5gnr_fec.rst                | 123 +++++++++++++
 drivers/baseband/fpga_5gnr_fec/Makefile            |   3 +
 drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h     |  74 ++++++++
 drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 196 +++++++++++++++++++++
 .../rte_pmd_bbdev_fpga_5gnr_fec_version.map        |   7 +
 6 files changed, 460 insertions(+)
 create mode 100644 drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
  

Reviewed-by: Niall Power <niall.power@intel.com>

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Chautru, Nicolas
> Sent: Monday, March 30, 2020 1:03 AM
> To: dev@dpdk.org; akhil.goyal@nxp.com
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Chautru, Nicolas
> <nicolas.chautru@intel.com>
> Subject: [dpdk-dev] [PATCH v2 10/13] baseband/fpga_5gnr_fec: add configure
> function
> 
> Add configure function to configure the PF from within the bbdev-test itself
> without external application configuration the device.
> 
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>  app/test-bbdev/test_bbdev_perf.c                   |  57 ++++++
>  doc/guides/bbdevs/fpga_5gnr_fec.rst                | 123 +++++++++++++
>  drivers/baseband/fpga_5gnr_fec/Makefile            |   3 +
>  drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h     |  74 ++++++++
>  drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c | 196
> +++++++++++++++++++++
>  .../rte_pmd_bbdev_fpga_5gnr_fec_version.map        |   7 +
>  6 files changed, 460 insertions(+)
>  create mode 100644 drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> 
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-
> bbdev/test_bbdev_perf.c
> index d8db58e..19ad65c 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -42,6 +42,19 @@
>  #define FLR_TIMEOUT 610
>  #endif
> 
> +#ifdef RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC
> +#include <fpga_5gnr_fec.h>
> +#define FPGA_5GNR_PF_DRIVER_NAME ("intel_fpga_5gnr_fec_pf") #define
> +FPGA_5GNR_VF_DRIVER_NAME ("intel_fpga_5gnr_fec_vf") #define
> +VF_UL_5G_QUEUE_VALUE 4 #define VF_DL_5G_QUEUE_VALUE 4 #define
> +UL_5G_BANDWIDTH 3 #define DL_5G_BANDWIDTH 3 #define
> UL_5G_LOAD_BALANCE
> +128 #define DL_5G_LOAD_BALANCE 128 #define FLR_5G_TIMEOUT 610
> #endif
> +
>  #define OPS_CACHE_SIZE 256U
>  #define OPS_POOL_SIZE_MIN 511U /* 0.5K per queue */
> 
> @@ -563,6 +576,50 @@ typedef int (test_case_function)(struct active_device
> *ad,
>  				info->dev_name);
>  	}
>  #endif
> +#ifdef RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC
> +	if ((get_init_device() == true) &&
> +		(!strcmp(info->drv.driver_name,
> FPGA_5GNR_PF_DRIVER_NAME))) {
> +		struct fpga_5gnr_fec_conf conf;
> +		unsigned int i;
> +
> +		printf("Configure FPGA 5GNR FEC Driver %s with default
> values\n",
> +				info->drv.driver_name);
> +
> +		/* clear default configuration before initialization */
> +		memset(&conf, 0, sizeof(struct fpga_5gnr_fec_conf));
> +
> +		/* Set PF mode :
> +		 * true if PF is used for data plane
> +		 * false for VFs
> +		 */
> +		conf.pf_mode_en = true;
> +
> +		for (i = 0; i < FPGA_5GNR_FEC_NUM_VFS; ++i) {
> +			/* Number of UL queues per VF (fpga supports 8 VFs) */
> +			conf.vf_ul_queues_number[i] =
> VF_UL_5G_QUEUE_VALUE;
> +			/* Number of DL queues per VF (fpga supports 8 VFs) */
> +			conf.vf_dl_queues_number[i] =
> VF_DL_5G_QUEUE_VALUE;
> +		}
> +
> +		/* UL bandwidth. Needed for schedule algorithm */
> +		conf.ul_bandwidth = UL_5G_BANDWIDTH;
> +		/* DL bandwidth */
> +		conf.dl_bandwidth = DL_5G_BANDWIDTH;
> +
> +		/* UL & DL load Balance Factor to 64 */
> +		conf.ul_load_balance = UL_5G_LOAD_BALANCE;
> +		conf.dl_load_balance = DL_5G_LOAD_BALANCE;
> +
> +		/**< FLR timeout value */
> +		conf.flr_time_out = FLR_5G_TIMEOUT;
> +
> +		/* setup FPGA PF with configuration information */
> +		ret = fpga_5gnr_fec_configure(info->dev_name, &conf);
> +		TEST_ASSERT_SUCCESS(ret,
> +				"Failed to configure 5G FPGA PF for bbdev %s",
> +				info->dev_name);
> +	}
> +#endif
>  	nb_queues = RTE_MIN(rte_lcore_count(), info->drv.max_num_queues);
>  	nb_queues = RTE_MIN(nb_queues, (unsigned int) MAX_QUEUES);
> 
> diff --git a/doc/guides/bbdevs/fpga_5gnr_fec.rst
> b/doc/guides/bbdevs/fpga_5gnr_fec.rst
> index 7eab7a4..5641b1a 100644
> --- a/doc/guides/bbdevs/fpga_5gnr_fec.rst
> +++ b/doc/guides/bbdevs/fpga_5gnr_fec.rst
> @@ -166,6 +166,129 @@ name is different:
>    echo <num-of-vfs> > /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_numvfs
> 
> 
> +Configure the VFs through PF
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The PCI virtual functions must be configured before working or getting
> +assigned to VMs/Containers. The configuration involves allocating the
> +number of hardware queues, priorities, load balance, bandwidth and
> +other settings necessary for the device to perform FEC functions.
> +
> +This configuration needs to be executed at least once after reboot or
> +PCI FLR and can be achieved by using the function
> +``fpga_5gnr_fec_configure()``, which sets up the parameters defined in
> ``fpga_5gnr_fec_conf`` structure:
> +
> +.. code-block:: c
> +
> +  struct fpga_5gnr_fec_conf {
> +      bool pf_mode_en;
> +      uint8_t vf_ul_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +      uint8_t vf_dl_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +      uint8_t ul_bandwidth;
> +      uint8_t dl_bandwidth;
> +      uint8_t ul_load_balance;
> +      uint8_t dl_load_balance;
> +      uint16_t flr_time_out;
> +  };
> +
> +- ``pf_mode_en``: identifies whether only PF is to be used, or the VFs.
> +PF and
> +  VFs are mutually exclusive and cannot run simultaneously.
> +  Set to 1 for PF mode enabled.
> +  If PF mode is enabled all queues available in the device are assigned
> +  exclusively to PF and 0 queues given to VFs.
> +
> +- ``vf_*l_queues_number``: defines the hardware queue mapping for every
> VF.
> +
> +- ``*l_bandwidth``: in case of congestion on PCIe interface. The device
> +  allocates different bandwidth to UL and DL. The weight is configured
> +by this
> +  setting. The unit of weight is 3 code blocks. For example, if the
> +code block
> +  cbps (code block per second) ratio between UL and DL is 12:1, then
> +the
> +  configuration value should be set to 36:3. The schedule algorithm is
> +based
> +  on code block regardless the length of each block.
> +
> +- ``*l_load_balance``: hardware queues are load-balanced in a
> +round-robin
> +  fashion. Queues get filled first-in first-out until they reach a
> +pre-defined
> +  watermark level, if exceeded, they won't get assigned new code blocks..
> +  This watermark is defined by this setting.
> +
> +  If all hardware queues exceeds the watermark, no code blocks will be
> + streamed in from UL/DL code block FIFO.
> +
> +- ``flr_time_out``: specifies how many 16.384us to be FLR time out. The
> +  time_out = flr_time_out x 16.384us. For instance, if you want to set
> +10ms for
> +  the FLR time out then set this setting to 0x262=610.
> +
> +
> +An example configuration code calling the function
> +``fpga_5gnr_fec_configure()`` is shown
> +below:
> +
> +.. code-block:: c
> +
> +  struct fpga_5gnr_fec_conf conf;
> +  unsigned int i;
> +
> +  memset(&conf, 0, sizeof(struct fpga_5gnr_fec_conf));  conf.pf_mode_en
> + = 1;
> +
> +  for (i = 0; i < FPGA_5GNR_FEC_NUM_VFS; ++i) {
> +      conf.vf_ul_queues_number[i] = 4;
> +      conf.vf_dl_queues_number[i] = 4;
> +  }
> +  conf.ul_bandwidth = 12;
> +  conf.dl_bandwidth = 5;
> +  conf.dl_load_balance = 64;
> +  conf.ul_load_balance = 64;
> +
> +  /* setup FPGA PF */
> +  ret = fpga_5gnr_fec_configure(info->dev_name, &conf);
> + TEST_ASSERT_SUCCESS(ret,
> +      "Failed to configure 4G FPGA PF for bbdev %s",
> +      info->dev_name);
> +
> +
> +Test Application
> +----------------
> +
> +BBDEV provides a test application, ``test-bbdev.py`` and range of test
> +data for testing the functionality of FPGA 5GNR FEC encode and decode,
> +depending on the device's capabilities. The test application is located
> +under app->test-bbdev folder and has the following options:
> +
> +.. code-block:: console
> +
> +  "-p", "--testapp-path": specifies path to the bbdev test app.
> +  "-e", "--eal-params"	: EAL arguments which are passed to the test app.
> +  "-t", "--timeout"	: Timeout in seconds (default=300).
> +  "-c", "--test-cases"	: Defines test cases to run. Run all if not specified.
> +  "-v", "--test-vector"	: Test vector path (default=dpdk_path+/app/test-
> bbdev/test_vectors/bbdev_null.data).
> +  "-n", "--num-ops"	: Number of operations to process on device
> (default=32).
> +  "-b", "--burst-size"	: Operations enqueue/dequeue burst size (default=32).
> +  "-l", "--num-lcores"	: Number of lcores to run (default=16).
> +  "-i", "--init-device" : Initialise PF device with default values.
> +
> +
> +To execute the test application tool using simple decode or encode
> +data, type one of the following:
> +
> +.. code-block:: console
> +
> +  ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_dec_default.data
> + ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_enc_default.data
> +
> +
> +The test application ``test-bbdev.py``, supports the ability to
> +configure the PF device with a default set of values, if the "-i" or "-
> +-init-device" option is included. The default values are defined in
> test_bbdev_perf.c as:
> +
> +- VF_UL_QUEUE_VALUE 4
> +- VF_DL_QUEUE_VALUE 4
> +- UL_BANDWIDTH 3
> +- DL_BANDWIDTH 3
> +- UL_LOAD_BALANCE 128
> +- DL_LOAD_BALANCE 128
> +- FLR_TIMEOUT 610
> +
> +
>  Test Vectors
>  ~~~~~~~~~~~~
> 
> diff --git a/drivers/baseband/fpga_5gnr_fec/Makefile
> b/drivers/baseband/fpga_5gnr_fec/Makefile
> index 3f5c511..b68a79f 100644
> --- a/drivers/baseband/fpga_5gnr_fec/Makefile
> +++ b/drivers/baseband/fpga_5gnr_fec/Makefile
> @@ -23,4 +23,7 @@ LIBABIVER := 1
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC) +=
> rte_fpga_5gnr_fec.c
> 
> +# export include files
> +SYMLINK-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC)-include +=
> +fpga_5gnr_fec.h
> +
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> new file mode 100644
> index 0000000..7eebc7d
> --- /dev/null
> +++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation
> + */
> +
> +#ifndef _FPGA_5GNR_FEC_H_
> +#define _FPGA_5GNR_FEC_H_
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +/**
> + * @file fpga_5gnr_fec.h
> + *
> + * Interface for Intel(R) FGPA 5GNR FEC device configuration at the
> +host level,
> + * directly accessible by the application.
> + * Configuration related to 5GNR functionality is done through
> + * librte_bbdev library.
> + *
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice  */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**< Number of Virtual Functions FGPA 4G FEC supports */ #define
> +FPGA_5GNR_FEC_NUM_VFS 8
> +
> +/**
> + * Structure to pass FPGA 4G FEC configuration.
> + */
> +struct fpga_5gnr_fec_conf {
> +	/**< 1 if PF is used for dataplane, 0 for VFs */
> +	bool pf_mode_en;
> +	/**< Number of UL queues per VF */
> +	uint8_t vf_ul_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +	/**< Number of DL queues per VF */
> +	uint8_t vf_dl_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +	/**< UL bandwidth. Needed for schedule algorithm */
> +	uint8_t ul_bandwidth;
> +	/**< DL bandwidth. Needed for schedule algorithm */
> +	uint8_t dl_bandwidth;
> +	/**< UL Load Balance */
> +	uint8_t ul_load_balance;
> +	/**< DL Load Balance */
> +	uint8_t dl_load_balance;
> +	/**< FLR timeout value */
> +	uint16_t flr_time_out;
> +};
> +
> +/**
> + * Configure Intel(R) FPGA 5GNR FEC device
> + *
> + * @param dev_name
> + *   The name of the device. This is the short form of PCI BDF, e.g. 00:01.0.
> + *   It can also be retrieved for a bbdev device from the dev_name field in the
> + *   rte_bbdev_info structure returned by rte_bbdev_info_get().
> + * @param conf
> + *   Configuration to apply to FPGA 4G FEC.
> + *
> + * @return
> + *   Zero on success, negative value on failure.
> + */
> +__rte_experimental
> +int
> +fpga_5gnr_fec_configure(const char *dev_name,
> +		const struct fpga_5gnr_fec_conf *conf);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _FPGA_5GNR_FEC_H_ */
> diff --git a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> index 389257d..2559da5 100644
> --- a/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> +++ b/drivers/baseband/fpga_5gnr_fec/rte_fpga_5gnr_fec.c
> @@ -21,6 +21,7 @@
>  #include <rte_bbdev_pmd.h>
> 
>  #include "rte_fpga_5gnr_fec.h"
> +#include "fpga_5gnr_fec.h"
> 
>  /* 5GNR SW PMD logging ID */
>  static int fpga_5gnr_fec_logtype;
> @@ -1722,6 +1723,201 @@
>  	return 0;
>  }
> 
> +static inline void
> +set_default_fpga_conf(struct fpga_5gnr_fec_conf *def_conf) {
> +	/* clear default configuration before initialization */
> +	memset(def_conf, 0, sizeof(struct fpga_5gnr_fec_conf));
> +	/* Set pf mode to true */
> +	def_conf->pf_mode_en = true;
> +
> +	/* Set ratio between UL and DL to 1:1 (unit of weight is 3 CBs) */
> +	def_conf->ul_bandwidth = 3;
> +	def_conf->dl_bandwidth = 3;
> +
> +	/* Set Load Balance Factor to 64 */
> +	def_conf->dl_load_balance = 64;
> +	def_conf->ul_load_balance = 64;
> +}
> +
> +/* Initial configuration of FPGA 5GNR FEC device */ int
> +fpga_5gnr_fec_configure(const char *dev_name,
> +		const struct fpga_5gnr_fec_conf *conf) {
> +	uint32_t payload_32, address;
> +	uint16_t payload_16;
> +	uint8_t payload_8;
> +	uint16_t q_id, vf_id, total_q_id, total_ul_q_id, total_dl_q_id;
> +	struct rte_bbdev *bbdev = rte_bbdev_get_named_dev(dev_name);
> +	struct fpga_5gnr_fec_conf def_conf;
> +
> +	if (bbdev == NULL) {
> +		rte_bbdev_log(ERR,
> +				"Invalid dev_name (%s), or device is not yet
> initialised",
> +				dev_name);
> +		return -ENODEV;
> +	}
> +
> +	struct fpga_5gnr_fec_device *d = bbdev->data->dev_private;
> +
> +	if (conf == NULL) {
> +		rte_bbdev_log(ERR,
> +				"FPGA Configuration was not provided. Default
> configuration will be loaded.");
> +		set_default_fpga_conf(&def_conf);
> +		conf = &def_conf;
> +	}
> +
> +	/*
> +	 * Configure UL:DL ratio.
> +	 * [7:0]: UL weight
> +	 * [15:8]: DL weight
> +	 */
> +	payload_16 = (conf->dl_bandwidth << 8) | conf->ul_bandwidth;
> +	address = FPGA_5GNR_FEC_CONFIGURATION;
> +	fpga_reg_write_16(d->mmio_base, address, payload_16);
> +
> +	/* Clear all queues registers */
> +	payload_32 = FPGA_INVALID_HW_QUEUE_ID;
> +	for (q_id = 0; q_id < FPGA_TOTAL_NUM_QUEUES; ++q_id) {
> +		address = (q_id << 2) + FPGA_5GNR_FEC_QUEUE_MAP;
> +		fpga_reg_write_32(d->mmio_base, address, payload_32);
> +	}
> +
> +	/*
> +	 * If PF mode is enabled allocate all queues for PF only.
> +	 *
> +	 * For VF mode each VF can have different number of UL and DL queues.
> +	 * Total number of queues to configure cannot exceed FPGA
> +	 * capabilities - 64 queues - 32 queues for UL and 32 queues for DL.
> +	 * Queues mapping is done according to configuration:
> +	 *
> +	 * UL queues:
> +	 * |                Q_ID              | VF_ID |
> +	 * |                 0                |   0   |
> +	 * |                ...               |   0   |
> +	 * | conf->vf_dl_queues_number[0] - 1 |   0   |
> +	 * | conf->vf_dl_queues_number[0]     |   1   |
> +	 * |                ...               |   1   |
> +	 * | conf->vf_dl_queues_number[1] - 1 |   1   |
> +	 * |                ...               |  ...  |
> +	 * | conf->vf_dl_queues_number[7] - 1 |   7   |
> +	 *
> +	 * DL queues:
> +	 * |                Q_ID              | VF_ID |
> +	 * |                 32               |   0   |
> +	 * |                ...               |   0   |
> +	 * | conf->vf_ul_queues_number[0] - 1 |   0   |
> +	 * | conf->vf_ul_queues_number[0]     |   1   |
> +	 * |                ...               |   1   |
> +	 * | conf->vf_ul_queues_number[1] - 1 |   1   |
> +	 * |                ...               |  ...  |
> +	 * | conf->vf_ul_queues_number[7] - 1 |   7   |
> +	 *
> +	 * Example of configuration:
> +	 * conf->vf_ul_queues_number[0] = 4;  -> 4 UL queues for VF0
> +	 * conf->vf_dl_queues_number[0] = 4;  -> 4 DL queues for VF0
> +	 * conf->vf_ul_queues_number[1] = 2;  -> 2 UL queues for VF1
> +	 * conf->vf_dl_queues_number[1] = 2;  -> 2 DL queues for VF1
> +	 *
> +	 * UL:
> +	 * | Q_ID | VF_ID |
> +	 * |   0  |   0   |
> +	 * |   1  |   0   |
> +	 * |   2  |   0   |
> +	 * |   3  |   0   |
> +	 * |   4  |   1   |
> +	 * |   5  |   1   |
> +	 *
> +	 * DL:
> +	 * | Q_ID | VF_ID |
> +	 * |  32  |   0   |
> +	 * |  33  |   0   |
> +	 * |  34  |   0   |
> +	 * |  35  |   0   |
> +	 * |  36  |   1   |
> +	 * |  37  |   1   |
> +	 */
> +	if (conf->pf_mode_en) {
> +		payload_32 = 0x1;
> +		for (q_id = 0; q_id < FPGA_TOTAL_NUM_QUEUES; ++q_id) {
> +			address = (q_id << 2) + FPGA_5GNR_FEC_QUEUE_MAP;
> +			fpga_reg_write_32(d->mmio_base, address,
> payload_32);
> +		}
> +	} else {
> +		/* Calculate total number of UL and DL queues to configure */
> +		total_ul_q_id = total_dl_q_id = 0;
> +		for (vf_id = 0; vf_id < FPGA_5GNR_FEC_NUM_VFS; ++vf_id) {
> +			total_ul_q_id += conf->vf_ul_queues_number[vf_id];
> +			total_dl_q_id += conf->vf_dl_queues_number[vf_id];
> +		}
> +		total_q_id = total_dl_q_id + total_ul_q_id;
> +		/*
> +		 * Check if total number of queues to configure does not exceed
> +		 * FPGA capabilities (64 queues - 32 UL and 32 DL queues)
> +		 */
> +		if ((total_ul_q_id > FPGA_NUM_UL_QUEUES) ||
> +			(total_dl_q_id > FPGA_NUM_DL_QUEUES) ||
> +			(total_q_id > FPGA_TOTAL_NUM_QUEUES)) {
> +			rte_bbdev_log(ERR,
> +					"FPGA Configuration failed. Too many
> queues to configure: UL_Q %u, DL_Q %u, FPGA_Q %u",
> +					total_ul_q_id, total_dl_q_id,
> +					FPGA_TOTAL_NUM_QUEUES);
> +			return -EINVAL;
> +		}
> +		total_ul_q_id = 0;
> +		for (vf_id = 0; vf_id < FPGA_5GNR_FEC_NUM_VFS; ++vf_id) {
> +			for (q_id = 0; q_id < conf->vf_ul_queues_number[vf_id];
> +					++q_id, ++total_ul_q_id) {
> +				address = (total_ul_q_id << 2) +
> +
> 	FPGA_5GNR_FEC_QUEUE_MAP;
> +				payload_32 = ((0x80 + vf_id) << 16) | 0x1;
> +				fpga_reg_write_32(d->mmio_base, address,
> +						payload_32);
> +			}
> +		}
> +		total_dl_q_id = 0;
> +		for (vf_id = 0; vf_id < FPGA_5GNR_FEC_NUM_VFS; ++vf_id) {
> +			for (q_id = 0; q_id < conf->vf_dl_queues_number[vf_id];
> +					++q_id, ++total_dl_q_id) {
> +				address = ((total_dl_q_id +
> FPGA_NUM_UL_QUEUES)
> +						<< 2) +
> FPGA_5GNR_FEC_QUEUE_MAP;
> +				payload_32 = ((0x80 + vf_id) << 16) | 0x1;
> +				fpga_reg_write_32(d->mmio_base, address,
> +						payload_32);
> +			}
> +		}
> +	}
> +
> +	/* Setting Load Balance Factor */
> +	payload_16 = (conf->dl_load_balance << 8) | (conf->ul_load_balance);
> +	address = FPGA_5GNR_FEC_LOAD_BALANCE_FACTOR;
> +	fpga_reg_write_16(d->mmio_base, address, payload_16);
> +
> +	/* Setting length of ring descriptor entry */
> +	payload_16 = FPGA_RING_DESC_ENTRY_LENGTH;
> +	address = FPGA_5GNR_FEC_RING_DESC_LEN;
> +	fpga_reg_write_16(d->mmio_base, address, payload_16);
> +
> +	/* Setting FLR timeout value */
> +	payload_16 = conf->flr_time_out;
> +	address = FPGA_5GNR_FEC_FLR_TIME_OUT;
> +	fpga_reg_write_16(d->mmio_base, address, payload_16);
> +
> +	/* Queue PF/VF mapping table is ready */
> +	payload_8 = 0x1;
> +	address = FPGA_5GNR_FEC_QUEUE_PF_VF_MAP_DONE;
> +	fpga_reg_write_8(d->mmio_base, address, payload_8);
> +
> +	rte_bbdev_log_debug("PF FPGA 5GNR FEC configuration complete for
> %s",
> +			dev_name);
> +
> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> +	print_static_reg_debug_info(d->mmio_base);
> +#endif
> +	return 0;
> +}
> +
>  /* FPGA 5GNR FEC PCI PF address map */
>  static struct rte_pci_id pci_id_fpga_5gnr_fec_pf_map[] = {
>  	{
> diff --git
> a/drivers/baseband/fpga_5gnr_fec/rte_pmd_bbdev_fpga_5gnr_fec_version.ma
> p
> b/drivers/baseband/fpga_5gnr_fec/rte_pmd_bbdev_fpga_5gnr_fec_version.ma
> p
> index f9f17e4..b0fb971 100644
> ---
> a/drivers/baseband/fpga_5gnr_fec/rte_pmd_bbdev_fpga_5gnr_fec_version.ma
> p
> +++
> b/drivers/baseband/fpga_5gnr_fec/rte_pmd_bbdev_fpga_5gnr_fec_version
> +++ .map
> @@ -1,3 +1,10 @@
>  DPDK_20.0 {
>  	local: *;
>  };
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	fpga_5gnr_fec_configure;
> +
> +};
> --
> 1.8.3.1
  

> 
> diff --git a/drivers/baseband/fpga_5gnr_fec/Makefile
> b/drivers/baseband/fpga_5gnr_fec/Makefile
> index 3f5c511..b68a79f 100644
> --- a/drivers/baseband/fpga_5gnr_fec/Makefile
> +++ b/drivers/baseband/fpga_5gnr_fec/Makefile
> @@ -23,4 +23,7 @@ LIBABIVER := 1
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC) +=
> rte_fpga_5gnr_fec.c
> 
> +# export include files
> +SYMLINK-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC)-include +=
> fpga_5gnr_fec.h
> +
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> new file mode 100644
> index 0000000..7eebc7d
> --- /dev/null
> +++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation
> + */
> +
> +#ifndef _FPGA_5GNR_FEC_H_
> +#define _FPGA_5GNR_FEC_H_
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +/**
> + * @file fpga_5gnr_fec.h
> + *
> + * Interface for Intel(R) FGPA 5GNR FEC device configuration at the host level,
> + * directly accessible by the application.
> + * Configuration related to 5GNR functionality is done through
> + * librte_bbdev library.
> + *
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + */

The exposed PMD header files are normally prefixed as rte_pmd_

You should rename your other header file as fpga_5gnr_fec.h
And this one as rte_pmd_fpga_5gnr_fec.h

BTW what is the need of a pmd API to configure the fpga?
Is it not possible to do that as one of rte_bbdev_ops ?

I can see that the comments in rte_bbdev_ops are not in proper format.
'<' should not be there if comment is before the element
Could you please correct them in a separate patch? Please check other structures as well

> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**< Number of Virtual Functions FGPA 4G FEC supports */
> +#define FPGA_5GNR_FEC_NUM_VFS 8
> +
> +/**
> + * Structure to pass FPGA 4G FEC configuration.
> + */
> +struct fpga_5gnr_fec_conf {
> +	/**< 1 if PF is used for dataplane, 0 for VFs */
> +	bool pf_mode_en;
> +	/**< Number of UL queues per VF */
> +	uint8_t vf_ul_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +	/**< Number of DL queues per VF */
> +	uint8_t vf_dl_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> +	/**< UL bandwidth. Needed for schedule algorithm */
> +	uint8_t ul_bandwidth;
> +	/**< DL bandwidth. Needed for schedule algorithm */
> +	uint8_t dl_bandwidth;
> +	/**< UL Load Balance */
> +	uint8_t ul_load_balance;
> +	/**< DL Load Balance */
> +	uint8_t dl_load_balance;
> +	/**< FLR timeout value */
> +	uint16_t flr_time_out;

If you are adding comment before the element, then no need to add '<'
  

Hi Akhil, 

> > diff --git a/drivers/baseband/fpga_5gnr_fec/Makefile
> > b/drivers/baseband/fpga_5gnr_fec/Makefile
> > index 3f5c511..b68a79f 100644
> > --- a/drivers/baseband/fpga_5gnr_fec/Makefile
> > +++ b/drivers/baseband/fpga_5gnr_fec/Makefile
> > @@ -23,4 +23,7 @@ LIBABIVER := 1
> >  # library source files
> >  SRCS-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC) +=
> > rte_fpga_5gnr_fec.c
> >
> > +# export include files
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_PMD_BBDEV_FPGA_5GNR_FEC)-include
> +=
> > fpga_5gnr_fec.h
> > +
> >  include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > new file mode 100644
> > index 0000000..7eebc7d
> > --- /dev/null
> > +++ b/drivers/baseband/fpga_5gnr_fec/fpga_5gnr_fec.h
> > @@ -0,0 +1,74 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2020 Intel Corporation  */
> > +
> > +#ifndef _FPGA_5GNR_FEC_H_
> > +#define _FPGA_5GNR_FEC_H_
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +/**
> > + * @file fpga_5gnr_fec.h
> > + *
> > + * Interface for Intel(R) FGPA 5GNR FEC device configuration at the
> > +host level,
> > + * directly accessible by the application.
> > + * Configuration related to 5GNR functionality is done through
> > + * librte_bbdev library.
> > + *
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice  */
> 
> The exposed PMD header files are normally prefixed as rte_pmd_
> 
> You should rename your other header file as fpga_5gnr_fec.h And this one as
> rte_pmd_fpga_5gnr_fec.h
> 

OK

> BTW what is the need of a pmd API to configure the fpga?
> Is it not possible to do that as one of rte_bbdev_ops ?

This configuration is done agnostically of bbdev, the application doesn't need to know there is actual HW device requiring to be configured below the hood.
Also this operation would be possible for the VF driver which doesn't have access to related registers. 
Typical deployment is within a container environment and this configuration is done without any need of any dependency on anything from librte_bbdev. 

Something we were considering was to push a standalone application (without dependency on BBDEV or even DPDK) to do that configuration from PF. 
Currently DPDK community are only able to run the PF driver, since to run from the VF would need a separate application to configure the device to allow usage from VF. 
We need to provide such application externally to DPDK currently (pretty much that function wrapped up with args and xml parsing). 
So we were wondering whether this could still be provided with DPDK (even if application has no DPDK dependency) or just push it externally on git hub. I see pros and cons, any view on that or similar concern for other crypto drivers?

> 
> I can see that the comments in rte_bbdev_ops are not in proper format.
> '<' should not be there if comment is before the element Could you please
> correct them in a separate patch? Please check other structures as well
> 

Thanks. This was missed all over the place, will make a ticket for fixing in separate patch. 
For now will fix in that new serie today. 

> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**< Number of Virtual Functions FGPA 4G FEC supports */ #define
> > +FPGA_5GNR_FEC_NUM_VFS 8
> > +
> > +/**
> > + * Structure to pass FPGA 4G FEC configuration.
> > + */
> > +struct fpga_5gnr_fec_conf {
> > +	/**< 1 if PF is used for dataplane, 0 for VFs */
> > +	bool pf_mode_en;
> > +	/**< Number of UL queues per VF */
> > +	uint8_t vf_ul_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> > +	/**< Number of DL queues per VF */
> > +	uint8_t vf_dl_queues_number[FPGA_5GNR_FEC_NUM_VFS];
> > +	/**< UL bandwidth. Needed for schedule algorithm */
> > +	uint8_t ul_bandwidth;
> > +	/**< DL bandwidth. Needed for schedule algorithm */
> > +	uint8_t dl_bandwidth;
> > +	/**< UL Load Balance */
> > +	uint8_t ul_load_balance;
> > +	/**< DL Load Balance */
> > +	uint8_t dl_load_balance;
> > +	/**< FLR timeout value */
> > +	uint16_t flr_time_out;
> 
> If you are adding comment before the element, then no need to add '<'
  

Hi Akhil, Thomas, 

Following up on that previous discussion below so that to confirm whether there is an available option to handle this usecase within DPDK repo. 

Basically traditional deployment for VRAN relies on BBDEV/DPDK running within container where the workload is processed behind BBDEV API bounded to a VF of the accelerator, all that is fully covered currently in 20.05.
Conversely an application from baremetal still has to be run at initialization to do the required register poking to PF MMIO so that to configure the HW so that the VF is functional. Without this it is not possible to use the VF driver form within the container. Said otherwise the BBDEV VF PMD cannot be even tested with DPDK repo (only the PF PMD with the workaround discussed in the previous discussion).
That small userspace application is purely doing mmap and writing to register based on xml file input (relying on igb_uio bounded to PF, or other vanilla kernel module) and has no dependency on rest of DPDK (DPDK would not be installed outside of the container since no packet or wireless workload is actually run from there). 
Is that sensible to add such a small companion application within the related PMD directory even if it has no dependency on DPDK libraries per se, only the fact that is required just to be able to use BBDEV from the VF.
On one hand I see reason not to do this as this is not a DPDK application per se, but that companion HW application is still required to be able for anyone  to use BBDEV driver + being within the same repo enforces that there is no risk of version mismatch. The other option being to put that on a separate repo outside of DPDK causing fragmentation of ingredients across repos. 

I wanted to check whether you had any strong opinion on this topic and whether a patch with such a companion simple user application may be approved. 

Thanks and regards, 
Nic

 
> > BTW what is the need of a pmd API to configure the fpga?
> > Is it not possible to do that as one of rte_bbdev_ops ?
> 
> This configuration is done agnostically of bbdev, the application doesn't need to
> know there is actual HW device requiring to be configured below the hood.
> Also this operation would be possible for the VF driver which doesn't have
> access to related registers.
> Typical deployment is within a container environment and this configuration is
> done without any need of any dependency on anything from librte_bbdev.
> 
> Something we were considering was to push a standalone application (without
> dependency on BBDEV or even DPDK) to do that configuration from PF.
> Currently DPDK community are only able to run the PF driver, since to run from
> the VF would need a separate application to configure the device to allow
> usage from VF.
> We need to provide such application externally to DPDK currently (pretty much
> that function wrapped up with args and xml parsing).
> So we were wondering whether this could still be provided with DPDK (even if
> application has no DPDK dependency) or just push it externally on git hub. I see
> pros and cons, any view on that or similar concern for other crypto drivers?
> 
> >
  

02/05/2020 01:15, Chautru, Nicolas:
> Hi Akhil, Thomas, 
> 
> Following up on that previous discussion below so that to confirm whether there is an available option to handle this usecase within DPDK repo. 
> 
> Basically traditional deployment for VRAN relies on BBDEV/DPDK running within container where the workload is processed behind BBDEV API bounded to a VF of the accelerator, all that is fully covered currently in 20.05.
> Conversely an application from baremetal still has to be run at initialization to do the required register poking to PF MMIO so that to configure the HW so that the VF is functional. Without this it is not possible to use the VF driver form within the container. Said otherwise the BBDEV VF PMD cannot be even tested with DPDK repo (only the PF PMD with the workaround discussed in the previous discussion).
> That small userspace application is purely doing mmap and writing to register based on xml file input (relying on igb_uio bounded to PF, or other vanilla kernel module) and has no dependency on rest of DPDK (DPDK would not be installed outside of the container since no packet or wireless workload is actually run from there). 
> Is that sensible to add such a small companion application within the related PMD directory even if it has no dependency on DPDK libraries per se, only the fact that is required just to be able to use BBDEV from the VF.
> On one hand I see reason not to do this as this is not a DPDK application per se, but that companion HW application is still required to be able for anyone  to use BBDEV driver + being within the same repo enforces that there is no risk of version mismatch. The other option being to put that on a separate repo outside of DPDK causing fragmentation of ingredients across repos. 
> 
> I wanted to check whether you had any strong opinion on this topic and whether a patch with such a companion simple user application may be approved. 

I feel it is best to have the required app in the PMD directory,
as in "batteries included".



--------------------------

> > > BTW what is the need of a pmd API to configure the fpga?
> > > Is it not possible to do that as one of rte_bbdev_ops ?
> > 
> > This configuration is done agnostically of bbdev, the application doesn't need to
> > know there is actual HW device requiring to be configured below the hood.
> > Also this operation would be possible for the VF driver which doesn't have
> > access to related registers.
> > Typical deployment is within a container environment and this configuration is
> > done without any need of any dependency on anything from librte_bbdev.
> > 
> > Something we were considering was to push a standalone application (without
> > dependency on BBDEV or even DPDK) to do that configuration from PF.
> > Currently DPDK community are only able to run the PF driver, since to run from
> > the VF would need a separate application to configure the device to allow
> > usage from VF.
> > We need to provide such application externally to DPDK currently (pretty much
> > that function wrapped up with args and xml parsing).
> > So we were wondering whether this could still be provided with DPDK (even if
> > application has no DPDK dependency) or just push it externally on git hub. I see
> > pros and cons, any view on that or similar concern for other crypto drivers?
  

> From: Thomas Monjalon :
> 02/05/2020 01:15, Chautru, Nicolas:
> > Hi Akhil, Thomas,
> >
> > Following up on that previous discussion below so that to confirm whether
> there is an available option to handle this usecase within DPDK repo.
> >
> > Basically traditional deployment for VRAN relies on BBDEV/DPDK running
> within container where the workload is processed behind BBDEV API bounded
> to a VF of the accelerator, all that is fully covered currently in 20.05.
> > Conversely an application from baremetal still has to be run at initialization
> to do the required register poking to PF MMIO so that to configure the HW so
> that the VF is functional. Without this it is not possible to use the VF driver
> form within the container. Said otherwise the BBDEV VF PMD cannot be even
> tested with DPDK repo (only the PF PMD with the workaround discussed in
> the previous discussion).
> > That small userspace application is purely doing mmap and writing to
> register based on xml file input (relying on igb_uio bounded to PF, or other
> vanilla kernel module) and has no dependency on rest of DPDK (DPDK would
> not be installed outside of the container since no packet or wireless workload
> is actually run from there).
> > Is that sensible to add such a small companion application within the
> related PMD directory even if it has no dependency on DPDK libraries per se,
> only the fact that is required just to be able to use BBDEV from the VF.
> > On one hand I see reason not to do this as this is not a DPDK application per
> se, but that companion HW application is still required to be able for anyone
> to use BBDEV driver + being within the same repo enforces that there is no
> risk of version mismatch. The other option being to put that on a separate
> repo outside of DPDK causing fragmentation of ingredients across repos.
> >
> > I wanted to check whether you had any strong opinion on this topic and
> whether a patch with such a companion simple user application may be
> approved.
> 
> I feel it is best to have the required app in the PMD directory, as in "batteries
> included".
> 

Hi Thomas, 
For such a companion application to configure the HW within the PMD directory I want to confirm two things before pushing a patch : 
	- This is okay with you for it to build outside of the DPDK build flow. Ie. Separate Makefile, not planning meson support. Again zero DPDK libraries dependency. 
	- This is okay with you for it to have dependency on other open-source library to build it. Ie. we are currently linking to this https://github.com/benhoyt/inih (BSD license) as a simple input config file parsing. 

Thanks, 
Nic
  

25/06/2020 02:30, Chautru, Nicolas:
> > From: Thomas Monjalon :
> > 02/05/2020 01:15, Chautru, Nicolas:
> > > Hi Akhil, Thomas,
> > >
> > > Following up on that previous discussion below so that to confirm whether
> > there is an available option to handle this usecase within DPDK repo.
> > >
> > > Basically traditional deployment for VRAN relies on BBDEV/DPDK running
> > within container where the workload is processed behind BBDEV API bounded
> > to a VF of the accelerator, all that is fully covered currently in 20.05.
> > > Conversely an application from baremetal still has to be run at initialization
> > to do the required register poking to PF MMIO so that to configure the HW so
> > that the VF is functional. Without this it is not possible to use the VF driver
> > form within the container. Said otherwise the BBDEV VF PMD cannot be even
> > tested with DPDK repo (only the PF PMD with the workaround discussed in
> > the previous discussion).
> > > That small userspace application is purely doing mmap and writing to
> > register based on xml file input (relying on igb_uio bounded to PF, or other
> > vanilla kernel module) and has no dependency on rest of DPDK (DPDK would
> > not be installed outside of the container since no packet or wireless workload
> > is actually run from there).
> > > Is that sensible to add such a small companion application within the
> > related PMD directory even if it has no dependency on DPDK libraries per se,
> > only the fact that is required just to be able to use BBDEV from the VF.
> > > On one hand I see reason not to do this as this is not a DPDK application per
> > se, but that companion HW application is still required to be able for anyone
> > to use BBDEV driver + being within the same repo enforces that there is no
> > risk of version mismatch. The other option being to put that on a separate
> > repo outside of DPDK causing fragmentation of ingredients across repos.
> > >
> > > I wanted to check whether you had any strong opinion on this topic and
> > whether a patch with such a companion simple user application may be
> > approved.
> > 
> > I feel it is best to have the required app in the PMD directory, as in "batteries
> > included".
> > 
> 
> Hi Thomas, 
> For such a companion application to configure the HW within the PMD directory I want to confirm two things before pushing a patch : 
> 	- This is okay with you for it to build outside of the DPDK build flow. Ie. Separate Makefile, not planning meson support. Again zero DPDK libraries dependency.

I think it should be built as part of the PMD.
Why not?

> 	- This is okay with you for it to have dependency on other open-source library to build it. Ie. we are currently linking to this https://github.com/benhoyt/inih (BSD license) as a simple input config file parsing.

No problem with dependencies.
  

> From: Thomas Monjalon 
> 25/06/2020 02:30, Chautru, Nicolas:
> > > From: Thomas Monjalon :
> > > 02/05/2020 01:15, Chautru, Nicolas:
> > > > Hi Akhil, Thomas,
> > > >
> > > > Following up on that previous discussion below so that to confirm
> > > > whether
> > > there is an available option to handle this usecase within DPDK repo.
> > > >
> > > > Basically traditional deployment for VRAN relies on BBDEV/DPDK
> > > > running
> > > within container where the workload is processed behind BBDEV API
> > > bounded to a VF of the accelerator, all that is fully covered currently in
> 20.05.
> > > > Conversely an application from baremetal still has to be run at
> > > > initialization
> > > to do the required register poking to PF MMIO so that to configure
> > > the HW so that the VF is functional. Without this it is not possible
> > > to use the VF driver form within the container. Said otherwise the
> > > BBDEV VF PMD cannot be even tested with DPDK repo (only the PF PMD
> > > with the workaround discussed in the previous discussion).
> > > > That small userspace application is purely doing mmap and writing
> > > > to
> > > register based on xml file input (relying on igb_uio bounded to PF,
> > > or other vanilla kernel module) and has no dependency on rest of
> > > DPDK (DPDK would not be installed outside of the container since no
> > > packet or wireless workload is actually run from there).
> > > > Is that sensible to add such a small companion application within
> > > > the
> > > related PMD directory even if it has no dependency on DPDK libraries
> > > per se, only the fact that is required just to be able to use BBDEV from the
> VF.
> > > > On one hand I see reason not to do this as this is not a DPDK
> > > > application per
> > > se, but that companion HW application is still required to be able
> > > for anyone to use BBDEV driver + being within the same repo enforces
> > > that there is no risk of version mismatch. The other option being to
> > > put that on a separate repo outside of DPDK causing fragmentation of
> ingredients across repos.
> > > >
> > > > I wanted to check whether you had any strong opinion on this topic
> > > > and
> > > whether a patch with such a companion simple user application may be
> > > approved.
> > >
> > > I feel it is best to have the required app in the PMD directory, as
> > > in "batteries included".
> > >
> >
> > Hi Thomas,
> > For such a companion application to configure the HW within the PMD
> directory I want to confirm two things before pushing a patch :
> > 	- This is okay with you for it to build outside of the DPDK build flow.
> Ie. Separate Makefile, not planning meson support. Again zero DPDK libraries
> dependency.
> 
> I think it should be built as part of the PMD.
> Why not?

For the same reason as above : you would not deploy this companion application in the same OS or container/VM as DPDK; they would be built without dependency on each other, but still provided together so that you can actually have all the ingredients in one place without mismatch and be able to actually use the PMD will all required ingredients in one place. 
Also based on the dependency below, even if adding option to build within same DPDK meson framework, it would not build by default by anyone as the dependency repo would be lacking. 
For that reason that would be a bit artificial to me to be built with the PMD really, but I could be convinced otherwise. 
Any thought Thomas?

> 
> > 	- This is okay with you for it to have dependency on other open-
> source library to build it. Ie. we are currently linking to this
> https://github.com/benhoyt/inih (BSD license) as a simple input config file
> parsing.
> 
> No problem with dependencies.
> 
>
  

26/06/2020 03:14, Chautru, Nicolas:
> > From: Thomas Monjalon 
> > 25/06/2020 02:30, Chautru, Nicolas:
> > > > From: Thomas Monjalon :
> > > > 02/05/2020 01:15, Chautru, Nicolas:
> > > > > Hi Akhil, Thomas,
> > > > >
> > > > > Following up on that previous discussion below so that to confirm
> > > > > whether
> > > > there is an available option to handle this usecase within DPDK repo.
> > > > >
> > > > > Basically traditional deployment for VRAN relies on BBDEV/DPDK
> > > > > running
> > > > within container where the workload is processed behind BBDEV API
> > > > bounded to a VF of the accelerator, all that is fully covered currently in
> > 20.05.
> > > > > Conversely an application from baremetal still has to be run at
> > > > > initialization
> > > > to do the required register poking to PF MMIO so that to configure
> > > > the HW so that the VF is functional. Without this it is not possible
> > > > to use the VF driver form within the container. Said otherwise the
> > > > BBDEV VF PMD cannot be even tested with DPDK repo (only the PF PMD
> > > > with the workaround discussed in the previous discussion).
> > > > > That small userspace application is purely doing mmap and writing
> > > > > to
> > > > register based on xml file input (relying on igb_uio bounded to PF,
> > > > or other vanilla kernel module) and has no dependency on rest of
> > > > DPDK (DPDK would not be installed outside of the container since no
> > > > packet or wireless workload is actually run from there).
> > > > > Is that sensible to add such a small companion application within
> > > > > the
> > > > related PMD directory even if it has no dependency on DPDK libraries
> > > > per se, only the fact that is required just to be able to use BBDEV from the
> > VF.
> > > > > On one hand I see reason not to do this as this is not a DPDK
> > > > > application per
> > > > se, but that companion HW application is still required to be able
> > > > for anyone to use BBDEV driver + being within the same repo enforces
> > > > that there is no risk of version mismatch. The other option being to
> > > > put that on a separate repo outside of DPDK causing fragmentation of
> > ingredients across repos.
> > > > >
> > > > > I wanted to check whether you had any strong opinion on this topic
> > > > > and
> > > > whether a patch with such a companion simple user application may be
> > > > approved.
> > > >
> > > > I feel it is best to have the required app in the PMD directory, as
> > > > in "batteries included".
> > > >
> > >
> > > Hi Thomas,
> > > For such a companion application to configure the HW within the PMD
> > directory I want to confirm two things before pushing a patch :
> > > 	- This is okay with you for it to build outside of the DPDK build flow.
> > Ie. Separate Makefile, not planning meson support. Again zero DPDK libraries
> > dependency.
> > 
> > I think it should be built as part of the PMD.
> > Why not?
> 
> For the same reason as above : you would not deploy this companion application in the same OS or container/VM as DPDK; they would be built without dependency on each other, but still provided together so that you can actually have all the ingredients in one place without mismatch and be able to actually use the PMD will all required ingredients in one place.

OK I missed the DPDK environment is not the same as the environment
of the companion application.

> Also based on the dependency below, even if adding option to build within same DPDK meson framework, it would not build by default by anyone as the dependency repo would be lacking.

What is the dependency? I assume it is freely and easily downloadable.

> For that reason that would be a bit artificial to me to be built with the PMD really, but I could be convinced otherwise.
> Any thought Thomas?

OK
If you think the application is tightly related to the PMD,
I think it could be hosted with it.


> > > 	- This is okay with you for it to have dependency on other open-
> > source library to build it. Ie. we are currently linking to this
> > https://github.com/benhoyt/inih (BSD license) as a simple input config file
> > parsing.
> > 
> > No problem with dependencies.
  

> From: Thomas Monjalon <thomas@monjalon.net>
> 26/06/2020 03:14, Chautru, Nicolas:
> > > From: Thomas Monjalon
> > > 25/06/2020 02:30, Chautru, Nicolas:
> > > > > From: Thomas Monjalon :
> > > > > 02/05/2020 01:15, Chautru, Nicolas:
> > > > > > Hi Akhil, Thomas,
> > > > > >
> > > > > > Following up on that previous discussion below so that to
> > > > > > confirm whether
> > > > > there is an available option to handle this usecase within DPDK repo.
> > > > > >
> > > > > > Basically traditional deployment for VRAN relies on BBDEV/DPDK
> > > > > > running
> > > > > within container where the workload is processed behind BBDEV
> > > > > API bounded to a VF of the accelerator, all that is fully
> > > > > covered currently in
> > > 20.05.
> > > > > > Conversely an application from baremetal still has to be run
> > > > > > at initialization
> > > > > to do the required register poking to PF MMIO so that to
> > > > > configure the HW so that the VF is functional. Without this it
> > > > > is not possible to use the VF driver form within the container.
> > > > > Said otherwise the BBDEV VF PMD cannot be even tested with DPDK
> > > > > repo (only the PF PMD with the workaround discussed in the previous
> discussion).
> > > > > > That small userspace application is purely doing mmap and
> > > > > > writing to
> > > > > register based on xml file input (relying on igb_uio bounded to
> > > > > PF, or other vanilla kernel module) and has no dependency on
> > > > > rest of DPDK (DPDK would not be installed outside of the
> > > > > container since no packet or wireless workload is actually run from
> there).
> > > > > > Is that sensible to add such a small companion application
> > > > > > within the
> > > > > related PMD directory even if it has no dependency on DPDK
> > > > > libraries per se, only the fact that is required just to be able
> > > > > to use BBDEV from the
> > > VF.
> > > > > > On one hand I see reason not to do this as this is not a DPDK
> > > > > > application per
> > > > > se, but that companion HW application is still required to be
> > > > > able for anyone to use BBDEV driver + being within the same repo
> > > > > enforces that there is no risk of version mismatch. The other
> > > > > option being to put that on a separate repo outside of DPDK
> > > > > causing fragmentation of
> > > ingredients across repos.
> > > > > >
> > > > > > I wanted to check whether you had any strong opinion on this
> > > > > > topic and
> > > > > whether a patch with such a companion simple user application
> > > > > may be approved.
> > > > >
> > > > > I feel it is best to have the required app in the PMD directory,
> > > > > as in "batteries included".
> > > > >
> > > >
> > > > Hi Thomas,
> > > > For such a companion application to configure the HW within the
> > > > PMD
> > > directory I want to confirm two things before pushing a patch :
> > > > 	- This is okay with you for it to build outside of the DPDK build flow.
> > > Ie. Separate Makefile, not planning meson support. Again zero DPDK
> > > libraries dependency.
> > >
> > > I think it should be built as part of the PMD.
> > > Why not?
> >
> > For the same reason as above : you would not deploy this companion
> application in the same OS or container/VM as DPDK; they would be built
> without dependency on each other, but still provided together so that you
> can actually have all the ingredients in one place without mismatch and be
> able to actually use the PMD will all required ingredients in one place.
> 
> OK I missed the DPDK environment is not the same as the environment of the
> companion application.
> 
> > Also based on the dependency below, even if adding option to build within
> same DPDK meson framework, it would not build by default by anyone as the
> dependency repo would be lacking.
> 
> What is the dependency? I assume it is freely and easily downloadable.
> 
> > For that reason that would be a bit artificial to me to be built with the PMD
> really, but I could be convinced otherwise.
> > Any thought Thomas?
> 
> OK
> If you think the application is tightly related to the PMD, I think it could be
> hosted with it.
> 
> 
> > > > 	- This is okay with you for it to have dependency on other open-
> > > source library to build it. Ie. we are currently linking to this
> > > https://github.com/benhoyt/inih (BSD license) as a simple input
> > > config file parsing.
> > >
> > > No problem with dependencies.
> 
> 

Hi, 
Note that the related contribution is capture on this new patchset.
http://patches.dpdk.org/patch/73785/

Thanks
Nicolas