RE: [PATCH v3] net/ice: fix tm configuration cannot be cleared

2023-09-07 Thread Zhang, Qi Z



> -Original Message-
> From: Wu, Wenjun1 
> Sent: Thursday, September 7, 2023 10:35 AM
> To: Deng, KaiwenX ; dev@dpdk.org
> Cc: sta...@dpdk.org; Yang, Qiming ; Zhou, YidingX
> ; Deng, KaiwenX ;
> Zhang, Qi Z ; Xu, Ting 
> Subject: RE: [PATCH v3] net/ice: fix tm configuration cannot be cleared
> 
> 
> 
> > -Original Message-
> > From: Kaiwen Deng 
> > Sent: Wednesday, September 6, 2023 3:50 PM
> > To: dev@dpdk.org
> > Cc: sta...@dpdk.org; Yang, Qiming ; Zhou,
> > YidingX ; Deng, KaiwenX
> > ; Zhang, Qi Z ; Xu, Ting
> > 
> > Subject: [PATCH v3] net/ice: fix tm configuration cannot be cleared
> >
> > When the device is stopped, the PMD resets the commit flag so that we
> > can update the hierarchy configuration. The commit flag is also used
> > to determine if the hierarchy configuration needs to be cleared.
> > When the PMD exits, it always stops the device first and also resets
> > the commit flag result in the hierarchy configuration is not cleared.
> >
> > This commit changes the PMD to not reset the commit flag when the
> > device is stopped. And we prevent additional commit when device is
> > running by only checking the stop flag.
> >
> > Fixes: f5ec6a3a1987 ("net/ice: fix TM hierarchy commit flag reset")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Kaiwen Deng 
> > ---
> > Changes since v2:
> > - Replace DPDK with the PMD in commit log.
> >
> > Changes since v1:
> > - Prevent additional commit when device is running.
> > ---
> > ---
> >  drivers/net/ice/ice_dcf_ethdev.c |  2 --
> > drivers/net/ice/ice_dcf_sched.c  | 14
> > --
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_dcf_ethdev.c
> > b/drivers/net/ice/ice_dcf_ethdev.c
> > index 30ad18d8fc..065ec728c2 100644
> > --- a/drivers/net/ice/ice_dcf_ethdev.c
> > +++ b/drivers/net/ice/ice_dcf_ethdev.c
> > @@ -670,7 +670,6 @@ ice_dcf_dev_stop(struct rte_eth_dev *dev)
> > struct ice_dcf_adapter *dcf_ad = dev->data->dev_private;
> > struct rte_intr_handle *intr_handle = dev->intr_handle;
> > struct ice_adapter *ad = &dcf_ad->parent;
> > -   struct ice_dcf_hw *hw = &dcf_ad->real_hw;
> >
> > if (ad->pf.adapter_stopped == 1) {
> > PMD_DRV_LOG(DEBUG, "Port is already stopped"); @@ -
> > 697,7 +696,6 @@ ice_dcf_dev_stop(struct rte_eth_dev *dev)
> >
> > dev->data->dev_link.link_status = RTE_ETH_LINK_DOWN;
> > ad->pf.adapter_stopped = 1;
> > -   hw->tm_conf.committed = false;
> >
> > return 0;
> >  }
> > diff --git a/drivers/net/ice/ice_dcf_sched.c
> > b/drivers/net/ice/ice_dcf_sched.c index a231c1e60b..b08bc5f1de 100644
> > --- a/drivers/net/ice/ice_dcf_sched.c
> > +++ b/drivers/net/ice/ice_dcf_sched.c
> > @@ -237,6 +237,7 @@ ice_dcf_node_add(struct rte_eth_dev *dev,
> uint32_t
> > node_id,
> > enum ice_dcf_tm_node_type node_type =
> ICE_DCF_TM_NODE_TYPE_MAX;
> > struct ice_dcf_tm_shaper_profile *shaper_profile = NULL;
> > struct ice_dcf_adapter *adapter = dev->data->dev_private;
> > +   struct ice_adapter *ad = &adapter->parent;
> > struct ice_dcf_hw *hw = &adapter->real_hw;
> > struct ice_dcf_tm_node *parent_node;
> > struct ice_dcf_tm_node *tm_node;
> > @@ -246,10 +247,10 @@ ice_dcf_node_add(struct rte_eth_dev *dev,
> > uint32_t node_id,
> > if (!params || !error)
> > return -EINVAL;
> >
> > -   /* if already committed */
> > -   if (hw->tm_conf.committed) {
> > +   /* if port is running */
> > +   if (!ad->pf.adapter_stopped) {
> > error->type = RTE_TM_ERROR_TYPE_UNSPECIFIED;
> > -   error->message = "already committed";
> > +   error->message = "port is running";
> > return -EINVAL;
> > }
> >
> > @@ -400,16 +401,17 @@ ice_dcf_node_delete(struct rte_eth_dev *dev,
> > uint32_t node_id,  {
> > enum ice_dcf_tm_node_type node_type =
> ICE_DCF_TM_NODE_TYPE_MAX;
> > struct ice_dcf_adapter *adapter = dev->data->dev_private;
> > +   struct ice_adapter *ad = &adapter->parent;
> > struct ice_dcf_hw *hw = &adapter->real_hw;
> > struct ice_dcf_tm_node *tm_node;
> >
> > if (!error)
> > return -EINVAL;
> >
> > -   /* if already committed */
> > -   if (hw->tm_conf.committed) {
> > +   /* if port is running */
> > +   if (!ad->pf.adapter_stopped) {
> > error->type = RTE_TM_ERROR_TYPE_UNSPECIFIED;
> > -   error->message = "already committed";
> > +   error->message = "port is running";
> > return -EINVAL;
> > }
> >
> > --
> > 2.25.1
> 
> Acked-by: Wenjun Wu 

Applied to dpdk-next-net-intel.

Thanks
Qi


[PATCH v1 0/2] offload support to free dma source buffer

2023-09-07 Thread Amit Prakash Shukla
This series adds offload support to free source buffer in dma library
and adds a test support in dmadev_autotest to validate the
functionality.

v1:
- Implementation from RFC.
- Add test support to validate functionality.

Amit Prakash Shukla (2):
  dmadev: offload to free source buffer
  test/dma: add source buffer offload free test

 app/test/test_dmadev.c  | 132 +++-
 lib/dmadev/rte_dmadev.h |  27 
 2 files changed, 158 insertions(+), 1 deletion(-)

-- 
2.25.1



[PATCH v1 1/2] dmadev: offload to free source buffer

2023-09-07 Thread Amit Prakash Shukla
This changeset adds support in DMA library to free source DMA buffer by
hardware. On a supported hardware, application can pass on the mempool
information as part of vchan config when the DMA transfer direction is
configured as RTE_DMA_DIR_MEM_TO_DEV.

Signed-off-by: Amit Prakash Shukla 
Acked-by: Morten Brørup 
---
 lib/dmadev/rte_dmadev.h | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h
index b157ab7600..d6a685907f 100644
--- a/lib/dmadev/rte_dmadev.h
+++ b/lib/dmadev/rte_dmadev.h
@@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
 #define RTE_DMA_CAPA_OPS_COPY_SG   RTE_BIT64(33)
 /** Support fill operation. */
 #define RTE_DMA_CAPA_OPS_FILL  RTE_BIT64(34)
+/** Support for source buffer free for mem to dev transfer.
+ *
+ * @note Even though the DMA driver has this capability, it may not support all
+ * mempool drivers. If the mempool is not supported by the DMA driver,
+ * rte_dma_vchan_setup() will fail.
+ **/
+#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE RTE_BIT64(35)
 /**@}*/
 
 /**
@@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
 * @see struct rte_dma_port_param
 */
struct rte_dma_port_param dst_port;
+   /** mempool from which source buffer is allocated. mempool info is used
+* for freeing source buffer by hardware when configured direction is
+* RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by hardware,
+* RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling rte_dma_copy and
+* rte_dma_copy_sg().
+*
+* @note If the mempool is not supported by the DMA driver,
+* rte_dma_vchan_setup() will fail.
+*
+* @see RTE_DMA_OP_FLAG_FREE_SBUF
+*/
+   struct rte_mempool *mem_to_dev_src_buf_pool;
+
 };
 
 /**
@@ -818,6 +838,13 @@ struct rte_dma_sge {
  * capability bit for this, driver should not return error if this flag was 
set.
  */
 #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
+/** Mem to dev source buffer free flag.
+ * Used for freeing source DMA buffer by hardware when the transfer direction 
is
+ * configured as RTE_DMA_DIR_MEM_TO_DEV.
+ *
+ * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
+ */
+#define RTE_DMA_OP_FLAG_FREE_SBUF  RTE_BIT64(3)
 /**@}*/
 
 /**
-- 
2.25.1



[PATCH v1 2/2] test/dma: add source buffer offload free test

2023-09-07 Thread Amit Prakash Shukla
Add a test case to validate the functionality of drivers' dma
source buffer offload free. As part of dmadev_autotest, test case
will be executed only if the driver supports source buffer offload
free and if the test is exported by env variable DPDK_ADD_DMA_TEST.

Signed-off-by: Amit Prakash Shukla 
---
 app/test/test_dmadev.c | 132 -
 1 file changed, 131 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 6ef875e545..48da4664ae 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -18,11 +18,26 @@
 
 #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); 
return -1; } while (0)
 
+#define TEST_RINGSIZE 512
 #define COPY_LEN 1024
 
 static struct rte_mempool *pool;
 static uint16_t id_count;
 
+enum {
+   TEST_SRC_BUF_FREE = 0,
+   TEST_MAX,
+};
+
+struct dma_add_test {
+   const char *name;
+   bool enabled;
+};
+
+struct dma_add_test dma_add_test[] = {
+   [TEST_SRC_BUF_FREE] = {.name = "sbuf_free", .enabled = false},
+};
+
 static void
 __rte_format_printf(3, 4)
 print_err(const char *func, int lineno, const char *format, ...)
@@ -797,10 +812,93 @@ test_burst_capacity(int16_t dev_id, uint16_t vchan)
return 0;
 }
 
+static int
+test_sbuf_free(int16_t dev_id, uint16_t vchan)
+{
+#define NR_MBUF 256
+   int i, ret = 0;
+   int retry = 100;
+   uint16_t nb_done = 0;
+   bool dma_err = false;
+   uint32_t buf_cnt1, buf_cnt2;
+   struct rte_mempool_ops *ops;
+   uint64_t remote_addr = 0x4000ull;
+   struct rte_mbuf *src[NR_MBUF], *dst[NR_MBUF];
+   const struct rte_dma_vchan_conf qconf = {
+   .direction = RTE_DMA_DIR_MEM_TO_DEV,
+   .nb_desc = TEST_RINGSIZE,
+   .mem_to_dev_src_buf_pool = pool,
+   .dst_port.port_type = RTE_DMA_PORT_PCIE,
+   /* Assuming pemid as 0. */
+   .dst_port.pcie.coreid = 0,
+   };
+   static int dev_init;
+
+   if (!dev_init) {
+   /* Stop the device to reconfigure vchan. */
+   if (rte_dma_stop(dev_id) < 0)
+   ERR_RETURN("Error stopping device %u\n", dev_id);
+
+   if (rte_dma_vchan_setup(dev_id, vchan, &qconf) < 0)
+   ERR_RETURN("Error with queue configuration\n");
+
+   if (rte_dma_start(dev_id) != 0)
+   ERR_RETURN("Error with rte_dma_start()\n");
+
+   dev_init++;
+   }
+
+   if (rte_pktmbuf_alloc_bulk(pool, dst, NR_MBUF) != 0)
+   ERR_RETURN("alloc dst mbufs failed.\n");
+
+   for (i = 0; i < NR_MBUF; i++) {
+   /* Using mbuf structure to hold remote iova address. */
+   rte_mbuf_iova_set(dst[i], (rte_iova_t)remote_addr);
+   dst[i]->data_off = 0;
+   }
+
+   /* Capture buffer count before allocating source buffer. */
+   ops = rte_mempool_get_ops(pool->ops_index);
+   buf_cnt1 = ops->get_count(pool);
+
+   if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0)
+   ERR_RETURN("alloc src mbufs failed.\n");
+
+   if ((buf_cnt1 - NR_MBUF) != ops->get_count(pool))
+   ERR_RETURN("Buffer count check failed.\n");
+
+   for (i = 0; i < NR_MBUF; i++) {
+   ret = rte_dma_copy(dev_id, vchan, rte_mbuf_data_iova(src[i]),
+   rte_mbuf_data_iova(dst[i]), COPY_LEN,
+   RTE_DMA_OP_FLAG_FREE_SBUF);
+
+   if (ret < 0)
+   ERR_RETURN("rte_dma_copy returned error.\n");
+   }
+
+   rte_dma_submit(dev_id, vchan);
+   nb_done = 0;
+   do {
+   nb_done += rte_dma_completed(dev_id, vchan, (NR_MBUF - 
nb_done), NULL, &dma_err);
+   if (dma_err)
+   break;
+   /* Sleep for 1 millisecond */
+   rte_delay_us_sleep(1000);
+   } while (retry-- && (nb_done < NR_MBUF));
+
+   buf_cnt2 = ops->get_count(pool);
+   if ((buf_cnt1 != buf_cnt2) || dma_err)
+   ERR_RETURN("Free source buffer test failed.\n");
+
+   /* If the test passes source buffer will be freed in hardware. */
+   rte_pktmbuf_free_bulk(dst, NR_MBUF);
+
+   return 0;
+}
+
 static int
 test_dmadev_instance(int16_t dev_id)
 {
-#define TEST_RINGSIZE 512
 #define CHECK_ERRStrue
struct rte_dma_stats stats;
struct rte_dma_info info;
@@ -890,6 +988,12 @@ test_dmadev_instance(int16_t dev_id)
else if (runtest("fill", test_enqueue_fill, 1, dev_id, vchan, 
CHECK_ERRS) < 0)
goto err;
 
+   if ((info.dev_capa & RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE) &&
+   dma_add_test[TEST_SRC_BUF_FREE].enabled == true) {
+   if (runtest("sbuf_free", test_sbuf_free, 128, dev_id, vchan, 
CHECK_ERRS) < 0)
+   goto err;
+   }
+
rte_mempool_free(pool);
 
if (rte_

Re: [PATCH 01/11] devtools: warn when adding some pthread calls

2023-09-07 Thread David Marchand
Hello Thomas,

On Wed, Sep 6, 2023 at 6:22 PM Thomas Monjalon  wrote:
>
> All pthread functions below have an equivalent in rte_thread API:
> - pthread_create
> - pthread_join
> - pthread_detach
> - pthread_setname_np
> - pthread_set_name_np
> - pthread_setaffinity_np
> - pthread_attr_setinheritsched
> - pthread_attr_setschedpolicy
> Usage of these functions will be raised to encourage rte_thread adoption.
>
> The pthread functions for locks and cancel are still allowed.
>
> Signed-off-by: Thomas Monjalon 
> ---
>  devtools/checkpatches.sh | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 55fabc5458..131ffbcebe 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -119,6 +119,14 @@ check_forbidden_additions() { # 
> -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> "$1" || res=1
>
> +   # refrain from using some pthread functions
> +   awk -v FOLDERS="lib drivers app examples" \
> +   -v 
> EXPRESSIONS="pthread_(create|join|detach|set((|_)name_np|affinity_np)|attr_set(inheritsched|schedpolicy))\\\("
>  \

I remember some awk (was it Alpine Linux? or FreeBSD ?..) does not
like empty pattern like (|plop).
For this case here, it is better (and kind of more readable) to use _?


> +   -v RET_ON_FAIL=1 \
> +   -v MESSAGE='Using pthread functions, prefer rte_thread' \
> +   -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> +   "$1" || res=1
> +
> # forbid use of __reserved which is a reserved keyword in Windows 
> system headers
> awk -v FOLDERS="lib drivers app examples" \
> -v EXPRESSIONS='\\<__reserved\\>' \
> --
> 2.42.0
>

-- 
David Marchand



[PATCH v1] dma/cnxk: offload source buffer free

2023-09-07 Thread Amit Prakash Shukla
Added support in driver, to offload source buffer free to hardware
on completion of DMA transfer.

Signed-off-by: Amit Prakash Shukla 
---
Depends-on: series-29427 ("use mempool for DMA chunk pool")
Depends-on: series-29442 ("offload support to free dma source buffer")

v1:
- Driver implementation from RFC.

 drivers/dma/cnxk/cnxk_dmadev.c| 48 +++
 drivers/dma/cnxk/cnxk_dmadev_fp.c |  8 +++---
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/cnxk/cnxk_dmadev.c b/drivers/dma/cnxk/cnxk_dmadev.c
index 588b3783a9..3be1547793 100644
--- a/drivers/dma/cnxk/cnxk_dmadev.c
+++ b/drivers/dma/cnxk/cnxk_dmadev.c
@@ -16,7 +16,8 @@ cnxk_dmadev_info_get(const struct rte_dma_dev *dev, struct 
rte_dma_info *dev_inf
dev_info->nb_vchans = dpivf->num_vchans;
dev_info->dev_capa = RTE_DMA_CAPA_MEM_TO_MEM | RTE_DMA_CAPA_MEM_TO_DEV |
 RTE_DMA_CAPA_DEV_TO_MEM | RTE_DMA_CAPA_DEV_TO_DEV |
-RTE_DMA_CAPA_OPS_COPY | RTE_DMA_CAPA_OPS_COPY_SG;
+RTE_DMA_CAPA_OPS_COPY | RTE_DMA_CAPA_OPS_COPY_SG |
+RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE;
dev_info->max_desc = DPI_MAX_DESC;
dev_info->min_desc = DPI_MIN_DESC;
dev_info->max_sges = DPI_MAX_POINTER;
@@ -159,9 +160,26 @@ cnxk_dmadev_configure(struct rte_dma_dev *dev, const 
struct rte_dma_conf *conf,
return rc;
 }
 
-static void
+static int
+dmadev_src_buf_aura_get(struct rte_mempool *sb_mp, const char *mp_ops_name)
+{
+   struct rte_mempool_ops *ops;
+
+   if (sb_mp == NULL)
+   return 0;
+
+   ops = rte_mempool_get_ops(sb_mp->ops_index);
+   if (strcmp(ops->name, mp_ops_name) != 0)
+   return -EINVAL;
+
+   return roc_npa_aura_handle_to_aura(sb_mp->pool_id);
+}
+
+static int
 cn9k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, const struct 
rte_dma_vchan_conf *conf)
 {
+   int aura;
+
header->cn9k.pt = DPI_HDR_PT_ZBW_CA;
 
switch (conf->direction) {
@@ -184,6 +202,11 @@ cn9k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, 
const struct rte_dma_vch
header->cn9k.func = conf->dst_port.pcie.pfid << 12;
header->cn9k.func |= conf->dst_port.pcie.vfid;
}
+   aura = dmadev_src_buf_aura_get(conf->mem_to_dev_src_buf_pool, 
"cn9k_mempool_ops");
+   if (aura < 0)
+   return aura;
+   header->cn9k.aura = aura;
+   header->cn9k.ii = 1;
break;
case RTE_DMA_DIR_MEM_TO_MEM:
header->cn9k.xtype = DPI_XTYPE_INTERNAL_ONLY;
@@ -197,11 +220,15 @@ cn9k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, 
const struct rte_dma_vch
header->cn9k.fport = conf->dst_port.pcie.coreid;
header->cn9k.pvfe = 0;
};
+
+   return 0;
 }
 
-static void
+static int
 cn10k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, const struct 
rte_dma_vchan_conf *conf)
 {
+   int aura;
+
header->cn10k.pt = DPI_HDR_PT_ZBW_CA;
 
switch (conf->direction) {
@@ -224,6 +251,10 @@ cn10k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, 
const struct rte_dma_vc
header->cn10k.func = conf->dst_port.pcie.pfid << 12;
header->cn10k.func |= conf->dst_port.pcie.vfid;
}
+   aura = dmadev_src_buf_aura_get(conf->mem_to_dev_src_buf_pool, 
"cn10k_mempool_ops");
+   if (aura < 0)
+   return aura;
+   header->cn10k.aura = aura;
break;
case RTE_DMA_DIR_MEM_TO_MEM:
header->cn10k.xtype = DPI_XTYPE_INTERNAL_ONLY;
@@ -237,6 +268,8 @@ cn10k_dmadev_setup_hdr(union cnxk_dpi_instr_cmd *header, 
const struct rte_dma_vc
header->cn10k.fport = conf->dst_port.pcie.coreid;
header->cn10k.pvfe = 0;
};
+
+   return 0;
 }
 
 static int
@@ -248,7 +281,7 @@ cnxk_dmadev_vchan_setup(struct rte_dma_dev *dev, uint16_t 
vchan,
union cnxk_dpi_instr_cmd *header;
uint16_t max_desc;
uint32_t size;
-   int i;
+   int i, ret;
 
RTE_SET_USED(conf_sz);
 
@@ -257,9 +290,12 @@ cnxk_dmadev_vchan_setup(struct rte_dma_dev *dev, uint16_t 
vchan,
return 0;
 
if (dpivf->is_cn10k)
-   cn10k_dmadev_setup_hdr(header, conf);
+   ret = cn10k_dmadev_setup_hdr(header, conf);
else
-   cn9k_dmadev_setup_hdr(header, conf);
+   ret = cn9k_dmadev_setup_hdr(header, conf);
+
+   if (ret)
+   return ret;
 
/* Free up descriptor memory before allocating. */
cnxk_dmadev_vchan_free(dpivf, vchan);
diff --git a/drivers/dma/cnxk/cnxk_dmadev_fp.c 
b/drivers/dma/cnxk/cnxk_dmadev_fp.c
index d1f27ba2a6..5049ad503d 100644
--- a/drivers/dma/cnxk/cnxk_dmadev_fp.c
+

[PATCH v3] net/iavf: unregister intr handler before FD close

2023-09-07 Thread Saurabh Singhal
Unregister VFIO interrupt handler before the interrupt fd gets closed in
case iavf_dev_init() returns an error.

dpdk creates a standalone thread named eal-intr-thread for processing
interrupts for the PCI devices. The interrupt handler callbacks are
registered by the VF driver(iavf, in this case).

When we do a PCI probe of the network interfaces, we register an
interrupt handler, open a vfio-device fd using ioctl, and an eventfd in
dpdk. These interrupt sources are registered in a global linked list
that the eal-intr-thread keeps iterating over for handling the
interrupts. In our internal testing, we see eal-intr-thread crash in
these two ways:

Error adding fd 660 epoll_ctl, Operation not permitted

or

Error adding fd 660 epoll_ctl, Bad file descriptor

epoll_ctl() returns EPERM if the target fd does not support poll.
It returns EBADF when the epoll fd itself is closed or the target fd is
closed.

When the first type of crash happens, we see that the fd 660 is
anon_inode:[vfio-device] which does not support poll.

When the second type of crash happens, we could see from the fd map of
the crashing process that the fd 660 was already closed.

This means the said fd has been closed and in certain cases may have
been reassigned to a different device by the operating system but the
eal-intr-thread does not know about it.

We observed that these crashes were always accompanied by an error in
iavf_dev_init() after rte_intr_callback_register() and
iavf_enable_irq0() have already happened. In the error path, the
intr_handle_fd was being closed but the interrupt handler wasn't being
unregistered.

The fix is to unregister the interrupt handle in the
iavf_dev_init() error path.

Ensure proper cleanup if iavf_security_init() or
iavf_security_ctx_create() fail. Earlier, we were leaking memory by
simply returning from iavf_dev_init().

Signed-off-by: Saurabh Singhal 
---
 .mailmap   |  1 +
 drivers/net/iavf/iavf_ethdev.c | 22 --
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/.mailmap b/.mailmap
index 864d33ee46..4dac53011b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1227,6 +1227,7 @@ Satananda Burla 
 Satha Rao  
 Satheesh Paul 
 Sathesh Edara 
+Saurabh Singhal 
 Savinay Dharmappa 
 Scott Branden 
 Scott Daniels 
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f2fc5a5621..7d09246061 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -133,6 +133,8 @@ static int iavf_dev_rx_queue_intr_enable(struct rte_eth_dev 
*dev,
uint16_t queue_id);
 static int iavf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev,
 uint16_t queue_id);
+static void iavf_dev_interrupt_handler(void *param);
+static inline void iavf_disable_irq0(struct iavf_hw *hw);
 static int iavf_dev_flow_ops_get(struct rte_eth_dev *dev,
 const struct rte_flow_ops **ops);
 static int iavf_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -2709,13 +2711,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
ret = iavf_security_ctx_create(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to create ipsec crypto 
security instance");
-   return ret;
+   goto flow_init_err;
}
 
ret = iavf_security_init(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to initialized ipsec crypto 
resources");
-   return ret;
+   goto security_init_err;
}
}
 
@@ -2728,7 +2730,23 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 
return 0;
 
+security_init_err:
+   iavf_security_ctx_destroy(adapter);
+
 flow_init_err:
+   iavf_disable_irq0(hw);
+
+   if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+   /* disable uio intr before callback unregiser */
+   rte_intr_disable(pci_dev->intr_handle);
+
+   /* unregister callback func from eal lib */
+   rte_intr_callback_unregister(pci_dev->intr_handle,
+iavf_dev_interrupt_handler, 
eth_dev);
+   } else {
+   rte_eal_alarm_cancel(iavf_dev_alarm_handler, eth_dev);
+   }
+
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
 
-- 
2.41.0



[PATCH v4] net/iavf: unregister intr handler before FD close

2023-09-07 Thread Saurabh Singhal
Unregister VFIO interrupt handler before the interrupt fd gets closed in
case iavf_dev_init() returns an error.

dpdk creates a standalone thread named eal-intr-thread for processing
interrupts for the PCI devices. The interrupt handler callbacks are
registered by the VF driver(iavf, in this case).

When we do a PCI probe of the network interfaces, we register an
interrupt handler, open a vfio-device fd using ioctl, and an eventfd in
dpdk. These interrupt sources are registered in a global linked list
that the eal-intr-thread keeps iterating over for handling the
interrupts. In our internal testing, we see eal-intr-thread crash in
these two ways:

Error adding fd 660 epoll_ctl, Operation not permitted

or

Error adding fd 660 epoll_ctl, Bad file descriptor

epoll_ctl() returns EPERM if the target fd does not support poll.
It returns EBADF when the epoll fd itself is closed or the target fd is
closed.

When the first type of crash happens, we see that the fd 660 is
anon_inode:[vfio-device] which does not support poll.

When the second type of crash happens, we could see from the fd map of
the crashing process that the fd 660 was already closed.

This means the said fd has been closed and in certain cases may have
been reassigned to a different device by the operating system but the
eal-intr-thread does not know about it.

We observed that these crashes were always accompanied by an error in
iavf_dev_init() after rte_intr_callback_register() and
iavf_enable_irq0() have already happened. In the error path, the
intr_handle_fd was being closed but the interrupt handler wasn't being
unregistered.

The fix is to unregister the interrupt handle in the
iavf_dev_init() error path.

Ensure proper cleanup if iavf_security_init() or
iavf_security_ctx_create() fail. Earlier, we were leaking memory by
simply returning from iavf_dev_init().

Signed-off-by: Saurabh Singhal 
---
 .mailmap   |  1 +
 drivers/net/iavf/iavf_ethdev.c | 22 --
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/.mailmap b/.mailmap
index 864d33ee46..4dac53011b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1227,6 +1227,7 @@ Satananda Burla 
 Satha Rao  
 Satheesh Paul 
 Sathesh Edara 
+Saurabh Singhal 
 Savinay Dharmappa 
 Scott Branden 
 Scott Daniels 
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f2fc5a5621..47c1399a52 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -133,6 +133,8 @@ static int iavf_dev_rx_queue_intr_enable(struct rte_eth_dev 
*dev,
uint16_t queue_id);
 static int iavf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev,
 uint16_t queue_id);
+static void iavf_dev_interrupt_handler(void *param);
+static void iavf_disable_irq0(struct iavf_hw *hw);
 static int iavf_dev_flow_ops_get(struct rte_eth_dev *dev,
 const struct rte_flow_ops **ops);
 static int iavf_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -2709,13 +2711,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
ret = iavf_security_ctx_create(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to create ipsec crypto 
security instance");
-   return ret;
+   goto flow_init_err;
}
 
ret = iavf_security_init(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to initialized ipsec crypto 
resources");
-   return ret;
+   goto security_init_err;
}
}
 
@@ -2728,7 +2730,23 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 
return 0;
 
+security_init_err:
+   iavf_security_ctx_destroy(adapter);
+
 flow_init_err:
+   iavf_disable_irq0(hw);
+
+   if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+   /* disable uio intr before callback unregiser */
+   rte_intr_disable(pci_dev->intr_handle);
+
+   /* unregister callback func from eal lib */
+   rte_intr_callback_unregister(pci_dev->intr_handle,
+iavf_dev_interrupt_handler, 
eth_dev);
+   } else {
+   rte_eal_alarm_cancel(iavf_dev_alarm_handler, eth_dev);
+   }
+
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
 
-- 
2.41.0



[PATCH v4] net/iavf: unregister intr handler before FD close

2023-09-07 Thread Saurabh Singhal
Unregister VFIO interrupt handler before the interrupt fd gets closed in
case iavf_dev_init() returns an error.

dpdk creates a standalone thread named eal-intr-thread for processing
interrupts for the PCI devices. The interrupt handler callbacks are
registered by the VF driver(iavf, in this case).

When we do a PCI probe of the network interfaces, we register an
interrupt handler, open a vfio-device fd using ioctl, and an eventfd in
dpdk. These interrupt sources are registered in a global linked list
that the eal-intr-thread keeps iterating over for handling the
interrupts. In our internal testing, we see eal-intr-thread crash in
these two ways:

Error adding fd 660 epoll_ctl, Operation not permitted

or

Error adding fd 660 epoll_ctl, Bad file descriptor

epoll_ctl() returns EPERM if the target fd does not support poll.
It returns EBADF when the epoll fd itself is closed or the target fd is
closed.

When the first type of crash happens, we see that the fd 660 is
anon_inode:[vfio-device] which does not support poll.

When the second type of crash happens, we could see from the fd map of
the crashing process that the fd 660 was already closed.

This means the said fd has been closed and in certain cases may have
been reassigned to a different device by the operating system but the
eal-intr-thread does not know about it.

We observed that these crashes were always accompanied by an error in
iavf_dev_init() after rte_intr_callback_register() and
iavf_enable_irq0() have already happened. In the error path, the
intr_handle_fd was being closed but the interrupt handler wasn't being
unregistered.

The fix is to unregister the interrupt handle in the
iavf_dev_init() error path.

Ensure proper cleanup if iavf_security_init() or
iavf_security_ctx_create() fail. Earlier, we were leaking memory by
simply returning from iavf_dev_init().

Signed-off-by: Saurabh Singhal 
---
 .mailmap   |  1 +
 drivers/net/iavf/iavf_ethdev.c | 22 --
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/.mailmap b/.mailmap
index 864d33ee46..4dac53011b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1227,6 +1227,7 @@ Satananda Burla 
 Satha Rao  
 Satheesh Paul 
 Sathesh Edara 
+Saurabh Singhal 
 Savinay Dharmappa 
 Scott Branden 
 Scott Daniels 
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index f2fc5a5621..47c1399a52 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -133,6 +133,8 @@ static int iavf_dev_rx_queue_intr_enable(struct rte_eth_dev 
*dev,
uint16_t queue_id);
 static int iavf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev,
 uint16_t queue_id);
+static void iavf_dev_interrupt_handler(void *param);
+static void iavf_disable_irq0(struct iavf_hw *hw);
 static int iavf_dev_flow_ops_get(struct rte_eth_dev *dev,
 const struct rte_flow_ops **ops);
 static int iavf_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -2709,13 +2711,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
ret = iavf_security_ctx_create(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to create ipsec crypto 
security instance");
-   return ret;
+   goto flow_init_err;
}
 
ret = iavf_security_init(adapter);
if (ret) {
PMD_INIT_LOG(ERR, "failed to initialized ipsec crypto 
resources");
-   return ret;
+   goto security_init_err;
}
}
 
@@ -2728,7 +2730,23 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 
return 0;
 
+security_init_err:
+   iavf_security_ctx_destroy(adapter);
+
 flow_init_err:
+   iavf_disable_irq0(hw);
+
+   if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+   /* disable uio intr before callback unregiser */
+   rte_intr_disable(pci_dev->intr_handle);
+
+   /* unregister callback func from eal lib */
+   rte_intr_callback_unregister(pci_dev->intr_handle,
+iavf_dev_interrupt_handler, 
eth_dev);
+   } else {
+   rte_eal_alarm_cancel(iavf_dev_alarm_handler, eth_dev);
+   }
+
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
 
-- 
2.41.0



Re: [PATCH 05/11] eal: force prefix for internal threads

2023-09-07 Thread David Marchand
On Wed, Sep 6, 2023 at 6:23 PM Thomas Monjalon  wrote:
>
> In order to make sure all threads created in DPDK drivers and libraries
> have the same prefix in their name, some wrapper functions are added
> for internal use when creating a control thread or setting a thread name:
> - rte_thread_create_internal_control
> - rte_thread_set_prefixed_name
>
> The equivalent public functions are then forbidden for internal use:
> - rte_thread_create_control
> - rte_thread_set_name
>
> Note: the libraries and drivers conversion is done in next patches,
> while doing other thread-related changes.
>
> Signed-off-by: Thomas Monjalon 
> ---
>  devtools/checkpatches.sh   |  8 +
>  lib/eal/common/eal_common_thread.c | 30 
>  lib/eal/include/rte_thread.h   | 57 +-
>  lib/eal/version.map|  2 ++
>  4 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 131ffbcebe..18ad6fbb7f 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -159,6 +159,14 @@ check_forbidden_additions() { # 
> -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> "$1" || res=1
>
> +   # forbid non-internal thread in drivers and libs
> +   awk -v FOLDERS='lib drivers' \
> +   -v EXPRESSIONS="rte_thread_(set_name|create_control)\\\(" \
> +   -v RET_ON_FAIL=1 \
> +   -v MESSAGE='Prefer 
> rte_thread_(set_prefixed_name|create_internal_control) in lib & drivers' \
> +   -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> +   "$1" || res=1
> +
> # forbid inclusion of driver specific headers in apps and examples
> awk -v FOLDERS='app examples' \
> -v EXPRESSIONS='include.*_driver\\.h include.*_pmd\\.h' \
> diff --git a/lib/eal/common/eal_common_thread.c 
> b/lib/eal/common/eal_common_thread.c
> index 07ac721da1..31c37e3102 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -392,6 +392,36 @@ rte_thread_create_control(rte_thread_t *thread, const 
> char *name,
> return ret;
>  }
>
> +static void
> +add_internal_prefix(char *prefixed_name, const char *name, size_t size)
> +{
> +   const char *prefix = "dpdk-";

Can you add some BUILD_BUG_ON to check RTE_THREAD_INTERNAL_NAME_SIZE
is still relevant to the prefix?


> +   size_t prefixlen;
> +
> +   prefixlen = strlen(prefix);
> +   strlcpy(prefixed_name, prefix, size);
> +   strlcpy(prefixed_name + prefixlen, name, size - prefixlen);
> +}
> +
> +int
> +rte_thread_create_internal_control(rte_thread_t *id, const char *name,
> +   rte_thread_func func, void *arg)
> +{
> +   char prefixed_name[RTE_THREAD_NAME_SIZE];
> +
> +   add_internal_prefix(prefixed_name, name, sizeof(prefixed_name));
> +   return rte_thread_create_control(id, prefixed_name, func, arg);
> +}
> +
> +void
> +rte_thread_set_prefixed_name(rte_thread_t id, const char *name)
> +{
> +   char prefixed_name[RTE_THREAD_NAME_SIZE];
> +
> +   add_internal_prefix(prefixed_name, name, sizeof(prefixed_name));
> +   rte_thread_set_name(id, prefixed_name);
> +}
> +
>  int
>  rte_thread_register(void)
>  {
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index dd1f62523f..4b1135df4f 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -28,6 +28,9 @@ extern "C" {
>  /* Old definition, aliased for compatibility. */
>  #define RTE_MAX_THREAD_NAME_LEN RTE_THREAD_NAME_SIZE
>
> +/** Maximum internal thread name length (including '\0'). */
> +#define RTE_THREAD_INTERNAL_NAME_SIZE 11
> +
>  /**
>   * Thread id descriptor.
>   */
> @@ -112,7 +115,7 @@ int rte_thread_create(rte_thread_t *thread_id,
>   * @param thread_func
>   *   Function to be executed by the new thread.
>   * @param arg
> - *   Argument passed to start_routine.
> + *   Argument passed to thread_func.
>   * @return
>   *   On success, returns 0; on error, it returns a negative value
>   *   corresponding to the error number.
> @@ -121,6 +124,36 @@ int
>  rte_thread_create_control(rte_thread_t *thread, const char *name,
> rte_thread_func thread_func, void *arg);
>
> +/**
> + * Create an internal control thread.
> + *
> + * Creates a control thread with the given name prefixed with "dpdk-".

I don't like having a hardcoded comment here.
Plus try to grep dpdk- and you will see it is likely we will miss some
if we change the prefix.


> + * If setting the name of the thread fails, the error is ignored and logged.
> + *
> + * The affinity of the new thread is based on the CPU affinity retrieved
> + * at the time rte_eal_init() was called, the EAL threads are then excluded.
> + *
> + * @param id
> + *   Filled with the thread ID of the new created thread.
> + * @param name
> + *   

RE: [PATCH 00/11] rework thread management

2023-09-07 Thread Morten Brørup
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Wednesday, 6 September 2023 18.12
> 
> The main effect of this patch series is to
> remove calls to pthread functions except for pthread_cancel and locks.
> 
> The function rte_thread_create_control() does not take attributes anymore
> as it looks a useless complication of the API.

Note for other reviewers: The "args" parameter, passed to the thread function, 
is still there.

> Then the rte_thread API is made stable,
> so we can remove the old deprecated functions
> rte_thread_setname() and rte_ctrl_thread_create().
> 
> Some new internal functions are added in rte_thread to make sure
> all internal thread names are prefixed with "dpdk-".
> 
> Few other cleanups are done.
> 
> Future work about pthread portability are about:
>   - cancel
>   - mutex
> 
> 
> Thomas Monjalon (11):
>   devtools: warn when adding some pthread calls
>   eal: rename thread name length definition
>   eal: remove attributes from control thread creation
>   eal: promote thread API as stable
>   eal: force prefix for internal threads
>   lib: convert to internal control threads
>   drivers: convert to internal control threads
>   examples: convert to normal control threads
>   test: convert threads creation
>   eal: remove deprecated thread functions
>   lib: remove pthread.h from includes

Thank you for cleaning all this up, Thomas.

Series-acked-by: Morten Brørup 



RE: [PATCH 05/11] eal: force prefix for internal threads

2023-09-07 Thread Morten Brørup
> From: David Marchand [mailto:david.march...@redhat.com]
> Sent: Thursday, 7 September 2023 10.28
> 
> On Wed, Sep 6, 2023 at 6:23 PM Thomas Monjalon  wrote:
> >
> > In order to make sure all threads created in DPDK drivers and libraries
> > have the same prefix in their name, some wrapper functions are added
> > for internal use when creating a control thread or setting a thread name:
> > - rte_thread_create_internal_control
> > - rte_thread_set_prefixed_name
> >
> > The equivalent public functions are then forbidden for internal use:
> > - rte_thread_create_control
> > - rte_thread_set_name
> >
> > Note: the libraries and drivers conversion is done in next patches,
> > while doing other thread-related changes.
> >
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  devtools/checkpatches.sh   |  8 +
> >  lib/eal/common/eal_common_thread.c | 30 
> >  lib/eal/include/rte_thread.h   | 57 +-
> >  lib/eal/version.map|  2 ++
> >  4 files changed, 96 insertions(+), 1 deletion(-)
> >
> > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> > index 131ffbcebe..18ad6fbb7f 100755
> > --- a/devtools/checkpatches.sh
> > +++ b/devtools/checkpatches.sh
> > @@ -159,6 +159,14 @@ check_forbidden_additions() { # 
> > -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> > "$1" || res=1
> >
> > +   # forbid non-internal thread in drivers and libs
> > +   awk -v FOLDERS='lib drivers' \
> > +   -v EXPRESSIONS="rte_thread_(set_name|create_control)\\\(" \
> > +   -v RET_ON_FAIL=1 \
> > +   -v MESSAGE='Prefer
> rte_thread_(set_prefixed_name|create_internal_control) in lib & drivers' \
> > +   -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> > +   "$1" || res=1
> > +
> > # forbid inclusion of driver specific headers in apps and examples
> > awk -v FOLDERS='app examples' \
> > -v EXPRESSIONS='include.*_driver\\.h include.*_pmd\\.h' \
> > diff --git a/lib/eal/common/eal_common_thread.c
> b/lib/eal/common/eal_common_thread.c
> > index 07ac721da1..31c37e3102 100644
> > --- a/lib/eal/common/eal_common_thread.c
> > +++ b/lib/eal/common/eal_common_thread.c
> > @@ -392,6 +392,36 @@ rte_thread_create_control(rte_thread_t *thread, const
> char *name,
> > return ret;
> >  }
> >
> > +static void
> > +add_internal_prefix(char *prefixed_name, const char *name, size_t size)
> > +{
> > +   const char *prefix = "dpdk-";
> 
> Can you add some BUILD_BUG_ON to check RTE_THREAD_INTERNAL_NAME_SIZE
> is still relevant to the prefix?
> 
> 
> > +   size_t prefixlen;
> > +
> > +   prefixlen = strlen(prefix);
> > +   strlcpy(prefixed_name, prefix, size);
> > +   strlcpy(prefixed_name + prefixlen, name, size - prefixlen);
> > +}
> > +
> > +int
> > +rte_thread_create_internal_control(rte_thread_t *id, const char *name,
> > +   rte_thread_func func, void *arg)
> > +{
> > +   char prefixed_name[RTE_THREAD_NAME_SIZE];
> > +
> > +   add_internal_prefix(prefixed_name, name, sizeof(prefixed_name));
> > +   return rte_thread_create_control(id, prefixed_name, func, arg);
> > +}
> > +
> > +void
> > +rte_thread_set_prefixed_name(rte_thread_t id, const char *name)
> > +{
> > +   char prefixed_name[RTE_THREAD_NAME_SIZE];
> > +
> > +   add_internal_prefix(prefixed_name, name, sizeof(prefixed_name));
> > +   rte_thread_set_name(id, prefixed_name);
> > +}
> > +
> >  int
> >  rte_thread_register(void)
> >  {
> > diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> > index dd1f62523f..4b1135df4f 100644
> > --- a/lib/eal/include/rte_thread.h
> > +++ b/lib/eal/include/rte_thread.h
> > @@ -28,6 +28,9 @@ extern "C" {
> >  /* Old definition, aliased for compatibility. */
> >  #define RTE_MAX_THREAD_NAME_LEN RTE_THREAD_NAME_SIZE
> >
> > +/** Maximum internal thread name length (including '\0'). */
> > +#define RTE_THREAD_INTERNAL_NAME_SIZE 11
> > +
> >  /**
> >   * Thread id descriptor.
> >   */
> > @@ -112,7 +115,7 @@ int rte_thread_create(rte_thread_t *thread_id,
> >   * @param thread_func
> >   *   Function to be executed by the new thread.
> >   * @param arg
> > - *   Argument passed to start_routine.
> > + *   Argument passed to thread_func.
> >   * @return
> >   *   On success, returns 0; on error, it returns a negative value
> >   *   corresponding to the error number.
> > @@ -121,6 +124,36 @@ int
> >  rte_thread_create_control(rte_thread_t *thread, const char *name,
> > rte_thread_func thread_func, void *arg);
> >
> > +/**
> > + * Create an internal control thread.
> > + *
> > + * Creates a control thread with the given name prefixed with "dpdk-".
> 
> I don't like having a hardcoded comment here.
> Plus try to grep dpdk- and you will see it is likely we will miss some
> if we change the prefix.

You cou

Re: [PATCH 05/11] eal: force prefix for internal threads

2023-09-07 Thread David Marchand
On Thu, Sep 7, 2023 at 10:50 AM Morten Brørup  
wrote:
> > This 10 value in the comment is easy to miss if some change with the
> > prefix is done.
> > Mentionning RTE_THREAD_INTERNAL_NAME_SIZE is enough.
>
> I disagree with David's comment to this.
>
> The function documentation is easier to read if the actual number is also 
> mentioned.
>
> For the best of both worlds, you can add something like this nearby:
>
> _Static_assert(sizeof(RTE_THREAD_NAME_PREFIX) == sizeof("dpdk-"),
> "Length of RTE_THREAD_NAME_PREFIX has changed; "
> "the documentation needs updating.");

And how will it catch the comment about 10 characters ?


-- 
David Marchand



Re: [PATCH 05/11] eal: force prefix for internal threads

2023-09-07 Thread David Marchand
On Thu, Sep 7, 2023 at 10:53 AM David Marchand
 wrote:
>
> On Thu, Sep 7, 2023 at 10:50 AM Morten Brørup  
> wrote:
> > > This 10 value in the comment is easy to miss if some change with the
> > > prefix is done.
> > > Mentionning RTE_THREAD_INTERNAL_NAME_SIZE is enough.
> >
> > I disagree with David's comment to this.
> >
> > The function documentation is easier to read if the actual number is also 
> > mentioned.
> >
> > For the best of both worlds, you can add something like this nearby:
> >
> > _Static_assert(sizeof(RTE_THREAD_NAME_PREFIX) == sizeof("dpdk-"),
> > "Length of RTE_THREAD_NAME_PREFIX has changed; "
> > "the documentation needs updating.");
>
> And how will it catch the comment about 10 characters ?

I mean you still have to re-read the whole documentation and look for
some reference somewhere about 10 characters.


-- 
David Marchand



RE: [PATCH v1 1/2] dmadev: offload to free source buffer

2023-09-07 Thread Amit Prakash Shukla
Driver implementation of the spec: 
http://patches.dpdk.org/project/dpdk/patch/20230907082443.1002665-1-amitpraka...@marvell.com/


> -Original Message-
> From: Amit Prakash Shukla 
> Sent: Thursday, September 7, 2023 1:41 PM
> To: Chengwen Feng ; Kevin Laatz
> ; Bruce Richardson 
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran ;
> conor.wa...@intel.com; Vamsi Krishna Attunuru ;
> g.si...@nxp.com; sachin.sax...@oss.nxp.com; hemant.agra...@nxp.com;
> cheng1.ji...@intel.com; Nithin Kumar Dabilpuram
> ; Anoob Joseph ; Amit
> Prakash Shukla ; Morten Brørup
> 
> Subject: [PATCH v1 1/2] dmadev: offload to free source buffer
> 
> This changeset adds support in DMA library to free source DMA buffer by
> hardware. On a supported hardware, application can pass on the mempool
> information as part of vchan config when the DMA transfer direction is
> configured as RTE_DMA_DIR_MEM_TO_DEV.
> 
> Signed-off-by: Amit Prakash Shukla 
> Acked-by: Morten Brørup 
> ---
>  lib/dmadev/rte_dmadev.h | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index
> b157ab7600..d6a685907f 100644
> --- a/lib/dmadev/rte_dmadev.h
> +++ b/lib/dmadev/rte_dmadev.h
> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id);
>  #define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33)
>  /** Support fill operation. */
>  #define RTE_DMA_CAPA_OPS_FILLRTE_BIT64(34)
> +/** Support for source buffer free for mem to dev transfer.
> + *
> + * @note Even though the DMA driver has this capability, it may not
> +support all
> + * mempool drivers. If the mempool is not supported by the DMA driver,
> + * rte_dma_vchan_setup() will fail.
> + **/
> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE
>   RTE_BIT64(35)
>  /**@}*/
> 
>  /**
> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf {
>* @see struct rte_dma_port_param
>*/
>   struct rte_dma_port_param dst_port;
> + /** mempool from which source buffer is allocated. mempool info is
> used
> +  * for freeing source buffer by hardware when configured direction is
> +  * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by
> hardware,
> +  * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling
> rte_dma_copy and
> +  * rte_dma_copy_sg().
> +  *
> +  * @note If the mempool is not supported by the DMA driver,
> +  * rte_dma_vchan_setup() will fail.
> +  *
> +  * @see RTE_DMA_OP_FLAG_FREE_SBUF
> +  */
> + struct rte_mempool *mem_to_dev_src_buf_pool;
> +
>  };
> 
>  /**
> @@ -818,6 +838,13 @@ struct rte_dma_sge {
>   * capability bit for this, driver should not return error if this flag was 
> set.
>   */
>  #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2)
> +/** Mem to dev source buffer free flag.
> + * Used for freeing source DMA buffer by hardware when the transfer
> +direction is
> + * configured as RTE_DMA_DIR_MEM_TO_DEV.
> + *
> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool
> + */
> +#define RTE_DMA_OP_FLAG_FREE_SBUFRTE_BIT64(3)
>  /**@}*/
> 
>  /**
> --
> 2.25.1



[PATCH v5 0/2] Add Digest Encrypted to aesni_mb PMD

2023-09-07 Thread Brian Dooley
This series adds the Digest Encrypted feature to the AESNI_MB PMD.
It also fixes an issue where IV data in SNOW3G and ZUC algorithms
were incorrect and are required to be non-zero length.

v2:
Fixed CHECKPATCH warning
v3:
Add Digest encrypted support to docs
v4:
add comments and refactor
v5:
Fix checkpatch warnings
v6:
Add skipping tests for synchronous crypto

Brian Dooley (2):
  crypto/ipsec_mb: add digest encrypted feature
  test/crypto: fix IV in some vectors

 app/test/test_cryptodev_mixed_test_vectors.h |   8 +-
 doc/guides/cryptodevs/features/aesni_mb.ini  |   1 +
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c   | 107 ++-
 3 files changed, 109 insertions(+), 7 deletions(-)

-- 
2.25.1



[PATCH v6 1/2] crypto/ipsec_mb: add digest encrypted feature

2023-09-07 Thread Brian Dooley
AESNI_MB PMD does not support Digest Encrypted. This patch adds a check and
support for this feature.

Signed-off-by: Brian Dooley 
---
v2:
Fixed CHECKPATCH warning
v3:
Add Digest encrypted support to docs
v4:
Add comments and small refactor
v5:
Fix checkpatch warnings
v6:
Add skipping tests for synchronous crypto
---
 app/test/test_cryptodev.c   |   6 ++
 doc/guides/cryptodevs/features/aesni_mb.ini |   1 +
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c  | 109 +++-
 3 files changed, 111 insertions(+), 5 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 956268bfcd..70f6b7ece1 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -6394,6 +6394,9 @@ test_zuc_auth_cipher(const struct wireless_test_data 
*tdata,
tdata->digest.len) < 0)
return TEST_SKIPPED;
 
+   if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
+   return TEST_SKIPPED;
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
 
uint64_t feat_flags = dev_info.feature_flags;
@@ -7829,6 +7832,9 @@ test_mixed_auth_cipher(const struct 
mixed_cipher_auth_test_data *tdata,
if (global_api_test_type == CRYPTODEV_RAW_API_TEST)
return TEST_SKIPPED;
 
+   if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
+   return TEST_SKIPPED;
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
 
uint64_t feat_flags = dev_info.feature_flags;
diff --git a/doc/guides/cryptodevs/features/aesni_mb.ini 
b/doc/guides/cryptodevs/features/aesni_mb.ini
index e4e965c35a..8df5fa2c85 100644
--- a/doc/guides/cryptodevs/features/aesni_mb.ini
+++ b/doc/guides/cryptodevs/features/aesni_mb.ini
@@ -20,6 +20,7 @@ OOP LB  In LB  Out = Y
 CPU crypto = Y
 Symmetric sessionless  = Y
 Non-Byte aligned data  = Y
+Digest encrypted   = Y
 
 ;
 ; Supported crypto algorithms of the 'aesni_mb' crypto driver.
diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c 
b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
index 9e298023d7..7f61065939 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
@@ -1438,6 +1438,54 @@ set_gcm_job(IMB_MGR *mb_mgr, IMB_JOB *job, const uint8_t 
sgl,
return 0;
 }
 
+/** Check if conditions are met for digest-appended operations */
+static uint8_t *
+aesni_mb_digest_appended_in_src(struct rte_crypto_op *op, IMB_JOB *job,
+   uint32_t oop)
+{
+   unsigned int auth_size, cipher_size;
+   uint8_t *end_cipher;
+   uint8_t *start_cipher;
+
+   if (job->cipher_mode == IMB_CIPHER_NULL)
+   return NULL;
+
+   if (job->cipher_mode == IMB_CIPHER_ZUC_EEA3 ||
+   job->cipher_mode == IMB_CIPHER_SNOW3G_UEA2_BITLEN ||
+   job->cipher_mode == IMB_CIPHER_KASUMI_UEA1_BITLEN) {
+   cipher_size = (op->sym->cipher.data.offset >> 3) +
+   (op->sym->cipher.data.length >> 3);
+   } else {
+   cipher_size = (op->sym->cipher.data.offset) +
+   (op->sym->cipher.data.length);
+   }
+   if (job->hash_alg == IMB_AUTH_ZUC_EIA3_BITLEN ||
+   job->hash_alg == IMB_AUTH_SNOW3G_UIA2_BITLEN ||
+   job->hash_alg == IMB_AUTH_KASUMI_UIA1 ||
+   job->hash_alg == IMB_AUTH_ZUC256_EIA3_BITLEN) {
+   auth_size = (op->sym->auth.data.offset >> 3) +
+   (op->sym->auth.data.length >> 3);
+   } else {
+   auth_size = (op->sym->auth.data.offset) +
+   (op->sym->auth.data.length);
+   }
+
+   if (!oop) {
+   end_cipher = rte_pktmbuf_mtod_offset(op->sym->m_src, uint8_t *, 
cipher_size);
+   start_cipher = rte_pktmbuf_mtod(op->sym->m_src, uint8_t *);
+   } else {
+   end_cipher = rte_pktmbuf_mtod_offset(op->sym->m_dst, uint8_t *, 
cipher_size);
+   start_cipher = rte_pktmbuf_mtod(op->sym->m_dst, uint8_t *);
+   }
+
+   if (start_cipher < op->sym->auth.digest.data &&
+   op->sym->auth.digest.data < end_cipher) {
+   return rte_pktmbuf_mtod_offset(op->sym->m_src, uint8_t *, 
auth_size);
+   } else {
+   return NULL;
+   }
+}
+
 /**
  * Process a crypto operation and complete a IMB_JOB job structure for
  * submission to the multi buffer library for processing.
@@ -1580,9 +1628,12 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
} else {
if (aead)
job->auth_tag_output = op->sym->aead.digest.data;
-   else
-   job->auth_tag_output = op->sym->auth.digest.data;
-
+   else {
+   job->auth_tag_output = 
aesni_mb_digest_appended_in_src(op, job, oop);
+   if (job->auth_tag_output == NULL) {
+   job->auth_tag_outpu

[PATCH v6 2/2] test/crypto: fix IV in some vectors

2023-09-07 Thread Brian Dooley
SNOW3G and ZUC algorithms require non-zero length IVs.

Fixes: c6c267a00a92 ("test/crypto: add mixed encypted-digest")
Cc: adamx.dybkow...@intel.com

Signed-off-by: Brian Dooley 
---
 app/test/test_cryptodev_mixed_test_vectors.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/test/test_cryptodev_mixed_test_vectors.h 
b/app/test/test_cryptodev_mixed_test_vectors.h
index 161e2d905f..9c4313185e 100644
--- a/app/test/test_cryptodev_mixed_test_vectors.h
+++ b/app/test/test_cryptodev_mixed_test_vectors.h
@@ -478,8 +478,10 @@ struct mixed_cipher_auth_test_data 
auth_aes_cmac_cipher_snow_test_case_1 = {
},
.cipher_iv = {
.data = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
},
-   .len = 0,
+   .len = 16,
},
.cipher = {
.len_bits = 516 << 3,
@@ -917,8 +919,10 @@ struct mixed_cipher_auth_test_data 
auth_aes_cmac_cipher_zuc_test_case_1 = {
},
.cipher_iv = {
.data = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
},
-   .len = 0,
+   .len = 16,
},
.cipher = {
.len_bits = 516 << 3,
-- 
2.25.1



DPDK Release Status Meeting 2023-09-07

2023-09-07 Thread Mcnamara, John
Release status meeting minutes 2023-09-07
=

Agenda:
* Release Dates
* Subtrees
* Roadmaps
* LTS
* Defects
* Opens

Participants:
* AMD
* ARM
* Intel
* Marvell
* Nvidia
* Red Hat

Release Dates
-

The following are the proposed working dates for 23.11:

* V1:  12 August 2023
* RC1: 29 September 2023
* RC2: 20 October 2023
* RC3: 27 October 2023
* Release: 15 November 2023


Subtrees


* next-net
  * New driver from Napatech
  * New driver RNP: https://patchwork.dpdk.org/project/dpdk/list/?series=29118
  * 1 other new driver
  * More reviews are required.

* next-net-intel
  * 12 patches awaiting merge.

* next-net-mlx
  * Some patches coming next week.

* next-net-mvl
  * All patches merged.

* next-eventdev
  * Intel patches merged
  * 12 patches pending.

* next-baseband
  * 2 series under review.

* next-virtio
  * Working on improvements for VDUSE.
  * Other patches under review.
  * No big features for now.

* next-crypto
  * Started merged patches and giving comments.
  * ~55 patches.
  * 2-3 new features from Marvell
  * RX inject patchset for review.
  * SSL/TLS patches for review.
  * SM2 patches.

* main
  * Updates to the MAINTAINERs file to help with automated delegation.
  * Some updates to bit count helpers for Windows but affects other
trees as well.
  * Patch for Atomics
* This needs review since it is an important API that will touch a lot of 
components later.
* https://patchwork.dpdk.org/project/dpdk/list/?series=29318
  * Preparing for LTS release.
  * Removing deprecated libraries.
  * Meeting cancelled next week due to DPDK Summit.


Proposed Schedule for 2023
--

See also http://core.dpdk.org/roadmap/#dates

23.11
  * Proposal deadline (RFC/v1 patches): 12 August 2023
  * API freeze (-rc1): 29 September 2023
  * PMD features freeze (-rc2): 20 October 2023
  * Builtin applications features freeze (-rc3): 27 October 2023
  * Release: 15 November 2023


LTS
---

Backports ongoing. Awaiting test results.

Next LTS releases:

* 22.11.2 - Aug 31 set as release date. In validation.
* 21.11.5 - Released
* 20.11.9 - Released
* 19.11.15
  * Will only be updated with CVE and critical fixes.


* Distros
  * v22.11 in Debian 12
  * Ubuntu 22.04-LTS contains 21.11
  * Ubuntu 23.04 contains 22.11

Defects
---

* Bugzilla links, 'Bugs',  added for hosted projects
  * https://www.dpdk.org/hosted-projects/



DPDK Release Status Meetings


The DPDK Release Status Meeting is intended for DPDK Committers to discuss the
status of the master tree and sub-trees, and for project managers to track
progress or milestone dates.

The meeting occurs on every Thursday at 9:30 UTC over Jitsi on 
https://meet.jit.si/DPDK

You don't need an invite to join the meeting but if you want a calendar 
reminder just
send an email to "John McNamara john.mcnam...@intel.com" for the invite.


RE: [PATCH 05/11] eal: force prefix for internal threads

2023-09-07 Thread Morten Brørup
> From: David Marchand [mailto:david.march...@redhat.com]
> Sent: Thursday, 7 September 2023 10.55
> 
> On Thu, Sep 7, 2023 at 10:53 AM David Marchand
>  wrote:
> >
> > On Thu, Sep 7, 2023 at 10:50 AM Morten Brørup 
> wrote:
> > > > This 10 value in the comment is easy to miss if some change with the
> > > > prefix is done.
> > > > Mentionning RTE_THREAD_INTERNAL_NAME_SIZE is enough.
> > >
> > > I disagree with David's comment to this.
> > >
> > > The function documentation is easier to read if the actual number is also
> mentioned.
> > >
> > > For the best of both worlds, you can add something like this nearby:
> > >
> > > _Static_assert(sizeof(RTE_THREAD_NAME_PREFIX) == sizeof("dpdk-"),
> > > "Length of RTE_THREAD_NAME_PREFIX has changed; "
> > > "the documentation needs updating.");
> >
> > And how will it catch the comment about 10 characters ?
> 
> I mean you still have to re-read the whole documentation and look for
> some reference somewhere about 10 characters.

The trick is to put the _Static_assert close to where the expectation occurs. 
That makes it easier to find where changes are necessary.

And the _Static_assert can be added at all the locations where changes would be 
necessary. (Generally, we should add a lot more _Static_assert to the code 
where it makes assumptions about e.g. the ordering of fields in a struct, such 
as the vector optimized code.)

Also, the failure message could be improved to include help about what to look 
for.

PS: The reference to RTE_THREAD_INTERNAL_NAME_SIZE should remain in the 
documentation, so perhaps look for "RTE_THREAD_INTERNAL_NAME_SIZE".



RE: [PATCH v2 04/15] bus/pci: find PCI capability

2023-09-07 Thread Xia, Chenbo
Hi David,

> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Burakov, Anatoly ;
> Jay Zhou ; McDaniel, Timothy
> ; Julien Aube ; Rahul
> Lakkireddy ; Guo, Junfeng
> ; Jeroen de Borst ; Rushil
> Gupta ; Joshua Washington ;
> Dongdong Liu ; Yisen Zhuang
> ; Maxime Coquelin ;
> Gaetan Rivet 
> Subject: [PATCH v2 04/15] bus/pci: find PCI capability
> 
> Introduce two helpers so that drivers stop reinventing the wheel when it
> comes to finding capabilities in a device PCI configuration space.
> Use it in existing drivers.
> 
> Note:
> - base/ drivers code is left untouched, only some wrappers in cxgbe
>   are touched,
> - bnx2x maintained a per device cache of capabilities, this code has been
>   reworked to only cache the capabilities used in this driver,
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
> Changes since v1:
> - updated commitlog,
> - separated VFIO changes for using standard PCI helper in a separate
>   patch,
> - marked new experimental symbols with current version,
> - reordered defines in rte_pci.h,
> 
> ---
>  drivers/bus/pci/linux/pci_vfio.c   |  74 --
>  drivers/bus/pci/pci_common.c   |  45 +++
>  drivers/bus/pci/rte_bus_pci.h  |  31 
>  drivers/bus/pci/version.map|   4 +
>  drivers/crypto/virtio/virtio_pci.c |  57 +-
>  drivers/event/dlb2/pf/dlb2_main.c  |  42 +-
>  drivers/net/bnx2x/bnx2x.c  |  41 +-
>  drivers/net/cxgbe/base/adapter.h   |  28 +--
>  drivers/net/gve/gve_ethdev.c   |  46 ++-
>  drivers/net/gve/gve_ethdev.h   |   4 -
>  drivers/net/hns3/hns3_ethdev_vf.c  |  79 +++
>  drivers/net/virtio/virtio_pci.c| 121 +
>  lib/pci/rte_pci.h  |  11 +++
>  13 files changed, 186 insertions(+), 397 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 958f8b3b52..614ed5d696 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -110,74 +110,34 @@ static int
>  pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
>   struct pci_msix_table *msix_table)
>  {
> - int ret;
> - uint32_t reg;
> - uint16_t flags;
> - uint8_t cap_id, cap_offset;
> + off_t cap_offset;
> 
> - /* read PCI capability pointer from config space */
> - ret = rte_pci_read_config(dev, ®, sizeof(reg),
> PCI_CAPABILITY_LIST);
> - if (ret != sizeof(reg)) {
> - RTE_LOG(ERR, EAL,
> - "Cannot read capability pointer from PCI config
> space!\n");
> + cap_offset = rte_pci_find_capability(dev, PCI_CAP_ID_MSIX);

I notice in some cases we use rte_pci_has_capability_list() to check first,
then looking for specific cap, in other cases we don't use
rte_pci_has_capability_list(). Since we define this API, should we always do
the check?


> + if (cap_offset < 0)
>   return -1;
> - }
> 
> - /* we need first byte */
> - cap_offset = reg & 0xFF;
> + if (cap_offset != 0) {
> + uint16_t flags;
> + uint32_t reg;
> 
> - while (cap_offset) {
> -
> - /* read PCI capability ID */
> - ret = rte_pci_read_config(dev, ®, sizeof(reg), cap_offset);
> - if (ret != sizeof(reg)) {
> + /* table offset resides in the next 4 bytes */
> + if (rte_pci_read_config(dev, ®, sizeof(reg), cap_offset + 4)
> < 0) {
>   RTE_LOG(ERR, EAL,
> - "Cannot read capability ID from PCI config
> space!\n");
> + "Cannot read MSIX table from PCI config 
> space!\n");
>   return -1;
>   }
> 
> - /* we need first byte */
> - cap_id = reg & 0xFF;
> -
> - /* if we haven't reached MSI-X, check next capability */
> - if (cap_id != PCI_CAP_ID_MSIX) {
> - ret = rte_pci_read_config(dev, ®, sizeof(reg),
> cap_offset);
> - if (ret != sizeof(reg)) {
> - RTE_LOG(ERR, EAL,
> - "Cannot read capability pointer from PCI
> config space!\n");
> - return -1;
> - }
> -
> - /* we need second byte */
> - cap_offset = (reg & 0xFF00) >> 8;
> -
> - continue;
> + if (rte_pci_read_config(dev, &flags, sizeof(flags), cap_offset
> + 2) < 0) {
> + RTE_LOG(ERR, EAL,
> + "Cannot read MSIX flags from PCI config 
> space!\n");
> + return -1;
>   }
> - /* else, read table offset */
> - else {
> -  

[PATCH] cryptodev: add missing doc

2023-09-07 Thread Anoob Joseph
Description for rte_cryptodev_get_sec_ctx is missing. Add the same.

Signed-off-by: Anoob Joseph 
---
 lib/cryptodev/rte_cryptodev.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/lib/cryptodev/rte_cryptodev.h b/lib/cryptodev/rte_cryptodev.h
index ba730373fb..8876e0427f 100644
--- a/lib/cryptodev/rte_cryptodev.h
+++ b/lib/cryptodev/rte_cryptodev.h
@@ -973,6 +973,15 @@ struct rte_cryptodev_cb_rcu {
/**< RCU QSBR variable per queue pair */
 };
 
+/**
+ * Get the security context for the cryptodev.
+ *
+ * @param dev_id
+ *   The device identifier.
+ * @return
+ *   - NULL on error.
+ *   - Pointer to security context on success.
+ */
 void *
 rte_cryptodev_get_sec_ctx(uint8_t dev_id);
 
-- 
2.25.1



RE: [PATCH v2 05/15] pci: define some capability constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Burakov, Anatoly ;
> Jay Zhou ; McDaniel, Timothy
> ; Julien Aube ; Rahul
> Lakkireddy ; Guo, Junfeng
> ; Jeroen de Borst ; Rushil
> Gupta ; Joshua Washington ;
> Dongdong Liu ; Yisen Zhuang
> ; Maxime Coquelin ;
> Wang, Xiao W ; Gaetan Rivet 
> Subject: [PATCH v2 05/15] pci: define some capability constants
> 
> Define some PCI capability constants and use them in existing drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/bus/pci/linux/pci_vfio.c|  2 +-
>  drivers/crypto/virtio/virtio_pci.c  | 12 ++--
>  drivers/event/dlb2/pf/dlb2_main.c   |  6 ++
>  drivers/net/bnx2x/bnx2x.c   | 16 
>  drivers/net/bnx2x/bnx2x.h   |  4 
>  drivers/net/cxgbe/base/adapter.h|  3 +--
>  drivers/net/gve/gve_ethdev.c|  2 +-
>  drivers/net/gve/gve_ethdev.h|  2 +-
>  drivers/net/hns3/hns3_ethdev_vf.c   |  2 +-
>  drivers/net/virtio/virtio_pci.c | 12 ++--
>  drivers/vdpa/ifc/base/ifcvf_osdep.h |  4 +++-
>  lib/pci/rte_pci.h   |  5 +
>  12 files changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 614ed5d696..bfedbc1bed 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -112,7 +112,7 @@ pci_vfio_get_msix_bar(const struct rte_pci_device *dev,
>  {
>   off_t cap_offset;
> 
> - cap_offset = rte_pci_find_capability(dev, PCI_CAP_ID_MSIX);
> + cap_offset = rte_pci_find_capability(dev, RTE_PCI_CAP_ID_MSIX);
>   if (cap_offset < 0)
>   return -1;
> 
> diff --git a/drivers/crypto/virtio/virtio_pci.c
> b/drivers/crypto/virtio/virtio_pci.c
> index abc52b4701..9e340f2b0d 100644
> --- a/drivers/crypto/virtio/virtio_pci.c
> +++ b/drivers/crypto/virtio/virtio_pci.c
> @@ -14,14 +14,6 @@
>  #include "virtio_pci.h"
>  #include "virtqueue.h"
> 
> -/*
> - * Following macros are derived from linux/pci_regs.h, however,
> - * we can't simply include that header here, as there is no such
> - * file for non-Linux platform.
> - */
> -#define PCI_CAP_ID_VNDR  0x09
> -#define PCI_CAP_ID_MSIX  0x11
> -
>  /*
>   * The remaining space is defined by each driver as the per-driver
>   * configuration space.
> @@ -356,7 +348,7 @@ virtio_read_caps(struct rte_pci_device *dev, struct
> virtio_crypto_hw *hw)
>* Transitional devices would also have this capability,
>* that's why we also check if msix is enabled.
>*/
> - pos = rte_pci_find_capability(dev, PCI_CAP_ID_MSIX);
> + pos = rte_pci_find_capability(dev, RTE_PCI_CAP_ID_MSIX);
>   if (pos > 0 && rte_pci_read_config(dev, &flags, sizeof(flags),
>   pos + 2) == sizeof(flags)) {
>   if (flags & PCI_MSIX_ENABLE)
> @@ -367,7 +359,7 @@ virtio_read_caps(struct rte_pci_device *dev, struct
> virtio_crypto_hw *hw)
>   hw->use_msix = VIRTIO_MSIX_NONE;
>   }
> 
> - pos = rte_pci_find_capability(dev, PCI_CAP_ID_VNDR);
> + pos = rte_pci_find_capability(dev, RTE_PCI_CAP_ID_VNDR);
>   if (pos > 0 && rte_pci_read_config(dev, &cap, sizeof(cap), pos) ==
> sizeof(cap)) {
>   VIRTIO_CRYPTO_INIT_LOG_DBG(
>   "[%2x] cfg type: %u, bar: %u, offset: %04x, len: %u",
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c
> b/drivers/event/dlb2/pf/dlb2_main.c
> index 40e5cb594f..1a229baee0 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -38,8 +38,6 @@
>  #define DLB2_PCI_EXP_DEVSTA_TRPND 0x20
>  #define DLB2_PCI_EXP_DEVCTL_BCR_FLR 0x8000
> 
> -#define DLB2_PCI_CAP_ID_EXP   0x10
> -#define DLB2_PCI_CAP_ID_MSIX  0x11
>  #define DLB2_PCI_EXT_CAP_ID_PRI   0x13
>  #define DLB2_PCI_EXT_CAP_ID_ACS   0xD
> 
> @@ -244,7 +242,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
>   }
> 
> - pcie_cap_offset = rte_pci_find_capability(pdev, DLB2_PCI_CAP_ID_EXP);
> + pcie_cap_offset = rte_pci_find_capability(pdev, RTE_PCI_CAP_ID_EXP);
> 
>   if (pcie_cap_offset < 0) {
>   DLB2_LOG_ERR("[%s()] failed to find the pcie capability\n",
> @@ -483,7 +481,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   }
>   }
> 
> - msix_cap_offset = rte_pci_find_capability(pdev,
> DLB2_PCI_CAP_ID_MSIX);
> + msix_cap_offset = rte_pci_find_capability(pdev, RTE_PCI_CAP_ID_MSIX);
>   if (msix_cap_offset >= 0) {
>   off = msix_cap_offset + DLB2_PCI_MSIX_FLAGS;
>   if (rte_pci_read_config(pdev, &cmd, 2, off) == 2) {
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 06f2949885..8a97de8806 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/driv

RE: [PATCH v2 06/15] pci: define some MSIX constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Burakov, Anatoly ;
> Jay Zhou ; McDaniel, Timothy
> ; Julien Aube ; Guo,
> Junfeng ; Jeroen de Borst ;
> Rushil Gupta ; Joshua Washington ;
> Dongdong Liu ; Yisen Zhuang
> ; Maxime Coquelin ;
> Gaetan Rivet 
> Subject: [PATCH v2 06/15] pci: define some MSIX constants
> 
> Define some PCI MSIX constants and use them in existing drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/bus/pci/linux/pci_init.h   | 18 --
>  drivers/bus/pci/linux/pci_vfio.c   |  7 ---
>  drivers/crypto/virtio/virtio_pci.c |  6 ++
>  drivers/event/dlb2/pf/dlb2_main.c  | 13 +
>  drivers/net/bnx2x/bnx2x.c  |  4 ++--
>  drivers/net/bnx2x/bnx2x.h  |  2 --
>  drivers/net/gve/gve_ethdev.c   |  4 ++--
>  drivers/net/gve/gve_ethdev.h   |  8 
>  drivers/net/hns3/hns3_ethdev_vf.c  |  9 -
>  drivers/net/virtio/virtio_pci.c|  6 ++
>  lib/pci/rte_pci.h  | 10 ++
>  11 files changed, 31 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_init.h
> b/drivers/bus/pci/linux/pci_init.h
> index d842809ccd..a4d37c0d0a 100644
> --- a/drivers/bus/pci/linux/pci_init.h
> +++ b/drivers/bus/pci/linux/pci_init.h
> @@ -52,24 +52,6 @@ int pci_uio_ioport_unmap(struct rte_pci_ioport *p);
> 
>  #ifdef VFIO_PRESENT
> 
> -#ifdef PCI_MSIX_TABLE_BIR
> -#define RTE_PCI_MSIX_TABLE_BIRPCI_MSIX_TABLE_BIR
> -#else
> -#define RTE_PCI_MSIX_TABLE_BIR0x7
> -#endif
> -
> -#ifdef PCI_MSIX_TABLE_OFFSET
> -#define RTE_PCI_MSIX_TABLE_OFFSET PCI_MSIX_TABLE_OFFSET
> -#else
> -#define RTE_PCI_MSIX_TABLE_OFFSET 0xfff8
> -#endif
> -
> -#ifdef PCI_MSIX_FLAGS_QSIZE
> -#define RTE_PCI_MSIX_FLAGS_QSIZE  PCI_MSIX_FLAGS_QSIZE
> -#else
> -#define RTE_PCI_MSIX_FLAGS_QSIZE  0x07ff
> -#endif
> -
>  /* access config space */
>  int pci_vfio_read_config(const struct rte_pci_device *dev,
>void *buf, size_t len, off_t offs);
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index bfedbc1bed..7881b7a946 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -120,14 +120,15 @@ pci_vfio_get_msix_bar(const struct rte_pci_device
> *dev,
>   uint16_t flags;
>   uint32_t reg;
> 
> - /* table offset resides in the next 4 bytes */
> - if (rte_pci_read_config(dev, ®, sizeof(reg), cap_offset + 4)
> < 0) {
> + if (rte_pci_read_config(dev, ®, sizeof(reg), cap_offset +
> + RTE_PCI_MSIX_TABLE) < 0) {
>   RTE_LOG(ERR, EAL,
>   "Cannot read MSIX table from PCI config 
> space!\n");
>   return -1;
>   }
> 
> - if (rte_pci_read_config(dev, &flags, sizeof(flags), cap_offset
> + 2) < 0) {
> + if (rte_pci_read_config(dev, &flags, sizeof(flags), cap_offset
> +
> + RTE_PCI_MSIX_FLAGS) < 0) {
>   RTE_LOG(ERR, EAL,
>   "Cannot read MSIX flags from PCI config 
> space!\n");
>   return -1;
> diff --git a/drivers/crypto/virtio/virtio_pci.c
> b/drivers/crypto/virtio/virtio_pci.c
> index 9e340f2b0d..c9fb1087a9 100644
> --- a/drivers/crypto/virtio/virtio_pci.c
> +++ b/drivers/crypto/virtio/virtio_pci.c
> @@ -329,8 +329,6 @@ get_cfg_addr(struct rte_pci_device *dev, struct
> virtio_pci_cap *cap)
>   return base + offset;
>  }
> 
> -#define PCI_MSIX_ENABLE 0x8000
> -
>  static int
>  virtio_read_caps(struct rte_pci_device *dev, struct virtio_crypto_hw *hw)
>  {
> @@ -350,8 +348,8 @@ virtio_read_caps(struct rte_pci_device *dev, struct
> virtio_crypto_hw *hw)
>*/
>   pos = rte_pci_find_capability(dev, RTE_PCI_CAP_ID_MSIX);
>   if (pos > 0 && rte_pci_read_config(dev, &flags, sizeof(flags),
> - pos + 2) == sizeof(flags)) {
> - if (flags & PCI_MSIX_ENABLE)
> + pos + RTE_PCI_MSIX_FLAGS) == sizeof(flags)) {
> + if (flags & RTE_PCI_MSIX_FLAGS_ENABLE)
>   hw->use_msix = VIRTIO_MSIX_ENABLED;
>   else
>   hw->use_msix = VIRTIO_MSIX_DISABLED;
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c
> b/drivers/event/dlb2/pf/dlb2_main.c
> index 1a229baee0..c6606a9bee 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -44,9 +44,6 @@
>  #define DLB2_PCI_PRI_CTRL_ENABLE 0x1
>  #define DLB2_PCI_PRI_ALLOC_REQ   0xC
>  #define DLB2_PCI_PRI_CTRL0x4
> -#define DLB2_PCI_MSIX_FLAGS  0x2
> -#define DLB2_PCI_MSIX_FLAGS_ENABLE   0x8000
> -#define DLB2_PCI_MSIX_FLAGS_MASK

RE: [PATCH v2 07/15] pci: define some command constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Burakov, Anatoly ;
> McDaniel, Timothy ; Gaetan Rivet
> 
> Subject: [PATCH v2 07/15] pci: define some command constants
> 
> Define some PCI command constants and use them in existing drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/bus/pci/linux/pci_vfio.c  | 8 
>  drivers/event/dlb2/pf/dlb2_main.c | 8 +++-
>  lib/pci/rte_pci.h | 6 --
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 7881b7a946..bf91492dd9 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -156,18 +156,18 @@ pci_vfio_enable_bus_memory(struct rte_pci_device
> *dev, int dev_fd)
>   return -1;
>   }
> 
> - ret = pread64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND);
> + ret = pread64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMMAND);
> 
>   if (ret != sizeof(cmd)) {
>   RTE_LOG(ERR, EAL, "Cannot read command from PCI config
> space!\n");
>   return -1;
>   }
> 
> - if (cmd & PCI_COMMAND_MEMORY)
> + if (cmd & RTE_PCI_COMMAND_MEMORY)
>   return 0;
> 
> - cmd |= PCI_COMMAND_MEMORY;
> - ret = pwrite64(dev_fd, &cmd, sizeof(cmd), offset + PCI_COMMAND);
> + cmd |= RTE_PCI_COMMAND_MEMORY;
> + ret = pwrite64(dev_fd, &cmd, sizeof(cmd), offset + RTE_PCI_COMMAND);
> 
>   if (ret != sizeof(cmd)) {
>   RTE_LOG(ERR, EAL, "Cannot write command to PCI config
> space!\n");
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c
> b/drivers/event/dlb2/pf/dlb2_main.c
> index c6606a9bee..6dbaa2ff97 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -33,7 +33,6 @@
>  #define DLB2_PCI_EXP_DEVCTL2 40
>  #define DLB2_PCI_LNKCTL2 48
>  #define DLB2_PCI_SLTCTL2 56
> -#define DLB2_PCI_CMD 4
>  #define DLB2_PCI_EXP_DEVSTA 10
>  #define DLB2_PCI_EXP_DEVSTA_TRPND 0x20
>  #define DLB2_PCI_EXP_DEVCTL_BCR_FLR 0x8000
> @@ -47,7 +46,6 @@
>  #define DLB2_PCI_ERR_ROOT_STATUS 0x30
>  #define DLB2_PCI_ERR_COR_STATUS  0x10
>  #define DLB2_PCI_ERR_UNCOR_STATUS0x4
> -#define DLB2_PCI_COMMAND_INTX_DISABLE0x400
>  #define DLB2_PCI_ACS_CAP 0x4
>  #define DLB2_PCI_ACS_CTRL0x6
>  #define DLB2_PCI_ACS_SV  0x1
> @@ -286,7 +284,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
> 
>   /* clear the PCI command register before issuing the FLR */
> 
> - off = DLB2_PCI_CMD;
> + off = RTE_PCI_COMMAND;
>   cmd = 0;
>   if (rte_pci_write_config(pdev, &cmd, 2, off) != 2) {
>   DLB2_LOG_ERR("[%s()] failed to write the pci command\n",
> @@ -468,9 +466,9 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   }
>   }
> 
> - off = DLB2_PCI_CMD;
> + off = RTE_PCI_COMMAND;
>   if (rte_pci_read_config(pdev, &cmd, 2, off) == 2) {
> - cmd &= ~DLB2_PCI_COMMAND_INTX_DISABLE;
> + cmd &= ~RTE_PCI_COMMAND_INTX_DISABLE;
>   if (rte_pci_write_config(pdev, &cmd, 2, off) != 2) {
>   DLB2_LOG_ERR("[%s()] failed to write the pci command\n",
>  __func__);
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index 41dc725cc4..9eb8f85ceb 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -37,8 +37,10 @@ extern "C" {
>  #define RTE_PCI_STATUS   0x06/* 16 bits */
>  #define RTE_PCI_CAPABILITY_LIST  0x34/* 32 bits */
> 
> -/* PCI Command Register */
> -#define RTE_PCI_COMMAND_MASTER   0x4 /* Bus Master Enable */
> +/* PCI Command Register (RTE_PCI_COMMAND) */
> +#define RTE_PCI_COMMAND_MEMORY   0x2 /* Enable response in
> Memory space */
> +#define RTE_PCI_COMMAND_MASTER   0x4 /* Bus Master Enable */
> +#define RTE_PCI_COMMAND_INTX_DISABLE 0x400   /* INTx Emulation Disable
> */
> 
>  /* PCI Status Register (RTE_PCI_STATUS) */
>  #define RTE_PCI_STATUS_CAP_LIST  0x10/* Support Capability 
> List
> */
> --
> 2.41.0

Reviewed-by: Chenbo Xia  


RE: [PATCH v2 08/15] pci: define some BAR constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Burakov, Anatoly ;
> Gaetan Rivet 
> Subject: [PATCH v2 08/15] pci: define some BAR constants
> 
> Define some PCI BAR constants and use them in existing drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/bus/pci/linux/pci_vfio.c | 7 +++
>  lib/pci/rte_pci.h| 4 
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index bf91492dd9..3f3201daf2 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -5,7 +5,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -427,14 +426,14 @@ pci_vfio_is_ioport_bar(const struct rte_pci_device
> *dev, int vfio_dev_fd,
>   }
> 
>   ret = pread64(vfio_dev_fd, &ioport_bar, sizeof(ioport_bar),
> -   offset + PCI_BASE_ADDRESS_0 + bar_index * 4);
> +   offset + RTE_PCI_BASE_ADDRESS_0 + bar_index * 4);
>   if (ret != sizeof(ioport_bar)) {
>   RTE_LOG(ERR, EAL, "Cannot read command (%x) from config
> space!\n",
> - PCI_BASE_ADDRESS_0 + bar_index*4);
> + RTE_PCI_BASE_ADDRESS_0 + bar_index*4);
>   return -1;
>   }
> 
> - return (ioport_bar & PCI_BASE_ADDRESS_SPACE_IO) != 0;
> + return (ioport_bar & RTE_PCI_BASE_ADDRESS_SPACE_IO) != 0;
>  }
> 
>  static int
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index 9eb8f85ceb..62bf87aa10 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -35,6 +35,7 @@ extern "C" {
>  #define RTE_PCI_DEVICE_ID0x02/* 16 bits */
>  #define RTE_PCI_COMMAND  0x04/* 16 bits */
>  #define RTE_PCI_STATUS   0x06/* 16 bits */
> +#define RTE_PCI_BASE_ADDRESS_0   0x10/* 32 bits */
>  #define RTE_PCI_CAPABILITY_LIST  0x34/* 32 bits */
> 
>  /* PCI Command Register (RTE_PCI_COMMAND) */
> @@ -45,6 +46,9 @@ extern "C" {
>  /* PCI Status Register (RTE_PCI_STATUS) */
>  #define RTE_PCI_STATUS_CAP_LIST  0x10/* Support Capability 
> List
> */
> 
> +/* Base addresses (RTE_PCI_BASE_ADDRESS_*) */
> +#define RTE_PCI_BASE_ADDRESS_SPACE_IO0x01
> +
>  /* Capability registers (RTE_PCI_CAPABILITY_LIST) */
>  #define RTE_PCI_CAP_ID_PM0x01/* Power Management */
>  #define RTE_PCI_CAP_ID_MSI   0x05/* Message Signalled Interrupts
> */
> --
> 2.41.0

Reviewed-by: Chenbo Xia  


RE: [PATCH v2 09/15] pci: define some PM constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; Julien Aube ; Gaetan
> Rivet 
> Subject: [PATCH v2 09/15] pci: define some PM constants
> 
> Define some PCI Power Management constants and use them in existing
> drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/net/bnx2x/bnx2x.c | 17 +
>  drivers/net/bnx2x/bnx2x.h |  5 -
>  lib/pci/rte_pci.h |  6 ++
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index e3f14400cc..faf061beba 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -5843,17 +5843,17 @@ static int bnx2x_set_power_state(struct
> bnx2x_softc *sc, uint8_t state)
>   return 0;
>   }
> 
> - pci_read(sc, (sc->devinfo.pcie_pm_cap_reg + PCIR_POWER_STATUS),
> &pmcsr,
> + pci_read(sc, (sc->devinfo.pcie_pm_cap_reg + RTE_PCI_PM_CTRL), &pmcsr,
>2);
> 
>   switch (state) {
>   case PCI_PM_D0:
>   pci_write_word(sc,
>  (sc->devinfo.pcie_pm_cap_reg +
> - PCIR_POWER_STATUS),
> -((pmcsr & ~PCIM_PSTAT_DMASK) | PCIM_PSTAT_PME));
> + RTE_PCI_PM_CTRL),
> +((pmcsr & ~RTE_PCI_PM_CTRL_STATE_MASK) |
> RTE_PCI_PM_CTRL_PME_STATUS));
> 
> - if (pmcsr & PCIM_PSTAT_DMASK) {
> + if (pmcsr & RTE_PCI_PM_CTRL_STATE_MASK) {
>   /* delay required during transition out of D3hot */
>   DELAY(2);
>   }
> @@ -5866,16 +5866,17 @@ static int bnx2x_set_power_state(struct
> bnx2x_softc *sc, uint8_t state)
>   return 0;
>   }
> 
> - pmcsr &= ~PCIM_PSTAT_DMASK;
> - pmcsr |= PCIM_PSTAT_D3;
> + pmcsr &= ~RTE_PCI_PM_CTRL_STATE_MASK;
> + /* D3 power state */
> + pmcsr |= 0x3;
> 
>   if (sc->wol) {
> - pmcsr |= PCIM_PSTAT_PMEENABLE;
> + pmcsr |= RTE_PCI_PM_CTRL_PME_ENABLE;
>   }
> 
>   pci_write_long(sc,
>  (sc->devinfo.pcie_pm_cap_reg +
> - PCIR_POWER_STATUS), pmcsr);
> + RTE_PCI_PM_CTRL), pmcsr);
> 
>   /*
>* No more memory access after this point until device is
> brought back
> diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
> index 60af75d336..1efa166316 100644
> --- a/drivers/net/bnx2x/bnx2x.h
> +++ b/drivers/net/bnx2x/bnx2x.h
> @@ -41,11 +41,6 @@
>  #define PCIR_EXPRESS_DEVICE_CTLPCI_EXP_DEVCTL
>  #define PCIM_EXP_CTL_MAX_PAYLOAD   PCI_EXP_DEVCTL_PAYLOAD
>  #define PCIM_EXP_CTL_MAX_READ_REQUEST  PCI_EXP_DEVCTL_READRQ
> -#define PCIR_POWER_STATUS  PCI_PM_CTRL
> -#define PCIM_PSTAT_DMASK   PCI_PM_CTRL_STATE_MASK
> -#define PCIM_PSTAT_PME PCI_PM_CTRL_PME_STATUS
> -#define PCIM_PSTAT_D3  0x3
> -#define PCIM_PSTAT_PMEENABLE   PCI_PM_CTRL_PME_ENABLE
>  #else
>  #include 
>  #endif
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index 62bf87aa10..542d142dfb 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -57,6 +57,12 @@ extern "C" {
>  #define RTE_PCI_CAP_ID_MSIX  0x11/* MSI-X */
>  #define RTE_PCI_CAP_SIZEOF   4
> 
> +/* Power Management Registers (RTE_PCI_CAP_ID_PM) */
> +#define RTE_PCI_PM_CTRL  4   /* PM control and status
> register */
> +#define RTE_PCI_PM_CTRL_STATE_MASK   0x0003  /* Current power state (D0
> to D3) */
> +#define RTE_PCI_PM_CTRL_PME_ENABLE   0x0100  /* PME pin enable */
> +#define RTE_PCI_PM_CTRL_PME_STATUS   0x8000  /* PME pin status */
> +
>  /* MSI-X registers (RTE_PCI_CAP_ID_MSIX) */
>  #define RTE_PCI_MSIX_FLAGS   2   /* Message Control */
>  #define RTE_PCI_MSIX_FLAGS_QSIZE 0x07ff  /* Table size */
> --
> 2.41.0

Reviewed-by: Chenbo Xia  


RE: [PATCH v2 10/15] pci: define some PCIe constants

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; McDaniel, Timothy
> ; Julien Aube ; Gaetan
> Rivet 
> Subject: [PATCH v2 10/15] pci: define some PCIe constants
> 
> Define some PCI Express constants and use them in existing drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/event/dlb2/pf/dlb2_main.c | 40 ---
>  drivers/net/bnx2x/bnx2x.c | 16 ++---
>  drivers/net/bnx2x/bnx2x.h | 35 ---
>  lib/pci/rte_pci.h | 21 +---
>  4 files changed, 41 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c
> b/drivers/event/dlb2/pf/dlb2_main.c
> index 6dbaa2ff97..8d960edef6 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -27,16 +27,6 @@
>  #define NO_OWNER_VF 0/* PF ONLY! */
>  #define NOT_VF_REQ false /* PF ONLY! */
> 
> -#define DLB2_PCI_LNKCTL 16
> -#define DLB2_PCI_SLTCTL 24
> -#define DLB2_PCI_RTCTL 28
> -#define DLB2_PCI_EXP_DEVCTL2 40
> -#define DLB2_PCI_LNKCTL2 48
> -#define DLB2_PCI_SLTCTL2 56
> -#define DLB2_PCI_EXP_DEVSTA 10
> -#define DLB2_PCI_EXP_DEVSTA_TRPND 0x20
> -#define DLB2_PCI_EXP_DEVCTL_BCR_FLR 0x8000
> -
>  #define DLB2_PCI_EXT_CAP_ID_PRI   0x13
>  #define DLB2_PCI_EXT_CAP_ID_ACS   0xD
> 
> @@ -249,27 +239,27 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   if (rte_pci_read_config(pdev, &dev_ctl_word, 2, off) != 2)
>   dev_ctl_word = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_LNKCTL;
> + off = pcie_cap_offset + RTE_PCI_EXP_LNKCTL;
>   if (rte_pci_read_config(pdev, &lnk_word, 2, off) != 2)
>   lnk_word = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_SLTCTL;
> + off = pcie_cap_offset + RTE_PCI_EXP_SLTCTL;
>   if (rte_pci_read_config(pdev, &slt_word, 2, off) != 2)
>   slt_word = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_RTCTL;
> + off = pcie_cap_offset + RTE_PCI_EXP_RTCTL;
>   if (rte_pci_read_config(pdev, &rt_ctl_word, 2, off) != 2)
>   rt_ctl_word = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_EXP_DEVCTL2;
> + off = pcie_cap_offset + RTE_PCI_EXP_DEVCTL2;
>   if (rte_pci_read_config(pdev, &dev_ctl2_word, 2, off) != 2)
>   dev_ctl2_word = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_LNKCTL2;
> + off = pcie_cap_offset + RTE_PCI_EXP_LNKCTL2;
>   if (rte_pci_read_config(pdev, &lnk_word2, 2, off) != 2)
>   lnk_word2 = 0;
> 
> - off = pcie_cap_offset + DLB2_PCI_SLTCTL2;
> + off = pcie_cap_offset + RTE_PCI_EXP_SLTCTL2;
>   if (rte_pci_read_config(pdev, &slt_word2, 2, off) != 2)
>   slt_word2 = 0;
> 
> @@ -296,7 +286,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   for (wait_count = 0; wait_count < 4; wait_count++) {
>   int sleep_time;
> 
> - off = pcie_cap_offset + DLB2_PCI_EXP_DEVSTA;
> + off = pcie_cap_offset + RTE_PCI_EXP_DEVSTA;
>   ret = rte_pci_read_config(pdev, &devsta_busy_word, 2, off);
>   if (ret != 2) {
>   DLB2_LOG_ERR("[%s()] failed to read the pci device
> status\n",
> @@ -304,7 +294,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
>   }
> 
> - if (!(devsta_busy_word & DLB2_PCI_EXP_DEVSTA_TRPND))
> + if (!(devsta_busy_word & RTE_PCI_EXP_DEVSTA_TRPND))
>   break;
> 
>   sleep_time = (1 << (wait_count)) * 100;
> @@ -325,7 +315,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
>   }
> 
> - devctl_word |= DLB2_PCI_EXP_DEVCTL_BCR_FLR;
> + devctl_word |= RTE_PCI_EXP_DEVCTL_BCR_FLR;
> 
>   ret = rte_pci_write_config(pdev, &devctl_word, 2, off);
>   if (ret != 2) {
> @@ -347,7 +337,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
>   }
> 
> - off = pcie_cap_offset + DLB2_PCI_LNKCTL;
> + off = pcie_cap_offset + RTE_PCI_EXP_LNKCTL;
>   ret = rte_pci_write_config(pdev, &lnk_word, 2, off);
>   if (ret != 2) {
>   DLB2_LOG_ERR("[%s()] failed to write the pcie config
> space at offset %d\n",
> @@ -355,7 +345,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
>   }
> 
> - off = pcie_cap_offset + DLB2_PCI_SLTCTL;
> + off = pcie_cap_offset + RTE_PCI_EXP_SLTCTL;
>   ret = rte_pci_write_config(pdev, &slt_word, 2, off);
>   if (ret != 2) {
>   DLB2_LOG_ERR("[%s()] failed to write the pcie config
> space at offset %d\n",
> @@ -363,7 +353,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   return ret;
> 

RE: [PATCH v2 11/15] pci: define some extended capability constants

2023-09-07 Thread Xia, Chenbo
Hi David,

> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> ; McDaniel, Timothy
> ; Gaetan Rivet 
> Subject: [PATCH v2 11/15] pci: define some extended capability constants
> 
> Define some PCI extended capability constants and use them in existing
> drivers.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  drivers/event/dlb2/pf/dlb2_main.c | 7 ++-
>  lib/pci/rte_pci.h | 4 +++-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/event/dlb2/pf/dlb2_main.c
> b/drivers/event/dlb2/pf/dlb2_main.c
> index 8d960edef6..29e3001627 100644
> --- a/drivers/event/dlb2/pf/dlb2_main.c
> +++ b/drivers/event/dlb2/pf/dlb2_main.c
> @@ -27,9 +27,6 @@
>  #define NO_OWNER_VF 0/* PF ONLY! */
>  #define NOT_VF_REQ false /* PF ONLY! */
> 
> -#define DLB2_PCI_EXT_CAP_ID_PRI   0x13
> -#define DLB2_PCI_EXT_CAP_ID_ACS   0xD
> -
>  #define DLB2_PCI_PRI_CTRL_ENABLE 0x1
>  #define DLB2_PCI_PRI_ALLOC_REQ   0xC
>  #define DLB2_PCI_PRI_CTRL0x4
> @@ -263,7 +260,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   if (rte_pci_read_config(pdev, &slt_word2, 2, off) != 2)
>   slt_word2 = 0;
> 
> - off = DLB2_PCI_EXT_CAP_ID_PRI;
> + off = RTE_PCI_EXT_CAP_ID_PRI;
>   pri_cap_offset = rte_pci_find_ext_capability(pdev, off);
> 
>   if (pri_cap_offset >= 0) {
> @@ -490,7 +487,7 @@ dlb2_pf_reset(struct dlb2_dev *dlb2_dev)
>   }
>   }
> 
> - off = DLB2_PCI_EXT_CAP_ID_ACS;
> + off = RTE_PCI_EXT_CAP_ID_ACS;
>   acs_cap_offset = rte_pci_find_ext_capability(pdev, off);
> 
>   if (acs_cap_offset >= 0) {
> diff --git a/lib/pci/rte_pci.h b/lib/pci/rte_pci.h
> index a82f073a7d..d2f7a96f17 100644
> --- a/lib/pci/rte_pci.h
> +++ b/lib/pci/rte_pci.h
> @@ -97,9 +97,11 @@ extern "C" {
> 
>  #define RTE_PCI_EXT_CAP_ID_ERR   0x01/* Advanced Error
> Reporting */
>  #define RTE_PCI_EXT_CAP_ID_DSN   0x03/* Device Serial Number 
> */
> +#define RTE_PCI_EXT_CAP_ID_ACS   0x0d/* Access Control 
> Services
> */
>  #define RTE_PCI_EXT_CAP_ID_SRIOV 0x10/* SR-IOV*/

Maybe we could also do the small clean-up: it should be one space after 'IOV' :)

With this fixed:

Reviewed-by: Chenbo Xia 

> +#define RTE_PCI_EXT_CAP_ID_PRI   0x13/* Page Request 
> Interface
> */
> 
> -/* Single Root I/O Virtualization */
> +/* Single Root I/O Virtualization (RTE_PCI_EXT_CAP_ID_SRIOV) */
>  #define RTE_PCI_SRIOV_CAP0x04/* SR-IOV Capabilities */
>  #define RTE_PCI_SRIOV_CTRL   0x08/* SR-IOV Control */
>  #define RTE_PCI_SRIOV_INITIAL_VF 0x0c/* Initial VFs */
> --
> 2.41.0



RE: [PATCH v2 15/15] devtools: forbid inclusion of Linux header for PCI

2023-09-07 Thread Xia, Chenbo
> -Original Message-
> From: David Marchand 
> Sent: Monday, August 21, 2023 7:36 PM
> To: dev@dpdk.org
> Cc: tho...@monjalon.net; ferruh.yi...@amd.com; Xia, Chenbo
> ; nipun.gu...@amd.com; Richardson, Bruce
> 
> Subject: [PATCH v2 15/15] devtools: forbid inclusion of Linux header for
> PCI
> 
> Refrain from including Linux-only pci_regs.h header.
> Instead, prefer our own definitions from the pci library.
> 
> Signed-off-by: David Marchand 
> Acked-by: Bruce Richardson 
> ---
>  devtools/checkpatches.sh | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 43f5e36a18..5d3c5aaba5 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -127,6 +127,14 @@ check_forbidden_additions() { # 
>   -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
>   "$1" || res=1
> 
> + # forbid inclusion of Linux header for PCI constants
> + awk -v FOLDERS="lib drivers app examples" \
> + -v EXPRESSIONS='include.*linux/pci_regs\\.h' \
> + -v RET_ON_FAIL=1 \
> + -v MESSAGE='Using linux/pci_regs.h, prefer rte_pci.h' \
> + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> + "$1" || res=1
> +
>   # forbid use of experimental build flag except in examples
>   awk -v FOLDERS='lib drivers app' \
>   -v EXPRESSIONS='-DALLOW_EXPERIMENTAL_API
> allow_experimental_apis' \
> --
> 2.41.0

Reviewed-by: Chenbo Xia  


[PATCH v2 0/2] fixes to rte_random for non-EAL threads

2023-09-07 Thread Stephen Hemminger
While examining the code for rte_random, noticed a couple of
existing bugs around use of rte_rand() by non-EAL threads

Stephen Hemminger (2):
  random: initialize the random state for non-EAL threads
  random: make rte_rand() thread safe for non-EAL threads

 lib/eal/common/rte_random.c | 56 -
 1 file changed, 37 insertions(+), 19 deletions(-)

-- 
2.39.2



[PATCH v2 1/2] random: initialize the random state for non-EAL threads

2023-09-07 Thread Stephen Hemminger
The per-lcore PRNG was not initializing the rand_state of all
the lcores. Any usage of rte_random by a non-EAL lcore would
use rand_states[RTE_MAX_LCORE] which was never initialized.

Fix by using RTE_DIM() which will get all lcores.

Fixes: 3f002f069612 ("eal: replace libc-based random generation with LFSR")
Cc: mattias.ronnb...@ericsson.com
Acked-by: Morten Brørup 
Signed-off-by: Stephen Hemminger 
---
 lib/eal/common/rte_random.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 53636331a27b..812e5b4757b5 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -84,7 +84,7 @@ rte_srand(uint64_t seed)
unsigned int lcore_id;
 
/* add lcore_id to seed to avoid having the same sequence */
-   for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
+   for (lcore_id = 0; lcore_id < RTE_DIM(rand_states); lcore_id++)
__rte_srand_lfsr258(seed + lcore_id, &rand_states[lcore_id]);
 }
 
-- 
2.39.2



[PATCH v2 2/2] random: make rte_rand() thread safe for non-EAL threads

2023-09-07 Thread Stephen Hemminger
Add missing locking so that if two non-EAL threads call rte_rand()
they will not corrupt the per-thread state.

Fixes: 3f002f069612 ("eal: replace libc-based random generation with LFSR")
Signed-off-by: Stephen Hemminger 
---
 lib/eal/common/rte_random.c | 54 -
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 812e5b4757b5..02b6b6b97bc0 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 struct rte_rand_state {
@@ -21,6 +22,9 @@ struct rte_rand_state {
uint64_t z5;
 } __rte_cache_aligned;
 
+/* Used for thread safety for non EAL threads. */
+static rte_spinlock_t rte_rand_lock = RTE_SPINLOCK_INITIALIZER;
+
 /* One instance each for every lcore id-equipped thread, and one
  * additional instance to be shared by all others threads (i.e., all
  * unregistered non-EAL threads).
@@ -124,20 +128,32 @@ struct rte_rand_state *__rte_rand_get_state(void)
idx = rte_lcore_id();
 
/* last instance reserved for unregistered non-EAL threads */
-   if (unlikely(idx == LCORE_ID_ANY))
+   if (unlikely(idx == LCORE_ID_ANY)) {
idx = RTE_MAX_LCORE;
+   rte_spinlock_lock(&rte_rand_lock);
+   }
 
return &rand_states[idx];
 }
 
+static __rte_always_inline
+void __rte_rand_put_state(struct rte_rand_state *state)
+{
+   if (state == &rand_states[RTE_MAX_LCORE])
+   rte_spinlock_unlock(&rte_rand_lock);
+}
+
 uint64_t
 rte_rand(void)
 {
struct rte_rand_state *state;
+   uint64_t res;
 
state = __rte_rand_get_state();
+   res = __rte_rand_lfsr258(state);
+   __rte_rand_put_state(state);
 
-   return __rte_rand_lfsr258(state);
+   return res;
 }
 
 uint64_t
@@ -159,22 +175,24 @@ rte_rand_max(uint64_t upper_bound)
/* Handle power-of-2 upper_bound as a special case, since it
 * has no bias issues.
 */
-   if (unlikely(ones == 1))
-   return __rte_rand_lfsr258(state) & (upper_bound - 1);
-
-   /* The approach to avoiding bias is to create a mask that
-* stretches beyond the request value range, and up to the
-* next power-of-2. In case the masked generated random value
-* is equal to or greater than the upper bound, just discard
-* the value and generate a new one.
-*/
-
-   leading_zeros = rte_clz64(upper_bound);
-   mask >>= leading_zeros;
-
-   do {
-   res = __rte_rand_lfsr258(state) & mask;
-   } while (unlikely(res >= upper_bound));
+   if (unlikely(ones == 1)) {
+   res = __rte_rand_lfsr258(state) & (upper_bound - 1);
+   } else {
+   /* The approach to avoiding bias is to create a mask that
+* stretches beyond the request value range, and up to the
+* next power-of-2. In case the masked generated random value
+* is equal to or greater than the upper bound, just discard
+* the value and generate a new one.
+*/
+
+   leading_zeros = rte_clz64(upper_bound);
+   mask >>= leading_zeros;
+
+   do {
+   res = __rte_rand_lfsr258(state) & mask;
+   } while (unlikely(res >= upper_bound));
+   }
+   __rte_rand_put_state(state);
 
return res;
 }
-- 
2.39.2



RE: [PATCH v6 2/2] test/crypto: fix IV in some vectors

2023-09-07 Thread Power, Ciara



> -Original Message-
> From: Brian Dooley 
> Sent: Thursday, September 7, 2023 11:26 AM
> To: Akhil Goyal ; Fan Zhang 
> Cc: dev@dpdk.org; Dooley, Brian ;
> adamx.dybkow...@intel.com
> Subject: [PATCH v6 2/2] test/crypto: fix IV in some vectors
> 
> SNOW3G and ZUC algorithms require non-zero length IVs.
> 
> Fixes: c6c267a00a92 ("test/crypto: add mixed encypted-digest")
> Cc: adamx.dybkow...@intel.com
> 
> Signed-off-by: Brian Dooley 
> ---
>  app/test/test_cryptodev_mixed_test_vectors.h | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Acked-by: Ciara Power 


RE: [PATCH v6 1/2] crypto/ipsec_mb: add digest encrypted feature

2023-09-07 Thread Power, Ciara


Hi Brian,

> -Original Message-
> From: Brian Dooley 
> Sent: Thursday, September 7, 2023 11:26 AM
> To: Akhil Goyal ; Fan Zhang ;
> Ji, Kai ; De Lara Guarch, Pablo
> 
> Cc: dev@dpdk.org; Dooley, Brian 
> Subject: [PATCH v6 1/2] crypto/ipsec_mb: add digest encrypted feature
> 
> AESNI_MB PMD does not support Digest Encrypted. This patch adds a check
> and support for this feature.
> 
> Signed-off-by: Brian Dooley 
> ---
> v2:
> Fixed CHECKPATCH warning
> v3:
> Add Digest encrypted support to docs
> v4:
> Add comments and small refactor
> v5:
> Fix checkpatch warnings
> v6:
> Add skipping tests for synchronous crypto
> ---
>  app/test/test_cryptodev.c   |   6 ++
>  doc/guides/cryptodevs/features/aesni_mb.ini |   1 +
>  drivers/crypto/ipsec_mb/pmd_aesni_mb.c  | 109
> +++-
>  3 files changed, 111 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> 956268bfcd..70f6b7ece1 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -6394,6 +6394,9 @@ test_zuc_auth_cipher(const struct
> wireless_test_data *tdata,
>   tdata->digest.len) < 0)
>   return TEST_SKIPPED;
> 
> + if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> + return TEST_SKIPPED;
> +
>   rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
> 
>   uint64_t feat_flags = dev_info.feature_flags; @@ -7829,6 +7832,9
> @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data
> *tdata,
>   if (global_api_test_type == CRYPTODEV_RAW_API_TEST)
>   return TEST_SKIPPED;
> 
> + if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
> + return TEST_SKIPPED;
> +
>   rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
> 


Small thing, I think the above fixes should be in their own fix patch.

Code changes look good to me. Can keep my ack on v7 with the fixes split out.

Acked-by: Ciara Power 


[PATCH] crypto/qat: fix raw API null algorithm digest

2023-09-07 Thread Ciara Power
QAT HW generates bytes of 0x00 digest, even when a digest of len 0 is
requested for NULL. This caused test failures when the test vector had
digest len 0, as the buffer has unexpected changed bytes.

By placing the digest into the cookie for NULL authentication,
the buffer remains unchanged as expected, and the digest
is placed to the side, as it won't be used anyway.

This fix was previously added for the main QAT code path, but it also
needs to be included for the raw API code path.

Fixes: db0e952a5c01 ("crypto/qat: add NULL capability")
Cc: sta...@dpdk.org

Signed-off-by: Ciara Power 
---
 drivers/crypto/qat/dev/qat_crypto_pmd_gen3.c | 19 -
 drivers/crypto/qat/dev/qat_sym_pmd_gen1.c| 41 +---
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/qat/dev/qat_crypto_pmd_gen3.c 
b/drivers/crypto/qat/dev/qat_crypto_pmd_gen3.c
index d25e1b2f3a..0a939161f9 100644
--- a/drivers/crypto/qat/dev/qat_crypto_pmd_gen3.c
+++ b/drivers/crypto/qat/dev/qat_crypto_pmd_gen3.c
@@ -637,6 +637,8 @@ qat_sym_dp_enqueue_single_auth_gen3(void *qp_data, uint8_t 
*drv_ctx,
struct icp_qat_fw_la_bulk_req *req;
int32_t data_len;
uint32_t tail = dp_ctx->tail;
+   struct rte_crypto_va_iova_ptr null_digest;
+   struct rte_crypto_va_iova_ptr *job_digest = digest;
 
req = (struct icp_qat_fw_la_bulk_req *)(
(uint8_t *)tx_queue->base_addr + tail);
@@ -650,7 +652,12 @@ qat_sym_dp_enqueue_single_auth_gen3(void *qp_data, uint8_t 
*drv_ctx,
if (unlikely(data_len < 0))
return -1;
 
-   enqueue_one_auth_job_gen3(ctx, cookie, req, digest, auth_iv, ofs,
+   if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_NULL) {
+   null_digest.iova = cookie->digest_null_phys_addr;
+   job_digest = &null_digest;
+   }
+
+   enqueue_one_auth_job_gen3(ctx, cookie, req, job_digest, auth_iv, ofs,
(uint32_t)data_len);
 
dp_ctx->tail = tail;
@@ -672,6 +679,8 @@ qat_sym_dp_enqueue_auth_jobs_gen3(void *qp_data, uint8_t 
*drv_ctx,
uint32_t tail;
struct icp_qat_fw_la_bulk_req *req;
int32_t data_len;
+   struct rte_crypto_va_iova_ptr null_digest;
+   struct rte_crypto_va_iova_ptr *job_digest = NULL;
 
n = QAT_SYM_DP_GET_MAX_ENQ(qp, dp_ctx->cached_enqueue, vec->num);
if (unlikely(n == 0)) {
@@ -704,7 +713,13 @@ qat_sym_dp_enqueue_auth_jobs_gen3(void *qp_data, uint8_t 
*drv_ctx,
 
if (unlikely(data_len < 0))
break;
-   enqueue_one_auth_job_gen3(ctx, cookie, req, &vec->digest[i],
+   if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_NULL) {
+   null_digest.iova = cookie->digest_null_phys_addr;
+   job_digest = &null_digest;
+   } else
+   job_digest = &vec->digest[i];
+
+   enqueue_one_auth_job_gen3(ctx, cookie, req, job_digest,
&vec->auth_iv[i], ofs, (uint32_t)data_len);
tail = (tail + tx_queue->msg_size) & tx_queue->modulo_mask;
}
diff --git a/drivers/crypto/qat/dev/qat_sym_pmd_gen1.c 
b/drivers/crypto/qat/dev/qat_sym_pmd_gen1.c
index 70938ba508..e4bcfa59e7 100644
--- a/drivers/crypto/qat/dev/qat_sym_pmd_gen1.c
+++ b/drivers/crypto/qat/dev/qat_sym_pmd_gen1.c
@@ -598,6 +598,8 @@ qat_sym_dp_enqueue_single_auth_gen1(void *qp_data, uint8_t 
*drv_ctx,
struct icp_qat_fw_la_bulk_req *req;
int32_t data_len;
uint32_t tail = dp_ctx->tail;
+   struct rte_crypto_va_iova_ptr null_digest;
+   struct rte_crypto_va_iova_ptr *job_digest = digest;
 
req = (struct icp_qat_fw_la_bulk_req *)(
(uint8_t *)tx_queue->base_addr + tail);
@@ -611,8 +613,13 @@ qat_sym_dp_enqueue_single_auth_gen1(void *qp_data, uint8_t 
*drv_ctx,
if (unlikely(data_len < 0))
return -1;
 
-   enqueue_one_auth_job_gen1(ctx, req, digest, auth_iv, ofs,
-   (uint32_t)data_len);
+   if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_NULL) {
+   null_digest.iova = cookie->digest_null_phys_addr;
+   job_digest = &null_digest;
+   }
+
+   enqueue_one_auth_job_gen1(ctx, req, job_digest, auth_iv, ofs,
+   (uint32_t)data_len);
 
dp_ctx->tail = tail;
dp_ctx->cached_enqueue++;
@@ -636,6 +643,8 @@ qat_sym_dp_enqueue_auth_jobs_gen1(void *qp_data, uint8_t 
*drv_ctx,
uint32_t tail;
struct icp_qat_fw_la_bulk_req *req;
int32_t data_len;
+   struct rte_crypto_va_iova_ptr null_digest;
+   struct rte_crypto_va_iova_ptr *job_digest = NULL;
 
n = QAT_SYM_DP_GET_MAX_ENQ(qp, dp_ctx->cached_enqueue, vec->num);
if (unlikely(n == 0)) {
@@ -668,7 +677,14 @@ qat_sym_dp_enqueue_auth_jobs_gen1(void *qp_data, uint8_t 
*drv_ctx,
 
if (unlikely(data_len < 0))
break;
-   

Re: [PATCH v2 2/2] random: make rte_rand() thread safe for non-EAL threads

2023-09-07 Thread David Marchand
On Thu, Sep 7, 2023 at 5:48 PM Stephen Hemminger
 wrote:
>
> On Thu,  7 Sep 2023 08:24:56 -0700
> Stephen Hemminger  wrote:
>
> >
> > +static __rte_always_inline
> > +void __rte_rand_put_state(struct rte_rand_state *state)
> > +{
> > + if (state == &rand_states[RTE_MAX_LCORE])
> > + rte_spinlock_unlock(&rte_rand_lock);
> > +}
>
> Conditional locking like this make clang lock analyzer unhappy though.

Ugly, but some macro can do the job...

diff --git a/lib/eal/common/rte_random.c b/lib/eal/common/rte_random.c
index 02b6b6b97b..3f2a4830fd 100644
--- a/lib/eal/common/rte_random.c
+++ b/lib/eal/common/rte_random.c
@@ -128,20 +128,22 @@ struct rte_rand_state *__rte_rand_get_state(void)
idx = rte_lcore_id();

/* last instance reserved for unregistered non-EAL threads */
-   if (unlikely(idx == LCORE_ID_ANY)) {
+   if (unlikely(idx == LCORE_ID_ANY))
idx = RTE_MAX_LCORE;
-   rte_spinlock_lock(&rte_rand_lock);
-   }

return &rand_states[idx];
 }

-static __rte_always_inline
-void __rte_rand_put_state(struct rte_rand_state *state)
-{
-   if (state == &rand_states[RTE_MAX_LCORE])
-   rte_spinlock_unlock(&rte_rand_lock);
-}
+#define PROTECT_NON_EAL_THREADS(...) do { \
+   unsigned int idx = rte_lcore_id(); \
+   if (idx == LCORE_ID_ANY) { \
+   rte_spinlock_lock(&rte_rand_lock); \
+   __VA_ARGS__ \
+   rte_spinlock_unlock(&rte_rand_lock); \
+   } else { \
+   __VA_ARGS__ \
+   } \
+} while (0)

 uint64_t
 rte_rand(void)
@@ -149,9 +151,10 @@ rte_rand(void)
struct rte_rand_state *state;
uint64_t res;

+   PROTECT_NON_EAL_THREADS(
state = __rte_rand_get_state();
res = __rte_rand_lfsr258(state);
-   __rte_rand_put_state(state);
+   );

return res;
 }
@@ -168,6 +171,7 @@ rte_rand_max(uint64_t upper_bound)
if (unlikely(upper_bound < 2))
return 0;

+   PROTECT_NON_EAL_THREADS(
state = __rte_rand_get_state();

ones = rte_popcount64(upper_bound);
@@ -192,7 +196,7 @@ rte_rand_max(uint64_t upper_bound)
res = __rte_rand_lfsr258(state) & mask;
} while (unlikely(res >= upper_bound));
}
-   __rte_rand_put_state(state);
+   );

return res;
 }


-- 
David Marchand



[PATCH v7 0/3] Add Digest Encrypted to aesni_mb PMD

2023-09-07 Thread Brian Dooley
This series adds the Digest Encrypted feature to the AESNI_MB PMD.
It also fixes an issue where IV data in SNOW3G and ZUC algorithms
were incorrect and are required to be non-zero length.

v2:
Fixed CHECKPATCH warning
v3:
Add Digest encrypted support to docs
v4:
add comments and refactor
v5:
Fix checkpatch warnings
v6:
Add skipping tests for synchronous crypto
v7:
Separate synchronous fix into separate commit

Brian Dooley (3):
  crypto/ipsec_mb: add digest encrypted feature
  test/crypto: fix IV in some vectors
  test/crypto: fix failing synchronous tests

 app/test/test_cryptodev.c|   6 +
 app/test/test_cryptodev_mixed_test_vectors.h |   8 +-
 doc/guides/cryptodevs/features/aesni_mb.ini  |   1 +
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c   | 109 ++-
 4 files changed, 117 insertions(+), 7 deletions(-)

-- 
2.25.1



[PATCH v7 1/3] crypto/ipsec_mb: add digest encrypted feature

2023-09-07 Thread Brian Dooley
AESNI_MB PMD does not support Digest Encrypted. This patch adds a check and
support for this feature.

Acked-by: Ciara Power 
Signed-off-by: Brian Dooley 
---
v2:
Fixed CHECKPATCH warning
v3:
Add Digest encrypted support to docs
v4:
Add comments and small refactor
v5:
Fix checkpatch warnings
v6:
Add skipping tests for synchronous crypto
v7:
Separate synchronous fix into separate commit
---
 doc/guides/cryptodevs/features/aesni_mb.ini |   1 +
 drivers/crypto/ipsec_mb/pmd_aesni_mb.c  | 109 +++-
 2 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/doc/guides/cryptodevs/features/aesni_mb.ini 
b/doc/guides/cryptodevs/features/aesni_mb.ini
index e4e965c35a..8df5fa2c85 100644
--- a/doc/guides/cryptodevs/features/aesni_mb.ini
+++ b/doc/guides/cryptodevs/features/aesni_mb.ini
@@ -20,6 +20,7 @@ OOP LB  In LB  Out = Y
 CPU crypto = Y
 Symmetric sessionless  = Y
 Non-Byte aligned data  = Y
+Digest encrypted   = Y
 
 ;
 ; Supported crypto algorithms of the 'aesni_mb' crypto driver.
diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c 
b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
index 9e298023d7..7f61065939 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
@@ -1438,6 +1438,54 @@ set_gcm_job(IMB_MGR *mb_mgr, IMB_JOB *job, const uint8_t 
sgl,
return 0;
 }
 
+/** Check if conditions are met for digest-appended operations */
+static uint8_t *
+aesni_mb_digest_appended_in_src(struct rte_crypto_op *op, IMB_JOB *job,
+   uint32_t oop)
+{
+   unsigned int auth_size, cipher_size;
+   uint8_t *end_cipher;
+   uint8_t *start_cipher;
+
+   if (job->cipher_mode == IMB_CIPHER_NULL)
+   return NULL;
+
+   if (job->cipher_mode == IMB_CIPHER_ZUC_EEA3 ||
+   job->cipher_mode == IMB_CIPHER_SNOW3G_UEA2_BITLEN ||
+   job->cipher_mode == IMB_CIPHER_KASUMI_UEA1_BITLEN) {
+   cipher_size = (op->sym->cipher.data.offset >> 3) +
+   (op->sym->cipher.data.length >> 3);
+   } else {
+   cipher_size = (op->sym->cipher.data.offset) +
+   (op->sym->cipher.data.length);
+   }
+   if (job->hash_alg == IMB_AUTH_ZUC_EIA3_BITLEN ||
+   job->hash_alg == IMB_AUTH_SNOW3G_UIA2_BITLEN ||
+   job->hash_alg == IMB_AUTH_KASUMI_UIA1 ||
+   job->hash_alg == IMB_AUTH_ZUC256_EIA3_BITLEN) {
+   auth_size = (op->sym->auth.data.offset >> 3) +
+   (op->sym->auth.data.length >> 3);
+   } else {
+   auth_size = (op->sym->auth.data.offset) +
+   (op->sym->auth.data.length);
+   }
+
+   if (!oop) {
+   end_cipher = rte_pktmbuf_mtod_offset(op->sym->m_src, uint8_t *, 
cipher_size);
+   start_cipher = rte_pktmbuf_mtod(op->sym->m_src, uint8_t *);
+   } else {
+   end_cipher = rte_pktmbuf_mtod_offset(op->sym->m_dst, uint8_t *, 
cipher_size);
+   start_cipher = rte_pktmbuf_mtod(op->sym->m_dst, uint8_t *);
+   }
+
+   if (start_cipher < op->sym->auth.digest.data &&
+   op->sym->auth.digest.data < end_cipher) {
+   return rte_pktmbuf_mtod_offset(op->sym->m_src, uint8_t *, 
auth_size);
+   } else {
+   return NULL;
+   }
+}
+
 /**
  * Process a crypto operation and complete a IMB_JOB job structure for
  * submission to the multi buffer library for processing.
@@ -1580,9 +1628,12 @@ set_mb_job_params(IMB_JOB *job, struct ipsec_mb_qp *qp,
} else {
if (aead)
job->auth_tag_output = op->sym->aead.digest.data;
-   else
-   job->auth_tag_output = op->sym->auth.digest.data;
-
+   else {
+   job->auth_tag_output = 
aesni_mb_digest_appended_in_src(op, job, oop);
+   if (job->auth_tag_output == NULL) {
+   job->auth_tag_output = 
op->sym->auth.digest.data;
+   }
+   }
if (session->auth.req_digest_len !=
job->auth_tag_output_len_in_bytes) {
job->auth_tag_output =
@@ -1917,6 +1968,7 @@ post_process_mb_job(struct ipsec_mb_qp *qp, IMB_JOB *job)
struct aesni_mb_session *sess = NULL;
uint8_t *linear_buf = NULL;
int sgl = 0;
+   uint8_t oop = 0;
uint8_t is_docsis_sec = 0;
 
if (op->sess_type == RTE_CRYPTO_OP_SECURITY_SESSION) {
@@ -1962,8 +2014,54 @@ post_process_mb_job(struct ipsec_mb_qp *qp, IMB_JOB *job)
op->sym->auth.digest.data,
sess->auth.req_digest_len,
&op->status);
-   } else
+   } else {
+   if (!op->sym->m_dst || op->sym->m_ds

[PATCH v7 2/3] test/crypto: fix IV in some vectors

2023-09-07 Thread Brian Dooley
SNOW3G and ZUC algorithms require non-zero length IVs.

Fixes: c6c267a00a92 ("test/crypto: add mixed encypted-digest")
Cc: adamx.dybkow...@intel.com

Acked-by: Ciara Power 
Signed-off-by: Brian Dooley 
---
 app/test/test_cryptodev_mixed_test_vectors.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/test/test_cryptodev_mixed_test_vectors.h 
b/app/test/test_cryptodev_mixed_test_vectors.h
index 161e2d905f..9c4313185e 100644
--- a/app/test/test_cryptodev_mixed_test_vectors.h
+++ b/app/test/test_cryptodev_mixed_test_vectors.h
@@ -478,8 +478,10 @@ struct mixed_cipher_auth_test_data 
auth_aes_cmac_cipher_snow_test_case_1 = {
},
.cipher_iv = {
.data = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
},
-   .len = 0,
+   .len = 16,
},
.cipher = {
.len_bits = 516 << 3,
@@ -917,8 +919,10 @@ struct mixed_cipher_auth_test_data 
auth_aes_cmac_cipher_zuc_test_case_1 = {
},
.cipher_iv = {
.data = {
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
},
-   .len = 0,
+   .len = 16,
},
.cipher = {
.len_bits = 516 << 3,
-- 
2.25.1



[PATCH v7 3/3] test/crypto: fix failing synchronous tests

2023-09-07 Thread Brian Dooley
Some synchronous tests do not support digest encrypted and need to be
skipped. This commit adds in extra skips for these tests.

Fixes: 55ab4a8c4fb5 ("test/crypto: disable wireless cases for CPU crypto API")
Cc: pablo.de.lara.gua...@intel.com

Acked-by: Ciara Power 
Signed-off-by: Brian Dooley 
---
 app/test/test_cryptodev.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 956268bfcd..70f6b7ece1 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -6394,6 +6394,9 @@ test_zuc_auth_cipher(const struct wireless_test_data 
*tdata,
tdata->digest.len) < 0)
return TEST_SKIPPED;
 
+   if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
+   return TEST_SKIPPED;
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
 
uint64_t feat_flags = dev_info.feature_flags;
@@ -7829,6 +7832,9 @@ test_mixed_auth_cipher(const struct 
mixed_cipher_auth_test_data *tdata,
if (global_api_test_type == CRYPTODEV_RAW_API_TEST)
return TEST_SKIPPED;
 
+   if (gbl_action_type == RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO)
+   return TEST_SKIPPED;
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
 
uint64_t feat_flags = dev_info.feature_flags;
-- 
2.25.1



RE: quick thread in DLB2

2023-09-07 Thread Sevincer, Abdullah
Hi Stephen,
It is probing ports for best CPU. Yes it collects cycles. We may rework in the 
future.
Open to suggestions.

-Original Message-
From: Stephen Hemminger  
Sent: Wednesday, September 6, 2023 12:45 PM
To: Thomas Monjalon 
Cc: Sevincer, Abdullah ; dev@dpdk.org; Tyler 
Retzlaff 
Subject: Re: quick thread in DLB2

On Fri, 01 Sep 2023 16:08:48 +0200
Thomas Monjalon  wrote:

> Hello Abdullah,
> 
> In the DLB2 code, I see a thread is created for a single operation:
> In drivers/event/dlb2/pf/base/dlb2_resource.c
> pthread_create(&pthread, NULL, &dlb2_pp_profile_func, 
> &dlb2_thread_data[i]); and just after:
> pthread_join(pthread, NULL);
> 
> Can we avoid creating this thread?
> I guess no, because it must spawn on a specific CPU.
> 
> 

The per thread data seems to break lots of expectations in EAL.
It all seems to be about capturing the number of cycles on different cores.
Looks like a mess.


Re: quick thread in DLB2

2023-09-07 Thread Stephen Hemminger
On Thu, 7 Sep 2023 22:09:06 +
"Sevincer, Abdullah"  wrote:

> Hi Stephen,
> It is probing ports for best CPU. Yes it collects cycles. We may rework in 
> the future.
> Open to suggestions.

Why is this device a special snowflake? All threading should be using the 
existing
infrastructure of service and control threads. Doing it this way looks like a 
special
case optimization for testpmd.  What if the user is using a multi-stage 
pipeline,
flow rules, secondary processes, rings etc. Look at some of the real 
applications
using DPDK.  And talk to some of the network equipment vendors.


[PATCH v4 00/10] net/cpfl: support port representor

2023-09-07 Thread beilei . xing
From: Beilei Xing 

1. code refine for representor support
2. support port representor

v4 changes:
 - change the patch order
 - merge two patches
 - revert enum change
v3 changes:
 - Refine commit log.
 - Add macro and enum.
 - Refine doc.
 - Refine error handling.
v2 changes:
 - Remove representor data path.
 - Fix coding style.

Beilei Xing (10):
  net/cpfl: refine devargs parse and process
  net/cpfl: introduce interface structure
  net/cpfl: refine handle virtual channel message
  net/cpfl: introduce CP channel API
  net/cpfl: enable vport mapping
  net/cpfl: parse representor devargs
  net/cpfl: support probe again
  net/cpfl: support vport list/info get
  net/cpfl: create port representor
  net/cpfl: support link update for representor

 doc/guides/nics/cpfl.rst   |  36 ++
 doc/guides/rel_notes/release_23_11.rst |   3 +
 drivers/net/cpfl/cpfl_cpchnl.h | 340 ++
 drivers/net/cpfl/cpfl_ethdev.c | 619 +
 drivers/net/cpfl/cpfl_ethdev.h |  91 +++-
 drivers/net/cpfl/cpfl_representor.c| 607 
 drivers/net/cpfl/cpfl_representor.h|  26 ++
 drivers/net/cpfl/cpfl_vchnl.c  |  72 +++
 drivers/net/cpfl/meson.build   |   4 +-
 9 files changed, 1692 insertions(+), 106 deletions(-)
 create mode 100644 drivers/net/cpfl/cpfl_cpchnl.h
 create mode 100644 drivers/net/cpfl/cpfl_representor.c
 create mode 100644 drivers/net/cpfl/cpfl_representor.h
 create mode 100644 drivers/net/cpfl/cpfl_vchnl.c

-- 
2.34.1



[PATCH v4 01/10] net/cpfl: refine devargs parse and process

2023-09-07 Thread beilei . xing
From: Beilei Xing 

1. Keep devargs in adapter.
2. Refine handling the case with no vport be specified in devargs.
3. Separate devargs parse and devargs process

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c | 154 ++---
 drivers/net/cpfl/cpfl_ethdev.h |   1 +
 2 files changed, 84 insertions(+), 71 deletions(-)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index c4ca9343c3..46b3a52e49 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -1407,12 +1407,12 @@ parse_bool(const char *key, const char *value, void 
*args)
 }
 
 static int
-cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter,
-  struct cpfl_devargs *cpfl_args)
+cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter)
 {
struct rte_devargs *devargs = pci_dev->device.devargs;
+   struct cpfl_devargs *cpfl_args = &adapter->devargs;
struct rte_kvargs *kvlist;
-   int i, ret;
+   int ret;
 
cpfl_args->req_vport_nb = 0;
 
@@ -1445,31 +1445,6 @@ cpfl_parse_devargs(struct rte_pci_device *pci_dev, 
struct cpfl_adapter_ext *adap
if (ret != 0)
goto fail;
 
-   /* check parsed devargs */
-   if (adapter->cur_vport_nb + cpfl_args->req_vport_nb >
-   adapter->max_vport_nb) {
-   PMD_INIT_LOG(ERR, "Total vport number can't be > %d",
-adapter->max_vport_nb);
-   ret = -EINVAL;
-   goto fail;
-   }
-
-   for (i = 0; i < cpfl_args->req_vport_nb; i++) {
-   if (cpfl_args->req_vports[i] > adapter->max_vport_nb - 1) {
-   PMD_INIT_LOG(ERR, "Invalid vport id %d, it should be 0 
~ %d",
-cpfl_args->req_vports[i], 
adapter->max_vport_nb - 1);
-   ret = -EINVAL;
-   goto fail;
-   }
-
-   if (adapter->cur_vports & RTE_BIT32(cpfl_args->req_vports[i])) {
-   PMD_INIT_LOG(ERR, "Vport %d has been requested",
-cpfl_args->req_vports[i]);
-   ret = -EINVAL;
-   goto fail;
-   }
-   }
-
 fail:
rte_kvargs_free(kvlist);
return ret;
@@ -1915,15 +1890,79 @@ cpfl_adapter_ext_deinit(struct cpfl_adapter_ext 
*adapter)
adapter->vports = NULL;
 }
 
+static int
+cpfl_vport_devargs_process(struct cpfl_adapter_ext *adapter)
+{
+   struct cpfl_devargs *devargs = &adapter->devargs;
+   int i;
+
+   /* refine vport number, at least 1 vport */
+   if (devargs->req_vport_nb == 0) {
+   devargs->req_vport_nb = 1;
+   devargs->req_vports[0] = 0;
+   }
+
+   /* check parsed devargs */
+   if (adapter->cur_vport_nb + devargs->req_vport_nb >
+   adapter->max_vport_nb) {
+   PMD_INIT_LOG(ERR, "Total vport number can't be > %d",
+adapter->max_vport_nb);
+   return -EINVAL;
+   }
+
+   for (i = 0; i < devargs->req_vport_nb; i++) {
+   if (devargs->req_vports[i] > adapter->max_vport_nb - 1) {
+   PMD_INIT_LOG(ERR, "Invalid vport id %d, it should be 0 
~ %d",
+devargs->req_vports[i], 
adapter->max_vport_nb - 1);
+   return -EINVAL;
+   }
+
+   if (adapter->cur_vports & RTE_BIT32(devargs->req_vports[i])) {
+   PMD_INIT_LOG(ERR, "Vport %d has been requested",
+devargs->req_vports[i]);
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
+static int
+cpfl_vport_create(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter)
+{
+   struct cpfl_vport_param vport_param;
+   char name[RTE_ETH_NAME_MAX_LEN];
+   int ret, i;
+
+   for (i = 0; i < adapter->devargs.req_vport_nb; i++) {
+   vport_param.adapter = adapter;
+   vport_param.devarg_id = adapter->devargs.req_vports[i];
+   vport_param.idx = cpfl_vport_idx_alloc(adapter);
+   if (vport_param.idx == CPFL_INVALID_VPORT_IDX) {
+   PMD_INIT_LOG(ERR, "No space for vport %u", 
vport_param.devarg_id);
+   break;
+   }
+   snprintf(name, sizeof(name), "net_%s_vport_%d",
+pci_dev->device.name,
+adapter->devargs.req_vports[i]);
+   ret = rte_eth_dev_create(&pci_dev->device, name,
+   sizeof(struct cpfl_vport),
+   NULL, NULL, cpfl_dev_vport_init,
+   &vport_param);
+   if (ret != 0)
+   P

[PATCH v4 02/10] net/cpfl: introduce interface structure

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Introduce cplf interface structure to distinguish vport and port
representor.

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c |  3 +++
 drivers/net/cpfl/cpfl_ethdev.h | 16 
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 46b3a52e49..92fe92c00f 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -1803,6 +1803,9 @@ cpfl_dev_vport_init(struct rte_eth_dev *dev, void 
*init_params)
goto err;
}
 
+   cpfl_vport->itf.type = CPFL_ITF_TYPE_VPORT;
+   cpfl_vport->itf.adapter = adapter;
+   cpfl_vport->itf.data = dev->data;
adapter->vports[param->idx] = cpfl_vport;
adapter->cur_vports |= RTE_BIT32(param->devarg_id);
adapter->cur_vport_nb++;
diff --git a/drivers/net/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
index b637bf2e45..53e45035e8 100644
--- a/drivers/net/cpfl/cpfl_ethdev.h
+++ b/drivers/net/cpfl/cpfl_ethdev.h
@@ -86,7 +86,19 @@ struct p2p_queue_chunks_info {
uint32_t rx_buf_qtail_spacing;
 };
 
+enum cpfl_itf_type {
+   CPFL_ITF_TYPE_VPORT,
+   CPFL_ITF_TYPE_REPRESENTOR
+};
+
+struct cpfl_itf {
+   enum cpfl_itf_type type;
+   struct cpfl_adapter_ext *adapter;
+   void *data;
+};
+
 struct cpfl_vport {
+   struct cpfl_itf itf;
struct idpf_vport base;
struct p2p_queue_chunks_info *p2p_q_chunks_info;
 
@@ -124,5 +136,9 @@ TAILQ_HEAD(cpfl_adapter_list, cpfl_adapter_ext);
RTE_DEV_TO_PCI((eth_dev)->device)
 #define CPFL_ADAPTER_TO_EXT(p) \
container_of((p), struct cpfl_adapter_ext, base)
+#define CPFL_DEV_TO_VPORT(dev) \
+   ((struct cpfl_vport *)((dev)->data->dev_private))
+#define CPFL_DEV_TO_ITF(dev)   \
+   ((struct cpfl_itf *)((dev)->data->dev_private))
 
 #endif /* _CPFL_ETHDEV_H_ */
-- 
2.34.1



[PATCH v4 03/10] net/cpfl: refine handle virtual channel message

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Refine handle virtual channel event message.

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c | 46 --
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 92fe92c00f..6b6e9b37b1 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -1450,40 +1450,50 @@ cpfl_parse_devargs(struct rte_pci_device *pci_dev, 
struct cpfl_adapter_ext *adap
return ret;
 }
 
-static struct idpf_vport *
+static struct cpfl_vport *
 cpfl_find_vport(struct cpfl_adapter_ext *adapter, uint32_t vport_id)
 {
-   struct idpf_vport *vport = NULL;
+   struct cpfl_vport *vport = NULL;
int i;
 
for (i = 0; i < adapter->cur_vport_nb; i++) {
-   vport = &adapter->vports[i]->base;
-   if (vport->vport_id != vport_id)
+   vport = adapter->vports[i];
+   if (vport->base.vport_id != vport_id)
continue;
else
return vport;
}
 
-   return vport;
+   return NULL;
 }
 
 static void
-cpfl_handle_event_msg(struct idpf_vport *vport, uint8_t *msg, uint16_t msglen)
+cpfl_handle_vchnl_event_msg(struct cpfl_adapter_ext *adapter, uint8_t *msg, 
uint16_t msglen)
 {
struct virtchnl2_event *vc_event = (struct virtchnl2_event *)msg;
-   struct rte_eth_dev_data *data = vport->dev_data;
-   struct rte_eth_dev *dev = &rte_eth_devices[data->port_id];
+   struct cpfl_vport *vport;
+   struct rte_eth_dev_data *data;
+   struct rte_eth_dev *dev;
 
if (msglen < sizeof(struct virtchnl2_event)) {
PMD_DRV_LOG(ERR, "Error event");
return;
}
 
+   vport = cpfl_find_vport(adapter, vc_event->vport_id);
+   if (!vport) {
+   PMD_DRV_LOG(ERR, "Can't find vport.");
+   return;
+   }
+
+   data = vport->itf.data;
+   dev = &rte_eth_devices[data->port_id];
+
switch (vc_event->event) {
case VIRTCHNL2_EVENT_LINK_CHANGE:
PMD_DRV_LOG(DEBUG, "VIRTCHNL2_EVENT_LINK_CHANGE");
-   vport->link_up = !!(vc_event->link_status);
-   vport->link_speed = vc_event->link_speed;
+   vport->base.link_up = !!(vc_event->link_status);
+   vport->base.link_speed = vc_event->link_speed;
cpfl_dev_link_update(dev, 0);
break;
default:
@@ -1498,10 +1508,8 @@ cpfl_handle_virtchnl_msg(struct cpfl_adapter_ext 
*adapter)
struct idpf_adapter *base = &adapter->base;
struct idpf_dma_mem *dma_mem = NULL;
struct idpf_hw *hw = &base->hw;
-   struct virtchnl2_event *vc_event;
struct idpf_ctlq_msg ctlq_msg;
enum idpf_mbx_opc mbx_op;
-   struct idpf_vport *vport;
uint16_t pending = 1;
uint32_t vc_op;
int ret;
@@ -1523,18 +1531,8 @@ cpfl_handle_virtchnl_msg(struct cpfl_adapter_ext 
*adapter)
switch (mbx_op) {
case idpf_mbq_opc_send_msg_to_peer_pf:
if (vc_op == VIRTCHNL2_OP_EVENT) {
-   if (ctlq_msg.data_len < sizeof(struct 
virtchnl2_event)) {
-   PMD_DRV_LOG(ERR, "Error event");
-   return;
-   }
-   vc_event = (struct virtchnl2_event 
*)base->mbx_resp;
-   vport = cpfl_find_vport(adapter, 
vc_event->vport_id);
-   if (!vport) {
-   PMD_DRV_LOG(ERR, "Can't find vport.");
-   return;
-   }
-   cpfl_handle_event_msg(vport, base->mbx_resp,
- ctlq_msg.data_len);
+   cpfl_handle_vchnl_event_msg(adapter, 
adapter->base.mbx_resp,
+   ctlq_msg.data_len);
} else {
if (vc_op == base->pend_cmd)
notify_cmd(base, base->cmd_retval);
-- 
2.34.1



[PATCH v4 04/10] net/cpfl: introduce CP channel API

2023-09-07 Thread beilei . xing
From: Beilei Xing 

The CPCHNL2 defines the API (v2) used for communication between the
CPF driver and its on-chip management software. The CPFL PMD is a
specific CPF driver to utilize CPCHNL2 for device configuration and
event probing.

Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_cpchnl.h | 340 +
 1 file changed, 340 insertions(+)
 create mode 100644 drivers/net/cpfl/cpfl_cpchnl.h

diff --git a/drivers/net/cpfl/cpfl_cpchnl.h b/drivers/net/cpfl/cpfl_cpchnl.h
new file mode 100644
index 00..2eefcbcc10
--- /dev/null
+++ b/drivers/net/cpfl/cpfl_cpchnl.h
@@ -0,0 +1,340 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Intel Corporation
+ */
+
+#ifndef _CPFL_CPCHNL_H_
+#define _CPFL_CPCHNL_H_
+
+/** @brief  Command Opcodes
+ *  Values are to be different from virtchnl.h opcodes
+ */
+enum cpchnl2_ops {
+   /* vport info */
+   CPCHNL2_OP_GET_VPORT_LIST   = 0x8025,
+   CPCHNL2_OP_GET_VPORT_INFO   = 0x8026,
+
+   /* DPHMA Event notifications */
+   CPCHNL2_OP_EVENT= 0x8050,
+};
+
+/* Note! This affects the size of structs below */
+#define CPCHNL2_MAX_TC_AMOUNT  8
+
+#define CPCHNL2_ETH_LENGTH_OF_ADDRESS  6
+
+#define CPCHNL2_FUNC_TYPE_PF   0
+#define CPCHNL2_FUNC_TYPE_SRIOV1
+
+/* vport statuses - must match the DB ones - see enum cp_vport_status*/
+#define CPCHNL2_VPORT_STATUS_CREATED   0
+#define CPCHNL2_VPORT_STATUS_ENABLED   1
+#define CPCHNL2_VPORT_STATUS_DISABLED  2
+#define CPCHNL2_VPORT_STATUS_DESTROYED 3
+
+/* Queue Groups Extension */
+/**/
+
+#define MAX_Q_REGIONS 16
+/* TBD - with current structure sizes, in order not to exceed 4KB ICQH buffer
+ * no more than 11 queue groups are allowed per a single vport..
+ * More will be possible only with future msg fragmentation.
+ */
+#define MAX_Q_VPORT_GROUPS 11
+
+#define CPCHNL2_CHECK_STRUCT_LEN(n, X) enum static_assert_enum_##X \
+   { static_assert_##X = (n) / ((sizeof(struct X) == (n)) ? 1 : 0) }
+
+struct cpchnl2_queue_chunk {
+   u32 type;  /* 0:QUEUE_TYPE_TX, 1:QUEUE_TYPE_RX */ /* enum 
nsl_lan_queue_type */
+   u32 start_queue_id;
+   u32 num_queues;
+   u8 pad[4];
+};
+CPCHNL2_CHECK_STRUCT_LEN(16, cpchnl2_queue_chunk);
+
+/* structure to specify several chunks of contiguous queues */
+struct cpchnl2_queue_grp_chunks {
+   u16 num_chunks;
+   u8 reserved[6];
+   struct cpchnl2_queue_chunk chunks[MAX_Q_REGIONS];
+};
+CPCHNL2_CHECK_STRUCT_LEN(264, cpchnl2_queue_grp_chunks);
+
+struct cpchnl2_rx_queue_group_info {
+   /* User can ask to update rss_lut size originally allocated
+* by CreateVport command. New size will be returned if allocation 
succeeded,
+* otherwise original rss_size from CreateVport will be returned.
+*/
+   u16 rss_lut_size;
+   u8 pad[6]; /*Future extension purpose*/
+};
+CPCHNL2_CHECK_STRUCT_LEN(8, cpchnl2_rx_queue_group_info);
+
+struct cpchnl2_tx_queue_group_info {
+   u8 tx_tc; /*TX TC queue group will be connected to*/
+   /* Each group can have its own priority, value 0-7, while each group 
with unique
+* priority is strict priority. It can be single set of queue groups 
which configured with
+* same priority, then they are assumed part of WFQ arbitration group 
and are expected to be
+* assigned with weight.
+*/
+   u8 priority;
+   /* Determines if queue group is expected to be Strict Priority 
according to its priority */
+   u8 is_sp;
+   u8 pad;
+   /* Peak Info Rate Weight in case Queue Group is part of WFQ arbitration 
set.
+* The weights of the groups are independent of each other. Possible 
values: 1-200.
+*/
+   u16 pir_weight;
+   /* Future extension purpose for CIR only */
+   u8 cir_pad[2];
+   u8 pad2[8]; /* Future extension purpose*/
+};
+CPCHNL2_CHECK_STRUCT_LEN(16, cpchnl2_tx_queue_group_info);
+
+struct cpchnl2_queue_group_id {
+   /* Queue group ID - depended on it's type:
+* Data & p2p - is an index which is relative to Vport.
+* Config & Mailbox - is an ID which is relative to func.
+* This ID is used in future calls, i.e. delete.
+* Requested by host and assigned by Control plane.
+*/
+   u16 queue_group_id;
+   /* Functional type: see CPCHNL2_QUEUE_GROUP_TYPE definitions */
+   u16 queue_group_type;
+   u8 pad[4];
+};
+CPCHNL2_CHECK_STRUCT_LEN(8, cpchnl2_queue_group_id);
+
+struct cpchnl2_queue_group_info {
+   /* IN */
+   struct cpchnl2_queue_group_id qg_id;
+
+   /* IN, Number of queues of different types in the group. */
+   u16 num_tx_q;
+   u16 num_tx_complq;
+   u16 num_rx_q;
+   u16 num_rx_bufq;
+
+   struct cpchnl2_tx_queue_group_info tx_q_grp_info;
+   struct cpchnl2_rx_queue_group_info rx_q_grp_info

[PATCH v4 05/10] net/cpfl: enable vport mapping

2023-09-07 Thread beilei . xing
From: Beilei Xing 

1. Handle cpchnl event for vport create/destroy
2. Use hash table to store vport_id to vport_info mapping
3. Use spinlock for thread safe.

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c | 157 +
 drivers/net/cpfl/cpfl_ethdev.h |  21 -
 drivers/net/cpfl/meson.build   |   2 +-
 3 files changed, 177 insertions(+), 3 deletions(-)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 6b6e9b37b1..f51aa6e95a 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "cpfl_ethdev.h"
 #include "cpfl_rxtx.h"
@@ -1502,6 +1503,108 @@ cpfl_handle_vchnl_event_msg(struct cpfl_adapter_ext 
*adapter, uint8_t *msg, uint
}
 }
 
+static int
+cpfl_vport_info_create(struct cpfl_adapter_ext *adapter,
+  struct cpfl_vport_id *vport_identity,
+  struct cpchnl2_vport_info *vport_info)
+{
+   struct cpfl_vport_info *info = NULL;
+   int ret;
+
+   rte_spinlock_lock(&adapter->vport_map_lock);
+   ret = rte_hash_lookup_data(adapter->vport_map_hash, vport_identity, 
(void **)&info);
+   if (ret >= 0) {
+   PMD_DRV_LOG(WARNING, "vport already exist, overwrite info 
anyway");
+   /* overwrite info */
+   if (info)
+   info->vport_info = *vport_info;
+   goto fini;
+   }
+
+   info = rte_zmalloc(NULL, sizeof(*info), 0);
+   if (info == NULL) {
+   PMD_DRV_LOG(ERR, "Failed to alloc memory for vport map info");
+   ret = -ENOMEM;
+   goto err;
+   }
+
+   info->vport_info = *vport_info;
+
+   ret = rte_hash_add_key_data(adapter->vport_map_hash, vport_identity, 
info);
+   if (ret < 0) {
+   PMD_DRV_LOG(ERR, "Failed to add vport map into hash");
+   rte_free(info);
+   goto err;
+   }
+
+fini:
+   rte_spinlock_unlock(&adapter->vport_map_lock);
+   return 0;
+err:
+   rte_spinlock_unlock(&adapter->vport_map_lock);
+   return ret;
+}
+
+static int
+cpfl_vport_info_destroy(struct cpfl_adapter_ext *adapter, struct cpfl_vport_id 
*vport_identity)
+{
+   struct cpfl_vport_info *info;
+   int ret;
+
+   rte_spinlock_lock(&adapter->vport_map_lock);
+   ret = rte_hash_lookup_data(adapter->vport_map_hash, vport_identity, 
(void **)&info);
+   if (ret < 0) {
+   PMD_DRV_LOG(ERR, "vport id not exist");
+   goto err;
+   }
+
+   rte_hash_del_key(adapter->vport_map_hash, vport_identity);
+   rte_spinlock_unlock(&adapter->vport_map_lock);
+   rte_free(info);
+
+   return 0;
+
+err:
+   rte_spinlock_unlock(&adapter->vport_map_lock);
+   return ret;
+}
+
+static void
+cpfl_handle_cpchnl_event_msg(struct cpfl_adapter_ext *adapter, uint8_t *msg, 
uint16_t msglen)
+{
+   struct cpchnl2_event_info *cpchnl2_event = (struct cpchnl2_event_info 
*)msg;
+   struct cpchnl2_vport_info *info;
+   struct cpfl_vport_id vport_identity = { 0 };
+
+   if (msglen < sizeof(struct cpchnl2_event_info)) {
+   PMD_DRV_LOG(ERR, "Error event");
+   return;
+   }
+
+   switch (cpchnl2_event->header.type) {
+   case CPCHNL2_EVENT_VPORT_CREATED:
+   vport_identity.vport_id = 
cpchnl2_event->data.vport_created.vport.vport_id;
+   info = &cpchnl2_event->data.vport_created.info;
+   vport_identity.func_type = info->func_type;
+   vport_identity.pf_id = info->pf_id;
+   vport_identity.vf_id = info->vf_id;
+   if (cpfl_vport_info_create(adapter, &vport_identity, info))
+   PMD_DRV_LOG(WARNING, "Failed to handle 
CPCHNL2_EVENT_VPORT_CREATED");
+   break;
+   case CPCHNL2_EVENT_VPORT_DESTROYED:
+   vport_identity.vport_id = 
cpchnl2_event->data.vport_destroyed.vport.vport_id;
+   vport_identity.func_type = 
cpchnl2_event->data.vport_destroyed.func.func_type;
+   vport_identity.pf_id = 
cpchnl2_event->data.vport_destroyed.func.pf_id;
+   vport_identity.vf_id = 
cpchnl2_event->data.vport_destroyed.func.vf_id;
+   if (cpfl_vport_info_destroy(adapter, &vport_identity))
+   PMD_DRV_LOG(WARNING, "Failed to handle 
CPCHNL2_EVENT_VPORT_DESTROY");
+   break;
+   default:
+   PMD_DRV_LOG(ERR, " unknown event received %u", 
cpchnl2_event->header.type);
+   break;
+   }
+}
+
 static void
 cpfl_handle_virtchnl_msg(struct cpfl_adapter_ext *adapter)
 {
@@ -1533,6 +1636,9 @@ cpfl_handle_virtchnl_msg(struct cpfl_adapter_ext *adapter)
if (vc_op == VIRTCHNL2_OP_EVENT) {
cpfl_handle_vchnl_event_msg(adapter, 
adapter->base.mbx_resp,
 

[PATCH v4 06/10] net/cpfl: parse representor devargs

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Format:

[[c]pf]vf

  controller_id:

  0 : host (default)
  1:  acc

  pf_id:

  0 : apf (default)
  1 : cpf

Example:

representor=c0pf0vf[0-3]
  -- host > apf > vf 0,1,2,3
 same as pf0vf[0-3] and vf[0-3] if omit default value.

representor=c0pf0
  -- host > apf
 same as pf0 if omit default value.

representor=c1pf0
  -- accelerator core > apf

multiple representor devargs are supported.
e.g.: create 4 representors for 4 vfs on host APF and one
representor for APF on accelerator core.

  -- representor=vf[0-3],representor=c1pf0

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 doc/guides/nics/cpfl.rst   |  36 +
 doc/guides/rel_notes/release_23_11.rst |   3 +
 drivers/net/cpfl/cpfl_ethdev.c | 179 +
 drivers/net/cpfl/cpfl_ethdev.h |   8 ++
 4 files changed, 226 insertions(+)

diff --git a/doc/guides/nics/cpfl.rst b/doc/guides/nics/cpfl.rst
index 39a2b603f3..83a18c3f2e 100644
--- a/doc/guides/nics/cpfl.rst
+++ b/doc/guides/nics/cpfl.rst
@@ -92,6 +92,42 @@ Runtime Configuration
   Then the PMD will configure Tx queue with single queue mode.
   Otherwise, split queue mode is chosen by default.
 
+- ``representor`` (default ``not enabled``)
+
+  The cpfl PMD supports the creation of APF/CPF/VF port representors.
+  Each port representor corresponds to a single function of that device.
+  Using the ``devargs`` option ``representor`` the user can specify
+  which functions to create port representors.
+
+  Format is::
+
+[[c]pf]vf
+
+  Controller_id 0 is host (default), while 1 is accelerator core.
+  Pf_id 0 is APF (default), while 1 is CPF.
+  Default value can be omitted.
+
+  Create 4 representors for 4 vfs on host APF::
+
+-a BDF,representor=c0pf0vf[0-3]
+
+  Or::
+
+-a BDF,representor=pf0vf[0-3]
+
+  Or::
+
+-a BDF,representor=vf[0-3]
+
+  Create a representor for CPF on accelerator core::
+
+-a BDF,representor=c1pf1
+
+  Multiple representor devargs are supported. Create 4 representors for 4
+  vfs on host APF and one representor for CPF on accelerator core::
+
+-a BDF,representor=vf[0-3],representor=c1pf1
+
 
 Driver compilation and testing
 --
diff --git a/doc/guides/rel_notes/release_23_11.rst 
b/doc/guides/rel_notes/release_23_11.rst
index 333e1d95a2..3d9be208d0 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -78,6 +78,9 @@ New Features
 * build: Optional libraries can now be selected with the new ``enable_libs``
   build option similarly to the existing ``enable_drivers`` build option.
 
+* **Updated Intel cpfl driver.**
+
+  * Added support for port representor.
 
 Removed Items
 -
diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index f51aa6e95a..1b21134ec1 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -13,8 +13,10 @@
 #include 
 
 #include "cpfl_ethdev.h"
+#include 
 #include "cpfl_rxtx.h"
 
+#define CPFL_REPRESENTOR   "representor"
 #define CPFL_TX_SINGLE_Q   "tx_single"
 #define CPFL_RX_SINGLE_Q   "rx_single"
 #define CPFL_VPORT "vport"
@@ -25,6 +27,7 @@ struct cpfl_adapter_list cpfl_adapter_list;
 bool cpfl_adapter_list_init;
 
 static const char * const cpfl_valid_args[] = {
+   CPFL_REPRESENTOR,
CPFL_TX_SINGLE_Q,
CPFL_RX_SINGLE_Q,
CPFL_VPORT,
@@ -1407,6 +1410,128 @@ parse_bool(const char *key, const char *value, void 
*args)
return 0;
 }
 
+static int
+enlist(uint16_t *list, uint16_t *len_list, const uint16_t max_list, uint16_t 
val)
+{
+   uint16_t i;
+
+   for (i = 0; i < *len_list; i++) {
+   if (list[i] == val)
+   return 0;
+   }
+   if (*len_list >= max_list)
+   return -1;
+   list[(*len_list)++] = val;
+   return 0;
+}
+
+static const char *
+process_range(const char *str, uint16_t *list, uint16_t *len_list,
+   const uint16_t max_list)
+{
+   uint16_t lo, hi, val;
+   int result, n = 0;
+   const char *pos = str;
+
+   result = sscanf(str, "%hu%n-%hu%n", &lo, &n, &hi, &n);
+   if (result == 1) {
+   if (enlist(list, len_list, max_list, lo) != 0)
+   return NULL;
+   } else if (result == 2) {
+   if (lo > hi)
+   return NULL;
+   for (val = lo; val <= hi; val++) {
+   if (enlist(list, len_list, max_list, val) != 0)
+   return NULL;
+   }
+   } else {
+   return NULL;
+   }
+   return pos + n;
+}
+
+static const char *
+process_list(const char *str, uint16_t *list, uint16_t *len_list, const 
uint16_t max_list)
+{
+   const char *pos = str;
+
+   if (*pos == '[')
+   pos++;
+   while (1) {
+   pos = process_range(pos, list, len_list, max_list);
+   if (pos == 

[PATCH v4 07/10] net/cpfl: support probe again

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Only representor will be parsed for probe again.

Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c | 69 +++---
 1 file changed, 56 insertions(+), 13 deletions(-)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 1b21134ec1..236347eeb3 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -26,7 +26,7 @@ rte_spinlock_t cpfl_adapter_lock;
 struct cpfl_adapter_list cpfl_adapter_list;
 bool cpfl_adapter_list_init;
 
-static const char * const cpfl_valid_args[] = {
+static const char * const cpfl_valid_args_first[] = {
CPFL_REPRESENTOR,
CPFL_TX_SINGLE_Q,
CPFL_RX_SINGLE_Q,
@@ -34,6 +34,11 @@ static const char * const cpfl_valid_args[] = {
NULL
 };
 
+static const char * const cpfl_valid_args_again[] = {
+   CPFL_REPRESENTOR,
+   NULL
+};
+
 uint32_t cpfl_supported_speeds[] = {
RTE_ETH_SPEED_NUM_NONE,
RTE_ETH_SPEED_NUM_10M,
@@ -1533,7 +1538,7 @@ parse_repr(const char *key __rte_unused, const char 
*value, void *args)
 }
 
 static int
-cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter)
+cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter, bool first)
 {
struct rte_devargs *devargs = pci_dev->device.devargs;
struct cpfl_devargs *cpfl_args = &adapter->devargs;
@@ -1545,7 +1550,8 @@ cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct 
cpfl_adapter_ext *adap
if (devargs == NULL)
return 0;
 
-   kvlist = rte_kvargs_parse(devargs->args, cpfl_valid_args);
+   kvlist = rte_kvargs_parse(devargs->args,
+   first ? cpfl_valid_args_first : cpfl_valid_args_again);
if (kvlist == NULL) {
PMD_INIT_LOG(ERR, "invalid kvargs key");
return -EINVAL;
@@ -1562,6 +1568,9 @@ cpfl_parse_devargs(struct rte_pci_device *pci_dev, struct 
cpfl_adapter_ext *adap
if (ret != 0)
goto fail;
 
+   if (!first)
+   return 0;
+
ret = rte_kvargs_process(kvlist, CPFL_VPORT, &parse_vport,
 cpfl_args);
if (ret != 0)
@@ -2289,18 +2298,11 @@ cpfl_vport_create(struct rte_pci_device *pci_dev, 
struct cpfl_adapter_ext *adapt
 }
 
 static int
-cpfl_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
-  struct rte_pci_device *pci_dev)
+cpfl_pci_probe_first(struct rte_pci_device *pci_dev)
 {
struct cpfl_adapter_ext *adapter;
int retval;
 
-   if (!cpfl_adapter_list_init) {
-   rte_spinlock_init(&cpfl_adapter_lock);
-   TAILQ_INIT(&cpfl_adapter_list);
-   cpfl_adapter_list_init = true;
-   }
-
adapter = rte_zmalloc("cpfl_adapter_ext",
  sizeof(struct cpfl_adapter_ext), 0);
if (adapter == NULL) {
@@ -2308,7 +2310,7 @@ cpfl_pci_probe(struct rte_pci_driver *pci_drv 
__rte_unused,
return -ENOMEM;
}
 
-   retval = cpfl_parse_devargs(pci_dev, adapter);
+   retval = cpfl_parse_devargs(pci_dev, adapter, true);
if (retval != 0) {
PMD_INIT_LOG(ERR, "Failed to parse private devargs");
return retval;
@@ -2353,6 +2355,46 @@ cpfl_pci_probe(struct rte_pci_driver *pci_drv 
__rte_unused,
return retval;
 }
 
+static int
+cpfl_pci_probe_again(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter)
+{
+   int ret;
+
+   ret = cpfl_parse_devargs(pci_dev, adapter, false);
+   if (ret != 0) {
+   PMD_INIT_LOG(ERR, "Failed to parse private devargs");
+   return ret;
+   }
+
+   ret = cpfl_repr_devargs_process(adapter);
+   if (ret != 0) {
+   PMD_INIT_LOG(ERR, "Failed to process reprenstor devargs");
+   return ret;
+   }
+
+   return 0;
+}
+
+static int
+cpfl_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
+  struct rte_pci_device *pci_dev)
+{
+   struct cpfl_adapter_ext *adapter;
+
+   if (!cpfl_adapter_list_init) {
+   rte_spinlock_init(&cpfl_adapter_lock);
+   TAILQ_INIT(&cpfl_adapter_list);
+   cpfl_adapter_list_init = true;
+   }
+
+   adapter = cpfl_find_adapter_ext(pci_dev);
+
+   if (adapter == NULL)
+   return cpfl_pci_probe_first(pci_dev);
+   else
+   return cpfl_pci_probe_again(pci_dev, adapter);
+}
+
 static int
 cpfl_pci_remove(struct rte_pci_device *pci_dev)
 {
@@ -2375,7 +2417,8 @@ cpfl_pci_remove(struct rte_pci_device *pci_dev)
 
 static struct rte_pci_driver rte_cpfl_pmd = {
.id_table   = pci_id_cpfl_map,
-   .drv_flags  = RTE_PCI_DRV_NEED_MAPPING,
+   .drv_flags  = RTE_PCI_DRV_NEED_MAPPING |
+ RTE_PCI_DRV_PROBE_AGAIN,
.probe  = cpfl_pci_probe,
  

[PATCH v4 08/10] net/cpfl: support vport list/info get

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Support cp channel ops CPCHNL2_OP_CPF_GET_VPORT_LIST and
CPCHNL2_OP_CPF_GET_VPORT_INFO.

Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.h |  8 
 drivers/net/cpfl/cpfl_vchnl.c  | 72 ++
 drivers/net/cpfl/meson.build   |  1 +
 3 files changed, 81 insertions(+)
 create mode 100644 drivers/net/cpfl/cpfl_vchnl.c

diff --git a/drivers/net/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
index 9c4d8d3ea1..a501ccde14 100644
--- a/drivers/net/cpfl/cpfl_ethdev.h
+++ b/drivers/net/cpfl/cpfl_ethdev.h
@@ -157,6 +157,14 @@ struct cpfl_adapter_ext {
 
 TAILQ_HEAD(cpfl_adapter_list, cpfl_adapter_ext);
 
+int cpfl_cc_vport_list_get(struct cpfl_adapter_ext *adapter,
+  struct cpfl_vport_id *vi,
+  struct cpchnl2_get_vport_list_response *response);
+int cpfl_cc_vport_info_get(struct cpfl_adapter_ext *adapter,
+  struct cpchnl2_vport_id *vport_id,
+  struct cpfl_vport_id *vi,
+  struct cpchnl2_get_vport_info_response *response);
+
 #define CPFL_DEV_TO_PCI(eth_dev)   \
RTE_DEV_TO_PCI((eth_dev)->device)
 #define CPFL_ADAPTER_TO_EXT(p) \
diff --git a/drivers/net/cpfl/cpfl_vchnl.c b/drivers/net/cpfl/cpfl_vchnl.c
new file mode 100644
index 00..a21a4a451f
--- /dev/null
+++ b/drivers/net/cpfl/cpfl_vchnl.c
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Intel Corporation
+ */
+
+#include "cpfl_ethdev.h"
+#include 
+
+int
+cpfl_cc_vport_list_get(struct cpfl_adapter_ext *adapter,
+  struct cpfl_vport_id *vi,
+  struct cpchnl2_get_vport_list_response *response)
+{
+   struct cpchnl2_get_vport_list_request request;
+   struct idpf_cmd_info args;
+   int err;
+
+   memset(&request, 0, sizeof(request));
+   request.func_type = vi->func_type;
+   request.pf_id = vi->pf_id;
+   request.vf_id = vi->vf_id;
+
+   memset(&args, 0, sizeof(args));
+   args.ops = CPCHNL2_OP_GET_VPORT_LIST;
+   args.in_args = (uint8_t *)&request;
+   args.in_args_size = sizeof(struct cpchnl2_get_vport_list_request);
+   args.out_buffer = adapter->base.mbx_resp;
+   args.out_size = IDPF_DFLT_MBX_BUF_SIZE;
+
+   err = idpf_vc_cmd_execute(&adapter->base, &args);
+   if (err != 0) {
+   PMD_DRV_LOG(ERR, "Failed to execute command of 
CPCHNL2_OP_GET_VPORT_LIST");
+   return err;
+   }
+
+   rte_memcpy(response, args.out_buffer, IDPF_DFLT_MBX_BUF_SIZE);
+
+   return 0;
+}
+
+int
+cpfl_cc_vport_info_get(struct cpfl_adapter_ext *adapter,
+  struct cpchnl2_vport_id *vport_id,
+  struct cpfl_vport_id *vi,
+  struct cpchnl2_get_vport_info_response *response)
+{
+   struct cpchnl2_get_vport_info_request request;
+   struct idpf_cmd_info args;
+   int err;
+
+   request.vport.vport_id = vport_id->vport_id;
+   request.vport.vport_type = vport_id->vport_type;
+   request.func.func_type = vi->func_type;
+   request.func.pf_id = vi->pf_id;
+   request.func.vf_id = vi->vf_id;
+
+   memset(&args, 0, sizeof(args));
+   args.ops = CPCHNL2_OP_GET_VPORT_INFO;
+   args.in_args = (uint8_t *)&request;
+   args.in_args_size = sizeof(struct cpchnl2_get_vport_info_request);
+   args.out_buffer = adapter->base.mbx_resp;
+   args.out_size = IDPF_DFLT_MBX_BUF_SIZE;
+
+   err = idpf_vc_cmd_execute(&adapter->base, &args);
+   if (err != 0) {
+   PMD_DRV_LOG(ERR, "Failed to execute command of 
CPCHNL2_OP_GET_VPORT_INFO");
+   return err;
+   }
+
+   rte_memcpy(response, args.out_buffer, sizeof(*response));
+
+   return 0;
+}
diff --git a/drivers/net/cpfl/meson.build b/drivers/net/cpfl/meson.build
index 28167bb81d..2f0f5d8434 100644
--- a/drivers/net/cpfl/meson.build
+++ b/drivers/net/cpfl/meson.build
@@ -16,6 +16,7 @@ deps += ['hash', 'common_idpf']
 sources = files(
 'cpfl_ethdev.c',
 'cpfl_rxtx.c',
+'cpfl_vchnl.c',
 )
 
 if arch_subdir == 'x86'
-- 
2.34.1



[PATCH v4 09/10] net/cpfl: create port representor

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Track representor request in the allowlist.
Representor will only be created for active vport.

Signed-off-by: Jingjing Wu 
Signed-off-by: Qi Zhang 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.c  | 109 +++---
 drivers/net/cpfl/cpfl_ethdev.h  |  36 ++
 drivers/net/cpfl/cpfl_representor.c | 586 
 drivers/net/cpfl/cpfl_representor.h |  26 ++
 drivers/net/cpfl/meson.build|   1 +
 5 files changed, 714 insertions(+), 44 deletions(-)
 create mode 100644 drivers/net/cpfl/cpfl_representor.c
 create mode 100644 drivers/net/cpfl/cpfl_representor.h

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 236347eeb3..330a865e3c 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -1643,7 +1643,7 @@ cpfl_handle_vchnl_event_msg(struct cpfl_adapter_ext 
*adapter, uint8_t *msg, uint
}
 }
 
-static int
+int
 cpfl_vport_info_create(struct cpfl_adapter_ext *adapter,
   struct cpfl_vport_id *vport_identity,
   struct cpchnl2_vport_info *vport_info)
@@ -1896,6 +1896,42 @@ cpfl_vport_map_uninit(struct cpfl_adapter_ext *adapter)
rte_hash_free(adapter->vport_map_hash);
 }
 
+static int
+cpfl_repr_allowlist_init(struct cpfl_adapter_ext *adapter)
+{
+   char hname[32];
+
+   snprintf(hname, 32, "%s-repr_wl", adapter->name);
+
+   rte_spinlock_init(&adapter->repr_lock);
+
+#define CPFL_REPR_HASH_ENTRY_NUM 2048
+
+   struct rte_hash_parameters params = {
+   .name = hname,
+   .entries = CPFL_REPR_HASH_ENTRY_NUM,
+   .key_len = sizeof(struct cpfl_repr_id),
+   .hash_func = rte_hash_crc,
+   .socket_id = SOCKET_ID_ANY,
+   };
+
+   adapter->repr_allowlist_hash = rte_hash_create(¶ms);
+
+   if (adapter->repr_allowlist_hash == NULL) {
+   PMD_INIT_LOG(ERR, "Failed to create repr allowlist hash");
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static void
+cpfl_repr_allowlist_uninit(struct cpfl_adapter_ext *adapter)
+{
+   rte_hash_free(adapter->repr_allowlist_hash);
+}
+
+
 static int
 cpfl_adapter_ext_init(struct rte_pci_device *pci_dev, struct cpfl_adapter_ext 
*adapter)
 {
@@ -1926,6 +1962,12 @@ cpfl_adapter_ext_init(struct rte_pci_device *pci_dev, 
struct cpfl_adapter_ext *a
goto err_vport_map_init;
}
 
+   ret = cpfl_repr_allowlist_init(adapter);
+   if (ret) {
+   PMD_INIT_LOG(ERR, "Failed to init representor allowlist");
+   goto err_repr_allowlist_init;
+   }
+
rte_eal_alarm_set(CPFL_ALARM_INTERVAL, cpfl_dev_alarm_handler, adapter);
 
adapter->max_vport_nb = adapter->base.caps.max_vports > 
CPFL_MAX_VPORT_NUM ?
@@ -1950,6 +1992,8 @@ cpfl_adapter_ext_init(struct rte_pci_device *pci_dev, 
struct cpfl_adapter_ext *a
 
 err_vports_alloc:
rte_eal_alarm_cancel(cpfl_dev_alarm_handler, adapter);
+   cpfl_repr_allowlist_uninit(adapter);
+err_repr_allowlist_init:
cpfl_vport_map_uninit(adapter);
 err_vport_map_init:
idpf_adapter_deinit(base);
@@ -2225,48 +2269,6 @@ cpfl_vport_devargs_process(struct cpfl_adapter_ext 
*adapter)
return 0;
 }
 
-static int
-cpfl_repr_devargs_process(struct cpfl_adapter_ext *adapter)
-{
-   struct cpfl_devargs *devargs = &adapter->devargs;
-   int i, j;
-
-   /* check and refine repr args */
-   for (i = 0; i < devargs->repr_args_num; i++) {
-   struct rte_eth_devargs *eth_da = &devargs->repr_args[i];
-
-   /* set default host_id to xeon host */
-   if (eth_da->nb_mh_controllers == 0) {
-   eth_da->nb_mh_controllers = 1;
-   eth_da->mh_controllers[0] = CPFL_HOST_ID_HOST;
-   } else {
-   for (j = 0; j < eth_da->nb_mh_controllers; j++) {
-   if (eth_da->mh_controllers[j] > 
CPFL_HOST_ID_ACC) {
-   PMD_INIT_LOG(ERR, "Invalid Host ID %d",
-eth_da->mh_controllers[j]);
-   return -EINVAL;
-   }
-   }
-   }
-
-   /* set default pf to APF */
-   if (eth_da->nb_ports == 0) {
-   eth_da->nb_ports = 1;
-   eth_da->ports[0] = CPFL_PF_TYPE_APF;
-   } else {
-   for (j = 0; j < eth_da->nb_ports; j++) {
-   if (eth_da->ports[j] > CPFL_PF_TYPE_CPF) {
-   PMD_INIT_LOG(ERR, "Invalid Host ID %d",
-eth_da->ports[j]);
-   return -EINVAL;
-   }
-   }
-   }
-   }
-
-   

[PATCH v4 10/10] net/cpfl: support link update for representor

2023-09-07 Thread beilei . xing
From: Beilei Xing 

Add link update ops for representor.

Signed-off-by: Jingjing Wu 
Signed-off-by: Beilei Xing 
---
 drivers/net/cpfl/cpfl_ethdev.h  |  1 +
 drivers/net/cpfl/cpfl_representor.c | 21 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
index 4937d2c6e3..0dd9d4e7f9 100644
--- a/drivers/net/cpfl/cpfl_ethdev.h
+++ b/drivers/net/cpfl/cpfl_ethdev.h
@@ -162,6 +162,7 @@ struct cpfl_repr {
struct cpfl_repr_id repr_id;
struct rte_ether_addr mac_addr;
struct cpfl_vport_info *vport_info;
+   bool func_up; /* If the represented function is up */
 };
 
 struct cpfl_adapter_ext {
diff --git a/drivers/net/cpfl/cpfl_representor.c 
b/drivers/net/cpfl/cpfl_representor.c
index 0cd92b1351..3c0fa957de 100644
--- a/drivers/net/cpfl/cpfl_representor.c
+++ b/drivers/net/cpfl/cpfl_representor.c
@@ -308,6 +308,23 @@ cpfl_repr_tx_queue_setup(__rte_unused struct rte_eth_dev 
*dev,
return 0;
 }
 
+static int
+cpfl_repr_link_update(struct rte_eth_dev *ethdev,
+ __rte_unused int wait_to_complete)
+{
+   struct cpfl_repr *repr = CPFL_DEV_TO_REPR(ethdev);
+   struct rte_eth_link *dev_link = ðdev->data->dev_link;
+
+   if (!(ethdev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR)) {
+   PMD_INIT_LOG(ERR, "This ethdev is not representor.");
+   return -EINVAL;
+   }
+   dev_link->link_status = repr->func_up ?
+   RTE_ETH_LINK_UP : RTE_ETH_LINK_DOWN;
+
+   return 0;
+}
+
 static const struct eth_dev_ops cpfl_repr_dev_ops = {
.dev_start  = cpfl_repr_dev_start,
.dev_stop   = cpfl_repr_dev_stop,
@@ -317,6 +334,8 @@ static const struct eth_dev_ops cpfl_repr_dev_ops = {
 
.rx_queue_setup = cpfl_repr_rx_queue_setup,
.tx_queue_setup = cpfl_repr_tx_queue_setup,
+
+   .link_update= cpfl_repr_link_update,
 };
 
 static int
@@ -331,6 +350,8 @@ cpfl_repr_init(struct rte_eth_dev *eth_dev, void 
*init_param)
repr->itf.type = CPFL_ITF_TYPE_REPRESENTOR;
repr->itf.adapter = adapter;
repr->itf.data = eth_dev->data;
+   if (repr->vport_info->vport_info.vport_status == 
CPCHNL2_VPORT_STATUS_ENABLED)
+   repr->func_up = true;
 
eth_dev->dev_ops = &cpfl_repr_dev_ops;
 
-- 
2.34.1



[PATCH v2] ethdev: add TCP/IP modify field IDs

2023-09-07 Thread Suanming Mou
Currently, get TCP/IP header or data length information from traffic
is missing in the modify field IDs. This commit adds the missing
TCP data_offset, IPv4 IHL/total_len, IPv6 payload_len to modify filed
IDs. This allows users be able to manager more TCP/IP fields.

Signed-off-by: Suanming Mou 
---

v2: fix typo tcp_date_off -> tcp_data_off

---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h   | 4 
 2 files changed, 5 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 94827bcc4a..310068ce88 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -937,6 +937,7 @@ static const char *const modify_field_ids[] = {
"flex_item",
"hash_result",
"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
+   "tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 2ebb76dbc0..43ba51da6e 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3875,6 +3875,10 @@ enum rte_flow_field_id {
RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class. */
RTE_FLOW_FIELD_GENEVE_OPT_DATA, /**< GENEVE option data. */
RTE_FLOW_FIELD_MPLS,/**< MPLS header. */
+   RTE_FLOW_FIELD_TCP_DATA_OFFSET, /**< TCP data offset. */
+   RTE_FLOW_FIELD_IPV4_IHL,/**< IPv4 IHL. */
+   RTE_FLOW_FIELD_IPV4_TOTAL_LEN,  /**< IPv4 total length. */
+   RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN /**< IPv6 payload length. */
 };
 
 /**
-- 
2.34.1



Re: [PATCH 02/11] eal: rename thread name length definition

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:19PM +0200, Thomas Monjalon wrote:
> RTE_MAX_THREAD_NAME_LEN is including the NUL character,
> so it should be named "size" instead of "length".
> A new constant RTE_THREAD_NAME_SIZE is introduced for naming accuracy.
> For API compatibility, the old name is kept.
> 
> At the same time, the original definition is moved
> from rte_eal.h to rte_thread.h.
> 
> Signed-off-by: Thomas Monjalon 
> ---
lgtm

Acked-by: Tyler Retzlaff 



Re: [PATCH 03/11] eal: remove attributes from control thread creation

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:20PM +0200, Thomas Monjalon wrote:
> The experimental function rte_thread_create_control()
> is supposed to wrap actions needed to create a control thread in DPDK.
> This function should be easy to port on any OS.
> 
> As such, the thread attributes should not be customizable in this API.
> The thread priority should be normal, and the affinity is on "free cores".
> That's why the custom attributes parameter thread_attr is dropped.
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Tyler Retzlaff 



Re: [PATCH 04/11] eal: promote thread API as stable

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:21PM +0200, Thomas Monjalon wrote:
> The rte_thread API must be used to ease OS porting.
> One step of this process is to mark the necessary API as stable.
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Tyler Retzlaff 



Re: [PATCH 08/11] examples: convert to normal control threads

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:25PM +0200, Thomas Monjalon wrote:
> Calls to rte_ctrl_thread_create() are replaced with
> rte_thread_create_control().
> 
> In vhost_blk, the control thread is not forced
> to be scheduled on core 0 anymore.
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Tyler Retzlaff 



Re: [PATCH 09/11] test: convert threads creation

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:26PM +0200, Thomas Monjalon wrote:
> Calls to pthread for thread creation are replaced with the rte_thread API.
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Tyler Retzlaff 



Re: [PATCH 10/11] eal: remove deprecated thread functions

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:27PM +0200, Thomas Monjalon wrote:
> The deprecated functions rte_thread_setname() and rte_ctrl_thread_create()
> are replaced with the new rte_thread API:
> 
>   rte_thread_setname()
> can be replaced with
>   rte_thread_set_name()
> orrte_thread_set_prefixed_name()
> 
>   rte_ctrl_thread_create()
> can be replaced with
>   rte_thread_create_control()
> orrte_thread_create_internal_control()
> 
> Signed-off-by: Thomas Monjalon 
> ---

Acked-by: Tyler Retzlaff 

with suggestions.

...

>  ABI Changes
> diff --git a/lib/eal/common/eal_common_thread.c 
> b/lib/eal/common/eal_common_thread.c
> index 31c37e3102..78f643af73 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -248,7 +248,7 @@ struct rte_thread_ctrl_params {
>   enum __rte_ctrl_thread_status ctrl_thread_status;
>  };

the code above here

struct rte_thread_ctrl_params i think can now get renamed to
rte_thread_control_params and i think we can get rid of the union.
if i look through the code history i only added it to help maintain
compatibility while we had both ctrl and control thread APIs.

>  
> -static int ctrl_thread_init(void *arg)
> +static int control_thread_init(void *arg)
>  {
>   struct internal_config *internal_conf =
>   eal_get_internal_configuration();
> @@ -273,80 +273,18 @@ static int ctrl_thread_init(void *arg)
>   return 0;
>  }


Re: [PATCH 11/11] lib: remove pthread.h from includes

2023-09-07 Thread Tyler Retzlaff
On Wed, Sep 06, 2023 at 06:12:28PM +0200, Thomas Monjalon wrote:
> The header files should have the minimum embedded includes.
> The file pthread.h can logically be removed from
> rte_per_lcore.h and rte_ethdev_core.h files.
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Tyler Retzlaff 



Re: [PATCH 00/11] rework thread management

2023-09-07 Thread Tyler Retzlaff
On Thu, Sep 07, 2023 at 10:30:59AM +0200, Morten Brørup wrote:
> > From: Thomas Monjalon [mailto:tho...@monjalon.net]
> > Sent: Wednesday, 6 September 2023 18.12
> > 
> > The main effect of this patch series is to
> > remove calls to pthread functions except for pthread_cancel and locks.
> > 
> > The function rte_thread_create_control() does not take attributes anymore
> > as it looks a useless complication of the API.
> 
> Note for other reviewers: The "args" parameter, passed to the thread 
> function, is still there.
> 
> > Then the rte_thread API is made stable,
> > so we can remove the old deprecated functions
> > rte_thread_setname() and rte_ctrl_thread_create().
> > 
> > Some new internal functions are added in rte_thread to make sure
> > all internal thread names are prefixed with "dpdk-".
> > 
> > Few other cleanups are done.
> > 
> > Future work about pthread portability are about:
> >   - cancel
> >   - mutex
> > 
> > 
> > Thomas Monjalon (11):
> >   devtools: warn when adding some pthread calls
> >   eal: rename thread name length definition
> >   eal: remove attributes from control thread creation
> >   eal: promote thread API as stable
> >   eal: force prefix for internal threads
> >   lib: convert to internal control threads
> >   drivers: convert to internal control threads
> >   examples: convert to normal control threads
> >   test: convert threads creation
> >   eal: remove deprecated thread functions
> >   lib: remove pthread.h from includes
> 
> Thank you for cleaning all this up, Thomas.
> 
> Series-acked-by: Morten Brørup 

+1

thank you very much, this has been on my todo list it's really
appreciated!

Series-acked-by: Tyler Retzlaff 


Re: [PATCH 11/11] lib: remove pthread.h from includes

2023-09-07 Thread Ajit Khaparde
On Thu, Sep 7, 2023 at 9:25 PM Tyler Retzlaff
 wrote:
>
> On Wed, Sep 06, 2023 at 06:12:28PM +0200, Thomas Monjalon wrote:
> > The header files should have the minimum embedded includes.
> > The file pthread.h can logically be removed from
> > rte_per_lcore.h and rte_ethdev_core.h files.
> >
> > Signed-off-by: Thomas Monjalon 
> > ---
> Acked-by: Tyler Retzlaff 

Acked-by: Ajit Khaparde 

>


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH v4] node: add IPv4 reassembly node

2023-09-07 Thread Nithin Dabilpuram
Acked-by: Nithin Dabilpuram 


On Thu, Jul 27, 2023 at 8:07 PM  wrote:
>
> From: Pavan Nikhilesh 
>
> Add IPv4 reassembly node.
>
> Signed-off-by: Pavan Nikhilesh 
> ---
>  v4 Changes:
>  - Add packet drop node as the 0th edge.
>  - Free deathrow packets to packet drop node.
>  v3 Changes:
>  - Actually include the changes that fix compilation.
>  v2 Changes:
>  - Fix compilation.
>
>  doc/guides/prog_guide/graph_lib.rst |   8 ++
>  lib/node/ethdev_rx.c|   1 +
>  lib/node/ethdev_rx_priv.h   |   1 +
>  lib/node/ip4_reassembly.c   | 186 
>  lib/node/ip4_reassembly_priv.h  |  28 +
>  lib/node/meson.build|   3 +-
>  lib/node/rte_node_ip4_api.h |  37 ++
>  lib/node/version.map|   1 +
>  8 files changed, 264 insertions(+), 1 deletion(-)
>  create mode 100644 lib/node/ip4_reassembly.c
>  create mode 100644 lib/node/ip4_reassembly_priv.h
>
> diff --git a/doc/guides/prog_guide/graph_lib.rst 
> b/doc/guides/prog_guide/graph_lib.rst
> index e7b6e12004..10d146e2f6 100644
> --- a/doc/guides/prog_guide/graph_lib.rst
> +++ b/doc/guides/prog_guide/graph_lib.rst
> @@ -453,6 +453,14 @@ to determine the L2 header to be written to the packet 
> before sending
>  the packet out to a particular ethdev_tx node.
>  ``rte_node_ip4_rewrite_add()`` is control path API to add next-hop info.
>
> +ip4_reassembly
> +~~
> +This node is an intermediate node that reassembles ipv4 fragmented packets,
> +non-fragmented packets pass through the node un-effected. The node rewrites
> +it's stream and moves it to the next node.
> +The fragment table and death row table should be setup via the
> +``rte_node_ip4_reassembly_configure`` API.
> +
>  ip6_lookup
>  ~~
>  This node is an intermediate node that does LPM lookup for the received
> diff --git a/lib/node/ethdev_rx.c b/lib/node/ethdev_rx.c
> index d131034991..3e8fac1df4 100644
> --- a/lib/node/ethdev_rx.c
> +++ b/lib/node/ethdev_rx.c
> @@ -215,6 +215,7 @@ static struct rte_node_register ethdev_rx_node_base = {
> .next_nodes = {
> [ETHDEV_RX_NEXT_PKT_CLS] = "pkt_cls",
> [ETHDEV_RX_NEXT_IP4_LOOKUP] = "ip4_lookup",
> +   [ETHDEV_RX_NEXT_IP4_REASSEMBLY] = "ip4_reassembly",
> },
>  };
>
> diff --git a/lib/node/ethdev_rx_priv.h b/lib/node/ethdev_rx_priv.h
> index 7f24cf962e..574a76c2a6 100644
> --- a/lib/node/ethdev_rx_priv.h
> +++ b/lib/node/ethdev_rx_priv.h
> @@ -39,6 +39,7 @@ struct ethdev_rx_node_elem {
>  enum ethdev_rx_next_nodes {
> ETHDEV_RX_NEXT_IP4_LOOKUP,
> ETHDEV_RX_NEXT_PKT_CLS,
> +   ETHDEV_RX_NEXT_IP4_REASSEMBLY,
> ETHDEV_RX_NEXT_MAX,
>  };
>
> diff --git a/lib/node/ip4_reassembly.c b/lib/node/ip4_reassembly.c
> new file mode 100644
> index 00..04823cc596
> --- /dev/null
> +++ b/lib/node/ip4_reassembly.c
> @@ -0,0 +1,186 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Marvell.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "rte_node_ip4_api.h"
> +
> +#include "ip4_reassembly_priv.h"
> +#include "node_private.h"
> +
> +struct ip4_reassembly_elem {
> +   struct ip4_reassembly_elem *next;
> +   struct ip4_reassembly_ctx ctx;
> +   rte_node_t node_id;
> +};
> +
> +/* IP4 reassembly global data struct */
> +struct ip4_reassembly_node_main {
> +   struct ip4_reassembly_elem *head;
> +};
> +
> +typedef struct ip4_reassembly_ctx ip4_reassembly_ctx_t;
> +typedef struct ip4_reassembly_elem ip4_reassembly_elem_t;
> +
> +static struct ip4_reassembly_node_main ip4_reassembly_main;
> +
> +static uint16_t
> +ip4_reassembly_node_process(struct rte_graph *graph, struct rte_node *node, 
> void **objs,
> +   uint16_t nb_objs)
> +{
> +#define PREFETCH_OFFSET 4
> +   struct rte_mbuf *mbuf, *mbuf_out;
> +   struct rte_ip_frag_death_row *dr;
> +   struct ip4_reassembly_ctx *ctx;
> +   struct rte_ipv4_hdr *ipv4_hdr;
> +   struct rte_ip_frag_tbl *tbl;
> +   void **to_next, **to_free;
> +   uint16_t idx = 0;
> +   int i;
> +
> +   ctx = (struct ip4_reassembly_ctx *)node->ctx;
> +
> +   /* Get core specific reassembly tbl */
> +   tbl = ctx->tbl;
> +   dr = ctx->dr;
> +
> +   for (i = 0; i < PREFETCH_OFFSET && i < nb_objs; i++) {
> +   rte_prefetch0(rte_pktmbuf_mtod_offset((struct rte_mbuf 
> *)objs[i], void *,
> + sizeof(struct 
> rte_ether_hdr)));
> +   }
> +
> +   to_next = node->objs;
> +   for (i = 0; i < nb_objs - PREFETCH_OFFSET; i++) {
> +#if RTE_GRAPH_BURST_SIZE > 64
> +   /* Prefetch next-next mbufs */
> +   if (likely(i + 8 < nb_objs))
> +   rte_prefetch0(objs[i + 8]);
> +#endif
> +

[PATCH 1/4] security: remove redundant cast

2023-09-07 Thread Anoob Joseph
The API 'rte_cryptodev_get_sec_ctx' returns void *. Type cast is not
required.

Signed-off-by: Anoob Joseph 
---
 lib/security/rte_security.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index c4d64bb8e9..71910863bc 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -385,7 +385,7 @@ security_capabilities_from_dev_id(int dev_id, const void 
**caps)
if (rte_cryptodev_is_valid_dev(dev_id) == 0)
return -EINVAL;
 
-   sec_ctx = (struct rte_security_ctx *)rte_cryptodev_get_sec_ctx(dev_id);
+   sec_ctx = rte_cryptodev_get_sec_ctx(dev_id);
RTE_PTR_OR_ERR_RET(sec_ctx, -EINVAL);
 
capabilities = rte_security_capabilities_get(sec_ctx);
-- 
2.25.1



[PATCH 2/4] test/crypto: remove redundant cast

2023-09-07 Thread Anoob Joseph
The API 'rte_cryptodev_get_sec_ctx' returns void *. Type cast is not
required.

Signed-off-by: Anoob Joseph 
---
 app/test/test_cryptodev.c | 29 -
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index fb2af40b99..589b9860ce 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8920,15 +8920,12 @@ security_proto_supported(enum 
rte_security_session_action_type action,
enum rte_security_session_protocol proto)
 {
struct crypto_testsuite_params *ts_params = &testsuite_params;
-
const struct rte_security_capability *capabilities;
const struct rte_security_capability *capability;
+   struct rte_security_ctx *ctx;
uint16_t i = 0;
 
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(
-   ts_params->valid_devs[0]);
-
+   ctx = rte_cryptodev_get_sec_ctx(ts_params->valid_devs[0]);
 
capabilities = rte_security_capabilities_get(ctx);
 
@@ -8967,12 +8964,12 @@ static int test_pdcp_proto(int i, int oop, enum 
rte_crypto_cipher_operation opc,
struct crypto_unittest_params *ut_params = &unittest_params;
uint8_t *plaintext;
int ret = TEST_SUCCESS;
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(
-   ts_params->valid_devs[0]);
struct rte_cryptodev_info dev_info;
+   struct rte_security_ctx *ctx;
uint64_t feat_flags;
 
+   ctx = rte_cryptodev_get_sec_ctx(ts_params->valid_devs[0]);
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
feat_flags = dev_info.feature_flags;
 
@@ -9174,11 +9171,11 @@ test_pdcp_proto_SGL(int i, int oop,
unsigned int trn_data = 0;
struct rte_cryptodev_info dev_info;
uint64_t feat_flags;
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(
-   ts_params->valid_devs[0]);
+   struct rte_security_ctx *ctx;
struct rte_mbuf *temp_mbuf;
 
+   ctx = rte_cryptodev_get_sec_ctx(ts_params->valid_devs[0]);
+
rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
feat_flags = dev_info.feature_flags;
 
@@ -10898,6 +10895,7 @@ test_docsis_proto_uplink(const void *data)
const struct docsis_test_data *d_td = data;
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct crypto_unittest_params *ut_params = &unittest_params;
+   struct rte_security_ctx *ctx;
uint8_t *plaintext = NULL;
uint8_t *ciphertext = NULL;
uint8_t *iv_ptr;
@@ -10905,9 +10903,7 @@ test_docsis_proto_uplink(const void *data)
uint32_t crc_data_len;
int ret = TEST_SUCCESS;
 
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(
-   ts_params->valid_devs[0]);
+   ctx = rte_cryptodev_get_sec_ctx(ts_params->valid_devs[0]);
 
/* Verify the capabilities */
struct rte_security_capability_idx sec_cap_idx;
@@ -11083,15 +11079,14 @@ test_docsis_proto_downlink(const void *data)
const struct docsis_test_data *d_td = data;
struct crypto_testsuite_params *ts_params = &testsuite_params;
struct crypto_unittest_params *ut_params = &unittest_params;
+   struct rte_security_ctx *ctx;
uint8_t *plaintext = NULL;
uint8_t *ciphertext = NULL;
uint8_t *iv_ptr;
int32_t cipher_len, crc_len;
int ret = TEST_SUCCESS;
 
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(
-   ts_params->valid_devs[0]);
+   ctx = rte_cryptodev_get_sec_ctx(ts_params->valid_devs[0]);
 
/* Verify the capabilities */
struct rte_security_capability_idx sec_cap_idx;
-- 
2.25.1



[PATCH 3/4] app/crypto-perf: remove redundant cast

2023-09-07 Thread Anoob Joseph
The API 'rte_cryptodev_get_sec_ctx' returns void *. Type cast is not
required.

Signed-off-by: Anoob Joseph 
---
 app/test-crypto-perf/cperf_ops.c | 12 ++--
 app/test-crypto-perf/cperf_test_pmd_cyclecount.c |  4 +---
 app/test-crypto-perf/cperf_test_throughput.c |  4 +---
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
index 93b9bfb240..5bb2ce954a 100644
--- a/app/test-crypto-perf/cperf_ops.c
+++ b/app/test-crypto-perf/cperf_ops.c
@@ -647,6 +647,7 @@ create_ipsec_session(struct rte_mempool *sess_mp,
struct rte_crypto_sym_xform auth_xform = {0};
struct rte_crypto_sym_xform *crypto_xform;
struct rte_crypto_sym_xform xform = {0};
+   struct rte_security_ctx *ctx;
 
if (options->aead_algo != 0) {
/* Setup AEAD Parameters */
@@ -749,8 +750,7 @@ create_ipsec_session(struct rte_mempool *sess_mp,
else
sess_conf.ipsec.direction = RTE_SECURITY_IPSEC_SA_DIR_INGRESS;
 
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(dev_id);
+   ctx = rte_cryptodev_get_sec_ctx(dev_id);
 
/* Create security session */
return (void *)rte_security_session_create(ctx, &sess_conf, sess_mp);
@@ -766,6 +766,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
struct rte_crypto_sym_xform cipher_xform;
struct rte_crypto_sym_xform auth_xform;
struct rte_crypto_sym_xform aead_xform;
+   struct rte_security_ctx *ctx;
void *sess = NULL;
void *asym_sess = NULL;
struct rte_crypto_asym_xform xform = {0};
@@ -853,8 +854,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
.crypto_xform = &cipher_xform
};
 
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(dev_id);
+   ctx = rte_cryptodev_get_sec_ctx(dev_id);
 
/* Create security session */
return (void *)rte_security_session_create(ctx, &sess_conf, 
sess_mp);
@@ -901,8 +901,8 @@ cperf_create_session(struct rte_mempool *sess_mp,
} },
.crypto_xform = &cipher_xform
};
-   struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(dev_id);
+
+   ctx = rte_cryptodev_get_sec_ctx(dev_id);
 
/* Create security session */
return (void *)rte_security_session_create(ctx, &sess_conf, 
sess_mp);
diff --git a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c 
b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
index 0307e82996..d6d4130195 100644
--- a/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
+++ b/app/test-crypto-perf/cperf_test_pmd_cyclecount.c
@@ -67,9 +67,7 @@ cperf_pmd_cyclecount_test_free(struct 
cperf_pmd_cyclecount_ctx *ctx)
 #ifdef RTE_LIB_SECURITY
if (ctx->options->op_type == CPERF_PDCP ||
ctx->options->op_type == CPERF_DOCSIS) {
-   struct rte_security_ctx *sec_ctx =
-   (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(ctx->dev_id);
+   struct rte_security_ctx *sec_ctx = 
rte_cryptodev_get_sec_ctx(ctx->dev_id);
rte_security_session_destroy(sec_ctx,
(void *)ctx->sess);
} else
diff --git a/app/test-crypto-perf/cperf_test_throughput.c 
b/app/test-crypto-perf/cperf_test_throughput.c
index e892a70699..21738e8425 100644
--- a/app/test-crypto-perf/cperf_test_throughput.c
+++ b/app/test-crypto-perf/cperf_test_throughput.c
@@ -44,9 +44,7 @@ cperf_throughput_test_free(struct cperf_throughput_ctx *ctx)
else if (ctx->options->op_type == CPERF_PDCP ||
 ctx->options->op_type == CPERF_DOCSIS ||
 ctx->options->op_type == CPERF_IPSEC) {
-   struct rte_security_ctx *sec_ctx =
-   (struct rte_security_ctx *)
-   rte_cryptodev_get_sec_ctx(ctx->dev_id);
+   struct rte_security_ctx *sec_ctx = 
rte_cryptodev_get_sec_ctx(ctx->dev_id);
rte_security_session_destroy(
sec_ctx,
(void *)ctx->sess);
-- 
2.25.1



[PATCH 4/4] examples/ipsec-secgw: remove redundant cast

2023-09-07 Thread Anoob Joseph
The API 'rte_cryptodev_get_sec_ctx' returns void *. Type cast is not
required.

Signed-off-by: Anoob Joseph 
---
 examples/ipsec-secgw/ipsec.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index a5706bed24..984fb7a2ec 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -327,9 +327,7 @@ create_lookaside_session(struct ipsec_ctx 
*ipsec_ctx_lcore[],
};
 
if (ips->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
-   struct rte_security_ctx *ctx = (struct rte_security_ctx 
*)
-   
rte_cryptodev_get_sec_ctx(
-   cdev_id);
+   struct rte_security_ctx *ctx = 
rte_cryptodev_get_sec_ctx(cdev_id);
 
/* Set IPsec parameters in conf */
set_ipsec_conf(sa, &(sess_conf.ipsec));
-- 
2.25.1



RE: [PATCH v3 2/9] net/cpfl: add flow json parser

2023-09-07 Thread Liu, Mingxia



> -Original Message-
> From: Qiao, Wenjing 
> Sent: Wednesday, September 6, 2023 5:34 PM
> To: Zhang, Yuying ; dev@dpdk.org; Zhang, Qi Z
> ; Wu, Jingjing ; Xing, Beilei
> 
> Cc: Liu, Mingxia ; Qiao, Wenjing
> 
> Subject: [PATCH v3 2/9] net/cpfl: add flow json parser
> 
> A JSON file will be used to direct DPDK CPF PMD to
> parse rte_flow tokens into low level hardware resources
> defined in a DDP package file.
> 
> Signed-off-by: Wenjing Qiao 
> ---
>  drivers/net/cpfl/cpfl_ethdev.h  |   70 +
>  drivers/net/cpfl/cpfl_flow_parser.c | 1910 +++
>  drivers/net/cpfl/cpfl_flow_parser.h |  236 
>  drivers/net/cpfl/meson.build|3 +
>  4 files changed, 2219 insertions(+)
>  create mode 100644 drivers/net/cpfl/cpfl_flow_parser.c
>  create mode 100644 drivers/net/cpfl/cpfl_flow_parser.h
> 
> +static int
> +cpfl_flow_js_pattern_key_attr(json_object *cjson_pr_key_attr, struct
> cpfl_flow_js_pr *js_pr)
> +{
> + int i, len;
> + struct cpfl_flow_js_pr_key_attr *attr;
> +
> + len = json_object_array_length(cjson_pr_key_attr);
> + js_pr->key.attributes = rte_malloc(NULL, sizeof(struct
> cpfl_flow_js_pr_key_attr), 0);
> + if (!js_pr->key.attributes) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> + js_pr->key.attr_size = len;
> + attr = js_pr->key.attributes;
> + for (i = 0; i < len; i++) {
> + json_object *object;
> + const char *name;
> + uint16_t value = 0;
> + int ret;
> +
> + object = json_object_array_get_idx(cjson_pr_key_attr, i);
> + name = cpfl_json_object_to_string(object, "Name");
> + if (!name) {
> + rte_free(js_pr->key.attributes);
> + PMD_DRV_LOG(ERR, "Can not parse string 'Name'.");
> + return -EINVAL;
[Liu, Mingxia] Better to use goto statement as other similar function do?

> + }
> + ret = cpfl_json_object_to_uint16(object, "Value", &value);
> + if (ret < 0) {
> + rte_free(js_pr->key.attributes);
> + PMD_DRV_LOG(ERR, "Can not parse 'value'.");
> + return -EINVAL;
> + }
> + if (strcmp(name, "ingress") == 0) {
> + attr->ingress = value;
> + } else if (strcmp(name, "egress") == 0) {
> + attr->egress = value;
> + } else {
> + /* TODO: more... */
> + rte_free(js_pr->key.attributes);
> + PMD_DRV_LOG(ERR, "Not support attr name: %s.",
> name);
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_flow_js_pattern_key_proto_field(json_object *cjson_field,
> +  struct cpfl_flow_js_pr_key_proto *js_field)
> +{
> + int len, i;
> +
> + if (!cjson_field)
> + return 0;
> + len = json_object_array_length(cjson_field);
> + js_field->fields_size = len;
> + if (len == 0)
> + return 0;
> + js_field->fields =
> + rte_malloc(NULL, sizeof(struct cpfl_flow_js_pr_key_proto_field) * 
> len,
> 0);
> + if (!js_field->fields) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> + for (i = 0; i < len; i++) {
> + json_object *object;
> + const char *name, *mask;
> +
> + object = json_object_array_get_idx(cjson_field, i);
> + name = cpfl_json_object_to_string(object, "name");
> + if (!name) {
> + PMD_DRV_LOG(ERR, "Can not parse string 'name'.");
> + goto err;
> + }
> + if (strlen(name) > CPFL_FLOW_JSON_STR_SIZE_MAX) {
> + PMD_DRV_LOG(ERR, "The 'name' is too long.");
> + goto err;
> + }
> + memcpy(js_field->fields[i].name, name, strlen(name));
> +
> + if (js_field->type == RTE_FLOW_ITEM_TYPE_ETH ||
> + js_field->type == RTE_FLOW_ITEM_TYPE_IPV4) {
> + mask = cpfl_json_object_to_string(object, "mask");
> + if (!mask) {
> + PMD_DRV_LOG(ERR, "Can not parse string
> 'mask'.");
> + goto err;
> + }
> + memcpy(js_field->fields[i].mask, mask, strlen(mask));
[Liu, Mingxia] Need to check the length and validation of mask?

> + } else {
> + uint32_t mask_32b;
> + int ret;
> +
> + ret = cpfl_json_object_to_uint32(object, "mask",
> &mask_32b);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse uint32
> 'mask'.");
> + goto er

Troubleshooting DPDK in Intel Ethernet NIC

2023-09-07 Thread Antón Rey Villaverde
Hi,
I have a problem while trying to manage my physical Ethernet interface from
DPDK (latest version compiled from source).
I have a:
NIC: :00:1f.6 Ethernet controller: Intel Corporation Ethernet
Connection (13) I219-LM (rev 20)

which belongs to the list of supported NICs in DPDK.

Also, I have bound my NICto vfio-pci, not the kernel:

dpdk-23.07$ usertools/dpdk-devbind.py --status
Network devices using DPDK-compatible driver
 :00:1f.6 'Ethernet
Connection (13) I219-LM 15fb' drv=vfio-pci unused=e1000e

I also enabled hugepages (2G).

when I enter "sudo build/app/dpdk-testpmd -c7 -- -i" I get:
EAL: Detected CPU lcores: 8
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: VFIO support initialized
TELEMETRY: No legacy callbacks, legacy socket not created
testpmd: No probed ethernet devices
Interactive-mode selected
testpmd: create a new mbuf pool : n=163456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Done
testpmd>

When I enter "show port info all" in the testpmd CLI I get nothing (no
ports).

When I start testpmd with "-a :00:1f.6" (the address of the ethernet
NIC I want to manage from DPDK), The same happens (no ports).

If I try to start testpmd with "-d build/drivers/librte_net_e1000.so" (the
shared library of the driver of that device), I get a crash:

dpdk-23.07$ sudo build/app/dpdk-testpmd -c7 -a :00:1f.6 -d
build/drivers/librte_net_e1000.so -- -i
EAL: Detected CPU lcores: 8
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: UIO_RESOURCE_LIST tailq is already registered PANIC in
tailqinitfn_rte_uio_tailq(): Cannot initialize tailq: UIO_RESOURCE_LIST
0: build/app/dpdk-testpmd (rte_dump_stack+0x32) [55be1839afe2]
1: build/app/dpdk-testpmd (__rte_panic+0xf1) [55be18369e68]
2: /home/anton/tdr/dpdk-23.07/build/drivers/librte_bus_pci.so.23
(7f52153f6000+0x5d86) [7f52153fbd86]
3: /lib64/ld-linux-x86-64.so.2 (7f5215f9+0x11b9a) [7f5215fa1b9a]
4: /lib64/ld-linux-x86-64.so.2 (7f5215f9+0x11ca1) [7f5215fa1ca1]
5: /lib/x86_64-linux-gnu/libc.so.6 (_dl_catch_exception+0xe5)
[7f5215970985]
6: /lib64/ld-linux-x86-64.so.2 (7f5215f9+0x160cf) [7f5215fa60cf]
7: /lib/x86_64-linux-gnu/libc.so.6 (_dl_catch_exception+0x88)
[7f5215970928]
8: /lib64/ld-linux-x86-64.so.2 (7f5215f9+0x1560a) [7f5215fa560a]
9: /lib/x86_64-linux-gnu/libdl.so.2 (7f5215e15000+0x134c) [7f5215e1634c]
10: /lib/x86_64-linux-gnu/libc.so.6 (_dl_catch_exception+0x88)
[7f5215970928]
11: /lib/x86_64-linux-gnu/libc.so.6 (_dl_catch_error+0x33) [7f52159709f3]
12: /lib/x86_64-linux-gnu/libdl.so.2 (7f5215e15000+0x1b59) [7f5215e16b59]
13: /lib/x86_64-linux-gnu/libdl.so.2 (dlopen+0x4a) [7f5215e163da]
14: build/app/dpdk-testpmd (55be17b79000+0x8013ee) [55be1837a3ee]
15: build/app/dpdk-testpmd (eal_plugins_init+0x14f) [55be1837a6d8]
16: build/app/dpdk-testpmd (rte_eal_init+0x1af) [55be1839ec21]
17: build/app/dpdk-testpmd (main+0xde) [55be17da108b]
18: /lib/x86_64-linux-gnu/libc.so.6 (__libc_start_main+0xf3) [7f5215834083]
19: build/app/dpdk-testpmd (_start+0x2e) [55be17cef1fe]
Aborted

I have also checked that:
I have IOMMU enabled:
cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-5.15.0-83-generic
root=UUID=3c080ff1-1c00-4b5c-aa00-2084f9e68794 ro i915.enable_psr=1 quiet
splash intel_iommu=on vt.handoff=7
and other virtualization flags enabled (VT-x, VT-d), and the Secure Boot is
disabled.
Also the NIC works fine in normal circumstances (managed by the OS kernel).

I am running Ubuntu 20.04.6 on a Dell Precision 3560 laptop (1 year old).

I don't know whether this is a bug or I missed something, so I would
appreciate any guidance. Thanks in advance

Anton