RE: [PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency

[Zhang, Qi Z]

> -Original Message-
> From: Morten Brørup 
> Sent: Monday, January 9, 2023 3:48 PM
> To: He, ShiyangX ; dev@dpdk.org
> Cc: Zhou, YidingX ; sta...@dpdk.org; Yang, Qiming
> ; Wu, Wenjun1 ; Remy
> Horton 
> Subject: RE: [PATCH v2] net/ixgbe: fix ixgbe firmware version inconsistency
> 
> > From: Shiyang He [mailto:shiyangx...@intel.com]
> > Sent: Monday, 9 January 2023 07.53
> >
> > This patch follows the code of ixgbe kernel driver so that it keeps
> > the firmware version obtained by dpdk-ethtool consistent with that
> > obtained by linux-ethtool.
> >
> > Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Shiyang He 
> >
> > v2: follow the code of ixgbe kernel driver
> > ---
> 
> Thank you for the updated patch. LGTM.
> 
> Acked-by: Morten Brørup 

Applied to dpdk-next-net-intel.

Thanks
Qi

Re: [2nd Try]:Re: Traffic Management API Questions

[Venky Venkatesh]

Hi Jasvinder,
Thanks for the insights on the complexity of adding a layer.
As for the workaround that you suggested using multiple subports, if I
understand it correctly (*pls correct if I misunderstood*) it would not
meet our needs:

   - We require multiple heterogeneous ports (i.e. ports with different
   bandwidths -- with no excess sharing since these are port limits). That
   would probably need some shaper attached there too since WRR (at the
   application) would share the instantaneous excess among the siblings.
   - As for the 2nd second suggestion (increase the number of subports):
   our need (*in addition to* multiple ports of different bandwidths at the
   top level) is to have  4 more TM layers for a total of 5. I am *not*
   looking at the assignment of the terms port/subport/user/pipe etc in the
   DPDK documentation -- instead am looking at it as abstract scheduling
   (and/or shaping) layers with differing abilities in some layers. So in
   order to compensate for the missing shaper at the port level I was planning
   to add 1 additional layer (so that what in DPDK documentation is referred
   to as subport is actually a port -- since the subport layer has the
   property of *not sharing* excess between siblings. With that principle,
   I am not clear how adding width to the subport layer (as I understand your
   suggestion) would help.


Thanks
-Venky


On Wed, Jan 11, 2023 at 9:24 AM Singh, Jasvinder 
wrote:

> Hi Venky,
>
>
>
> Please see inline;
>
>
>
> Jasvinder
>
>
>
> *From:* Venky Venkatesh 
> *Sent:* Wednesday, January 11, 2023 11:56 AM
> *To:* Singh, Jasvinder 
> *Cc:* dev@dpdk.org
> *Subject:* Re: [2nd Try]:Re: Traffic Management API Questions
>
>
>
> Hi Jasvinder,
>
> Thanks for the detailed answers. Our need is to have shaping at the port
> level as well. I am trying to see what would be the way to accomplish this
> given the current limitations of the sched library implementation in this
> regard. I see 2 options:
>
>- The top level (i.e. port level) documentation says the following:
>"Output Ethernet port 1/10/40 GbE" and "Multiple ports are scheduled
>in round robin order with all ports having equal priority". Questions:
>
>
>- Do all the ports have to be of the same speed OR can it be a
>   heterogeneous set of port speeds?
>
> [JS] – the library supports single port (root node) of the hierarchy. Each
> port can have multiple subports configured using different shaping rates.
> If you desire to have multiple ports, each port would have separate
> hierarchical tree underneath. Different ports could have different speed.
>
> o If it can be a heterogeneous set of ports, is the scheduling across
> those ports *weighted* round robin as opposed to round robin?
>
> [JS] – Scheduling across multiple ports is not supported in current sched
> library. At the application level, one can think of invoking
> enqueue/dequeue sched API in round robin or weighted round robin manner.
>
>- Are Speeds other than  1/10/40 GbE not supported?
>
> [JS] – Speeds other than above is supported, for eg. 25G, 50G etc.
>
>- I suppose this heterogeneous mix of port speeds is implemented by
>   playing with the weights across ports, correct?
>
> [JS] -please see above answers
>
>- If so, what problem do you foresee if we provide arbitrary bandwidth
>   ports by regulating the above weights?
>
> [JS] – I don’t see any issue.
>
>- The other alternative would be to add another layer (which has a
>shaper)  to the hierarchy by mimicking one of the existing layers: how
>amenable is the current implementation to that?
>
> Do either of the above look like workable ideas? Are there any other
> approaches where we could accomplish our requirement with minimal changes
> to the code logic?
>
> [JS] – adding another layer will require considerable work in library. How
> about using multiple subports with different shaping bandwidth where each
> subport maintain #subcribers and send traffic out through single physical
> port (root node). It will need less effort and library supports multiple
> subports under single port (root node).
>
>
>
>
>
> Thanks
>
> -Venky
>
>
>
> On Tue, Jan 10, 2023 at 2:54 AM Singh, Jasvinder <
> jasvinder.si...@intel.com> wrote:
>
> Hi Venky,
>
>
>
> Please see inline.
>
>
>
> Thanks,
>
> Jasvinder
>
>
>
>
>
> *From:* Venky Venkatesh 
> *Sent:* Tuesday, January 10, 2023 8:52 AM
> *To:* dev@dpdk.org
> *Subject:* [2nd Try]:Re: Traffic Management API Questions
>
>
>
> Hi,
>
> Can someone pls get back on these
>
> Thanks
>
> -Venky
>
>
>
> On Thu, Jan 5, 2023 at 4:07 AM Venky Venkatesh <
> vvenkat...@paloaltonetworks.com> wrote:
>
> Hi,
>
> I was looking at the DPDK Traffic Management API. I wanted to clarify some
> things that I understand from the code (for software based TM
> implementation (at 20.11)) vs the documentation.
>
> · The documentation says "Traffic shaping: single/*dual rate**,* 
> private
> (*per node*) and shared (by

Re: [PATCH] build: fix dependencies lookup

[Bruce Richardson]

On Sun, Jan 15, 2023 at 05:53:15PM +0100, Thomas Monjalon wrote:
> The first parameter of the Meson function "find_library()"
> should be the library name without the "lib" prefix.
> 
> Otherwise Meson prints this warning:
>   WARNING: find_library('libexecinfo') starting in "lib"
>   only works by accident and is not portable
> 
> Fixes: 1cd512b2f532 ("build: detect execinfo library on Linux")
> Fixes: e1defba4cf66 ("raw/ifpga/base: support device tree")
> Fixes: a489f5dbf437 ("baseband/turbo_sw: support meson build")
> Fixes: 72c00ae9dba7 ("regex/cn9k: use cnxk infrastructure")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Thomas Monjalon 
> ---
Acked-by: Bruce Richardson 

Re: [PATCH] ipsec: remove unneccessary null check

[Zhang, Fan]

On 1/13/2023 6:44 PM, Stephen Hemminger wrote:

The function rte_ring_free() accepts NULL as vaild input
like free() and other functions.

Found with null_free_check.cocci.

Fixes: 16d6ebb65d59 ("crypto/ipsec_mb: fix null checks")
Cc: kai...@intel.com
Signed-off-by: Stephen Hemminger 
---
  drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 5 +
  1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c 
b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index 71e02cd0513d..3e52f9567401 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -139,15 +139,12 @@ int
  ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
  {
struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
-   struct rte_ring *r = NULL;
  
  	if (!qp)

return 0;
  
  	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {

-   r = rte_ring_lookup(qp->name);
-   if (r)
-   rte_ring_free(r);
+   rte_ring_free(rte_ring_lookup(qp->name));
  
  #if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM

if (qp->mb_mgr)

Acked-by: Fan Zhang 

Re: [PATCH] build: fix dependencies lookup

[David Marchand]

On Sun, Jan 15, 2023 at 5:53 PM Thomas Monjalon  wrote:
>
> The first parameter of the Meson function "find_library()"
> should be the library name without the "lib" prefix.
>
> Otherwise Meson prints this warning:
> WARNING: find_library('libexecinfo') starting in "lib"
> only works by accident and is not portable
>
> Fixes: 1cd512b2f532 ("build: detect execinfo library on Linux")
> Fixes: e1defba4cf66 ("raw/ifpga/base: support device tree")
> Fixes: a489f5dbf437 ("baseband/turbo_sw: support meson build")
> Fixes: 72c00ae9dba7 ("regex/cn9k: use cnxk infrastructure")
> Cc: sta...@dpdk.org

This patch depends on
https://patchwork.dpdk.org/project/dpdk/patch/20230108095920.327672-1-tho...@monjalon.net/
which is not marked for backport.
So either the latter is marked for backport too, or the order of those
two patches should be inverted (to make it easier for LTS
maintainers).

Otherwise, it lgtm.


>
> Signed-off-by: Thomas Monjalon 


-- 
David Marchand

[PATCH 2/9] common/cnxk: enable CQ late BP with valid CPT BPID

[Nithin Dabilpuram]

From: Satha Rao 

When FC enable requested for CPT, mbox returns allocated BPID.
While configuring CQ consider this value to enable late back pressure.

Signed-off-by: Satha Rao 
---
 drivers/common/cnxk/roc_nix_fc.c|  2 ++
 drivers/common/cnxk/roc_nix_priv.h  |  1 +
 drivers/common/cnxk/roc_nix_queue.c | 19 +--
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/common/cnxk/roc_nix_fc.c b/drivers/common/cnxk/roc_nix_fc.c
index 569fe8dc48..784e6e5416 100644
--- a/drivers/common/cnxk/roc_nix_fc.c
+++ b/drivers/common/cnxk/roc_nix_fc.c
@@ -83,6 +83,7 @@ nix_fc_rxchan_bpid_set(struct roc_nix *roc_nix, bool enable)
rc = mbox_process_msg(mbox, (void *)&rsp);
if (rc)
goto exit;
+   nix->cpt_lbpid = rsp->chan_bpid[0] & 0x1FF;
} else {
req = mbox_alloc_msg_nix_cpt_bp_disable(mbox);
if (req == NULL)
@@ -94,6 +95,7 @@ nix_fc_rxchan_bpid_set(struct roc_nix *roc_nix, bool enable)
rc = mbox_process_msg(mbox, (void *)&rsp);
if (rc)
goto exit;
+   nix->cpt_lbpid = 0;
}
 
 exit:
diff --git a/drivers/common/cnxk/roc_nix_priv.h 
b/drivers/common/cnxk/roc_nix_priv.h
index 0a9461c856..7d2e3626a3 100644
--- a/drivers/common/cnxk/roc_nix_priv.h
+++ b/drivers/common/cnxk/roc_nix_priv.h
@@ -205,6 +205,7 @@ struct nix {
uint16_t nb_cpt_lf;
uint16_t outb_se_ring_cnt;
uint16_t outb_se_ring_base;
+   uint16_t cpt_lbpid;
bool need_meta_aura;
/* Mode provided by driver */
bool inb_inl_dev;
diff --git a/drivers/common/cnxk/roc_nix_queue.c 
b/drivers/common/cnxk/roc_nix_queue.c
index 20a1e7d4d8..385f1ba04e 100644
--- a/drivers/common/cnxk/roc_nix_queue.c
+++ b/drivers/common/cnxk/roc_nix_queue.c
@@ -798,7 +798,7 @@ roc_nix_cq_init(struct roc_nix *roc_nix, struct roc_nix_cq 
*cq)
struct mbox *mbox = (&nix->dev)->mbox;
volatile struct nix_cq_ctx_s *cq_ctx;
uint16_t drop_thresh = NIX_CQ_THRESH_LEVEL;
-   uint16_t cpt_lbpid = nix->bpid[0];
+   uint16_t cpt_lbpid = nix->cpt_lbpid;
enum nix_q_size qsize;
size_t desc_sz;
int rc;
@@ -860,11 +860,14 @@ roc_nix_cq_init(struct roc_nix *roc_nix, struct 
roc_nix_cq *cq)
if (roc_model_is_cn10kb() && roc_nix_inl_inb_is_enabled(roc_nix)) {
cq_ctx->cq_err_int_ena |= BIT(NIX_CQERRINT_CPT_DROP);
cq_ctx->cpt_drop_err_en = 1;
-   cq_ctx->lbp_ena = 1;
-   cq_ctx->lbpid_low = cpt_lbpid & 0x7;
-   cq_ctx->lbpid_med = (cpt_lbpid >> 3) & 0x7;
-   cq_ctx->lbpid_high = (cpt_lbpid >> 6) & 0x7;
-   cq_ctx->lbp_frac = NIX_CQ_LPB_THRESH_FRAC;
+   /* Enable Late BP only when non zero CPT BPID */
+   if (cpt_lbpid) {
+   cq_ctx->lbp_ena = 1;
+   cq_ctx->lbpid_low = cpt_lbpid & 0x7;
+   cq_ctx->lbpid_med = (cpt_lbpid >> 3) & 0x7;
+   cq_ctx->lbpid_high = (cpt_lbpid >> 6) & 0x7;
+   cq_ctx->lbp_frac = NIX_CQ_LPB_THRESH_FRAC;
+   }
drop_thresh = NIX_CQ_SEC_THRESH_LEVEL;
}
 
@@ -959,6 +962,10 @@ roc_nix_cq_fini(struct roc_nix_cq *cq)
aq->cq.bp_ena = 0;
aq->cq_mask.ena = ~aq->cq_mask.ena;
aq->cq_mask.bp_ena = ~aq->cq_mask.bp_ena;
+   if (roc_model_is_cn10kb() && 
roc_nix_inl_inb_is_enabled(cq->roc_nix)) {
+   aq->cq.lbp_ena = 0;
+   aq->cq_mask.lbp_ena = ~aq->cq_mask.lbp_ena;
+   }
}
 
rc = mbox_process(mbox);
-- 
2.25.1

[PATCH 1/9] common/cnxk: get mbox lock before NDC sync

[Nithin Dabilpuram]

Take mbox lock before NDC sync to be thread safe.
Also release the lock only after access to response
is complete.

Fixes: 7a978bc4be6b ("common/cnxk: support mailbox locking")
Cc: rkuduruma...@marvell.com

Signed-off-by: Nithin Dabilpuram 
---
 drivers/common/cnxk/roc_nix_tm.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/common/cnxk/roc_nix_tm.c b/drivers/common/cnxk/roc_nix_tm.c
index 4e5f320712..4ced7a052f 100644
--- a/drivers/common/cnxk/roc_nix_tm.c
+++ b/drivers/common/cnxk/roc_nix_tm.c
@@ -690,13 +690,16 @@ roc_nix_tm_sq_free_pending_sqe(struct nix *nix, int q)
 
mbox = dev->mbox;
/* Sync NDC-NIX-TX for LF */
-   ndc_req = mbox_alloc_msg_ndc_sync_op(mbox);
-   if (ndc_req == NULL)
+   ndc_req = mbox_alloc_msg_ndc_sync_op(mbox_get(mbox));
+   if (ndc_req == NULL) {
+   mbox_put(mbox);
return -EFAULT;
+   }
 
ndc_req->nix_lf_tx_sync = 1;
if (mbox_process(mbox))
rc |= NIX_ERR_NDC_SYNC;
+   mbox_put(mbox);
 
if (rc)
plt_err("NDC_SYNC failed rc %d", rc);
@@ -1480,8 +1483,9 @@ nix_tm_alloc_txschq(struct nix *nix, enum roc_nix_tm_tree 
tree)
mbox_put(mbox);
goto alloc_err;
}
-   mbox_put(mbox);
+
nix_tm_copy_rsp_to_nix(nix, rsp);
+   mbox_put(mbox);
} while (pend);
 
nix->tm_link_cfg_lvl = rsp->link_cfg_lvl;
-- 
2.25.1

[PATCH 3/9] common/cnxk: configure fc hist bits

[Nithin Dabilpuram]

From: Satha Rao 

New parameter added inside SQ structure to control the fc_hyst_bits.
Instead of count on all updates each SQ can tune his own hysteresis
level.

Signed-off-by: Satha Rao 
---
 drivers/common/cnxk/roc_nix.h   | 1 +
 drivers/common/cnxk/roc_nix_queue.c | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/common/cnxk/roc_nix.h b/drivers/common/cnxk/roc_nix.h
index 47ee078c2e..96756b1a2b 100644
--- a/drivers/common/cnxk/roc_nix.h
+++ b/drivers/common/cnxk/roc_nix.h
@@ -354,6 +354,7 @@ struct roc_nix_sq {
uint16_t cq_drop_thresh;
bool sso_ena;
bool cq_ena;
+   uint8_t fc_hyst_bits;
/* End of Input parameters */
uint16_t sqes_per_sqb_log2;
struct roc_nix *roc_nix;
diff --git a/drivers/common/cnxk/roc_nix_queue.c 
b/drivers/common/cnxk/roc_nix_queue.c
index 385f1ba04e..287a489e7f 100644
--- a/drivers/common/cnxk/roc_nix_queue.c
+++ b/drivers/common/cnxk/roc_nix_queue.c
@@ -1025,9 +1025,8 @@ sqb_pool_populate(struct roc_nix *roc_nix, struct 
roc_nix_sq *sq)
else
aura.fc_stype = 0x3; /* STSTP */
aura.fc_addr = (uint64_t)sq->fc;
-   aura.fc_hyst_bits = 1; /* Store count on all updates */
-   rc = roc_npa_pool_create(&sq->aura_handle, blk_sz, nb_sqb_bufs, &aura,
-&pool, 0);
+   aura.fc_hyst_bits = sq->fc_hyst_bits & 0xF;
+   rc = roc_npa_pool_create(&sq->aura_handle, blk_sz, nb_sqb_bufs, &aura, 
&pool, 0);
if (rc)
goto fail;
 
-- 
2.25.1

[PATCH 4/9] net/cnxk: reset pfc mode and flow control

[Nithin Dabilpuram]

From: Rakesh Kudurumalla 

reset pfc and flow control if pfc mode and flow
control are set respectively during unintilization
of pf or vf

Signed-off-by: Rakesh Kudurumalla 
---
 drivers/net/cnxk/cnxk_ethdev.c | 38 --
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c
index d711eb6b27..22072d29b0 100644
--- a/drivers/net/cnxk/cnxk_ethdev.c
+++ b/drivers/net/cnxk/cnxk_ethdev.c
@@ -1934,6 +1934,8 @@ cnxk_eth_dev_uninit(struct rte_eth_dev *eth_dev, bool 
reset)
 {
struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
const struct eth_dev_ops *dev_ops = eth_dev->dev_ops;
+   struct cnxk_pfc_cfg *pfc_cfg = &dev->pfc_cfg;
+   struct cnxk_fc_cfg *fc_cfg = &dev->fc_cfg;
struct rte_eth_pfc_queue_conf pfc_conf;
struct roc_nix *nix = &dev->nix;
struct rte_eth_fc_conf fc_conf;
@@ -1957,21 +1959,27 @@ cnxk_eth_dev_uninit(struct rte_eth_dev *eth_dev, bool 
reset)
/* Restore 802.3 Flow control configuration */
memset(&pfc_conf, 0, sizeof(struct rte_eth_pfc_queue_conf));
memset(&fc_conf, 0, sizeof(struct rte_eth_fc_conf));
-   fc_conf.mode = RTE_ETH_FC_NONE;
-   rc = cnxk_nix_flow_ctrl_set(eth_dev, &fc_conf);
-
-   pfc_conf.mode = RTE_ETH_FC_NONE;
-   for (i = 0; i < RTE_MAX(eth_dev->data->nb_rx_queues,
-   eth_dev->data->nb_tx_queues);
-i++) {
-   pfc_conf.rx_pause.tc = ROC_NIX_PFC_CLASS_INVALID;
-   pfc_conf.rx_pause.tx_qid = i;
-   pfc_conf.tx_pause.tc = ROC_NIX_PFC_CLASS_INVALID;
-   pfc_conf.tx_pause.rx_qid = i;
-   rc = cnxk_nix_priority_flow_ctrl_queue_config(eth_dev,
- &pfc_conf);
-   if (rc && rc != -ENOTSUP)
-   plt_err("Failed to reset PFC. error code(%d)", rc);
+   if (fc_cfg->rx_pause || fc_cfg->tx_pause) {
+   fc_conf.mode = RTE_ETH_FC_NONE;
+   rc = cnxk_nix_flow_ctrl_set(eth_dev, &fc_conf);
+   if (rc < 0)
+   plt_err("Failed to reset control flow. error code(%d)",
+   rc);
+   }
+   if (pfc_cfg->rx_pause_en || pfc_cfg->tx_pause_en) {
+   for (i = 0; i < RTE_MAX(eth_dev->data->nb_rx_queues,
+   eth_dev->data->nb_tx_queues);
+i++) {
+   pfc_conf.mode = RTE_ETH_FC_NONE;
+   pfc_conf.rx_pause.tc = ROC_NIX_PFC_CLASS_INVALID;
+   pfc_conf.rx_pause.tx_qid = i;
+   pfc_conf.tx_pause.tc = ROC_NIX_PFC_CLASS_INVALID;
+   pfc_conf.tx_pause.rx_qid = i;
+   rc = cnxk_nix_priority_flow_ctrl_queue_config(eth_dev,
+   &pfc_conf);
+   if (rc && rc != -ENOTSUP)
+   plt_err("Failed to reset PFC. error code(%d)", 
rc);
+   }
}
 
/* Disable and free rte_meter entries */
-- 
2.25.1

[PATCH 5/9] common/cnxk: dump inline device RQ context

[Nithin Dabilpuram]

Dump inline device RQ context along with ethdev's RQ context.

Signed-off-by: Nithin Dabilpuram 
---
 drivers/common/cnxk/roc_nix_debug.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/common/cnxk/roc_nix_debug.c 
b/drivers/common/cnxk/roc_nix_debug.c
index 775325115b..2f8c595bd9 100644
--- a/drivers/common/cnxk/roc_nix_debug.c
+++ b/drivers/common/cnxk/roc_nix_debug.c
@@ -693,6 +693,7 @@ roc_nix_queues_ctx_dump(struct roc_nix *roc_nix, FILE *file)
struct npa_aq_enq_req *npa_aq;
struct dev *dev = &nix->dev;
int sq = nix->nb_tx_queues;
+   struct roc_nix_rq *inl_rq;
struct npa_lf *npa_lf;
volatile void *ctx;
uint32_t sqb_aura;
@@ -726,6 +727,25 @@ roc_nix_queues_ctx_dump(struct roc_nix *roc_nix, FILE 
*file)
nix_lf_rq_dump(ctx, file);
}
 
+   /* Dump inline dev RQ for this port */
+   inl_rq = roc_nix_inl_dev_rq(roc_nix);
+   if (inl_rq) {
+   struct idev_cfg *idev = idev_get_cfg();
+   struct nix_inl_dev *inl_dev = idev->nix_inl_dev;
+
+   rc = nix_q_ctx_get(&inl_dev->dev, NIX_AQ_CTYPE_RQ, inl_rq->qid, 
&ctx);
+   if (rc) {
+   plt_err("Failed to get rq context");
+   goto fail;
+   }
+   nix_dump(file, "== port=%d inl_rq=%d 
===", roc_nix->port_id,
+inl_rq->qid);
+   if (roc_model_is_cn9k())
+   nix_cn9k_lf_rq_dump(ctx, file);
+   else
+   nix_lf_rq_dump(ctx, file);
+   }
+
for (q = 0; q < sq; q++) {
rc = nix_q_ctx_get(dev, NIX_AQ_CTYPE_SQ, q, &ctx);
if (rc) {
-- 
2.25.1

[PATCH 6/9] common/cnxk: free tm resources in order from leaf to root

[Nithin Dabilpuram]

Now that kernel is clearing parent info that is needed for flush,
free the resources in order from leaf to root so that when SMQ flush
is called there is always hierarchy present from SMQ till TL1.

Signed-off-by: Nithin Dabilpuram 
---
 drivers/common/cnxk/roc_nix_tm.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/common/cnxk/roc_nix_tm.c b/drivers/common/cnxk/roc_nix_tm.c
index 4ced7a052f..6d470f424d 100644
--- a/drivers/common/cnxk/roc_nix_tm.c
+++ b/drivers/common/cnxk/roc_nix_tm.c
@@ -1817,6 +1817,7 @@ nix_tm_free_resources(struct roc_nix *roc_nix, uint32_t 
tree_mask, bool hw_only)
enum roc_nix_tm_tree tree;
uint32_t profile_id;
int rc = 0;
+   int hw_lvl;
 
for (tree = 0; tree < ROC_NIX_TM_TREE_MAX; tree++) {
if (!(tree_mask & BIT(tree)))
@@ -1825,20 +1826,25 @@ nix_tm_free_resources(struct roc_nix *roc_nix, uint32_t 
tree_mask, bool hw_only)
plt_tm_dbg("Freeing resources of tree %u", tree);
 
list = nix_tm_node_list(nix, tree);
-   next_node = TAILQ_FIRST(list);
-   while (next_node) {
-   node = next_node;
-   next_node = TAILQ_NEXT(node, node);
+   /* Flush and free resources from leaf */
+   for (hw_lvl = NIX_TXSCH_LVL_SMQ; hw_lvl < NIX_TXSCH_LVL_CNT; 
hw_lvl++) {
+   next_node = TAILQ_FIRST(list);
+   while (next_node) {
+   node = next_node;
+   next_node = TAILQ_NEXT(node, node);
+   if (node->hw_lvl != hw_lvl)
+   continue;
 
-   if (!nix_tm_is_leaf(nix, node->lvl) &&
-   node->flags & NIX_TM_NODE_HWRES) {
-   /* Clear xoff in path for flush to succeed */
-   rc = nix_tm_clear_path_xoff(nix, node);
-   if (rc)
-   return rc;
-   rc = nix_tm_free_node_resource(nix, node);
-   if (rc)
-   return rc;
+   if (!nix_tm_is_leaf(nix, node->lvl) &&
+   node->flags & NIX_TM_NODE_HWRES) {
+   /* Clear xoff in path for flush to 
succeed */
+   rc = nix_tm_clear_path_xoff(nix, node);
+   if (rc)
+   return rc;
+   rc = nix_tm_free_node_resource(nix, 
node);
+   if (rc)
+   return rc;
+   }
}
}
 
-- 
2.25.1

[PATCH 7/9] common/cnxk: update CPT inbound inline IPsec mailbox

[Nithin Dabilpuram]

From: Srujana Challa 

Updates CPT inbound inline configuration mailbox message
format to set CPT credit threshold and bpid, which are
introduced for CN10KB.
This patch also fixes inline inbound config read API.

Fixes: 37da58509579 ("common/cnxk: update inbound inline IPsec config mailbox")
Cc: scha...@marvell.com

Signed-off-by: Srujana Challa 
---
 drivers/common/cnxk/roc_cpt.c  | 14 +-
 drivers/common/cnxk/roc_cpt.h  | 14 --
 drivers/common/cnxk/roc_mbox.h |  6 +-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/common/cnxk/roc_cpt.c b/drivers/common/cnxk/roc_cpt.c
index 48430096dc..6d3052c9be 100644
--- a/drivers/common/cnxk/roc_cpt.c
+++ b/drivers/common/cnxk/roc_cpt.c
@@ -268,9 +268,10 @@ roc_cpt_inline_ipsec_cfg(struct dev *cpt_dev, uint8_t 
lf_id,
 
 int
 roc_cpt_inline_ipsec_inb_cfg_read(struct roc_cpt *roc_cpt,
- struct nix_inline_ipsec_cfg *inb_cfg)
+ struct roc_cpt_inline_ipsec_inb_cfg *cfg)
 {
struct cpt *cpt = roc_cpt_to_cpt_priv(roc_cpt);
+   struct nix_inline_ipsec_cfg *inb_cfg;
struct dev *dev = &cpt->dev;
struct mbox *mbox = mbox_get(dev->mbox);
struct msg_req *req;
@@ -283,6 +284,17 @@ roc_cpt_inline_ipsec_inb_cfg_read(struct roc_cpt *roc_cpt,
}
 
rc = mbox_process_msg(mbox, (void *)&inb_cfg);
+   if (rc) {
+   rc = -EIO;
+   goto exit;
+   }
+   cfg->cpt_credit = inb_cfg->cpt_credit;
+   cfg->egrp = inb_cfg->gen_cfg.egrp;
+   cfg->opcode = inb_cfg->gen_cfg.opcode;
+   cfg->param1 = inb_cfg->gen_cfg.param1;
+   cfg->param2 = inb_cfg->gen_cfg.param2;
+   cfg->bpid = inb_cfg->bpid;
+   cfg->credit_th = inb_cfg->credit_th;
 exit:
mbox_put(mbox);
return rc;
diff --git a/drivers/common/cnxk/roc_cpt.h b/drivers/common/cnxk/roc_cpt.h
index bc9cc19edd..96d066dee3 100644
--- a/drivers/common/cnxk/roc_cpt.h
+++ b/drivers/common/cnxk/roc_cpt.h
@@ -144,6 +144,16 @@ struct roc_cpt_rxc_time_cfg {
uint16_t zombie_thres;
 };
 
+struct roc_cpt_inline_ipsec_inb_cfg {
+   uint32_t cpt_credit;
+   uint16_t opcode;
+   uint16_t param1;
+   uint16_t param2;
+   uint16_t bpid;
+   uint32_t credit_th;
+   uint8_t egrp;
+};
+
 int __roc_api roc_cpt_rxc_time_cfg(struct roc_cpt *roc_cpt,
   struct roc_cpt_rxc_time_cfg *cfg);
 int __roc_api roc_cpt_dev_init(struct roc_cpt *roc_cpt);
@@ -159,8 +169,8 @@ int __roc_api roc_cpt_lf_ctx_flush(struct roc_cpt_lf *lf, 
void *cptr,
 int __roc_api roc_cpt_lf_ctx_reload(struct roc_cpt_lf *lf, void *cptr);
 int __roc_api roc_cpt_inline_ipsec_cfg(struct dev *dev, uint8_t slot,
   struct roc_nix *nix);
-int __roc_api roc_cpt_inline_ipsec_inb_cfg_read(
-   struct roc_cpt *roc_cpt, struct nix_inline_ipsec_cfg *inb_cfg);
+int __roc_api roc_cpt_inline_ipsec_inb_cfg_read(struct roc_cpt *roc_cpt,
+   struct roc_cpt_inline_ipsec_inb_cfg 
*cfg);
 int __roc_api roc_cpt_inline_ipsec_inb_cfg(struct roc_cpt *roc_cpt,
   uint16_t param1, uint16_t param2,
   uint16_t opcode);
diff --git a/drivers/common/cnxk/roc_mbox.h b/drivers/common/cnxk/roc_mbox.h
index b74eb71275..c1769567b5 100644
--- a/drivers/common/cnxk/roc_mbox.h
+++ b/drivers/common/cnxk/roc_mbox.h
@@ -266,7 +266,7 @@ struct mbox_msghdr {
  msg_rsp) \
M(NIX_RX_SW_SYNC, 0x8022, nix_rx_sw_sync, msg_req, msg_rsp)\
M(NIX_READ_INLINE_IPSEC_CFG, 0x8023, nix_read_inline_ipsec_cfg,\
- msg_req, nix_inline_ipsec_cfg)   \
+ msg_req, nix_inline_ipsec_cfg)   \
M(NIX_LF_INLINE_RQ_CFG, 0x8024, nix_lf_inline_rq_cfg,  \
  nix_rq_cpt_field_mask_cfg_req, msg_rsp)  \
M(NIX_SPI_TO_SA_ADD, 0x8026, nix_spi_to_sa_add, nix_spi_to_sa_add_req, \
@@ -1198,6 +1198,8 @@ struct nix_inline_ipsec_cfg {
uint8_t __io cpt_slot;
} inst_qsel;
uint8_t __io enable;
+   uint16_t __io bpid;
+   uint32_t __io credit_th;
 };
 
 /* Per NIX LF inline IPSec configuration */
@@ -1503,6 +1505,8 @@ struct cpt_rx_inline_lf_cfg_msg {
uint16_t __io param2;
uint16_t __io opcode;
uint32_t __io credit;
+   uint32_t __io credit_th;
+   uint16_t __io bpid;
uint32_t __io reserved;
 };
 
-- 
2.25.1

[PATCH 8/9] net/cnxk: make flow control op for SDP as no-op

[Nithin Dabilpuram]

From: Rakesh Kudurumalla 

no action is taken when application calls
rte_eth_dev_flow_ctrl_get(), for sdp port
which is inline with rte_eth_dev_flow_ctrl_set()
for sdp port

Signed-off-by: Rakesh Kudurumalla 
---
 drivers/net/cnxk/cnxk_ethdev_ops.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/cnxk/cnxk_ethdev_ops.c 
b/drivers/net/cnxk/cnxk_ethdev_ops.c
index 8f7287161b..a6ab493626 100644
--- a/drivers/net/cnxk/cnxk_ethdev_ops.c
+++ b/drivers/net/cnxk/cnxk_ethdev_ops.c
@@ -212,6 +212,9 @@ cnxk_nix_flow_ctrl_get(struct rte_eth_dev *eth_dev,
struct roc_nix *nix = &dev->nix;
int mode;
 
+   if (roc_nix_is_sdp(nix))
+   return 0;
+
mode = roc_nix_fc_mode_get(nix);
if (mode < 0)
return mode;
-- 
2.25.1

[PATCH 9/9] common/cnxk: skip L4 checks on inline IPsec traffic

[Nithin Dabilpuram]

Skip L4 checks on inline IPsec traffic as even first fragment
is set as valid ESP packet in order to send it via CPT.

Signed-off-by: Nithin Dabilpuram 
---
 drivers/common/cnxk/roc_nix_inl_dev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/common/cnxk/roc_nix_inl_dev.c 
b/drivers/common/cnxk/roc_nix_inl_dev.c
index b340b92e77..6f60961bc7 100644
--- a/drivers/common/cnxk/roc_nix_inl_dev.c
+++ b/drivers/common/cnxk/roc_nix_inl_dev.c
@@ -13,9 +13,7 @@
 #define NIX_INL_LF_RX_CFG  
\
(ROC_NIX_LF_RX_CFG_DROP_RE | ROC_NIX_LF_RX_CFG_L2_LEN_ERR |\
 ROC_NIX_LF_RX_CFG_IP6_UDP_OPT | ROC_NIX_LF_RX_CFG_DIS_APAD |  \
-ROC_NIX_LF_RX_CFG_CSUM_IL4 | ROC_NIX_LF_RX_CFG_CSUM_OL4 | \
-ROC_NIX_LF_RX_CFG_LEN_IL4 | ROC_NIX_LF_RX_CFG_LEN_IL3 |   \
-ROC_NIX_LF_RX_CFG_LEN_OL4 | ROC_NIX_LF_RX_CFG_LEN_OL3)
+ROC_NIX_LF_RX_CFG_LEN_IL3 | ROC_NIX_LF_RX_CFG_LEN_OL3)
 
 extern uint32_t soft_exp_consumer_cnt;
 static bool soft_exp_poll_thread_exit = true;
-- 
2.25.1

Re: [PATCH] build: fix dependencies lookup

[Thomas Monjalon]

16/01/2023 10:23, David Marchand:
> On Sun, Jan 15, 2023 at 5:53 PM Thomas Monjalon  wrote:
> >
> > The first parameter of the Meson function "find_library()"
> > should be the library name without the "lib" prefix.
> >
> > Otherwise Meson prints this warning:
> > WARNING: find_library('libexecinfo') starting in "lib"
> > only works by accident and is not portable
> >
> > Fixes: 1cd512b2f532 ("build: detect execinfo library on Linux")
> > Fixes: e1defba4cf66 ("raw/ifpga/base: support device tree")
> > Fixes: a489f5dbf437 ("baseband/turbo_sw: support meson build")
> > Fixes: 72c00ae9dba7 ("regex/cn9k: use cnxk infrastructure")
> > Cc: sta...@dpdk.org
> 
> This patch depends on
> https://patchwork.dpdk.org/project/dpdk/patch/20230108095920.327672-1-tho...@monjalon.net/
> which is not marked for backport.

It does not depend really (logically), it is just a patch conflict
because they are changing close lines.

> So either the latter is marked for backport too, or the order of those
> two patches should be inverted (to make it easier for LTS
> maintainers).

I understand what you mean: if this patch is applied first,
it is easier to backport because context did not change compared to old 
versions.

> Otherwise, it lgtm.

So let's merge it :)

RE: [PATCH v5 1/2] ethdev: fix ethdev configuration state on reset

[Hanumanth Reddy Pothula]

Ping

> -Original Message-
> From: Hanumanth Pothula 
> Sent: Wednesday, December 21, 2022 7:37 AM
> To: Thomas Monjalon ; Ferruh Yigit
> ; Andrew Rybchenko
> 
> Cc: dev@dpdk.org; viachesl...@nvidia.com; Jerin Jacob Kollanukkaran
> ; Nithin Kumar Dabilpuram
> ; Hanumanth Reddy Pothula
> 
> Subject: [PATCH v5 1/2] ethdev: fix ethdev configuration state on reset
> 
> Presently, on device reset, ethdev configuration state, dev_configured, is
> not reset.
> 
> On device reset, reset ethdev configuration state to make sure device
> reconfiguration happens cleanly.
> 
> Signed-off-by: Hanumanth Pothula 
> ---
> v5:
>  - Move nic-to-pmd-rx-metadata declaration to struct rte_port.
> v4:
>  - As per spec rte_eth_rx_metadata_negotiate() is processed only when
>dev_configured is set. Hence can't enable automatically when a flow
>command requests metadata.
>  - Add new testpmd command to allow NIC to PMD Rx metadata
> negotiation.
> v3:
>  - Updated run_app.rst with the new command line argument,
>nic-to-pmd-rx-metadata.
>  - Updated commit text.
> v2:
>  - taken cared alignment issues
>  - renamed command line argument from rx-metadata to nic-to-pmd-rx-
> metadata
>  - renamed variable name from rx-metadata to nic_to_pmd_rx_metadata
> ---
>  lib/ethdev/rte_ethdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> 5d5e18db1e..18c59044bc 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1629,6 +1629,8 @@ rte_eth_dev_reset(uint16_t port_id)
>   port_id, rte_strerror(-ret));
>   }
>   ret = dev->dev_ops->dev_reset(dev);
> + if (!ret)
> + dev->data->dev_configured = 0;
> 
>   return eth_err(port_id, ret);
>  }
> --
> 2.25.1

[PATCH] net/i40e: rework maximum frame size configuration

[Simei Su]

This patch removes unnecessary link status check.

Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
Cc: sta...@dpdk.org

Signed-off-by: Simei Su 
---
 drivers/net/i40e/i40e_ethdev.c | 47 +-
 1 file changed, 10 insertions(+), 37 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7726a89d..e21e4d9 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -387,7 +387,6 @@ static int i40e_set_default_mac_addr(struct rte_eth_dev 
*dev,
  struct rte_ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size);
 
 static int i40e_ethertype_filter_convert(
const struct rte_eth_ethertype_filter *input,
@@ -2467,8 +2466,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
"please call hierarchy_commit() "
"before starting the port");
 
-   max_frame_size = dev->data->mtu + I40E_ETH_OVERHEAD;
-   i40e_set_mac_max_frame(dev, max_frame_size);
+   max_frame_size = dev->data->mtu ?
+   dev->data->mtu + I40E_ETH_OVERHEAD :
+   I40E_FRAME_SIZE_MAX;
+
+   /* Set the max frame size to HW*/
+   ret = i40e_aq_set_mac_config(hw, max_frame_size, TRUE, false, 0, NULL);
+   if (ret) {
+   PMD_DRV_LOG(ERR, "Fail to set mac config");
+   return ret;
+   }
 
return I40E_SUCCESS;
 
@@ -12123,40 +12130,6 @@ i40e_cloud_filter_qinq_create(struct i40e_pf *pf)
return ret;
 }
 
-static void
-i40e_set_mac_max_frame(struct rte_eth_dev *dev, uint16_t size)
-{
-   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-   uint32_t rep_cnt = MAX_REPEAT_TIME;
-   struct rte_eth_link link;
-   enum i40e_status_code status;
-   bool can_be_set = true;
-
-   /*
-* I40E_MEDIA_TYPE_BASET link up can be ignored
-* I40E_MEDIA_TYPE_BASET link down that hw->phy.media_type
-* is I40E_MEDIA_TYPE_UNKNOWN
-*/
-   if (hw->phy.media_type != I40E_MEDIA_TYPE_BASET &&
-   hw->phy.media_type != I40E_MEDIA_TYPE_UNKNOWN) {
-   do {
-   update_link_reg(hw, &link);
-   if (link.link_status)
-   break;
-   rte_delay_ms(CHECK_INTERVAL);
-   } while (--rep_cnt);
-   can_be_set = !!link.link_status;
-   }
-
-   if (can_be_set) {
-   status = i40e_aq_set_mac_config(hw, size, TRUE, 0, false, NULL);
-   if (status != I40E_SUCCESS)
-   PMD_DRV_LOG(ERR, "Failed to set max frame size at port 
level");
-   } else {
-   PMD_DRV_LOG(ERR, "Set max frame size at port level not 
applicable on link down");
-   }
-}
-
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(i40e_logtype_driver, driver, NOTICE);
 #ifdef RTE_ETHDEV_DEBUG_RX
-- 
2.9.5

RE: [PATCH v2] net/i40e: don't check link status on device start

[Su, Simei]

Hi David,

> -Original Message-
> From: David Marchand 
> Sent: Friday, January 13, 2023 9:53 PM
> To: Zhang, Helin 
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Mcnamara, John ;
> dev@dpdk.org; sta...@dpdk.org; Zhang, Qi Z ; Dapeng
> Yu ; Wenxuan Wu 
> Subject: Re: [PATCH v2] net/i40e: don't check link status on device start
> 
> On Fri, Jan 13, 2023 at 2:51 PM Zhang, Helin  wrote:
> >
> >
> >
> > > -Original Message-
> > > From: David Marchand 
> > > Sent: Friday, January 13, 2023 9:47 PM
> > > To: Zhang, Helin 
> > > Cc: Zhang, Yuying ; Xing, Beilei
> > > ; Mcnamara, John ;
> > > dev@dpdk.org; sta...@dpdk.org; Zhang, Qi Z ;
> > > Dapeng Yu ; Wenxuan Wu
> 
> > > Subject: Re: [PATCH v2] net/i40e: don't check link status on device
> > > start
> > >
> > > On Fri, Jan 13, 2023 at 2:39 PM Zhang, Helin 
> wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: David Marchand 
> > > > > Sent: Friday, January 13, 2023 9:33 PM
> > > > > To: Zhang, Yuying ; Xing, Beilei
> > > > > ; Mcnamara, John
> > > > > 
> > > > > Cc: dev@dpdk.org; sta...@dpdk.org; Zhang, Qi Z
> > > > > ; Dapeng Yu ;
> > > > > Wenxuan
> > > Wu
> > > > > 
> > > > > Subject: Re: [PATCH v2] net/i40e: don't check link status on
> > > > > device start
> > > > >
> > > > > Hello i40e maintainers, John,
> > > > >
> > > > > On Mon, Jan 9, 2023 at 10:21 AM David Marchand
> > > > >  wrote:
> > > > > > On Tue, Jan 3, 2023 at 3:02 PM David Marchand
> > > > >  wrote:
> > > > > > > Hi i40e maintainers,
> > > > > > >
> > > > > > > On Tue, Dec 13, 2022 at 10:19 AM David Marchand
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > The mentioned changes broke existing applications when the
> > > > > > > > link status of i40e ports is down at the time the port is 
> > > > > > > > started.
> > > > > > > > Revert those changes, the original issue will need a different 
> > > > > > > > fix.
> > > > Hi David
> > > >
> > > > Does it break all the application or just a specific application?
> > >
> > > I don't see how it would not affect all applications seeing how the
> > > original patch is dumb.
> > >
> > > > We may need to understand the issue you met, and try to fix it later.
> > >
> > > Just unplug the cable or fake a link down on your i40e port, start
> > > your application or port, then plug the cable back.
> > > The max frame size will never get applied to hw.
> > Got it, I will forward to a right expert to check. Thank you very much for
> reaching out to us!
> 
> I hope I get a reply _soon_.
> Or I will just apply those reverts.

If applying those reverts, some issues still exist on our side. I sent one 
patch to patchwork:
https://patchwork.dpdk.org/project/dpdk/patch/20230116105318.19412-1-simei...@intel.com/.
You can try this patch to see whether it can solve the issue on your side.
At the same time, on our side, we need to do regression test further to check 
if this patch will affect other
cases, the regression takes some time.

Thanks,
Simei

> 
> 
> Thanks.
> 
> --
> David Marchand

Re: [PATCH] net/i40e: rework maximum frame size configuration

[David Marchand]

On Mon, Jan 16, 2023 at 11:54 AM Simei Su  wrote:
>
> This patch removes unnecessary link status check.
>
> Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port level")
> Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port level")
> Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Simei Su 

Thanks for looking into the issue.

This is rather close to what I had tried [1] along my original report,
but it failed in the CI.
Let's see how the validation of your patch goes.

1: 
https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1-david.march...@redhat.com/


-- 
David Marchand

RE: [2nd Try]:Re: Traffic Management API Questions

[Singh, Jasvinder]

Hi Venky,

Please see inline;

Thanks,
Jasvinder


From: Venky Venkatesh 
Sent: Monday, January 16, 2023 8:06 AM
To: Singh, Jasvinder 
Cc: dev@dpdk.org
Subject: Re: [2nd Try]:Re: Traffic Management API Questions

Hi Jasvinder,
Thanks for the insights on the complexity of adding a layer.
As for the workaround that you suggested using multiple subports, if I 
understand it correctly (pls correct if I misunderstood) it would not meet our 
needs:

  *   We require multiple heterogeneous ports (i.e. ports with different 
bandwidths -- with no excess sharing since these are port limits). That would 
probably need some shaper attached there too since WRR (at the application) 
would share the instantaneous excess among the siblings.
·  As for the 2nd second suggestion (increase the number of subports): our need 
(in addition to multiple ports of different bandwidths at the top level) is to 
have  4 more TM layers for a total of 5. I am not looking at the assignment of 
the terms port/subport/user/pipe etc in the DPDK documentation -- instead am 
looking at it as abstract scheduling (and/or shaping) layers with differing 
abilities in some layers. So in order to compensate for the missing shaper at 
the port level I was planning to add 1 additional layer (so that what in DPDK 
documentation is referred to as subport is actually a port -- since the subport 
layer has the property of not sharing excess between siblings. With that 
principle, I am not clear how adding width to the subport layer (as I 
understand your suggestion) would help.
[JS] – I was suggesting to assume subports as ports in the existing 
implementation and assign fixed bandwidth to each of the subports. By doing so, 
you would have multiple subports (re-named as ports) with shaper attached. Only 
limitation in such solution is that hierarchy would have single root node with 
the bandwidth equal to sum of subports bandwidth and all the subports would be 
served individually in round robin manner. If it doesn’t suit your  
requirement, you need to make changes as you suggested above.

Thanks
-Venky


On Wed, Jan 11, 2023 at 9:24 AM Singh, Jasvinder 
mailto:jasvinder.si...@intel.com>> wrote:
Hi Venky,

Please see inline;

Jasvinder

From: Venky Venkatesh 
mailto:vvenkat...@paloaltonetworks.com>>
Sent: Wednesday, January 11, 2023 11:56 AM
To: Singh, Jasvinder 
mailto:jasvinder.si...@intel.com>>
Cc: dev@dpdk.org
Subject: Re: [2nd Try]:Re: Traffic Management API Questions

Hi Jasvinder,
Thanks for the detailed answers. Our need is to have shaping at the port level 
as well. I am trying to see what would be the way to accomplish this given the 
current limitations of the sched library implementation in this regard. I see 2 
options:

  *   The top level (i.e. port level) documentation says the following: "Output 
Ethernet port 1/10/40 GbE" and "Multiple ports are scheduled in round robin 
order with all ports having equal priority". Questions:

 *   Do all the ports have to be of the same speed OR can it be a 
heterogeneous set of port speeds?
[JS] – the library supports single port (root node) of the hierarchy. Each port 
can have multiple subports configured using different shaping rates. If you 
desire to have multiple ports, each port would have separate hierarchical tree 
underneath. Different ports could have different speed.
o If it can be a heterogeneous set of ports, is the scheduling across those 
ports weighted round robin as opposed to round robin?
[JS] – Scheduling across multiple ports is not supported in current sched 
library. At the application level, one can think of invoking enqueue/dequeue 
sched API in round robin or weighted round robin manner.

 *   Are Speeds other than  1/10/40 GbE not supported?
[JS] – Speeds other than above is supported, for eg. 25G, 50G etc.

 *   I suppose this heterogeneous mix of port speeds is implemented by 
playing with the weights across ports, correct?
[JS] -please see above answers

 *   If so, what problem do you foresee if we provide arbitrary bandwidth 
ports by regulating the above weights?
[JS] – I don’t see any issue.

  *   The other alternative would be to add another layer (which has a shaper)  
to the hierarchy by mimicking one of the existing layers: how amenable is the 
current implementation to that?
Do either of the above look like workable ideas? Are there any other approaches 
where we could accomplish our requirement with minimal changes to the code 
logic?
[JS] – adding another layer will require considerable work in library. How 
about using multiple subports with different shaping bandwidth where each 
subport maintain #subcribers and send traffic out through single physical port 
(root node). It will need less effort and library supports multiple subports 
under single port (root node).


Thanks
-Venky

On Tue, Jan 10, 2023 at 2:54 AM Singh, Jasvinder 
mailto:jasvinder.si...@intel.com>> wrote:
Hi Venky,

Please see inline.

Thanks,
Jasvinder

Re: [PATCH V8 0/8] telemetry: fix data truncation and conversion error and add hex integer API

[lihuisong (C)]

Hi Ferruh and Andrew,

This patch series optimizes some codes and bug.
Can you take a look at this patch series?
If there are no other questions, can it be merged?

Best,
Huisong

在 2022/12/19 15:06, Huisong Li 写道:

Some lib telemetry interfaces add the 'u32' and 'u64' data by the
rte_tel_data_add_dict/array_int API. This may cause data conversion
error or data truncation. This patch series uses 'u64' functions to
do this.

In addition, this patch series introduces two APIs to store unsigned
integer values as hexadecimal encoded strings in telemetry library.

---
  -v8: fix the coding style in patch 7/8
  -v7: replace sprintf with snprintf in patch 6/8
  -v6: fix code alignment to keep in line with codes in the file
  -v5:
 - drop a refactor patch.
 - no limit the bit width for xxx_uint_hex API.
  -v4:
 - remove 'u32' value type.merg
 - add padding zero for hexadecimal value
  -v3: fix a misspelling mistake in commit log.
  -v2:
 - fix ABI break warning.
 - introduce two APIs to store u32 and u64 values as hexadecimal
   encoded strings.

Huisong Li (8):
   telemetry: move to header to controllable range
   ethdev: fix possible data truncation and conversion error
   mempool: fix possible data truncation and conversion error
   cryptodev: fix possible data conversion error
   mem: possible data truncation and conversion error
   telemetry: support adding integer value as hexadecimal
   test: add test cases for adding hex integer value API
   ethdev: display capability values in hexadecimal format

  app/test/test_telemetry_data.c | 150 +
  lib/cryptodev/rte_cryptodev.c  |   2 +-
  lib/eal/common/eal_common_memory.c |  10 +-
  lib/ethdev/rte_ethdev.c|  19 ++--
  lib/mempool/rte_mempool.c  |  24 ++---
  lib/telemetry/rte_telemetry.h  |  52 +-
  lib/telemetry/telemetry_data.c |  73 ++
  lib/telemetry/version.map  |   9 ++
  8 files changed, 309 insertions(+), 30 deletions(-)

RE: [PATCH] net/i40e: rework maximum frame size configuration

[Su, Simei]

Hi David,

> -Original Message-
> From: David Marchand 
> Sent: Monday, January 16, 2023 7:19 PM
> To: Su, Simei 
> Cc: Xing, Beilei ; Zhang, Yuying
> ; dev@dpdk.org; Zhang, Qi Z
> ; Yang, Qiming ;
> sta...@dpdk.org; Zhang, Helin 
> Subject: Re: [PATCH] net/i40e: rework maximum frame size configuration
> 
> On Mon, Jan 16, 2023 at 11:54 AM Simei Su  wrote:
> >
> > This patch removes unnecessary link status check.
> >
> > Fixes: a4ba77367923 ("net/i40e: enable maximum frame size at port
> > level")
> > Fixes: 2184f7cdeeaa ("net/i40e: fix max frame size config at port
> > level")
> > Fixes: 719469f13b11 ("net/i40e: fix jumbo frame Rx with X722")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Simei Su 
> 
> Thanks for looking into the issue.
> 
> This is rather close to what I had tried [1] along my original report, but it 
> failed
> in the CI.
> Let's see how the validation of your patch goes.
> 
> 1:
> https://patchwork.dpdk.org/project/dpdk/patch/20221212143715.29649-1-d
> avid.march...@redhat.com/
> 

OK. We will find one environment to see why the unit test failed.

Thanks,
Simei

> 
> --
> David Marchand

Re: [PATCH v2 0/3] net/bonding: support device private dump

[Ferruh Yigit]

On 12/15/2022 10:56 AM, lihuisong (C) wrote:

> 
> 在 2022/12/14 17:55, humin (Q) 写道:

>>
>> 在 2022/12/14 14:13, Chengwen Feng 写道:
>>> This patchset adds device private dump for bonding PMD, and use
>>> rte_eth_dev_priv_dump API to implement testpmd show bonding command.
>>>
>>> Chengwen Feng (3):
>>>    net/bonding: support private dump ops
>>>    net/bonding: support dump LACP info
>>>    net/bonding: use dump API to impl show bonding cmd
>>>
>>> ---
>>> v2:
>>> * address Min Hu's comment
>>> * use rte_eth_dev_priv_dump API to implement testpmd show bonding
>>>    command.
>>>
>>>   .../link_bonding_poll_mode_drv_lib.rst    |  13 +-
>>>   drivers/net/bonding/bonding_testpmd.c | 281 +-
>>>   drivers/net/bonding/rte_eth_bond_pmd.c    | 244 ++-
>>>   3 files changed, 249 insertions(+), 289 deletions(-)
>>>
>> Acked-by：Min Hu (Connor) 
>
> Indeed, it is better to move them to bonding PMD. lgtm
> Series-acked-by: Huisong Li 

+1 to move code to bonding PMD & merge LACP info withing show bonding
config command and remove the redundant one from bonding testpmd,

For series,
Acked-by: Ferruh Yigit 


Series applied to dpdk-next-net/main, thanks.

[PATCH v6 2/4] event/dlb2: remove superfluous rte_memcpy

[Morten Brørup]

Copying with the same src and dst address has no effect; removed to
avoid compiler warning with decorated rte_memcpy.

Fixes: e7c9971a857a ("event/dlb2: add probe-time hardware init")
Cc: timothy.mcdan...@intel.com

Signed-off-by: Morten Brørup 
Acked-by: Stephen Hemminger 
---
v6:
* Add Fixes to patch description. (Stephen)
v5:
* First patch in series.
---
 drivers/event/dlb2/dlb2.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
index 60c5cd4804..03d32c779f 100644
--- a/drivers/event/dlb2/dlb2.c
+++ b/drivers/event/dlb2/dlb2.c
@@ -215,7 +215,6 @@ static int
 dlb2_hw_query_resources(struct dlb2_eventdev *dlb2)
 {
struct dlb2_hw_dev *handle = &dlb2->qm_instance;
-   struct dlb2_hw_resource_info *dlb2_info = &handle->info;
int num_ldb_ports;
int ret;
 
@@ -277,8 +276,6 @@ dlb2_hw_query_resources(struct dlb2_eventdev *dlb2)
handle->info.hw_rsrc_max.reorder_window_size =
dlb2->hw_rsrc_query_results.num_hist_list_entries;
 
-   rte_memcpy(dlb2_info, &handle->info.hw_rsrc_max, sizeof(*dlb2_info));
-
return 0;
 }
 
-- 
2.17.1

[PATCH v6 1/4] net/bnx2x: fix warnings about rte_memcpy lengths

[Morten Brørup]

Bugfix: The vlan in the bulletin does not contain a VLAN header, only the
VLAN ID, so only copy 2 byte, not 4. The target structure has padding
after the field, so copying 2 byte too many is effectively harmless.
There is no need to backport this patch.

Use RTE_PTR_ADD where copying arrays to the offset of a first field in a
structure holding multiple fields, to avoid compiler warnings with
decorated rte_memcpy.

Bugzilla ID: 1146

Fixes: 540a211084a7695a1c7bc43068934c140d6989be ("bnx2x: driver core")
Cc: step...@networkplumber.org
Cc: rm...@marvell.com
Cc: shsha...@marvell.com

Signed-off-by: Morten Brørup 
---
v6:
* Add Fixes to patch description.
* Fix checkpatch warnings.
v5:
* No changes.
v4:
* Type casting did not fix the warnings, so use RTE_PTR_ADD instead.
v3:
* First patch in series.
---
 drivers/net/bnx2x/bnx2x_stats.c | 11 +++
 drivers/net/bnx2x/bnx2x_vfpf.c  |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_stats.c b/drivers/net/bnx2x/bnx2x_stats.c
index c07b01510a..bc4a8b8e71 100644
--- a/drivers/net/bnx2x/bnx2x_stats.c
+++ b/drivers/net/bnx2x/bnx2x_stats.c
@@ -819,8 +819,9 @@ bnx2x_hw_stats_update(struct bnx2x_softc *sc)
 
 rte_memcpy(old, new, sizeof(struct nig_stats));
 
-rte_memcpy(&(estats->rx_stat_ifhcinbadoctets_hi), &(pstats->mac_stx[1]),
-  sizeof(struct mac_stx));
+   rte_memcpy(RTE_PTR_ADD(estats,
+   offsetof(struct bnx2x_eth_stats, 
rx_stat_ifhcinbadoctets_hi)),
+   &pstats->mac_stx[1], sizeof(struct mac_stx));
 estats->brb_drop_hi = pstats->brb_drop_hi;
 estats->brb_drop_lo = pstats->brb_drop_lo;
 
@@ -1492,9 +1493,11 @@ bnx2x_stats_init(struct bnx2x_softc *sc)
REG_RD(sc, NIG_REG_STAT0_BRB_TRUNCATE + port*0x38);
if (!CHIP_IS_E3(sc)) {
REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT0 + port*0x50,
-   &(sc->port.old_nig_stats.egress_mac_pkt0_lo), 
2);
+   RTE_PTR_ADD(&sc->port.old_nig_stats,
+   offsetof(struct nig_stats, 
egress_mac_pkt0_lo)), 2);
REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT1 + port*0x50,
-   &(sc->port.old_nig_stats.egress_mac_pkt1_lo), 
2);
+   RTE_PTR_ADD(&sc->port.old_nig_stats,
+   offsetof(struct nig_stats, 
egress_mac_pkt1_lo)), 2);
}
 
/* function stats */
diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c b/drivers/net/bnx2x/bnx2x_vfpf.c
index 63953c2979..87631c76ca 100644
--- a/drivers/net/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/bnx2x/bnx2x_vfpf.c
@@ -54,7 +54,7 @@ bnx2x_check_bull(struct bnx2x_softc *sc)
if (valid_bitmap & (1 << MAC_ADDR_VALID) && memcmp(bull->mac, 
sc->old_bulletin.mac, ETH_ALEN))
rte_memcpy(&sc->link_params.mac_addr, bull->mac, ETH_ALEN);
if (valid_bitmap & (1 << VLAN_VALID))
-   rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, RTE_VLAN_HLEN);
+   rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, 
sizeof(bull->vlan));
 
sc->old_bulletin = *bull;
 
-- 
2.17.1

[PATCH v6 3/4] net/mlx5: fix warning about rte_memcpy length

[Morten Brørup]

Use RTE_PTR_ADD where copying to the offset of a field in a structure
holding multiple fields, to avoid compiler warnings with decorated
rte_memcpy.

Fixes: 16a7dbc4f69006cc1c96ca2a2c6d3e3c51a2ff50 ("net/mlx5: make flow modify 
action list thread safe")
Cc: xuemi...@nvidia.com
Cc: ma...@nvidia.com
Cc: viachesl...@nvidia.com

Signed-off-by: Morten Brørup 
---
v6:
* Add Fixes to patch description.
v5:
* First patch in series.
---
 drivers/net/mlx5/mlx5_flow_dv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 62c38b87a1..dd9f5fda1a 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5662,7 +5662,7 @@ flow_dv_modify_create_cb(void *tool_ctx, void *cb_ctx)
   "cannot allocate resource memory");
return NULL;
}
-   rte_memcpy(&entry->ft_type,
+   rte_memcpy(RTE_PTR_ADD(entry, offsetof(typeof(*entry), ft_type)),
   RTE_PTR_ADD(ref, offsetof(typeof(*ref), ft_type)),
   key_len + data_len);
if (entry->ft_type == MLX5DV_FLOW_TABLE_TYPE_FDB)
-- 
2.17.1

[PATCH v6 4/4] eal: add nonnull and access function attributes

[Morten Brørup]

Add nonnull function attribute to help the compiler detect a NULL
pointer being passed to a function not accepting NULL pointers as an
argument at build time.

Add access function attributes to tell the compiler how a function
accesses memory pointed to by its pointer arguments.

Add these attributes to the rte_memcpy() function, as the first in
hopefully many to come.

Signed-off-by: Morten Brørup 
Acked-by: Tyler Retzlaff 
Reviewed-by: Ruifeng Wang 
---
v6:
* Make this the last patch in the series, putting the fixes first.
  (David)
* Split the generic access macro into dedicated macros per access mode.
  (David)
v5:
* No changes.
v4:
* No changes.
v3:
* No changes.
v2:
* Only define "nonnull" for GCC and CLANG.
* Append _param/_params to prepare for possible future attributes
  attached directly to the individual parameters, like __rte_unused.
* Use RTE_TOOLCHAIN_GCC instead of RTE_CC_GCC, to fix complaints about
  GCC_VERSION being undefined.
* Try to fix Doxygen compliants.
---
 lib/eal/arm/include/rte_memcpy_32.h |  8 +++
 lib/eal/arm/include/rte_memcpy_64.h |  6 +++
 lib/eal/include/rte_common.h| 76 +
 lib/eal/ppc/include/rte_memcpy.h|  3 ++
 lib/eal/x86/include/rte_memcpy.h|  6 +++
 5 files changed, 99 insertions(+)

diff --git a/lib/eal/arm/include/rte_memcpy_32.h 
b/lib/eal/arm/include/rte_memcpy_32.h
index fb3245b59c..a625a91951 100644
--- a/lib/eal/arm/include/rte_memcpy_32.h
+++ b/lib/eal/arm/include/rte_memcpy_32.h
@@ -14,6 +14,8 @@ extern "C" {
 
 #include "generic/rte_memcpy.h"
 
+#include 
+
 #ifdef RTE_ARCH_ARM_NEON_MEMCPY
 
 #ifndef __ARM_NEON
@@ -125,6 +127,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
memcpy((dst), (src), (n)) :  \
rte_memcpy_func((dst), (src), (n)); })
 
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static inline void *
 rte_memcpy_func(void *dst, const void *src, size_t n)
 {
@@ -290,6 +295,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
memcpy(dst, src, 256);
 }
 
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static inline void *
 rte_memcpy(void *dst, const void *src, size_t n)
 {
diff --git a/lib/eal/arm/include/rte_memcpy_64.h 
b/lib/eal/arm/include/rte_memcpy_64.h
index 85ad587bd3..0c86237cc9 100644
--- a/lib/eal/arm/include/rte_memcpy_64.h
+++ b/lib/eal/arm/include/rte_memcpy_64.h
@@ -282,6 +282,9 @@ void rte_memcpy_ge64(uint8_t *dst, const uint8_t *src, 
size_t n)
 }
 
 #if RTE_CACHE_LINE_SIZE >= 128
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static __rte_always_inline
 void *rte_memcpy(void *dst, const void *src, size_t n)
 {
@@ -303,6 +306,9 @@ void *rte_memcpy(void *dst, const void *src, size_t n)
 }
 
 #else
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static __rte_always_inline
 void *rte_memcpy(void *dst, const void *src, size_t n)
 {
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 15765b408d..9ee4d4ff6a 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -149,6 +149,82 @@ typedef uint16_t unaligned_uint16_t;
__attribute__((format(printf, format_index, first_arg)))
 #endif
 
+/**
+ * Tells compiler that the pointer arguments must be non-null.
+ *
+ * @param ...
+ *Comma separated list of parameter indexes of pointer arguments.
+ */
+#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+#define __rte_nonnull_params(...) \
+   __attribute__((nonnull(__VA_ARGS__)))
+#else
+#define __rte_nonnull_params(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+*  is only read, not written to, by the function.
+*  It might not be accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and parameter index of size argument.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
+#define __rte_read_only_param(...) \
+   __attribute__((access(read_only, __VA_ARGS__)))
+#else
+#define __rte_read_only_param(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+*  is only written to, not read, by the function.
+*  It might not be accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and parameter index of size argument.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
+#define __rte_write_only_param(...) \
+   __attribute__((access(write_only, __VA_ARGS__)))
+#else
+#define __rte_write_only_param(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+*  is both read and written to by the function.
+*  It might not be read, written to, or accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and parameter index of size argument.
+ */

RE: [PATCH v5 1/4] eal: add nonnull and access function attributes

[Morten Brørup]

> From: Morten Brørup
> Sent: Monday, 9 January 2023 13.28
> 
> > From: David Marchand [mailto:david.march...@redhat.com]
> > Sent: Monday, 9 January 2023 12.22
> > attributes
> >
> > On Wed, Dec 28, 2022 at 4:10 PM Morten Brørup
> >  wrote:
> > >
> > > Add "nonnull" function attribute to help the compiler detect a NULL
> > > pointer being passed to a function not accepting NULL pointers as
> an
> > > argument at build time.
> > >
> > > Add "access" function attribute to tell the compiler how a function
> > > accesses its pointer arguments.
> > >
> > > Add these attributes to the rte_memcpy() function, as the first in
> > > hopefully many to come.
> > >
> >
> > Compilation is broken starting first patch, so patches must be
> > reordered to have the fixes first.
> 
> Will do.
> 
> >
> >
> > > v5:
> > > * No changes.
> > > v4:
> > > * No changes.
> > > v3:
> > > * No changes.
> > > v2:
> > > * Only define "nonnull" for GCC and CLANG.
> > > * Append _param/_params to prepare for possible future attributes
> > >   attached directly to the individual parameters, like
> __rte_unused.
> > > * Use RTE_TOOLCHAIN_GCC instead of RTE_CC_GCC, to fix complaints
> > about
> > >   GCC_VERSION being undefined.
> > > * Try to fix Doxygen compliants.
> >
> > Patch history should be put as annotations (i.e. after --- but before
> > patch content).
> 
> Will do.
> 
> [...]
> 
> > This is tightly bound to gcc syntax.
> > With dedicated macros (which I find easier to read too), we can hope
> > to adapt to other compilers if some of them add support for this kind
> > of code cookies.
> > __rte_read_only_params(indexes...)
> > __rte_write_only_params(indexes...)
> > __rte_no_access_params(indexes...)
> 
> I agree. Splitting the generic access macro up into dedicated access
> macros is probably better.

Version 6 of this patch series is here:
https://patchwork.dpdk.org/project/dpdk/list/?series=26560

Please ignore the name of the series.

-Morten

[PATCH v7 1/4] net/bnx2x: fix warnings about rte_memcpy lengths

[Morten Brørup]

Bugfix: The vlan in the bulletin does not contain a VLAN header, only the
VLAN ID, so only copy 2 byte, not 4. The target structure has padding
after the field, so copying 2 byte too many is effectively harmless.
There is no need to backport this patch.

Use RTE_PTR_ADD where copying arrays to the offset of a first field in a
structure holding multiple fields, to avoid compiler warnings with
decorated rte_memcpy.

Bugzilla ID: 1146

Fixes: 540a211084a7695a1c7bc43068934c140d6989be ("bnx2x: driver core")
Cc: step...@networkplumber.org
Cc: rm...@marvell.com
Cc: shsha...@marvell.com

Signed-off-by: Morten Brørup 
---
v7:
* No changes.
v6:
* Add Fixes to patch description.
* Fix checkpatch warnings.
v5:
* No changes.
v4:
* Type casting did not fix the warnings, so use RTE_PTR_ADD instead.
v3:
* First patch in series.
---
 drivers/net/bnx2x/bnx2x_stats.c | 11 +++
 drivers/net/bnx2x/bnx2x_vfpf.c  |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_stats.c b/drivers/net/bnx2x/bnx2x_stats.c
index c07b01510a..bc4a8b8e71 100644
--- a/drivers/net/bnx2x/bnx2x_stats.c
+++ b/drivers/net/bnx2x/bnx2x_stats.c
@@ -819,8 +819,9 @@ bnx2x_hw_stats_update(struct bnx2x_softc *sc)
 
 rte_memcpy(old, new, sizeof(struct nig_stats));
 
-rte_memcpy(&(estats->rx_stat_ifhcinbadoctets_hi), &(pstats->mac_stx[1]),
-  sizeof(struct mac_stx));
+   rte_memcpy(RTE_PTR_ADD(estats,
+   offsetof(struct bnx2x_eth_stats, 
rx_stat_ifhcinbadoctets_hi)),
+   &pstats->mac_stx[1], sizeof(struct mac_stx));
 estats->brb_drop_hi = pstats->brb_drop_hi;
 estats->brb_drop_lo = pstats->brb_drop_lo;
 
@@ -1492,9 +1493,11 @@ bnx2x_stats_init(struct bnx2x_softc *sc)
REG_RD(sc, NIG_REG_STAT0_BRB_TRUNCATE + port*0x38);
if (!CHIP_IS_E3(sc)) {
REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT0 + port*0x50,
-   &(sc->port.old_nig_stats.egress_mac_pkt0_lo), 
2);
+   RTE_PTR_ADD(&sc->port.old_nig_stats,
+   offsetof(struct nig_stats, 
egress_mac_pkt0_lo)), 2);
REG_RD_DMAE(sc, NIG_REG_STAT0_EGRESS_MAC_PKT1 + port*0x50,
-   &(sc->port.old_nig_stats.egress_mac_pkt1_lo), 
2);
+   RTE_PTR_ADD(&sc->port.old_nig_stats,
+   offsetof(struct nig_stats, 
egress_mac_pkt1_lo)), 2);
}
 
/* function stats */
diff --git a/drivers/net/bnx2x/bnx2x_vfpf.c b/drivers/net/bnx2x/bnx2x_vfpf.c
index 63953c2979..87631c76ca 100644
--- a/drivers/net/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/bnx2x/bnx2x_vfpf.c
@@ -54,7 +54,7 @@ bnx2x_check_bull(struct bnx2x_softc *sc)
if (valid_bitmap & (1 << MAC_ADDR_VALID) && memcmp(bull->mac, 
sc->old_bulletin.mac, ETH_ALEN))
rte_memcpy(&sc->link_params.mac_addr, bull->mac, ETH_ALEN);
if (valid_bitmap & (1 << VLAN_VALID))
-   rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, RTE_VLAN_HLEN);
+   rte_memcpy(&bull->vlan, &sc->old_bulletin.vlan, 
sizeof(bull->vlan));
 
sc->old_bulletin = *bull;
 
-- 
2.17.1

[PATCH v7 2/4] event/dlb2: remove superfluous rte_memcpy

[Morten Brørup]

Copying with the same src and dst address has no effect; removed to
avoid compiler warning with decorated rte_memcpy.

Fixes: e7c9971a857a ("event/dlb2: add probe-time hardware init")
Cc: timothy.mcdan...@intel.com

Signed-off-by: Morten Brørup 
Acked-by: Stephen Hemminger 
---
v7:
* No changes.
v6:
* Add Fixes to patch description. (Stephen)
v5:
* First patch in series.
---
 drivers/event/dlb2/dlb2.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
index 60c5cd4804..03d32c779f 100644
--- a/drivers/event/dlb2/dlb2.c
+++ b/drivers/event/dlb2/dlb2.c
@@ -215,7 +215,6 @@ static int
 dlb2_hw_query_resources(struct dlb2_eventdev *dlb2)
 {
struct dlb2_hw_dev *handle = &dlb2->qm_instance;
-   struct dlb2_hw_resource_info *dlb2_info = &handle->info;
int num_ldb_ports;
int ret;
 
@@ -277,8 +276,6 @@ dlb2_hw_query_resources(struct dlb2_eventdev *dlb2)
handle->info.hw_rsrc_max.reorder_window_size =
dlb2->hw_rsrc_query_results.num_hist_list_entries;
 
-   rte_memcpy(dlb2_info, &handle->info.hw_rsrc_max, sizeof(*dlb2_info));
-
return 0;
 }
 
-- 
2.17.1

[PATCH v7 3/4] net/mlx5: fix warning about rte_memcpy length

[Morten Brørup]

Use RTE_PTR_ADD where copying to the offset of a field in a structure
holding multiple fields, to avoid compiler warnings with decorated
rte_memcpy.

Fixes: 16a7dbc4f69006cc1c96ca2a2c6d3e3c51a2ff50 ("net/mlx5: make flow modify 
action list thread safe")
Cc: xuemi...@nvidia.com
Cc: ma...@nvidia.com
Cc: viachesl...@nvidia.com

Signed-off-by: Morten Brørup 
---
v7:
* No changes.
v6:
* Add Fixes to patch description.
v5:
* First patch in series.
---
 drivers/net/mlx5/mlx5_flow_dv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 62c38b87a1..dd9f5fda1a 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5662,7 +5662,7 @@ flow_dv_modify_create_cb(void *tool_ctx, void *cb_ctx)
   "cannot allocate resource memory");
return NULL;
}
-   rte_memcpy(&entry->ft_type,
+   rte_memcpy(RTE_PTR_ADD(entry, offsetof(typeof(*entry), ft_type)),
   RTE_PTR_ADD(ref, offsetof(typeof(*ref), ft_type)),
   key_len + data_len);
if (entry->ft_type == MLX5DV_FLOW_TABLE_TYPE_FDB)
-- 
2.17.1

[PATCH v7 4/4] eal: add nonnull and access function attributes

[Morten Brørup]

Add nonnull function attribute to help the compiler detect a NULL
pointer being passed to a function not accepting NULL pointers as an
argument at build time.

Add access function attributes to tell the compiler how a function
accesses memory pointed to by its pointer arguments.

Add these attributes to the rte_memcpy() function, as the first in
hopefully many to come.

Signed-off-by: Morten Brørup 
Acked-by: Tyler Retzlaff 
Reviewed-by: Ruifeng Wang 
---
v7:
* Fix indentation. (checkpatch)
v6:
* Make this the last patch in the series, putting the fixes first.
  (David)
* Split the generic access macro into dedicated macros per access mode.
  (David)
v5:
* No changes.
v4:
* No changes.
v3:
* No changes.
v2:
* Only define "nonnull" for GCC and CLANG.
* Append _param/_params to prepare for possible future attributes
  attached directly to the individual parameters, like __rte_unused.
* Use RTE_TOOLCHAIN_GCC instead of RTE_CC_GCC, to fix complaints about
  GCC_VERSION being undefined.
* Try to fix Doxygen compliants.
---
 lib/eal/arm/include/rte_memcpy_32.h |  8 +++
 lib/eal/arm/include/rte_memcpy_64.h |  6 +++
 lib/eal/include/rte_common.h| 76 +
 lib/eal/ppc/include/rte_memcpy.h|  3 ++
 lib/eal/x86/include/rte_memcpy.h|  6 +++
 5 files changed, 99 insertions(+)

diff --git a/lib/eal/arm/include/rte_memcpy_32.h 
b/lib/eal/arm/include/rte_memcpy_32.h
index fb3245b59c..a625a91951 100644
--- a/lib/eal/arm/include/rte_memcpy_32.h
+++ b/lib/eal/arm/include/rte_memcpy_32.h
@@ -14,6 +14,8 @@ extern "C" {
 
 #include "generic/rte_memcpy.h"
 
+#include 
+
 #ifdef RTE_ARCH_ARM_NEON_MEMCPY
 
 #ifndef __ARM_NEON
@@ -125,6 +127,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
memcpy((dst), (src), (n)) :  \
rte_memcpy_func((dst), (src), (n)); })
 
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static inline void *
 rte_memcpy_func(void *dst, const void *src, size_t n)
 {
@@ -290,6 +295,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
memcpy(dst, src, 256);
 }
 
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static inline void *
 rte_memcpy(void *dst, const void *src, size_t n)
 {
diff --git a/lib/eal/arm/include/rte_memcpy_64.h 
b/lib/eal/arm/include/rte_memcpy_64.h
index 85ad587bd3..0c86237cc9 100644
--- a/lib/eal/arm/include/rte_memcpy_64.h
+++ b/lib/eal/arm/include/rte_memcpy_64.h
@@ -282,6 +282,9 @@ void rte_memcpy_ge64(uint8_t *dst, const uint8_t *src, 
size_t n)
 }
 
 #if RTE_CACHE_LINE_SIZE >= 128
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static __rte_always_inline
 void *rte_memcpy(void *dst, const void *src, size_t n)
 {
@@ -303,6 +306,9 @@ void *rte_memcpy(void *dst, const void *src, size_t n)
 }
 
 #else
+__rte_nonnull_params(1, 2)
+__rte_write_only_param(1, 3)
+__rte_read_only_param(2, 3)
 static __rte_always_inline
 void *rte_memcpy(void *dst, const void *src, size_t n)
 {
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 15765b408d..c9cd2c7496 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -149,6 +149,82 @@ typedef uint16_t unaligned_uint16_t;
__attribute__((format(printf, format_index, first_arg)))
 #endif
 
+/**
+ * Tells compiler that the pointer arguments must be non-null.
+ *
+ * @param ...
+ *Comma separated list of parameter indexes of pointer arguments.
+ */
+#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
+#define __rte_nonnull_params(...) \
+   __attribute__((nonnull(__VA_ARGS__)))
+#else
+#define __rte_nonnull_params(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+ * is only read, not written to, by the function.
+ * It might not be accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and parameter index of size argument.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
+#define __rte_read_only_param(...) \
+   __attribute__((access(read_only, __VA_ARGS__)))
+#else
+#define __rte_read_only_param(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+ * is only written to, not read, by the function.
+ * It might not be accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and parameter index of size argument.
+ */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
+#define __rte_write_only_param(...) \
+   __attribute__((access(write_only, __VA_ARGS__)))
+#else
+#define __rte_write_only_param(...)
+#endif
+
+/**
+ * Tells compiler that the memory pointed to by a pointer argument
+ * is both read and written to by the function.
+ * It might not be read, written to, or accessed at all.
+ *
+ * @param ...
+ *Parameter index of pointer argument.
+ *Optionally followeded by comma and pa

Re: [PATCH] net/af_xdp: parse numa node id from sysfs

[Ferruh Yigit]

On 12/12/2022 12:48 AM, Frank Du wrote:
> Get from /sys/class/net/{if}/device/numa_node.
> 
> Signed-off-by: Frank Du 
> ---
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index b6ec9bf490..38b9d36ab5 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -39,6 +39,7 @@
>  #include 
>  
>  #include "compat.h"
> +#include "eal_filesystem.h"
>  
>  #ifndef SO_PREFER_BUSY_POLL
>  #define SO_PREFER_BUSY_POLL 69
> @@ -2038,9 +2039,6 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
>   return -EINVAL;
>   }
>  
> - if (dev->device.numa_node == SOCKET_ID_ANY)
> - dev->device.numa_node = rte_socket_id();
> -
>   if (parse_parameters(kvlist, if_name, &xsk_start_queue_idx,
>&xsk_queue_cnt, &shared_umem, prog_path,
>&busy_budget, &force_copy) < 0) {
> @@ -2053,6 +2051,19 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
>   return -EINVAL;
>   }
>  
> + /* get numa node id from net sysfs */
> + if (dev->device.numa_node == SOCKET_ID_ANY) {
> + unsigned long numa = 0;
> + char numa_path[PATH_MAX];
> +
> + snprintf(numa_path, sizeof(numa_path), 
> "/sys/class/net/%s/device/numa_node",
> +  if_name);
> + if (eal_parse_sysfs_value(numa_path, &numa) != 0)
> + dev->device.numa_node = rte_socket_id();
> + else
> + dev->device.numa_node = numa;
> + }
> +
>   busy_budget = busy_budget == -1 ? ETH_AF_XDP_DFLT_BUSY_BUDGET :
>   busy_budget;
>  

Hi Frank,

It looks reasonable to set virtual DPDK af_xdp device socket to actual
underlying device socket. And as I checked quickly, it works as expected.

But what is the impact and motivation of the patch? In other words why
you are doing this patch and what output you are expecting as a result?
Did you able to do any performance testing, and are you observing any
difference before and after this test?


Thanks,
ferruh

RE: [PATCH v3] net/ice: add devargs for disabling default mac

[Zhang, Ke1X]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Thursday, January 12, 2023 11:32 PM
> To: Zhang, Ke1X 
> Cc: Zhang, Qi Z ; Zhang, Yuying
> ; dev@dpdk.org
> Subject: Re: [PATCH v3] net/ice: add devargs for disabling default mac
> 
> On Thu, 12 Jan 2023 15:55:38 +0800
> Ke Zhang  wrote:
> 
> > From: "ke1x.zhang" 
> >
> > Add the feature that support to disable default mac which will be used
> > by ice driver when setting dpdk_devargs config field.
> >
> > Default mac is not disabled in default, user can choose to disable the
> > default mac by setting ``devargs`` parameter ``default-mac-disable``,
> >
> > for example::
> >  -a 80:00.0,default-mac-disable=1
> >
> > Signed-off-by: ke1x.zhang 
> 
> Why is the driver specific option needed? Can't it be handled more generally
> by the application.

Thanks for your comments.

This patch is to support the latency function of a trex, for example:

Trex wants to send a packet with tos=1. After the peer forwards it back, it 
only receives packets
with tos=1. Other packets will be dropped in the switch stage of pipeline.

1.If the following rules are set as below:
flow create 0 ingress pattern eth / ipv4 tos is 1 / end actions queue index 1 / 
end

result: the packets will pass as long as the destination mac address matches 
the default mac address.


2.If the following rules are set as below:
flow create 0 ingress pattern any / end actions drop / end
flow create 0 ingress pattern eth / ipv4 tos is 1 / end actions queue index 1 / 
end

result: all packages will be dropped.


3.The current solution is to disable the default mac address during 
initialization, and then set the following rules:
flow create 0 ingress pattern eth / ipv4 tos is 1 / end actions queue index 1 / 
end

result: only packages with tos=1 can pass。

Would you please share a better solution for me? @Stephen Hemminger
Thanks.

Re: [2nd Try]:Re: Traffic Management API Questions

[Venky Venkatesh]

Thanks Jasvinder. I guess we are on the same page. With the design that you
mention we run short by 1 level of hierarchy -- which is why I was
originally asking for the difficulty of adding a layer. I think I
understand your assessment in that regard i.e. it is easier to add a shaped
dequeue at the roots in the application as opposed to add an additional
layer

Pls correct me if I am wrong. Otherwise Thanks for all your inputs
-Venky

On Mon, Jan 16, 2023 at 3:39 AM Singh, Jasvinder 
wrote:

> Hi Venky,
>
>
>
> Please see inline;
>
>
>
> Thanks,
>
> Jasvinder
>
>
>
>
>
> *From:* Venky Venkatesh 
> *Sent:* Monday, January 16, 2023 8:06 AM
> *To:* Singh, Jasvinder 
> *Cc:* dev@dpdk.org
> *Subject:* Re: [2nd Try]:Re: Traffic Management API Questions
>
>
>
> Hi Jasvinder,
>
> Thanks for the insights on the complexity of adding a layer.
>
> As for the workaround that you suggested using multiple subports, if I
> understand it correctly (*pls correct if I misunderstood*) it would not
> meet our needs:
>
>- We require multiple heterogeneous ports (i.e. ports with different
>bandwidths -- with no excess sharing since these are port limits). That
>would probably need some shaper attached there too since WRR (at the
>application) would share the instantaneous excess among the siblings.
>
> ·  As for the 2nd second suggestion (increase the number of subports):
> our need (*in addition to* multiple ports of different bandwidths at the
> top level) is to have  4 more TM layers for a total of 5. I am *not*
> looking at the assignment of the terms port/subport/user/pipe etc in the
> DPDK documentation -- instead am looking at it as abstract scheduling
> (and/or shaping) layers with differing abilities in some layers. So in
> order to compensate for the missing shaper at the port level I was planning
> to add 1 additional layer (so that what in DPDK documentation is referred
> to as subport is actually a port -- since the subport layer has the
> property of *not sharing* excess between siblings. With that principle, I
> am not clear how adding width to the subport layer (as I understand your
> suggestion) would help.
>
> [JS] – I was suggesting to assume subports as ports in the existing
> implementation and assign fixed bandwidth to each of the subports. By doing
> so, you would have multiple subports (re-named as ports) with shaper
> attached. Only limitation in such solution is that hierarchy would have
> single root node with the bandwidth equal to sum of subports bandwidth and
> all the subports would be served individually in round robin manner. If it
> doesn’t suit your  requirement, you need to make changes as you suggested
> above.
>
>
>
> Thanks
>
> -Venky
>
>
>
>
>
> On Wed, Jan 11, 2023 at 9:24 AM Singh, Jasvinder <
> jasvinder.si...@intel.com> wrote:
>
> Hi Venky,
>
>
>
> Please see inline;
>
>
>
> Jasvinder
>
>
>
> *From:* Venky Venkatesh 
> *Sent:* Wednesday, January 11, 2023 11:56 AM
> *To:* Singh, Jasvinder 
> *Cc:* dev@dpdk.org
> *Subject:* Re: [2nd Try]:Re: Traffic Management API Questions
>
>
>
> Hi Jasvinder,
>
> Thanks for the detailed answers. Our need is to have shaping at the port
> level as well. I am trying to see what would be the way to accomplish this
> given the current limitations of the sched library implementation in this
> regard. I see 2 options:
>
>- The top level (i.e. port level) documentation says the following:
>"Output Ethernet port 1/10/40 GbE" and "Multiple ports are scheduled
>in round robin order with all ports having equal priority". Questions:
>
>
>- Do all the ports have to be of the same speed OR can it be a
>   heterogeneous set of port speeds?
>
> [JS] – the library supports single port (root node) of the hierarchy. Each
> port can have multiple subports configured using different shaping rates.
> If you desire to have multiple ports, each port would have separate
> hierarchical tree underneath. Different ports could have different speed.
>
> o If it can be a heterogeneous set of ports, is the scheduling across
> those ports *weighted* round robin as opposed to round robin?
>
> [JS] – Scheduling across multiple ports is not supported in current sched
> library. At the application level, one can think of invoking
> enqueue/dequeue sched API in round robin or weighted round robin manner.
>
>- Are Speeds other than  1/10/40 GbE not supported?
>
> [JS] – Speeds other than above is supported, for eg. 25G, 50G etc.
>
>- I suppose this heterogeneous mix of port speeds is implemented by
>   playing with the weights across ports, correct?
>
> [JS] -please see above answers
>
>- If so, what problem do you foresee if we provide arbitrary bandwidth
>   ports by regulating the above weights?
>
> [JS] – I don’t see any issue.
>
>- The other alternative would be to add another layer (which has a
>shaper)  to the hierarchy by mimicking one of the existing layers: how
>amenable is the current implementation to that?
>
> 

Minutes of Technical Board Meeting, 2022-Nov-16

[Maxime Coquelin]

Minutes of Technical Board Meeting, 2022-Nov-16

Members Attending
-
∘ Maxime Coquelin
∘ Thomas Monjalon
∘ Kevin Traynor
∘ Hemant Agrawal
∘ Bruce Richardson
∘ Konstantin Ananyev
∘ Jerin Jacob
∘ Honnappa Nagarahalli
∘ Aaron Conole

NOTE: The technical board meetings every second Wednesday at 
https://meet.jit.si/DPDK at 3 pm UTC.

Meetings are public, and DPDK community members are welcome to attend.

NOTE: Next meeting will be on Wednesday 2022-Nov-30 @3pm UTC, and will 
be chaired by Stephen


1) Governing Board updates
 - Nathan provided an update about the Governing board chair election.
   Rashid Khan (Red Hat) has been voted in as the new chair.
 - A vice-chair role creation is being drafted.

2) Techwritter hiring updates
 - One candidate has been identified, compensation discussions to be
   engaged.

3) DPDK blog update
 - Governing boards minutes are now placed in a dedicated section.

4) MIT license exception
 - Exception for GVE base driver has been approved
 - Generic approval process under discussions at the Governing board
   level, waiting for legal advice.

5) BBDEV tree maintainership
 - Crypto tree will no more pull BBDEV patches.
 - Techboard voted in for a dedicated repo creation, with Maxime
   Coquelin maintaining it.

6) LTS maintenance for 2023
 - Kevin Traynor sent a mail to LTS maintainers and Techboard members to
   collect feedback for 2023 LTS maintenance.

[PATCH 0/5] dma/ioat: fix issues with stopping and restarting device

[Bruce Richardson]

This patchset fixes a couple of problems with stopping and restarting an
ioat DMA device. Following the two fixes, a series of improvements are
made to the dmadev unit tests to properly validate that dmadevs work
correctly as they are started and stopped, and ensure that no other or
future drivers will suffer from issues.

Bruce Richardson (5):
  dma/ioat: fix device stop if no copies done
  dma/ioat: fix incorrectly set indexes after restart
  test/dmadev: check result for device stop
  test/dmadev: create separate function for single copy test
  test/dmadev: add tests for stopping and restarting dev

 app/test/test_dmadev.c | 172 ++---
 drivers/dma/ioat/ioat_dmadev.c |  26 -
 2 files changed, 137 insertions(+), 61 deletions(-)

--
2.37.2

[PATCH 1/5] dma/ioat: fix device stop if no copies done

[Bruce Richardson]

The HW DMA devices supported by IOAT driver do not transition to
the "active" state until the first operation is started by the HW.
Therefore, if the user calls "rte_dma_stop()" on a device without
triggering any operations, the sequence of commands to be sent to
the HW is different, as is the final device state.

Update the IOAT driver "stop" function to take account of this
difference.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.wa...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/dma/ioat/ioat_dmadev.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index 5906eb45aa..aff7bbbfde 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -166,17 +166,28 @@ static int
 ioat_dev_stop(struct rte_dma_dev *dev)
 {
struct ioat_dmadev *ioat = dev->fp_obj->dev_private;
+   unsigned int chansts;
uint32_t retry = 0;

-   ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+   chansts = (unsigned int)(ioat->regs->chansts & IOAT_CHANSTS_STATUS);
+   if (chansts == IOAT_CHANSTS_ACTIVE || chansts == IOAT_CHANSTS_IDLE)
+   ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+   else
+   ioat->regs->chancmd = IOAT_CHANCMD_RESET;

do {
rte_pause();
retry++;
-   } while ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) != 
IOAT_CHANSTS_SUSPENDED
-   && retry < 200);
+   chansts = (unsigned int)(ioat->regs->chansts & 
IOAT_CHANSTS_STATUS);
+   } while (chansts != IOAT_CHANSTS_SUSPENDED &&
+   chansts != IOAT_CHANSTS_HALTED && retry < 200);
+
+   if (chansts == IOAT_CHANSTS_SUSPENDED || chansts == IOAT_CHANSTS_HALTED)
+   return 0;

-   return ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) == 
IOAT_CHANSTS_SUSPENDED) ? 0 : -1;
+   IOAT_PMD_WARN("Channel could not be suspended on stop. (chansts = %u 
[%s])",
+   chansts, chansts_readable[chansts]);
+   return -1;
 }

 /* Get device information of a device. */
--
2.37.2

[PATCH 2/5] dma/ioat: fix incorrectly set indexes after restart

[Bruce Richardson]

As part of the process of restarting a dma instance, the IOAT driver
will reset the HW addresses and state values. The read and write
indexes for SW use need to be similarly reset to keep HW and SW in
sync.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.wa...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/dma/ioat/ioat_dmadev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index aff7bbbfde..072eb17cd9 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -146,6 +146,13 @@ ioat_dev_start(struct rte_dma_dev *dev)
/* Prime the status register to be set to the last element. */
ioat->status = ioat->ring_addr + ((ioat->qcfg.nb_desc - 1) * DESC_SZ);

+   /* reset all counters */
+   ioat->next_read = 0;
+   ioat->next_write = 0;
+   ioat->last_write = 0;
+   ioat->offset = 0;
+   ioat->failure = 0;
+
printf("IOAT.status: %s [0x%"PRIx64"]\n",
chansts_readable[ioat->status & IOAT_CHANSTS_STATUS],
ioat->status);
--
2.37.2

[PATCH 3/5] test/dmadev: check result for device stop

[Bruce Richardson]

The DMA device stop API can return an error value so check that return
value when running dmadev unit tests.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index fe62e98af8..4e1dbcaa19 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -837,7 +837,11 @@ test_dmadev_instance(int16_t dev_id)
goto err;

rte_mempool_free(pool);
-   rte_dma_stop(dev_id);
+
+   if (rte_dma_stop(dev_id) < 0) {
+   rte_mempool_free(pool);
+   ERR_RETURN("Error stopping device %u\n", dev_id);
+   }
rte_dma_stats_reset(dev_id, vchan);
return 0;

--
2.37.2

[PATCH 4/5] test/dmadev: create separate function for single copy test

[Bruce Richardson]

The copy tests for dmadev had separate blocks in the test function for
single copy and burst copies. Separate out the single-copy block to its
own function so that it can be re-used if necessary.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 120 ++---
 1 file changed, 64 insertions(+), 56 deletions(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 4e1dbcaa19..de787c14e2 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -175,77 +175,85 @@ do_multi_copies(int16_t dev_id, uint16_t vchan,
 }

 static int
-test_enqueue_copies(int16_t dev_id, uint16_t vchan)
+test_single_copy(int16_t dev_id, uint16_t vchan)
 {
-   enum rte_dma_status_code status;
-   unsigned int i;
+   uint16_t i;
uint16_t id;
+   enum rte_dma_status_code status;
+   struct rte_mbuf *src, *dst;
+   char *src_data, *dst_data;

-   /* test doing a single copy */
-   do {
-   struct rte_mbuf *src, *dst;
-   char *src_data, *dst_data;
+   src = rte_pktmbuf_alloc(pool);
+   dst = rte_pktmbuf_alloc(pool);
+   src_data = rte_pktmbuf_mtod(src, char *);
+   dst_data = rte_pktmbuf_mtod(dst, char *);

-   src = rte_pktmbuf_alloc(pool);
-   dst = rte_pktmbuf_alloc(pool);
-   src_data = rte_pktmbuf_mtod(src, char *);
-   dst_data = rte_pktmbuf_mtod(dst, char *);
+   for (i = 0; i < COPY_LEN; i++)
+   src_data[i] = rte_rand() & 0xFF;

-   for (i = 0; i < COPY_LEN; i++)
-   src_data[i] = rte_rand() & 0xFF;
+   id = rte_dma_copy(dev_id, vchan, rte_pktmbuf_iova(src), 
rte_pktmbuf_iova(dst),
+   COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
+   if (id != id_count)
+   ERR_RETURN("Error with rte_dma_copy, got %u, expected %u\n",
+   id, id_count);

-   id = rte_dma_copy(dev_id, vchan, rte_pktmbuf_iova(src), 
rte_pktmbuf_iova(dst),
-   COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
-   if (id != id_count)
-   ERR_RETURN("Error with rte_dma_copy, got %u, expected 
%u\n",
-   id, id_count);
+   /* give time for copy to finish, then check it was done */
+   await_hw(dev_id, vchan);

-   /* give time for copy to finish, then check it was done */
-   await_hw(dev_id, vchan);
+   for (i = 0; i < COPY_LEN; i++)
+   if (dst_data[i] != src_data[i])
+   ERR_RETURN("Data mismatch at char %u [Got %02x not 
%02x]\n", i,
+   dst_data[i], src_data[i]);
+
+   /* now check completion works */
+   id = ~id;
+   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 1)
+   ERR_RETURN("Error with rte_dma_completed\n");
+
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received, %u [expected 
%u]\n",
+   id, id_count);
+
+   /* check for completed and id when no job done */
+   id = ~id;
+   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
+   ERR_RETURN("Error with rte_dma_completed when no job done\n");
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received when no job done, 
%u [expected %u]\n",
+   id, id_count);
+
+   /* check for completed_status and id when no job done */
+   id = ~id;
+   if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
+   ERR_RETURN("Error with rte_dma_completed_status when no job 
done\n");
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received when no job done, 
%u [expected %u]\n",
+   id, id_count);

-   for (i = 0; i < COPY_LEN; i++)
-   if (dst_data[i] != src_data[i])
-   ERR_RETURN("Data mismatch at char %u [Got %02x 
not %02x]\n", i,
-   dst_data[i], src_data[i]);
-
-   /* now check completion works */
-   id = ~id;
-   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 1)
-   ERR_RETURN("Error with rte_dma_completed\n");
-
-   if (id != id_count)
-   ERR_RETURN("Error:incorrect job id received, %u 
[expected %u]\n",
-   id, id_count);
-
-   /* check for completed and id when no job done */
-   id = ~id;
-   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
-   ERR_RETURN("Error with rte_dma_completed when no job 
done\n");
-   if (id != id_count)
-   ERR_RETURN("Error:incorrect job id received when no job 
done, %u [expected %u]\n",
-   

[PATCH 5/5] test/dmadev: add tests for stopping and restarting dev

[Bruce Richardson]

Validate device operation when a device is stopped or restarted.

The only complication - and gap in the dmadev ABI specification - is
what happens to the job ids on restart. Some drivers reset them to 0,
while others continue where things left off. Take account of both
posibilities in the test case.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index de787c14e2..8fb73a41e2 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -304,6 +304,48 @@ test_enqueue_copies(int16_t dev_id, uint16_t vchan)
|| do_multi_copies(dev_id, vchan, 0, 0, 1);
 }
 
+static int
+test_stop_start(int16_t dev_id, uint16_t vchan)
+{
+   /* device is already started on input, should be (re)started on output 
*/
+
+   uint16_t id = 0;
+   enum rte_dma_status_code status = RTE_DMA_STATUS_SUCCESSFUL;
+
+   /* - test stopping a device works ok,
+* - then do a start-stop without doing a copy
+* - finally restart the device
+* checking for errors at each stage, and validating we can still copy 
at the end.
+*/
+   if (rte_dma_stop(dev_id) < 0)
+   ERR_RETURN("Error stopping device\n");
+
+   if (rte_dma_start(dev_id) < 0)
+   ERR_RETURN("Error restarting device\n");
+   if (rte_dma_stop(dev_id) < 0)
+   ERR_RETURN("Error stopping device after restart (no jobs 
executed)\n");
+
+   if (rte_dma_start(dev_id) < 0)
+   ERR_RETURN("Error restarting device after multiple 
stop-starts\n");
+
+   /* before doing a copy, we need to know what the next id will be it 
should
+* either be:
+* - the last completed job before start if driver does not reset id on 
stop
+* - or -1 i.e. next job is 0, if driver does reset the job ids on stop
+*/
+   if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
+   ERR_RETURN("Error with rte_dma_completed_status when no job 
done\n");
+   id += 1; /* id_count is next job id */
+   if (id != id_count && id != 0)
+   ERR_RETURN("Unexpected next id from device after stop-start. 
Got %u, expected %u or 0\n",
+   id, id_count);
+
+   id_count = id;
+   if (test_single_copy(dev_id, vchan) < 0)
+   ERR_RETURN("Error performing copy after device restart\n");
+   return 0;
+}
+
 /* Failure handling test cases - global macros and variables for those tests*/
 #define COMP_BURST_SZ  16
 #define OPT_FENCE(idx) ((fence && idx == 8) ? RTE_DMA_OP_FLAG_FENCE : 0)
@@ -819,6 +861,10 @@ test_dmadev_instance(int16_t dev_id)
if (runtest("copy", test_enqueue_copies, 640, dev_id, vchan, 
CHECK_ERRS) < 0)
goto err;
 
+   /* run tests stopping/starting devices and check jobs still work after 
restart */
+   if (runtest("stop-start", test_stop_start, 1, dev_id, vchan, 
CHECK_ERRS) < 0)
+   goto err;
+
/* run some burst capacity tests */
if (rte_dma_burst_capacity(dev_id, vchan) < 64)
printf("DMA Dev %u: insufficient burst capacity (64 required), 
skipping tests\n",
-- 
2.37.2

Minutes of Technical Board Meeting, 2022-07-13

[Aaron Conole]

Members Attending: 6
* Aaron Conole (chair)
* Hement Agarwal
* Maxime Coquelin
* Nathan Southern
* Stephen Hemminger
* Thomas Monjalon

NOTE: The Technical Board meetings take place every second Wednesday on 
https://meet.jit.si/DPDK at 3 pm UTC.
Meetings are public, and DPDK community members are welcome to attend.
Agenda and minutes can be found at http://core.dpdk.org/techboard/minutes

* DPDK Userspace Event updates
** CfP submissions are coming in.  More expected within the next week.
** Remote presenters can be accommodated (but not preferred, although
   remote speakers doing pre-recorded is preferred in that case)
** Remote attendance is an option - access to all sessions isn't clear
   at the moment.  First day stuff (hackathon, TB meetings, etc. aren't
   recorded) TBD, but likely will not be accessible to remote attendees.
** Some complimentary passes have been approved for qualified individuals
** Sponsorship packages were approved
** Some travel / transportation recommendataions are forthcoming to the
   website

* LF server backup
** Nathan submitted request
** Last message from Johnson @ gowdy.net is that the ticket
   is in process
** Are any additional ticket requests? - Seems no
** No ETA for completion
** Thomas says when it should be available we can do some testing and
   see if we missed anything.

* Tech writer update
** Discussion with a candidate between Thomas, Bruce, and Nathan
** Candidate represents a group, which was a surprise
** Asked for a proposal, but the proposal did not have a breakdown, just
   a flat payment for 3 months.
** Group wants hourly information, and an "out" in case the work isn't
   sufficient.
** Thomas concern is that the plan is to work "full time" but we are
   probably not able to absorb full time work.  Better if they would work
   something like 1 wk/month because that is probably closer to the workload
   we estimate.
** Maybe if they are able to split the work in small tasks we can work out a
   schedule, but the current proposal doesn't have that.
** Need an audit of work to do to generate tasks list.
** Do we pay for that?  Discussion
*** Probably do, this is work and we are asking them to do it.
*** Need to have a bigger discussion about how to come up with such an estimate,
etc.
*** Maybe other projects have some data that we can use for coming up with an
estimate?
*** Q: Has LF paid for this work before?  If so, what is the rate, etc.?

-- Discussing backlog --

* Security process
** Nothing new

* UNH CI / Testing
** Presented the gov board status
** UNH Git caching issue
*** Shallow clone should be sufficient for git pulls at UNH
*** Two solutions - git cache being out of sync is a rare case anyway
*** Maybe with self-retest this isn't an issue
** Question is UNH still on their schedule?
** There is a question about using UNH with OPI, doing some kind of global API
   on top of DPDK.  There is a need for a lab, and the suggestion is UNH.
** Thomas is monitoring the discussion
** Not sure about whether the teams would/could overlap

* Doc maintenance
** Need to see what will happen with tech writer candidate

* Bugzilla status
** Waiting on responses
** BZ doesn't seem overwhelmed, but needs maintenance

RE: [PATCH 0/5] dma/ioat: fix issues with stopping and restarting device

[Walsh, Conor]

Hi Bruce,

This patchset breaks the dmadev autotest for IOAT on IceLake.

Trace below:

### Test dmadev instance 0 [:00:01.0]
IOAT.status: ACTIVE [0x100242880]
DMA Dev 0: Running copy Tests
Ops submitted: 85120Ops completed: 85120Errors: 0
DMA Dev 0: Running stop-start Tests
IOAT.status: ACTIVE [0x100242880]
IOAT.status: ACTIVE [0x100242880]
Ops submitted: 1Ops completed: 1Errors: 0
DMA Dev 0: Running burst capacity Tests
Ops submitted: 65536Ops completed: 65536Errors: 0
DMA Dev 0: Running error handling Tests (errors expected)
In test_failure_in_full_burst:390 - Error, missing expected failed copy, 0. 
has_error is not set
In runtest:58 -
Error, not all submitted jobs are reported as completed
In test_dma:940 - Error, test failure for device 0
Test Faileded: 16   Ops completed: 0Errors: 0
RTE>>IOAT: ioat_dmadev_remove(): Closing :00:01.0 on NUMA node 0

Thanks,
Conor.


> -Original Message-
> From: Bruce Richardson 
> Sent: Monday 16 January 2023 15:37
> To: dev@dpdk.org
> Cc: Richardson, Bruce 
> Subject: [PATCH 0/5] dma/ioat: fix issues with stopping and restarting device
> 
> This patchset fixes a couple of problems with stopping and restarting an
> ioat DMA device. Following the two fixes, a series of improvements are
> made to the dmadev unit tests to properly validate that dmadevs work
> correctly as they are started and stopped, and ensure that no other or
> future drivers will suffer from issues.
> 
> Bruce Richardson (5):
>   dma/ioat: fix device stop if no copies done
>   dma/ioat: fix incorrectly set indexes after restart
>   test/dmadev: check result for device stop
>   test/dmadev: create separate function for single copy test
>   test/dmadev: add tests for stopping and restarting dev
> 
>  app/test/test_dmadev.c | 172 ++---
>  drivers/dma/ioat/ioat_dmadev.c |  26 -
>  2 files changed, 137 insertions(+), 61 deletions(-)
> 
> --
> 2.37.2

RE: [RFC 1/9] ethdev: add IPv6 routing extension header definition

[Ori Kam]

Hi Rongwei,


> -Original Message-
> From: Rongwei Liu 
> Sent: Wednesday, 21 December 2022 10:43
> 
> Add IPv6 routing extension header definition and no
> TLV support for now.
> At rte_flow layer, there are new items defined for matching
> type/nexthdr/segment_left field.
> 
> Signed-off-by: Rongwei Liu 
> ---
>  doc/guides/prog_guide/rte_flow.rst |  9 ++
>  doc/guides/rel_notes/release_22_03.rst |  5 
>  lib/ethdev/rte_flow.c  | 15 ++
>  lib/ethdev/rte_flow.h  | 39 ++
>  lib/net/rte_ip.h   | 21 ++
>  5 files changed, 89 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 3e6242803d..1ebc159893 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1544,6 +1544,15 @@ Matches Color Marker set by a Meter.
> 
>  - ``color``: Metering color marker.
> 
> +Item: ``IPV6_ROUTING_EXT``
> +^
> +
> +Matches ipv6 routing extension header.
> +
> +- ``nexthdr``: Next layer header type.
> +- ``type``: IPv6 routing extension header type.
> +- ``segments_left``: How many IPv6 destination addresses carries on
> +
>  Actions
>  ~~~
> 
> diff --git a/doc/guides/rel_notes/release_22_03.rst
> b/doc/guides/rel_notes/release_22_03.rst
> index 0923707cb8..cc050ff3e7 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -207,6 +207,11 @@ API Changes
>  * ethdev: Old public macros and enumeration constants without
> ``RTE_ETH_`` prefix,
>which are kept for backward compatibility, are marked as deprecated.
> 
> +* ethdev: added a new structure:
> +
> +- IPv6 routing extension header ``rte_flow_item_ipv6_routing_ext`` and
> +  ``rte_ipv6_routing_ext``
> +
>  * cryptodev: The asymmetric session handling was modified to use a single
>mempool object. An API ``rte_cryptodev_asym_session_pool_create`` was
> added
>to create a mempool with element size big enough to hold the generic
> asymmetric
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index 7d0c24366c..e08b690300 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -76,6 +76,19 @@ rte_flow_item_flex_conv(void *buf, const void *data)
>   return src->length;
>  }
> 
> +static size_t
> +rte_flow_item_ipv6_routing_ext_conv(void *buf, const void *data)
> +{
> + struct rte_flow_item_ipv6_routing_ext *dst = buf;
> + const struct rte_flow_item_ipv6_routing_ext *src = data;
> +
> + if (buf)
> + rte_memcpy((void *)((uintptr_t)(dst->hdr.segments)),
> +src->hdr.segments,
> +src->hdr.segments_left << 4);
> + return src->hdr.segments_left << 4;
> +}
> +
>  /** Generate flow_item[] entry. */
>  #define MK_FLOW_ITEM(t, s) \
>   [RTE_FLOW_ITEM_TYPE_ ## t] = { \
> @@ -157,6 +170,8 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
>   MK_FLOW_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
>   MK_FLOW_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
>   MK_FLOW_ITEM(METER_COLOR, sizeof(struct
> rte_flow_item_meter_color)),
> + MK_FLOW_ITEM_FN(IPV6_ROUTING_EXT, sizeof(struct
> rte_flow_item_ipv6_routing_ext),
> + rte_flow_item_ipv6_routing_ext_conv),
>  };
> 
>  /** Generate flow_action[] entry. */
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index b60987db4b..f8f1d6f9dd 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -624,6 +624,33 @@ enum rte_flow_item_type {
>* See struct rte_flow_item_meter_color.
>*/
>   RTE_FLOW_ITEM_TYPE_METER_COLOR,
> + /**
> +  * Matches the presence of IPv6 routing extension header.
> +  *
> +  * See struct rte_flow_item_ipv6_routing_ext.
> +  */
> + RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT,
> +
> + /**
> +  * Matches IPv6 routing extension header next header.
> +  *
> +  * See struct rte_flow_item_ipv6_routing_ext.
> +  */
> + RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT_NEXT_HDR,

Why do we need this item? Isn't it part of the rte_ipv6_routing_ext?

> +
> + /**
> +  * Matches IPv6 routing extension header type.
> +  *
> +  * See struct rte_flow_item_ipv6_routing_ext.
> +  */
> + RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT_TYPE,
> +

Why do we need this item? Isn't it part of the rte_ipv6_routing_ext?

> + /**
> +  * Matches IPv6 routing extension header segment left.
> +  *
> +  * See struct rte_flow_item_ipv6_routing_ext.
> +  */
> + RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT_SEG_LEFT,

Why do we need this item? Isn't it part of the rte_ipv6_routing_ext?

>  };
> 
>  /**
> @@ -873,6 +900,18 @@ struct rte_flow_item_ipv6 {
>   uint32_t reserved:23;
>  };
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RT

RE: [RFC 7/9] ethdev: add modify IPv6 protocol field

[Ori Kam]

Hi Rongwei,

> -Original Message-
> From: Rongwei Liu 
> Sent: Wednesday, 21 December 2022 10:43
> 
> Add IPv6 protocol modify field definition.
> 
> Signed-off-by: Rongwei Liu 
> ---
>  lib/ethdev/rte_flow.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index f8f1d6f9dd..b1240b0cb0 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3567,6 +3567,7 @@ enum rte_flow_field_id {
>   RTE_FLOW_FIELD_IPV6_ECN,/**< IPv6 ECN. */
>   RTE_FLOW_FIELD_GTP_PSC_QFI, /**< GTP QFI. */
>   RTE_FLOW_FIELD_METER_COLOR, /**< Meter color marker. */
> + RTE_FLOW_FIELD_IPV6_PROTO,  /**< IPv6 next header. */
>  };
> 
>  /**
> --
> 2.27.0

Acked-by: Ori Kam 
Best,
Ori

Re: [PATCH 0/5] dma/ioat: fix issues with stopping and restarting device

[Bruce Richardson]

On Mon, Jan 16, 2023 at 04:09:19PM +, Walsh, Conor wrote:
> Hi Bruce,
> 
> This patchset breaks the dmadev autotest for IOAT on IceLake.
> 
> Trace below:
> 
> ### Test dmadev instance 0 [:00:01.0] IOAT.status: ACTIVE
> [0x100242880] DMA Dev 0: Running copy Tests Ops submitted: 85120Ops
> completed: 85120Errors: 0 DMA Dev 0: Running stop-start Tests
> IOAT.status: ACTIVE [0x100242880] IOAT.status: ACTIVE [0x100242880] Ops
> submitted: 1Ops completed: 1Errors: 0 DMA Dev 0: Running
> burst capacity Tests Ops submitted: 65536Ops completed: 65536
> Errors: 0 DMA Dev 0: Running error handling Tests (errors expected) In
> test_failure_in_full_burst:390 - Error, missing expected failed copy, 0.
> has_error is not set In runtest:58 - Error, not all submitted jobs are
> reported as completed In test_dma:940 - Error, test failure for device 0
> Test Faileded: 16   Ops completed: 0Errors: 0 RTE>>IOAT:
> ioat_dmadev_remove(): Closing :00:01.0 on NUMA node 0
> 
> Thanks, Conor.
> 
Thanks for catching this, Conor. I was testing on a system using noiommu
mode, so those error tests were getting skipped. I'll investigate and do a
v2.

/Bruce

Re: [PATCH v7 4/4] eal: add nonnull and access function attributes

[Ferruh Yigit]

On 1/16/2023 1:07 PM, Morten Brørup wrote:
> Add nonnull function attribute to help the compiler detect a NULL
> pointer being passed to a function not accepting NULL pointers as an
> argument at build time.
> 
> Add access function attributes to tell the compiler how a function
> accesses memory pointed to by its pointer arguments.
> 
> Add these attributes to the rte_memcpy() function, as the first in
> hopefully many to come.
> 
> Signed-off-by: Morten Brørup 
> Acked-by: Tyler Retzlaff 
> Reviewed-by: Ruifeng Wang 
> ---
> v7:
> * Fix indentation. (checkpatch)
> v6:
> * Make this the last patch in the series, putting the fixes first.
>   (David)
> * Split the generic access macro into dedicated macros per access mode.
>   (David)
> v5:
> * No changes.
> v4:
> * No changes.
> v3:
> * No changes.
> v2:
> * Only define "nonnull" for GCC and CLANG.
> * Append _param/_params to prepare for possible future attributes
>   attached directly to the individual parameters, like __rte_unused.
> * Use RTE_TOOLCHAIN_GCC instead of RTE_CC_GCC, to fix complaints about
>   GCC_VERSION being undefined.
> * Try to fix Doxygen compliants.
> ---
>  lib/eal/arm/include/rte_memcpy_32.h |  8 +++
>  lib/eal/arm/include/rte_memcpy_64.h |  6 +++
>  lib/eal/include/rte_common.h| 76 +
>  lib/eal/ppc/include/rte_memcpy.h|  3 ++
>  lib/eal/x86/include/rte_memcpy.h|  6 +++
>  5 files changed, 99 insertions(+)
> 
> diff --git a/lib/eal/arm/include/rte_memcpy_32.h 
> b/lib/eal/arm/include/rte_memcpy_32.h
> index fb3245b59c..a625a91951 100644
> --- a/lib/eal/arm/include/rte_memcpy_32.h
> +++ b/lib/eal/arm/include/rte_memcpy_32.h
> @@ -14,6 +14,8 @@ extern "C" {
>  
>  #include "generic/rte_memcpy.h"
>  
> +#include 
> +
>  #ifdef RTE_ARCH_ARM_NEON_MEMCPY
>  
>  #ifndef __ARM_NEON
> @@ -125,6 +127,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
>   memcpy((dst), (src), (n)) :  \
>   rte_memcpy_func((dst), (src), (n)); })
>  
> +__rte_nonnull_params(1, 2)
> +__rte_write_only_param(1, 3)
> +__rte_read_only_param(2, 3)
>  static inline void *
>  rte_memcpy_func(void *dst, const void *src, size_t n)
>  {
> @@ -290,6 +295,9 @@ rte_mov256(uint8_t *dst, const uint8_t *src)
>   memcpy(dst, src, 256);
>  }
>  
> +__rte_nonnull_params(1, 2)
> +__rte_write_only_param(1, 3)
> +__rte_read_only_param(2, 3)
>  static inline void *
>  rte_memcpy(void *dst, const void *src, size_t n)
>  {
> diff --git a/lib/eal/arm/include/rte_memcpy_64.h 
> b/lib/eal/arm/include/rte_memcpy_64.h
> index 85ad587bd3..0c86237cc9 100644
> --- a/lib/eal/arm/include/rte_memcpy_64.h
> +++ b/lib/eal/arm/include/rte_memcpy_64.h
> @@ -282,6 +282,9 @@ void rte_memcpy_ge64(uint8_t *dst, const uint8_t *src, 
> size_t n)
>  }
>  
>  #if RTE_CACHE_LINE_SIZE >= 128
> +__rte_nonnull_params(1, 2)
> +__rte_write_only_param(1, 3)
> +__rte_read_only_param(2, 3)
>  static __rte_always_inline
>  void *rte_memcpy(void *dst, const void *src, size_t n)
>  {
> @@ -303,6 +306,9 @@ void *rte_memcpy(void *dst, const void *src, size_t n)
>  }
>  
>  #else
> +__rte_nonnull_params(1, 2)
> +__rte_write_only_param(1, 3)
> +__rte_read_only_param(2, 3)
>  static __rte_always_inline
>  void *rte_memcpy(void *dst, const void *src, size_t n)
>  {
> diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
> index 15765b408d..c9cd2c7496 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -149,6 +149,82 @@ typedef uint16_t unaligned_uint16_t;
>   __attribute__((format(printf, format_index, first_arg)))
>  #endif
>  
> +/**
> + * Tells compiler that the pointer arguments must be non-null.
> + *
> + * @param ...
> + *Comma separated list of parameter indexes of pointer arguments.
> + */
> +#if defined(RTE_CC_GCC) || defined(RTE_CC_CLANG)
> +#define __rte_nonnull_params(...) \
> + __attribute__((nonnull(__VA_ARGS__)))
> +#else
> +#define __rte_nonnull_params(...)
> +#endif
> +

What do you think to have a namespace for macros like '__rte_param_xxx',
so name macros as:
__rte_param_nonull
__rte_param_read_only
__rte_param_write_only

No strong opinion, it just feels tidier this way

> +/**
> + * Tells compiler that the memory pointed to by a pointer argument
> + * is only read, not written to, by the function.
> + * It might not be accessed at all.
> + *
> + * @param ...
> + *Parameter index of pointer argument.
> + *Optionally followeded by comma and parameter index of size argument.

s/followeded/followed/

multiple occurrences below

> + */
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 100400)
> +#define __rte_read_only_param(...) \
> + __attribute__((access(read_only, __VA_ARGS__)))
> +#else
> +#define __rte_read_only_param(...)
> +#endif
> +
> +/**
> + * Tells compiler that the memory pointed to by a pointer argument
> + * is only written to, not read, by the function.
> + * It might not be accessed at all.
> + *
> + * @param ...
> + *Parameter index of pointer arg

[PATCH 0/7] space after keyword cleanups

[Stephen Hemminger]

These are the results of a script replacing 'if(' with 'if ('
in various places.

Stephen Hemminger (7):
  test: fix whitespace
  testpmd: fix whitespace
  net/e1000: fix whitespace
  i40e: fix whitespace
  examples: fix whitespace
  cmdline: fix whitespace
  ip_frag: fix whitespace

 app/test-pmd/cmdline.c | 30 +++---
 app/test-pmd/parameters.c  |  8 +++---
 app/test-pmd/testpmd.c |  2 +-
 app/test/test_debug.c  |  4 +--
 app/test/test_hash.c   |  6 ++---
 app/test/test_malloc.c | 10 
 app/test/test_mbuf.c   | 12 -
 app/test/test_spinlock.c   |  2 +-
 drivers/net/e1000/em_ethdev.c  |  2 +-
 drivers/net/e1000/igb_ethdev.c |  8 +++---
 drivers/net/e1000/igb_pf.c |  2 +-
 drivers/net/e1000/igb_rxtx.c   |  6 ++---
 drivers/net/i40e/i40e_pf.c |  8 +++---
 examples/ip_reassembly/main.c  |  2 +-
 examples/l3fwd-power/main.c|  4 +--
 examples/l3fwd/main.c  |  4 +--
 examples/multi_process/symmetric_mp/main.c |  4 +--
 lib/cmdline/cmdline_rdline.c   |  6 ++---
 lib/ip_frag/rte_ipv4_reassembly.c  |  2 +-
 19 files changed, 61 insertions(+), 61 deletions(-)

-- 
2.39.0

[PATCH 1/7] test: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test/test_debug.c|  4 ++--
 app/test/test_hash.c |  6 +++---
 app/test/test_malloc.c   | 10 +-
 app/test/test_mbuf.c | 12 ++--
 app/test/test_spinlock.c |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 2704f5b92726..c66748019d9f 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -53,7 +53,7 @@ test_panic(void)
return -1;
}
wait(&status);
-   if(status == 0){
+   if (status == 0){
printf("Child process terminated normally!\n");
return -1;
} else
@@ -84,7 +84,7 @@ test_exit_val(int exit_val)
}
wait(&status);
printf("Child process status: %d\n", status);
-   if(!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
+   if (!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
printf("Child process terminated with incorrect status 
(expected = %d)!\n",
exit_val);
return -1;
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index 3e45afaa67fc..39e847530c1a 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -50,7 +50,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_FBK(cond, str, ...) do {   
\
if (cond) { \
@@ -58,7 +58,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_fbk_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_RCU_QSBR(cond, str, ...) do {  \
if (cond) { \
@@ -728,7 +728,7 @@ static int test_five_keys(void)
key_array[i] = &keys[i];
 
ret = rte_hash_lookup_bulk(handle, &key_array[0], 5, (int32_t *)pos);
-   if(ret == 0)
+   if (ret == 0)
for(i = 0; i < 5; i++) {
print_key_info("Lkp", key_array[i], pos[i]);
RETURN_IF_ERROR(pos[i] != expected_pos[i],
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index de40e506113a..f28a3f69d4fd 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -87,7 +87,7 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
break;
}
for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   if ( *(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
}
@@ -158,7 +158,7 @@ test_reordered_free_per_lcore(__rte_unused void *arg)
break;
}
for(j = 0; j < 1000 ; j++) {
-   if( *(char *)p1 != 0) {
+   if ( *(char *)p1 != 0) {
printf("rte_zmalloc didn't zero the allocated 
memory\n");
ret = -1;
}
@@ -331,12 +331,12 @@ test_multi_alloc_statistics(void)
/* After freeing both allocations check stats return to original */
rte_malloc_get_socket_stats(socket, &post_stats);
 
-   if(second_stats.heap_totalsz_bytes != first_stats.heap_totalsz_bytes) {
+   if (second_stats.heap_totalsz_bytes != first_stats.heap_totalsz_bytes) {
printf("Incorrect heap statistics: Total size \n");
return -1;
}
/* Check allocated size is equal to two additions plus overhead */
-   if(second_stats.heap_allocsz_bytes !=
+   if (second_stats.heap_allocsz_bytes !=
size + overhead + first_stats.heap_allocsz_bytes) {
printf("Incorrect heap statistics: Allocated size \n");
return -1;
@@ -495,7 +495,7 @@ test_realloc_socket(int socket)
return -1;
}
/* calc an alignment we don't already have */
-   while(RTE_PTR_ALIGN(ptr7, new_align) == ptr7)
+   while (RTE_PTR_ALIGN(ptr7, new_align) == ptr7)
new_align *= 2;
char *ptr8 = rte_realloc_socket(ptr7, size7, new_align, socket);
if (!ptr8){
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
i

[PATCH 2/7] testpmd: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/cmdline.c| 30 +++---
 app/test-pmd/parameters.c |  8 
 app/test-pmd/testpmd.c|  2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b32dc8bfd445..615a1ea295f7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2730,11 +2730,11 @@ parse_reta_config(const char *str,
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -3242,15 +3242,15 @@ cmd_config_thresh_parsed(void *parsed_result,
 
if (!strcmp(res->name, "txpt"))
tx_pthresh = res->value;
-   else if(!strcmp(res->name, "txht"))
+   else if (!strcmp(res->name, "txht"))
tx_hthresh = res->value;
-   else if(!strcmp(res->name, "txwt"))
+   else if (!strcmp(res->name, "txwt"))
tx_wthresh = res->value;
-   else if(!strcmp(res->name, "rxpt"))
+   else if (!strcmp(res->name, "rxpt"))
rx_pthresh = res->value;
-   else if(!strcmp(res->name, "rxht"))
+   else if (!strcmp(res->name, "rxht"))
rx_hthresh = res->value;
-   else if(!strcmp(res->name, "rxwt"))
+   else if (!strcmp(res->name, "rxwt"))
rx_wthresh = res->value;
else {
fprintf(stderr, "Unknown parameter\n");
@@ -4084,8 +4084,8 @@ cmd_vlan_offload_parsed(void *parsed_result,
len = strnlen(str, STR_TOKEN_SIZE);
i = 0;
/* Get port_id first */
-   while(i < len){
-   if(str[i] == ',')
+   while (i < len){
+   if (str[i] == ',')
break;
 
i++;
@@ -4093,7 +4093,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
str[i]='\0';
tmp = strtoul(str, NULL, 0);
/* If port_id greater that what portid_t can represent, return */
-   if(tmp >= RTE_MAX_ETHPORTS)
+   if (tmp >= RTE_MAX_ETHPORTS)
return;
port_id = (portid_t)tmp;
 
@@ -4104,17 +4104,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
 
if (!strcmp(res->what, "strip"))
rx_vlan_strip_set(port_id,  on);
-   else if(!strcmp(res->what, "stripq")){
+   else if (!strcmp(res->what, "stripq")){
uint16_t queue_id = 0;
 
/* No queue_id, return */
-   if(i + 1 >= len) {
+   if (i + 1 >= len) {
fprintf(stderr, "must specify (port,queue_id)\n");
return;
}
tmp = strtoul(str + i + 1, NULL, 0);
/* If queue_id greater that what 16-bits can represent, return 
*/
-   if(tmp > 0x)
+   if (tmp > 0x)
return;
 
queue_id = (uint16_t)tmp;
@@ -7252,7 +7252,7 @@ static void cmd_mac_addr_parsed(void *parsed_result,
ret = rte_eth_dev_mac_addr_remove(res->port_num, &res->address);
 
/* check the return value and print it if is < 0 */
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
 
 }
@@ -7825,7 +7825,7 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
res->vf_num);
 #endif
 
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
strerror(-ret));
 
 }
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index d597c209ba5e..e2371cfd7094 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -308,11 +308,11 @@ parse_portnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -368,11 +368,11 @@ parse_ringnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size 

[PATCH 4/7] i40e: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/i40e/i40e_pf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 15d9ff868f3a..7050e0057d8e 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
 
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
vid = vlan_filter_list->vlan_id;
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 * return if SRIOV not enabled, VF number not configured or
 * no queue assigned.
 */
-   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
+   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
return I40E_SUCCESS;
 
/* Allocate memory to store VF structure */
pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
-   if(pf->vfs == NULL)
+   if (pf->vfs == NULL)
return -ENOMEM;
 
/* Disable irq0 for VFR event */
-- 
2.39.0

[PATCH 3/7] net/e1000: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_ethdev.c  | 2 +-
 drivers/net/e1000/igb_ethdev.c | 8 
 drivers/net/e1000/igb_pf.c | 2 +-
 drivers/net/e1000/igb_rxtx.c   | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8ee9be12ad19..3cb09538b2df 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -872,7 +872,7 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
int pause_frames;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
stats->sec += E1000_READ_REG(hw, E1000_SEC);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8cc..5c35b8349063 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw, struct 
e1000_hw_stats *stats)
uint64_t old_rpthc = stats->rpthc;
uint64_t old_hgptc = stats->hgptc;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs +=
E1000_READ_REG(hw,E1000_SYMERRS);
@@ -3500,10 +3500,10 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, 
bool on)
 
for (i = 0; i < IGB_VFTA_SIZE; i++){
vfta = shadow_vfta->vfta[i];
-   if(vfta){
+   if (vfta){
mask = 1;
for (j = 0; j < 32; j++){
-   if(vfta & mask)
+   if (vfta & mask)
igbvf_set_vfta(hw,
(uint16_t)((i<<5)+j), on);
mask<<=1;
@@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 
/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
ret = igbvf_set_vfta(hw, vlan_id, !!on);
-   if(ret){
+   if (ret){
PMD_INIT_LOG(ERR, "Unable to set VF vlan");
return ret;
}
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index c7588ea57eaa..b1f74eb841d2 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -78,7 +78,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
 
if (hw->mac.type == e1000_i350)
nb_queue = 1;
-   else if(hw->mac.type == e1000_82576)
+   else if (hw->mac.type == e1000_82576)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index f32dee46df82..2d4a1292053e 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -196,13 +196,13 @@ struct igb_tx_queue {
 #ifdef RTE_PMD_USE_PREFETCH
 #define rte_igb_prefetch(p)rte_prefetch0(p)
 #else
-#define rte_igb_prefetch(p)do {} while(0)
+#define rte_igb_prefetch(p)do {} while (0)
 #endif
 
 #ifdef RTE_PMD_PACKET_PREFETCH
 #define rte_packet_prefetch(p) rte_prefetch1(p)
 #else
-#define rte_packet_prefetch(p) do {} while(0)
+#define rte_packet_prefetch(p) do {} while (0)
 #endif
 
 /*
@@ -2276,7 +2276,7 @@ igb_dev_mq_rx_configure(struct rte_eth_dev *dev)
/* 011b Def_Q ignore, according to VT_CTL.DEF_PL */
mrqc |= 0x3 << E1000_MRQC_DEF_Q_SHIFT;
E1000_WRITE_REG(hw, E1000_MRQC, mrqc);
-   } else if(RTE_ETH_DEV_SRIOV(dev).active == 0) {
+   } else if (RTE_ETH_DEV_SRIOV(dev).active == 0) {
/*
 * SRIOV inactive scheme
 */
-- 
2.39.0

[PATCH 5/7] examples: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 examples/ip_reassembly/main.c  | 2 +-
 examples/l3fwd-power/main.c| 4 ++--
 examples/l3fwd/main.c  | 4 ++--
 examples/multi_process/symmetric_mp/main.c | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index bd0b1d31decf..7e84b4944759 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -300,7 +300,7 @@ send_single_packet(struct rte_mbuf *m, uint16_t port)
 
TX_LCORE_STAT_UPDATE(&qconf->tx_stat, queue, 1);
txmb->m_table[txmb->head] = m;
-   if(++txmb->head == len)
+   if (++txmb->head == len)
txmb->head = 0;
 
return 0;
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index fd3ade330f82..3ef6e5a1ccc6 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1787,11 +1787,11 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5198ff30dd00..70054183e1bb 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -516,11 +516,11 @@ parse_config(const char *q_arg)
 
while ((p = strchr(p0,'(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   if ((p0 = strchr(p,')')) == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
diff --git a/examples/multi_process/symmetric_mp/main.c 
b/examples/multi_process/symmetric_mp/main.c
index 1ff85875dfdf..8d1119bd8483 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -156,7 +156,7 @@ smp_parse_args(int argc, char **argv)
 
/* get the port numbers from the port mask */
RTE_ETH_FOREACH_DEV(i)
-   if(port_mask & (1 << i))
+   if (port_mask & (1 << i))
ports[num_ports++] = (uint8_t)i;
 
ret = optind-1;
@@ -471,7 +471,7 @@ main(int argc, char **argv)
if (num_ports & 1)
rte_exit(EXIT_FAILURE, "Application must use an even number of 
ports\n");
for(i = 0; i < num_ports; i++){
-   if(proc_type == RTE_PROC_PRIMARY)
+   if (proc_type == RTE_PROC_PRIMARY)
if (smp_port_init(ports[i], mp, (uint16_t)num_procs) < 
0)
rte_exit(EXIT_FAILURE, "Error initialising 
ports\n");
}
-- 
2.39.0

[PATCH 6/7] cmdline: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/cmdline/cmdline_rdline.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 5cf723a0126a..28fc54cdfebf 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -301,7 +301,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* delete 1 char from the left */
case CMDLINE_KEY_BKSPACE:
case CMDLINE_KEY_BKSPACE2:
-   if(!cirbuf_del_tail_safe(&rdl->left)) {
+   if (!cirbuf_del_tail_safe(&rdl->left)) {
rdline_puts(rdl, vt100_bs);
display_right_buffer(rdl, 1);
}
@@ -354,7 +354,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* paste contents of kill buffer to the left side of caret */
case CMDLINE_KEY_CTRL_Y:
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < rdl->kill_size) {
cirbuf_add_tail(&rdl->left, rdl->kill_buf[i]);
@@ -404,7 +404,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* add chars */
if (ret == RDLINE_RES_COMPLETE) {
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < tmp_size) {
cirbuf_add_tail(&rdl->left, 
tmp_buf[i]);
-- 
2.39.0

[PATCH 7/7] ip_frag: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/ip_frag/rte_ipv4_reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c 
b/lib/ip_frag/rte_ipv4_reassembly.c
index 4a89a5f5365a..88ee0aa63f62 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -34,7 +34,7 @@ ipv4_frag_reassemble(struct ip_frag_pkt *fp)
for (i = n; i != IP_FIRST_FRAG_IDX && ofs != first_len; i--) {
 
/* previous fragment found. */
-   if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
+   if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
 
RTE_ASSERT(curr_idx != i);
 
-- 
2.39.0

[PATCH v2 0/6] dma/ioat: fix issues with stopping and restarting device

[Bruce Richardson]

This patchset fixes a couple of problems with stopping and restarting an
ioat DMA device. Following the three fixes, a series of improvements are
made to the dmadev unit tests to properly validate that dmadevs work
correctly as they are started and stopped, and ensure that no other or
future drivers will suffer from issues.

v2:
* extra patch to fix issues with error reporting, as noted by Conor W.

Bruce Richardson (6):
  dma/ioat: fix device stop if no copies done
  dma/ioat: fix incorrectly set indexes after restart
  dma/ioat: fix incorrect error reporting on restart
  test/dmadev: check result for device stop
  test/dmadev: create separate function for single copy test
  test/dmadev: add tests for stopping and restarting dev

 app/test/test_dmadev.c | 172 ++---
 drivers/dma/ioat/ioat_dmadev.c |  31 --
 2 files changed, 140 insertions(+), 63 deletions(-)

--
2.37.2

[PATCH v2 1/6] dma/ioat: fix device stop if no copies done

[Bruce Richardson]

The HW DMA devices supported by IOAT driver do not transition to
the "active" state until the first operation is started by the HW.
Therefore, if the user calls "rte_dma_stop()" on a device without
triggering any operations, the sequence of commands to be sent to
the HW is different, as is the final device state.

Update the IOAT driver "stop" function to take account of this
difference.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.wa...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/dma/ioat/ioat_dmadev.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index 5906eb45aa..aff7bbbfde 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -166,17 +166,28 @@ static int
 ioat_dev_stop(struct rte_dma_dev *dev)
 {
struct ioat_dmadev *ioat = dev->fp_obj->dev_private;
+   unsigned int chansts;
uint32_t retry = 0;
 
-   ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+   chansts = (unsigned int)(ioat->regs->chansts & IOAT_CHANSTS_STATUS);
+   if (chansts == IOAT_CHANSTS_ACTIVE || chansts == IOAT_CHANSTS_IDLE)
+   ioat->regs->chancmd = IOAT_CHANCMD_SUSPEND;
+   else
+   ioat->regs->chancmd = IOAT_CHANCMD_RESET;
 
do {
rte_pause();
retry++;
-   } while ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) != 
IOAT_CHANSTS_SUSPENDED
-   && retry < 200);
+   chansts = (unsigned int)(ioat->regs->chansts & 
IOAT_CHANSTS_STATUS);
+   } while (chansts != IOAT_CHANSTS_SUSPENDED &&
+   chansts != IOAT_CHANSTS_HALTED && retry < 200);
+
+   if (chansts == IOAT_CHANSTS_SUSPENDED || chansts == IOAT_CHANSTS_HALTED)
+   return 0;
 
-   return ((ioat->regs->chansts & IOAT_CHANSTS_STATUS) == 
IOAT_CHANSTS_SUSPENDED) ? 0 : -1;
+   IOAT_PMD_WARN("Channel could not be suspended on stop. (chansts = %u 
[%s])",
+   chansts, chansts_readable[chansts]);
+   return -1;
 }
 
 /* Get device information of a device. */
-- 
2.37.2

[PATCH v2 2/6] dma/ioat: fix incorrectly set indexes after restart

[Bruce Richardson]

As part of the process of restarting a dma instance, the IOAT driver
will reset the HW addresses and state values. The read and write
indexes for SW use need to be similarly reset to keep HW and SW in
sync.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.wa...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/dma/ioat/ioat_dmadev.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index aff7bbbfde..072eb17cd9 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -146,6 +146,13 @@ ioat_dev_start(struct rte_dma_dev *dev)
/* Prime the status register to be set to the last element. */
ioat->status = ioat->ring_addr + ((ioat->qcfg.nb_desc - 1) * DESC_SZ);
 
+   /* reset all counters */
+   ioat->next_read = 0;
+   ioat->next_write = 0;
+   ioat->last_write = 0;
+   ioat->offset = 0;
+   ioat->failure = 0;
+
printf("IOAT.status: %s [0x%"PRIx64"]\n",
chansts_readable[ioat->status & IOAT_CHANSTS_STATUS],
ioat->status);
-- 
2.37.2

[PATCH v2 3/6] dma/ioat: fix incorrect error reporting on restart

[Bruce Richardson]

When the DMA device was stopped and restarted by the driver, the control
register specifying the behaviour on error was not getting correctly
reset. This caused unit tests to fail as explicitly introduced errors
were got getting reported back.

Fix by moving the setting of the register to the start function from the
probe function.

Fixes: 583f046dd404 ("dma/ioat: add start and stop")
Cc: conor.wa...@intel.com
Cc: sta...@dpdk.org

Signed-off-by: Bruce Richardson 
---
 drivers/dma/ioat/ioat_dmadev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ioat/ioat_dmadev.c b/drivers/dma/ioat/ioat_dmadev.c
index 072eb17cd9..57c18c081d 100644
--- a/drivers/dma/ioat/ioat_dmadev.c
+++ b/drivers/dma/ioat/ioat_dmadev.c
@@ -142,6 +142,9 @@ ioat_dev_start(struct rte_dma_dev *dev)
ioat->regs->chainaddr = ioat->ring_addr;
/* Inform hardware of where to write the status/completions. */
ioat->regs->chancmp = ioat->status_addr;
+   /* Ensure channel control is set to abort on error, so we get status 
writeback. */
+   ioat->regs->chanctrl = IOAT_CHANCTRL_ANY_ERR_ABORT_EN |
+   IOAT_CHANCTRL_ERR_COMPLETION_EN;
 
/* Prime the status register to be set to the last element. */
ioat->status = ioat->ring_addr + ((ioat->qcfg.nb_desc - 1) * DESC_SZ);
@@ -682,8 +685,6 @@ ioat_dmadev_create(const char *name, struct rte_pci_device 
*dev)
return -EIO;
}
}
-   ioat->regs->chanctrl = IOAT_CHANCTRL_ANY_ERR_ABORT_EN |
-   IOAT_CHANCTRL_ERR_COMPLETION_EN;
 
dmadev->fp_obj->dev_private = ioat;
 
-- 
2.37.2

[PATCH v2 4/6] test/dmadev: check result for device stop

[Bruce Richardson]

The DMA device stop API can return an error value so check that return
value when running dmadev unit tests.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index fe62e98af8..4e1dbcaa19 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -837,7 +837,11 @@ test_dmadev_instance(int16_t dev_id)
goto err;
 
rte_mempool_free(pool);
-   rte_dma_stop(dev_id);
+
+   if (rte_dma_stop(dev_id) < 0) {
+   rte_mempool_free(pool);
+   ERR_RETURN("Error stopping device %u\n", dev_id);
+   }
rte_dma_stats_reset(dev_id, vchan);
return 0;
 
-- 
2.37.2

[PATCH v2 5/6] test/dmadev: create separate function for single copy test

[Bruce Richardson]

The copy tests for dmadev had separate blocks in the test function for
single copy and burst copies. Separate out the single-copy block to its
own function so that it can be re-used if necessary.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 120 ++---
 1 file changed, 64 insertions(+), 56 deletions(-)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index 4e1dbcaa19..de787c14e2 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -175,77 +175,85 @@ do_multi_copies(int16_t dev_id, uint16_t vchan,
 }
 
 static int
-test_enqueue_copies(int16_t dev_id, uint16_t vchan)
+test_single_copy(int16_t dev_id, uint16_t vchan)
 {
-   enum rte_dma_status_code status;
-   unsigned int i;
+   uint16_t i;
uint16_t id;
+   enum rte_dma_status_code status;
+   struct rte_mbuf *src, *dst;
+   char *src_data, *dst_data;
 
-   /* test doing a single copy */
-   do {
-   struct rte_mbuf *src, *dst;
-   char *src_data, *dst_data;
+   src = rte_pktmbuf_alloc(pool);
+   dst = rte_pktmbuf_alloc(pool);
+   src_data = rte_pktmbuf_mtod(src, char *);
+   dst_data = rte_pktmbuf_mtod(dst, char *);
 
-   src = rte_pktmbuf_alloc(pool);
-   dst = rte_pktmbuf_alloc(pool);
-   src_data = rte_pktmbuf_mtod(src, char *);
-   dst_data = rte_pktmbuf_mtod(dst, char *);
+   for (i = 0; i < COPY_LEN; i++)
+   src_data[i] = rte_rand() & 0xFF;
 
-   for (i = 0; i < COPY_LEN; i++)
-   src_data[i] = rte_rand() & 0xFF;
+   id = rte_dma_copy(dev_id, vchan, rte_pktmbuf_iova(src), 
rte_pktmbuf_iova(dst),
+   COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
+   if (id != id_count)
+   ERR_RETURN("Error with rte_dma_copy, got %u, expected %u\n",
+   id, id_count);
 
-   id = rte_dma_copy(dev_id, vchan, rte_pktmbuf_iova(src), 
rte_pktmbuf_iova(dst),
-   COPY_LEN, RTE_DMA_OP_FLAG_SUBMIT);
-   if (id != id_count)
-   ERR_RETURN("Error with rte_dma_copy, got %u, expected 
%u\n",
-   id, id_count);
+   /* give time for copy to finish, then check it was done */
+   await_hw(dev_id, vchan);
 
-   /* give time for copy to finish, then check it was done */
-   await_hw(dev_id, vchan);
+   for (i = 0; i < COPY_LEN; i++)
+   if (dst_data[i] != src_data[i])
+   ERR_RETURN("Data mismatch at char %u [Got %02x not 
%02x]\n", i,
+   dst_data[i], src_data[i]);
+
+   /* now check completion works */
+   id = ~id;
+   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 1)
+   ERR_RETURN("Error with rte_dma_completed\n");
+
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received, %u [expected 
%u]\n",
+   id, id_count);
+
+   /* check for completed and id when no job done */
+   id = ~id;
+   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
+   ERR_RETURN("Error with rte_dma_completed when no job done\n");
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received when no job done, 
%u [expected %u]\n",
+   id, id_count);
+
+   /* check for completed_status and id when no job done */
+   id = ~id;
+   if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
+   ERR_RETURN("Error with rte_dma_completed_status when no job 
done\n");
+   if (id != id_count)
+   ERR_RETURN("Error:incorrect job id received when no job done, 
%u [expected %u]\n",
+   id, id_count);
 
-   for (i = 0; i < COPY_LEN; i++)
-   if (dst_data[i] != src_data[i])
-   ERR_RETURN("Data mismatch at char %u [Got %02x 
not %02x]\n", i,
-   dst_data[i], src_data[i]);
-
-   /* now check completion works */
-   id = ~id;
-   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 1)
-   ERR_RETURN("Error with rte_dma_completed\n");
-
-   if (id != id_count)
-   ERR_RETURN("Error:incorrect job id received, %u 
[expected %u]\n",
-   id, id_count);
-
-   /* check for completed and id when no job done */
-   id = ~id;
-   if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0)
-   ERR_RETURN("Error with rte_dma_completed when no job 
done\n");
-   if (id != id_count)
-   ERR_RETURN("Error:incorrect job id received when no job 
done, %u [expected %u]\n",
-

[PATCH v2 6/6] test/dmadev: add tests for stopping and restarting dev

[Bruce Richardson]

Validate device operation when a device is stopped or restarted.

The only complication - and gap in the dmadev ABI specification - is
what happens to the job ids on restart. Some drivers reset them to 0,
while others continue where things left off. Take account of both
possibilities in the test case.

Signed-off-by: Bruce Richardson 
---
 app/test/test_dmadev.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c
index de787c14e2..8fb73a41e2 100644
--- a/app/test/test_dmadev.c
+++ b/app/test/test_dmadev.c
@@ -304,6 +304,48 @@ test_enqueue_copies(int16_t dev_id, uint16_t vchan)
|| do_multi_copies(dev_id, vchan, 0, 0, 1);
 }
 
+static int
+test_stop_start(int16_t dev_id, uint16_t vchan)
+{
+   /* device is already started on input, should be (re)started on output 
*/
+
+   uint16_t id = 0;
+   enum rte_dma_status_code status = RTE_DMA_STATUS_SUCCESSFUL;
+
+   /* - test stopping a device works ok,
+* - then do a start-stop without doing a copy
+* - finally restart the device
+* checking for errors at each stage, and validating we can still copy 
at the end.
+*/
+   if (rte_dma_stop(dev_id) < 0)
+   ERR_RETURN("Error stopping device\n");
+
+   if (rte_dma_start(dev_id) < 0)
+   ERR_RETURN("Error restarting device\n");
+   if (rte_dma_stop(dev_id) < 0)
+   ERR_RETURN("Error stopping device after restart (no jobs 
executed)\n");
+
+   if (rte_dma_start(dev_id) < 0)
+   ERR_RETURN("Error restarting device after multiple 
stop-starts\n");
+
+   /* before doing a copy, we need to know what the next id will be it 
should
+* either be:
+* - the last completed job before start if driver does not reset id on 
stop
+* - or -1 i.e. next job is 0, if driver does reset the job ids on stop
+*/
+   if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0)
+   ERR_RETURN("Error with rte_dma_completed_status when no job 
done\n");
+   id += 1; /* id_count is next job id */
+   if (id != id_count && id != 0)
+   ERR_RETURN("Unexpected next id from device after stop-start. 
Got %u, expected %u or 0\n",
+   id, id_count);
+
+   id_count = id;
+   if (test_single_copy(dev_id, vchan) < 0)
+   ERR_RETURN("Error performing copy after device restart\n");
+   return 0;
+}
+
 /* Failure handling test cases - global macros and variables for those tests*/
 #define COMP_BURST_SZ  16
 #define OPT_FENCE(idx) ((fence && idx == 8) ? RTE_DMA_OP_FLAG_FENCE : 0)
@@ -819,6 +861,10 @@ test_dmadev_instance(int16_t dev_id)
if (runtest("copy", test_enqueue_copies, 640, dev_id, vchan, 
CHECK_ERRS) < 0)
goto err;
 
+   /* run tests stopping/starting devices and check jobs still work after 
restart */
+   if (runtest("stop-start", test_stop_start, 1, dev_id, vchan, 
CHECK_ERRS) < 0)
+   goto err;
+
/* run some burst capacity tests */
if (rte_dma_burst_capacity(dev_id, vchan) < 64)
printf("DMA Dev %u: insufficient burst capacity (64 required), 
skipping tests\n",
-- 
2.37.2

[PATCH v2 0/7] space after keyword cleanups

[Stephen Hemminger]

These are the results of a script replacing 'if(' with 'if ('
in various places.

v2 - fix related style changes on same lines reported
by checkpatch

Stephen Hemminger (7):
  test: fix whitespace
  testpmd: fix whitespace
  net/e1000: fix whitespace
  i40e: fix whitespace
  examples: fix whitespace
  cmdline: fix whitespace
  ip_frag: fix whitespace

 app/test-pmd/cmdline.c | 33 +++---
 app/test-pmd/parameters.c  | 11 
 app/test-pmd/testpmd.c |  2 +-
 app/test/test_cmdline_cirbuf.c |  4 +--
 app/test/test_debug.c  |  6 ++--
 app/test/test_hash.c   | 10 +++
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 -
 app/test/test_mbuf.c   | 15 +-
 app/test/test_ring_perf.c  |  2 +-
 app/test/test_spinlock.c   |  2 +-
 drivers/net/e1000/em_ethdev.c  |  2 +-
 drivers/net/e1000/igb_ethdev.c | 16 +--
 drivers/net/e1000/igb_pf.c |  2 +-
 drivers/net/e1000/igb_rxtx.c   |  6 ++--
 drivers/net/i40e/i40e_pf.c |  8 +++---
 examples/ip_reassembly/main.c  |  2 +-
 examples/l3fwd-power/main.c|  7 +++--
 examples/l3fwd/main.c  |  7 +++--
 examples/multi_process/symmetric_mp/main.c |  4 +--
 lib/cmdline/cmdline_rdline.c   | 12 
 lib/ip_frag/rte_ipv4_reassembly.c  |  2 +-
 22 files changed, 86 insertions(+), 83 deletions(-)

-- 
2.39.0

[PATCH v2 1/7] test: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test/test_cmdline_cirbuf.c |  4 ++--
 app/test/test_debug.c  |  6 +++---
 app/test/test_hash.c   | 10 +-
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +++---
 app/test/test_mbuf.c   | 15 +++
 app/test/test_ring_perf.c  |  2 +-
 app/test/test_spinlock.c   |  2 +-
 8 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c
index 8ac326cb02e0..6f7aae6df59a 100644
--- a/app/test/test_cmdline_cirbuf.c
+++ b/app/test/test_cmdline_cirbuf.c
@@ -708,7 +708,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from tail */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_tail_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_tail_safe(&cb) >= 0) {
@@ -737,7 +737,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from head */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_head_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_head_safe(&cb) >= 0) {
diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 2704f5b92726..24f14bd11313 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -53,7 +53,7 @@ test_panic(void)
return -1;
}
wait(&status);
-   if(status == 0){
+   if (status == 0) {
printf("Child process terminated normally!\n");
return -1;
} else
@@ -78,13 +78,13 @@ test_exit_val(int exit_val)
 
if (pid == 0)
rte_exit(exit_val, __func__);
-   else if (pid < 0){
+   else if (pid < 0) {
printf("Fork Failed\n");
return -1;
}
wait(&status);
printf("Child process status: %d\n", status);
-   if(!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
+   if (!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val) {
printf("Child process terminated with incorrect status 
(expected = %d)!\n",
exit_val);
return -1;
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index 3e45afaa67fc..b1307ce42e6b 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -50,7 +50,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_FBK(cond, str, ...) do {   
\
if (cond) { \
@@ -58,7 +58,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_fbk_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_RCU_QSBR(cond, str, ...) do {  \
if (cond) { \
@@ -724,12 +724,12 @@ static int test_five_keys(void)
}
 
/* Lookup */
-   for(i = 0; i < 5; i++)
+   for (i = 0; i < 5; i++)
key_array[i] = &keys[i];
 
ret = rte_hash_lookup_bulk(handle, &key_array[0], 5, (int32_t *)pos);
-   if(ret == 0)
-   for(i = 0; i < 5; i++) {
+   if (ret == 0)
+   for (i = 0; i < 5; i++) {
print_key_info("Lkp", key_array[i], pos[i]);
RETURN_IF_ERROR(pos[i] != expected_pos[i],
"failed to find key (pos[%u]=%d)", i, 
pos[i]);
diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index aaf2773b6fac..2f9321345e4b 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -47,7 +47,7 @@ print_route_distribution(const struct rules_tbl_entry *table, 
uint32_t n)
printf("--- \n");
 
/* Count depths. */
-   for(i = 1; i <= 128; i++) {
+   for (i = 1; i <= 128; i++) {
unsigned depth_counter = 0;
double percent_hits;
 
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index de40e506113a..8d3750a3f1f0 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -86,8 +86,8 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
 

[PATCH v2 2/7] testpmd: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/cmdline.c| 33 +
 app/test-pmd/parameters.c | 11 ++-
 app/test-pmd/testpmd.c|  2 +-
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b32dc8bfd445..d2a52ac7b723 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2728,13 +2728,14 @@ parse_reta_config(const char *str,
unsigned long int_fld[_NUM_FLD];
char *str_fld[_NUM_FLD];
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -3242,15 +3243,15 @@ cmd_config_thresh_parsed(void *parsed_result,
 
if (!strcmp(res->name, "txpt"))
tx_pthresh = res->value;
-   else if(!strcmp(res->name, "txht"))
+   else if (!strcmp(res->name, "txht"))
tx_hthresh = res->value;
-   else if(!strcmp(res->name, "txwt"))
+   else if (!strcmp(res->name, "txwt"))
tx_wthresh = res->value;
-   else if(!strcmp(res->name, "rxpt"))
+   else if (!strcmp(res->name, "rxpt"))
rx_pthresh = res->value;
-   else if(!strcmp(res->name, "rxht"))
+   else if (!strcmp(res->name, "rxht"))
rx_hthresh = res->value;
-   else if(!strcmp(res->name, "rxwt"))
+   else if (!strcmp(res->name, "rxwt"))
rx_wthresh = res->value;
else {
fprintf(stderr, "Unknown parameter\n");
@@ -4084,8 +4085,8 @@ cmd_vlan_offload_parsed(void *parsed_result,
len = strnlen(str, STR_TOKEN_SIZE);
i = 0;
/* Get port_id first */
-   while(i < len){
-   if(str[i] == ',')
+   while (i < len) {
+   if (str[i] == ',')
break;
 
i++;
@@ -4093,7 +4094,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
str[i]='\0';
tmp = strtoul(str, NULL, 0);
/* If port_id greater that what portid_t can represent, return */
-   if(tmp >= RTE_MAX_ETHPORTS)
+   if (tmp >= RTE_MAX_ETHPORTS)
return;
port_id = (portid_t)tmp;
 
@@ -4104,17 +4105,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
 
if (!strcmp(res->what, "strip"))
rx_vlan_strip_set(port_id,  on);
-   else if(!strcmp(res->what, "stripq")){
+   else if (!strcmp(res->what, "stripq")) {
uint16_t queue_id = 0;
 
/* No queue_id, return */
-   if(i + 1 >= len) {
+   if (i + 1 >= len) {
fprintf(stderr, "must specify (port,queue_id)\n");
return;
}
tmp = strtoul(str + i + 1, NULL, 0);
/* If queue_id greater that what 16-bits can represent, return 
*/
-   if(tmp > 0x)
+   if (tmp > 0x)
return;
 
queue_id = (uint16_t)tmp;
@@ -7252,7 +7253,7 @@ static void cmd_mac_addr_parsed(void *parsed_result,
ret = rte_eth_dev_mac_addr_remove(res->port_num, &res->address);
 
/* check the return value and print it if is < 0 */
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
 
 }
@@ -7825,7 +7826,7 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
res->vf_num);
 #endif
 
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
strerror(-ret));
 
 }
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index d597c209ba5e..5f8f962bc93d 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -306,13 +306,14 @@ parse_portnuma_config(const char *q_arg)
char *str_fld[_NUM_FLD];
 
/* reset from value set at definition */
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -368,11 +369,11 @@ parse_ringnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {

[PATCH v2 3/7] net/e1000: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_ethdev.c  |  2 +-
 drivers/net/e1000/igb_ethdev.c | 16 
 drivers/net/e1000/igb_pf.c |  2 +-
 drivers/net/e1000/igb_rxtx.c   |  6 +++---
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8ee9be12ad19..3cb09538b2df 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -872,7 +872,7 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
int pause_frames;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
stats->sec += E1000_READ_REG(hw, E1000_SEC);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8cc..e89bfa248fb0 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -735,7 +735,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
/* for secondary processes, we don't initialise any further as primary
 * has already done this work. Only check we don't need a different
 * RX function */
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
return 0;
@@ -927,7 +927,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
/* for secondary processes, we don't initialise any further as primary
 * has already done this work. Only check we don't need a different
 * RX function */
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
return 0;
@@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw, struct 
e1000_hw_stats *stats)
uint64_t old_rpthc = stats->rpthc;
uint64_t old_hgptc = stats->hgptc;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs +=
E1000_READ_REG(hw,E1000_SYMERRS);
@@ -3498,12 +3498,12 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, 
bool on)
E1000_DEV_PRIVATE_TO_VFTA(dev->data->dev_private);
int i = 0, j = 0, vfta = 0, mask = 1;
 
-   for (i = 0; i < IGB_VFTA_SIZE; i++){
+   for (i = 0; i < IGB_VFTA_SIZE; i++) {
vfta = shadow_vfta->vfta[i];
-   if(vfta){
+   if (vfta) {
mask = 1;
-   for (j = 0; j < 32; j++){
-   if(vfta & mask)
+   for (j = 0; j < 32; j++) {
+   if (vfta & mask)
igbvf_set_vfta(hw,
(uint16_t)((i<<5)+j), on);
mask<<=1;
@@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 
/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
ret = igbvf_set_vfta(hw, vlan_id, !!on);
-   if(ret){
+   if (ret) {
PMD_INIT_LOG(ERR, "Unable to set VF vlan");
return ret;
}
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index c7588ea57eaa..b1f74eb841d2 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -78,7 +78,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
 
if (hw->mac.type == e1000_i350)
nb_queue = 1;
-   else if(hw->mac.type == e1000_82576)
+   else if (hw->mac.type == e1000_82576)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index f32dee46df82..2d4a1292053e 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -196,13 +196,13 @@ struct igb_tx_queue {
 #ifdef RTE_PMD_USE_PREFETCH
 #define rte_igb_prefetch(p)rte_prefetch0(p)
 #else
-#define rte_igb_prefetch(p)do {} while(0)
+#define rte_igb_prefetch(p)do {} while (0)
 #endif
 
 #ifdef RTE_PMD_PACKET_PREFETCH
 #define rte_packet_prefetch(p) rte_prefetch1(p)
 #else
-#define rte_packet_prefetch(p) do {} while(0)
+#define rte_packet_pre

[PATCH v2 4/7] i40e: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/i40e/i40e_pf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 15d9ff868f3a..7050e0057d8e 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
 
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
vid = vlan_filter_list->vlan_id;
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 * return if SRIOV not enabled, VF number not configured or
 * no queue assigned.
 */
-   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
+   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
return I40E_SUCCESS;
 
/* Allocate memory to store VF structure */
pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
-   if(pf->vfs == NULL)
+   if (pf->vfs == NULL)
return -ENOMEM;
 
/* Disable irq0 for VFR event */
-- 
2.39.0

[PATCH v2 5/7] examples: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 examples/ip_reassembly/main.c  | 2 +-
 examples/l3fwd-power/main.c| 7 ---
 examples/l3fwd/main.c  | 7 ---
 examples/multi_process/symmetric_mp/main.c | 4 ++--
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index bd0b1d31decf..7e84b4944759 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -300,7 +300,7 @@ send_single_packet(struct rte_mbuf *m, uint16_t port)
 
TX_LCORE_STAT_UPDATE(&qconf->tx_stat, queue, 1);
txmb->m_table[txmb->head] = m;
-   if(++txmb->head == len)
+   if (++txmb->head == len)
txmb->head = 0;
 
return 0;
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index fd3ade330f82..d10102fa282e 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1785,13 +1785,14 @@ parse_config(const char *q_arg)
 
nb_lcore_params = 0;
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5198ff30dd00..e2bdfe451a2c 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -514,13 +514,14 @@ parse_config(const char *q_arg)
 
nb_lcore_params = 0;
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
diff --git a/examples/multi_process/symmetric_mp/main.c 
b/examples/multi_process/symmetric_mp/main.c
index 1ff85875dfdf..8d1119bd8483 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -156,7 +156,7 @@ smp_parse_args(int argc, char **argv)
 
/* get the port numbers from the port mask */
RTE_ETH_FOREACH_DEV(i)
-   if(port_mask & (1 << i))
+   if (port_mask & (1 << i))
ports[num_ports++] = (uint8_t)i;
 
ret = optind-1;
@@ -471,7 +471,7 @@ main(int argc, char **argv)
if (num_ports & 1)
rte_exit(EXIT_FAILURE, "Application must use an even number of 
ports\n");
for(i = 0; i < num_ports; i++){
-   if(proc_type == RTE_PROC_PRIMARY)
+   if (proc_type == RTE_PROC_PRIMARY)
if (smp_port_init(ports[i], mp, (uint16_t)num_procs) < 
0)
rte_exit(EXIT_FAILURE, "Error initialising 
ports\n");
}
-- 
2.39.0

[PATCH v2 6/7] cmdline: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/cmdline/cmdline_rdline.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 5cf723a0126a..1ceba6b0b8d4 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -301,7 +301,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* delete 1 char from the left */
case CMDLINE_KEY_BKSPACE:
case CMDLINE_KEY_BKSPACE2:
-   if(!cirbuf_del_tail_safe(&rdl->left)) {
+   if (!cirbuf_del_tail_safe(&rdl->left)) {
rdline_puts(rdl, vt100_bs);
display_right_buffer(rdl, 1);
}
@@ -354,7 +354,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* paste contents of kill buffer to the left side of caret */
case CMDLINE_KEY_CTRL_Y:
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < rdl->kill_size) {
cirbuf_add_tail(&rdl->left, rdl->kill_buf[i]);
@@ -403,10 +403,10 @@ rdline_char_in(struct rdline *rdl, char c)
tmp_size = strnlen(tmp_buf, sizeof(tmp_buf));
/* add chars */
if (ret == RDLINE_RES_COMPLETE) {
-   i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
- RDLINE_BUF_SIZE &&
- i < tmp_size) {
+   i = 0;
+   while (CIRBUF_GET_LEN(&rdl->right)
+  + CIRBUF_GET_LEN(&rdl->left) < 
RDLINE_BUF_SIZE &&
+  i < tmp_size) {
cirbuf_add_tail(&rdl->left, 
tmp_buf[i]);
rdl->write_char(rdl, 
tmp_buf[i]);
i++;
-- 
2.39.0

[PATCH v2 7/7] ip_frag: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/ip_frag/rte_ipv4_reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c 
b/lib/ip_frag/rte_ipv4_reassembly.c
index 4a89a5f5365a..88ee0aa63f62 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -34,7 +34,7 @@ ipv4_frag_reassemble(struct ip_frag_pkt *fp)
for (i = n; i != IP_FIRST_FRAG_IDX && ofs != first_len; i--) {
 
/* previous fragment found. */
-   if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
+   if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
 
RTE_ASSERT(curr_idx != i);
 
-- 
2.39.0

Re: [PATCH] Memory Allocation: Adding a new UT for fb_array

[Stephen Hemminger]

On Fri, 13 Jan 2023 13:12:47 +
Vipin P R  wrote:

> add test case coverage to cover the ms_idx jump
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vipin P R 
> Acked-by: Kumara Parameshwaran 
> ---
> Depends-on: 0001-Memory-Allocation-Fixes-ms_idx-jump-lookahead-during.patch
> Depends-on: 0002-Memory-Allocation-Fixes-ms_idx-jump-lookbehind-durin.patch

This looks like a good idea but lots of style errors on this patch.
Please run checkpatch, fix and resubmit.

Re: [PATCH] Intel iavf: Return in the case of ADD/DEL ETH address

[Stephen Hemminger]

On Fri, 13 Jan 2023 13:19:23 +
Vipin P R  wrote:

> In case of i40vf, VIRTCHNL_OP_DEL_ETH_ADDR and VIRTCHNL_OP_ADD_ETH_ADDR are 
> unsupported.
> i40evf_execute_vf_cmd is invoked with these operations as part of 
> i40evf_set_mc_addr_list()
> 
> The cases are not handled in i40evf_execute_vf_cmd() thus hitting the default 
> case.
> There is a retry logic of upto 200 times (2000 in iavf) with a delay of 10ms 
> (1ms in iavf).
> This results in a needless delay of 2s in the init phase for each VNIC.
> 
> The patch aims to rectify that delay.
> In fe2a571c70cc397f2ad4e280f8d084148fea5d62, i40e_ethdev_vf.c was deleted. 
> Hence adding this in iavf_vchnl.c.
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vipin P R 
> ---
>  drivers/net/iavf/iavf_vchnl.c | 8 
>  1 file changed, 8 insertions(+)

Please run checkpatch, the DPDK style is to indent with tabs not spaces.

Re: [PATCH v4 1/1] eal/linux: reject mountpt not parent of --huge-dir

[Ashish Sadanandan]

On Wed, Jan 11, 2023 at 5:11 AM John Levon  wrote:

> On Sun, Jan 08, 2023 at 06:52:39PM -0700, Ashish Sadanandan wrote:
>
> > The code added for allowing --huge-dir to specify hugetlbfs
> > sub-directories has a bug where it incorrectly matches mounts that
> > contain a prefix of the specified --huge-dir.
> >
> > Fixes: 24d5a1ce6b85 ("eal/linux: allow hugetlbfs sub-directories")
> > Cc: john.le...@nutanix.com
> > Cc: sta...@dpdk.org
> > Signed-off-by: Ashish Sadanandan 
>
> Reviewed-by: John Levon 
>
> thanks
> john
>

Thanks for reviewing again, John. Do I need to CC anyone else to get this
committed or any other steps I need to follow? I was hoping to get this
merged into 22.11 stable branch too.

- Ashish

RE: [PATCH] ipsec: remove unneccessary null check

[Ji, Kai]

Acked-by: Kai Ji 

> -Original Message-
> From: Stephen Hemminger 
> Sent: Friday, January 13, 2023 6:44 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Ji, Kai
> ; De Lara Guarch, Pablo ;
> Power, Ciara 
> Subject: [PATCH] ipsec: remove unneccessary null check
> 
> The function rte_ring_free() accepts NULL as vaild input like free() and
> other functions.
> 
> Found with null_free_check.cocci.
> 
> Fixes: 16d6ebb65d59 ("crypto/ipsec_mb: fix null checks")
> Cc: kai...@intel.com
> Signed-off-by: Stephen Hemminger 
> ---
>  drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> index 71e02cd0513d..3e52f9567401 100644
> --- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> +++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
> @@ -139,15 +139,12 @@ int
>  ipsec_mb_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)  {
>   struct ipsec_mb_qp *qp = dev->data->queue_pairs[qp_id];
> - struct rte_ring *r = NULL;
> 
>   if (!qp)
>   return 0;
> 
>   if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> - r = rte_ring_lookup(qp->name);
> - if (r)
> - rte_ring_free(r);
> + rte_ring_free(rte_ring_lookup(qp->name));
> 
>  #if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
>   if (qp->mb_mgr)
> --
> 2.39.0

[PATCH v3 0/7] add space after keywords

[Stephen Hemminger]

These are the results of a script replacing 'if(' with 'if ('
in various places.

v3 - more checkpatch fallout
 also fix places where 'for(' is used.

Stephen Hemminger (7):
  test: fix whitespace
  testpmd: fix whitespace
  net/e1000: fix whitespace
  i40e: fix whitespace
  examples: fix whitespace
  cmdline: fix whitespace
  ip_frag: fix whitespace

 app/test-pmd/cmdline.c | 33 +++---
 app/test-pmd/parameters.c  | 12 
 app/test-pmd/testpmd.c |  2 +-
 app/test/test_cmdline_cirbuf.c |  4 +--
 app/test/test_debug.c  |  6 ++--
 app/test/test_hash.c   | 10 +++
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 -
 app/test/test_mbuf.c   | 15 +-
 app/test/test_ring_perf.c  |  2 +-
 app/test/test_spinlock.c   |  2 +-
 drivers/net/e1000/em_ethdev.c  |  2 +-
 drivers/net/e1000/igb_ethdev.c | 16 +--
 drivers/net/e1000/igb_pf.c |  2 +-
 drivers/net/e1000/igb_rxtx.c   |  6 ++--
 drivers/net/i40e/i40e_pf.c |  8 +++---
 examples/ip_reassembly/main.c  |  2 +-
 examples/l3fwd-power/main.c|  9 +++---
 examples/l3fwd/main.c  |  9 +++---
 examples/multi_process/symmetric_mp/main.c |  6 ++--
 examples/qos_sched/app_thread.c|  6 ++--
 examples/qos_sched/args.c  |  2 +-
 examples/qos_sched/init.c  |  2 +-
 examples/qos_sched/main.c  |  2 +-
 lib/cmdline/cmdline_rdline.c   | 12 
 lib/ip_frag/rte_ipv4_reassembly.c  |  2 +-
 26 files changed, 96 insertions(+), 92 deletions(-)

-- 
2.39.0

[PATCH v3 1/7] test: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test/test_cmdline_cirbuf.c |  4 ++--
 app/test/test_debug.c  |  6 +++---
 app/test/test_hash.c   | 10 +-
 app/test/test_lpm6_perf.c  |  2 +-
 app/test/test_malloc.c | 14 +++---
 app/test/test_mbuf.c   | 15 +++
 app/test/test_ring_perf.c  |  2 +-
 app/test/test_spinlock.c   |  2 +-
 8 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c
index 8ac326cb02e0..6f7aae6df59a 100644
--- a/app/test/test_cmdline_cirbuf.c
+++ b/app/test/test_cmdline_cirbuf.c
@@ -708,7 +708,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from tail */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_tail_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_tail_safe(&cb) >= 0) {
@@ -737,7 +737,7 @@ test_cirbuf_char_fill(void)
return -1;
}
/* delete buffer from head */
-   for(i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
+   for (i = 0; i < CMDLINE_TEST_BUFSIZE; i++)
cirbuf_del_head_safe(&cb);
/* try to delete from an empty buffer */
if (cirbuf_del_head_safe(&cb) >= 0) {
diff --git a/app/test/test_debug.c b/app/test/test_debug.c
index 2704f5b92726..24f14bd11313 100644
--- a/app/test/test_debug.c
+++ b/app/test/test_debug.c
@@ -53,7 +53,7 @@ test_panic(void)
return -1;
}
wait(&status);
-   if(status == 0){
+   if (status == 0) {
printf("Child process terminated normally!\n");
return -1;
} else
@@ -78,13 +78,13 @@ test_exit_val(int exit_val)
 
if (pid == 0)
rte_exit(exit_val, __func__);
-   else if (pid < 0){
+   else if (pid < 0) {
printf("Fork Failed\n");
return -1;
}
wait(&status);
printf("Child process status: %d\n", status);
-   if(!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val){
+   if (!WIFEXITED(status) || WEXITSTATUS(status) != (uint8_t)exit_val) {
printf("Child process terminated with incorrect status 
(expected = %d)!\n",
exit_val);
return -1;
diff --git a/app/test/test_hash.c b/app/test/test_hash.c
index 3e45afaa67fc..b1307ce42e6b 100644
--- a/app/test/test_hash.c
+++ b/app/test/test_hash.c
@@ -50,7 +50,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_FBK(cond, str, ...) do {   
\
if (cond) { \
@@ -58,7 +58,7 @@ static uint32_t hashtest_key_lens[] = {0, 2, 4, 5, 6, 7, 8, 
10, 11, 15, 16, 21,
if (handle) rte_fbk_hash_free(handle);  \
return -1;  \
}   \
-} while(0)
+} while (0)
 
 #define RETURN_IF_ERROR_RCU_QSBR(cond, str, ...) do {  \
if (cond) { \
@@ -724,12 +724,12 @@ static int test_five_keys(void)
}
 
/* Lookup */
-   for(i = 0; i < 5; i++)
+   for (i = 0; i < 5; i++)
key_array[i] = &keys[i];
 
ret = rte_hash_lookup_bulk(handle, &key_array[0], 5, (int32_t *)pos);
-   if(ret == 0)
-   for(i = 0; i < 5; i++) {
+   if (ret == 0)
+   for (i = 0; i < 5; i++) {
print_key_info("Lkp", key_array[i], pos[i]);
RETURN_IF_ERROR(pos[i] != expected_pos[i],
"failed to find key (pos[%u]=%d)", i, 
pos[i]);
diff --git a/app/test/test_lpm6_perf.c b/app/test/test_lpm6_perf.c
index aaf2773b6fac..2f9321345e4b 100644
--- a/app/test/test_lpm6_perf.c
+++ b/app/test/test_lpm6_perf.c
@@ -47,7 +47,7 @@ print_route_distribution(const struct rules_tbl_entry *table, 
uint32_t n)
printf("--- \n");
 
/* Count depths. */
-   for(i = 1; i <= 128; i++) {
+   for (i = 1; i <= 128; i++) {
unsigned depth_counter = 0;
double percent_hits;
 
diff --git a/app/test/test_malloc.c b/app/test/test_malloc.c
index de40e506113a..8d3750a3f1f0 100644
--- a/app/test/test_malloc.c
+++ b/app/test/test_malloc.c
@@ -86,8 +86,8 @@ test_align_overlap_per_lcore(__rte_unused void *arg)
 

[PATCH v3 2/7] testpmd: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 app/test-pmd/cmdline.c| 33 +
 app/test-pmd/parameters.c | 12 +++-
 app/test-pmd/testpmd.c|  2 +-
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b32dc8bfd445..d2a52ac7b723 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2728,13 +2728,14 @@ parse_reta_config(const char *str,
unsigned long int_fld[_NUM_FLD];
char *str_fld[_NUM_FLD];
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -3242,15 +3243,15 @@ cmd_config_thresh_parsed(void *parsed_result,
 
if (!strcmp(res->name, "txpt"))
tx_pthresh = res->value;
-   else if(!strcmp(res->name, "txht"))
+   else if (!strcmp(res->name, "txht"))
tx_hthresh = res->value;
-   else if(!strcmp(res->name, "txwt"))
+   else if (!strcmp(res->name, "txwt"))
tx_wthresh = res->value;
-   else if(!strcmp(res->name, "rxpt"))
+   else if (!strcmp(res->name, "rxpt"))
rx_pthresh = res->value;
-   else if(!strcmp(res->name, "rxht"))
+   else if (!strcmp(res->name, "rxht"))
rx_hthresh = res->value;
-   else if(!strcmp(res->name, "rxwt"))
+   else if (!strcmp(res->name, "rxwt"))
rx_wthresh = res->value;
else {
fprintf(stderr, "Unknown parameter\n");
@@ -4084,8 +4085,8 @@ cmd_vlan_offload_parsed(void *parsed_result,
len = strnlen(str, STR_TOKEN_SIZE);
i = 0;
/* Get port_id first */
-   while(i < len){
-   if(str[i] == ',')
+   while (i < len) {
+   if (str[i] == ',')
break;
 
i++;
@@ -4093,7 +4094,7 @@ cmd_vlan_offload_parsed(void *parsed_result,
str[i]='\0';
tmp = strtoul(str, NULL, 0);
/* If port_id greater that what portid_t can represent, return */
-   if(tmp >= RTE_MAX_ETHPORTS)
+   if (tmp >= RTE_MAX_ETHPORTS)
return;
port_id = (portid_t)tmp;
 
@@ -4104,17 +4105,17 @@ cmd_vlan_offload_parsed(void *parsed_result,
 
if (!strcmp(res->what, "strip"))
rx_vlan_strip_set(port_id,  on);
-   else if(!strcmp(res->what, "stripq")){
+   else if (!strcmp(res->what, "stripq")) {
uint16_t queue_id = 0;
 
/* No queue_id, return */
-   if(i + 1 >= len) {
+   if (i + 1 >= len) {
fprintf(stderr, "must specify (port,queue_id)\n");
return;
}
tmp = strtoul(str + i + 1, NULL, 0);
/* If queue_id greater that what 16-bits can represent, return 
*/
-   if(tmp > 0x)
+   if (tmp > 0x)
return;
 
queue_id = (uint16_t)tmp;
@@ -7252,7 +7253,7 @@ static void cmd_mac_addr_parsed(void *parsed_result,
ret = rte_eth_dev_mac_addr_remove(res->port_num, &res->address);
 
/* check the return value and print it if is < 0 */
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "mac_addr_cmd error: (%s)\n", strerror(-ret));
 
 }
@@ -7825,7 +7826,7 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
res->vf_num);
 #endif
 
-   if(ret < 0)
+   if (ret < 0)
fprintf(stderr, "vf_mac_addr_cmd error: (%s)\n", 
strerror(-ret));
 
 }
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index d597c209ba5e..1e619db64891 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -306,13 +306,14 @@ parse_portnuma_config(const char *q_arg)
char *str_fld[_NUM_FLD];
 
/* reset from value set at definition */
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -368,11 +369,12 @@ parse_ringnuma_config(const char *q_arg)
/* reset from value set at definition */
while ((p = strchr(p0,'(')) != NULL) {

[PATCH v3 3/7] net/e1000: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_ethdev.c  |  2 +-
 drivers/net/e1000/igb_ethdev.c | 16 
 drivers/net/e1000/igb_pf.c |  2 +-
 drivers/net/e1000/igb_rxtx.c   |  6 +++---
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8ee9be12ad19..3cb09538b2df 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -872,7 +872,7 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct 
rte_eth_stats *rte_stats)
E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
int pause_frames;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
stats->sec += E1000_READ_REG(hw, E1000_SEC);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 8858f975f8cc..e89bfa248fb0 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -735,7 +735,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
/* for secondary processes, we don't initialise any further as primary
 * has already done this work. Only check we don't need a different
 * RX function */
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
return 0;
@@ -927,7 +927,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
/* for secondary processes, we don't initialise any further as primary
 * has already done this work. Only check we don't need a different
 * RX function */
-   if (rte_eal_process_type() != RTE_PROC_PRIMARY){
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
if (eth_dev->data->scattered_rx)
eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
return 0;
@@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw, struct 
e1000_hw_stats *stats)
uint64_t old_rpthc = stats->rpthc;
uint64_t old_hgptc = stats->hgptc;
 
-   if(hw->phy.media_type == e1000_media_type_copper ||
+   if (hw->phy.media_type == e1000_media_type_copper ||
(E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
stats->symerrs +=
E1000_READ_REG(hw,E1000_SYMERRS);
@@ -3498,12 +3498,12 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, 
bool on)
E1000_DEV_PRIVATE_TO_VFTA(dev->data->dev_private);
int i = 0, j = 0, vfta = 0, mask = 1;
 
-   for (i = 0; i < IGB_VFTA_SIZE; i++){
+   for (i = 0; i < IGB_VFTA_SIZE; i++) {
vfta = shadow_vfta->vfta[i];
-   if(vfta){
+   if (vfta) {
mask = 1;
-   for (j = 0; j < 32; j++){
-   if(vfta & mask)
+   for (j = 0; j < 32; j++) {
+   if (vfta & mask)
igbvf_set_vfta(hw,
(uint16_t)((i<<5)+j), on);
mask<<=1;
@@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t 
vlan_id, int on)
 
/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
ret = igbvf_set_vfta(hw, vlan_id, !!on);
-   if(ret){
+   if (ret) {
PMD_INIT_LOG(ERR, "Unable to set VF vlan");
return ret;
}
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index c7588ea57eaa..b1f74eb841d2 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -78,7 +78,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
 
if (hw->mac.type == e1000_i350)
nb_queue = 1;
-   else if(hw->mac.type == e1000_82576)
+   else if (hw->mac.type == e1000_82576)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index f32dee46df82..2d4a1292053e 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -196,13 +196,13 @@ struct igb_tx_queue {
 #ifdef RTE_PMD_USE_PREFETCH
 #define rte_igb_prefetch(p)rte_prefetch0(p)
 #else
-#define rte_igb_prefetch(p)do {} while(0)
+#define rte_igb_prefetch(p)do {} while (0)
 #endif
 
 #ifdef RTE_PMD_PACKET_PREFETCH
 #define rte_packet_prefetch(p) rte_prefetch1(p)
 #else
-#define rte_packet_prefetch(p) do {} while(0)
+#define rte_packet_pre

[PATCH v3 4/7] i40e: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 drivers/net/i40e/i40e_pf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index 15d9ff868f3a..7050e0057d8e 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -956,7 +956,7 @@ i40e_pf_host_process_cmd_add_vlan(struct i40e_pf_vf *vf,
 
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_add_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -996,7 +996,7 @@ i40e_pf_host_process_cmd_del_vlan(struct i40e_pf_vf *vf,
vid = vlan_filter_list->vlan_id;
for (i = 0; i < vlan_filter_list->num_elements; i++) {
ret = i40e_vsi_delete_vlan(vf->vsi, vid[i]);
-   if(ret != I40E_SUCCESS)
+   if (ret != I40E_SUCCESS)
goto send_msg;
}
 
@@ -1577,12 +1577,12 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 * return if SRIOV not enabled, VF number not configured or
 * no queue assigned.
 */
-   if(!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
+   if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 || pf->vf_nb_qps == 0)
return I40E_SUCCESS;
 
/* Allocate memory to store VF structure */
pf->vfs = rte_zmalloc("i40e_pf_vf",sizeof(*pf->vfs) * pf->vf_num, 0);
-   if(pf->vfs == NULL)
+   if (pf->vfs == NULL)
return -ENOMEM;
 
/* Disable irq0 for VFR event */
-- 
2.39.0

[PATCH v3 5/7] examples: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 examples/ip_reassembly/main.c  | 2 +-
 examples/l3fwd-power/main.c| 9 +
 examples/l3fwd/main.c  | 9 +
 examples/multi_process/symmetric_mp/main.c | 6 +++---
 examples/qos_sched/app_thread.c| 6 +++---
 examples/qos_sched/args.c  | 2 +-
 examples/qos_sched/init.c  | 2 +-
 examples/qos_sched/main.c  | 2 +-
 8 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index bd0b1d31decf..7e84b4944759 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -300,7 +300,7 @@ send_single_packet(struct rte_mbuf *m, uint16_t port)
 
TX_LCORE_STAT_UPDATE(&qconf->tx_stat, queue, 1);
txmb->m_table[txmb->head] = m;
-   if(++txmb->head == len)
+   if (++txmb->head == len)
txmb->head = 0;
 
return 0;
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index fd3ade330f82..513a1b21a69e 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1785,13 +1785,14 @@ parse_config(const char *q_arg)
 
nb_lcore_params = 0;
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -2946,7 +2947,7 @@ main(int argc, char **argv)
fflush(stdout);
 
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5198ff30dd00..687a81892aad 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -514,13 +514,14 @@ parse_config(const char *q_arg)
 
nb_lcore_params = 0;
 
-   while ((p = strchr(p0,'(')) != NULL) {
+   while ((p = strchr(p0, '(')) != NULL) {
++p;
-   if((p0 = strchr(p,')')) == NULL)
+   p0 = strchr(p, ')');
+   if (p0 == NULL)
return -1;
 
size = p0 - p;
-   if(size >= sizeof(s))
+   if (size >= sizeof(s))
return -1;
 
snprintf(s, sizeof(s), "%.*s", size, p);
@@ -1366,7 +1367,7 @@ l3fwd_poll_resource_setup(void)
printf("\nInitializing rx queues on lcore %u ... ", lcore_id );
fflush(stdout);
/* init RX queues */
-   for(queue = 0; queue < qconf->n_rx_queue; ++queue) {
+   for (queue = 0; queue < qconf->n_rx_queue; ++queue) {
struct rte_eth_rxconf rxq_conf;
 
portid = qconf->rx_queue_list[queue].port_id;
diff --git a/examples/multi_process/symmetric_mp/main.c 
b/examples/multi_process/symmetric_mp/main.c
index 1ff85875dfdf..9c5348bd36ed 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -156,7 +156,7 @@ smp_parse_args(int argc, char **argv)
 
/* get the port numbers from the port mask */
RTE_ETH_FOREACH_DEV(i)
-   if(port_mask & (1 << i))
+   if (port_mask & (1 << i))
ports[num_ports++] = (uint8_t)i;
 
ret = optind-1;
@@ -470,8 +470,8 @@ main(int argc, char **argv)
/* Primary instance initialized. 8< */
if (num_ports & 1)
rte_exit(EXIT_FAILURE, "Application must use an even number of 
ports\n");
-   for(i = 0; i < num_ports; i++){
-   if(proc_type == RTE_PROC_PRIMARY)
+   for (i = 0; i < num_ports; i++){
+   if (proc_type == RTE_PROC_PRIMARY)
if (smp_port_init(ports[i], mp, (uint16_t)num_procs) < 
0)
rte_exit(EXIT_FAILURE, "Error initialising 
ports\n");
}
diff --git a/examples/qos_sched/app_thread.c b/examples/qos_sched/app_thread.c
index dbc878b55394..a49356fecf5f 100644
--- a/examples/qos_sched/app_thread.c
+++ b/examples/qos_sched/app_thread.c
@@ -79,7 +79,7 @@ app_rx_thread(struct thread_conf **confs)
if (likely(nb_rx != 0)) {
APP_STATS_ADD(conf->stat.nb_rx, nb_rx);
 
-   for(i = 0; i < nb_rx; i++) {
+   for (i = 0; i < nb_rx; i++) {
   

[PATCH v3 6/7] cmdline: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/cmdline/cmdline_rdline.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
index 5cf723a0126a..1ceba6b0b8d4 100644
--- a/lib/cmdline/cmdline_rdline.c
+++ b/lib/cmdline/cmdline_rdline.c
@@ -301,7 +301,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* delete 1 char from the left */
case CMDLINE_KEY_BKSPACE:
case CMDLINE_KEY_BKSPACE2:
-   if(!cirbuf_del_tail_safe(&rdl->left)) {
+   if (!cirbuf_del_tail_safe(&rdl->left)) {
rdline_puts(rdl, vt100_bs);
display_right_buffer(rdl, 1);
}
@@ -354,7 +354,7 @@ rdline_char_in(struct rdline *rdl, char c)
/* paste contents of kill buffer to the left side of caret */
case CMDLINE_KEY_CTRL_Y:
i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
+   while (CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
  RDLINE_BUF_SIZE &&
  i < rdl->kill_size) {
cirbuf_add_tail(&rdl->left, rdl->kill_buf[i]);
@@ -403,10 +403,10 @@ rdline_char_in(struct rdline *rdl, char c)
tmp_size = strnlen(tmp_buf, sizeof(tmp_buf));
/* add chars */
if (ret == RDLINE_RES_COMPLETE) {
-   i=0;
-   while(CIRBUF_GET_LEN(&rdl->right) + 
CIRBUF_GET_LEN(&rdl->left) <
- RDLINE_BUF_SIZE &&
- i < tmp_size) {
+   i = 0;
+   while (CIRBUF_GET_LEN(&rdl->right)
+  + CIRBUF_GET_LEN(&rdl->left) < 
RDLINE_BUF_SIZE &&
+  i < tmp_size) {
cirbuf_add_tail(&rdl->left, 
tmp_buf[i]);
rdl->write_char(rdl, 
tmp_buf[i]);
i++;
-- 
2.39.0

[PATCH v3 7/7] ip_frag: fix whitespace

[Stephen Hemminger]

The style standard is to use blank after keywords.
I.e "if (" not "if("

Signed-off-by: Stephen Hemminger 
---
 lib/ip_frag/rte_ipv4_reassembly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ip_frag/rte_ipv4_reassembly.c 
b/lib/ip_frag/rte_ipv4_reassembly.c
index 4a89a5f5365a..88ee0aa63f62 100644
--- a/lib/ip_frag/rte_ipv4_reassembly.c
+++ b/lib/ip_frag/rte_ipv4_reassembly.c
@@ -34,7 +34,7 @@ ipv4_frag_reassemble(struct ip_frag_pkt *fp)
for (i = n; i != IP_FIRST_FRAG_IDX && ofs != first_len; i--) {
 
/* previous fragment found. */
-   if(fp->frags[i].ofs + fp->frags[i].len == ofs) {
+   if (fp->frags[i].ofs + fp->frags[i].len == ofs) {
 
RTE_ASSERT(curr_idx != i);
 
-- 
2.39.0

RE: [PATCH v3 4/7] i40e: fix whitespace

[Xing, Beilei]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Tuesday, January 17, 2023 8:15 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Zhang, Yuying
> ; Xing, Beilei 
> Subject: [PATCH v3 4/7] i40e: fix whitespace
> 
> The style standard is to use blank after keywords.
> I.e "if (" not "if("
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Beilei Xing 

RE: [PATCH v2 07/15] common/idpf: add irq map/unmap

[Zhang, Qi Z]

> -Original Message-
> From: Xing, Beilei 
> Sent: Friday, January 6, 2023 5:16 PM
> To: Wu, Jingjing 
> Cc: dev@dpdk.org; Zhang, Qi Z ; Xing, Beilei
> 
> Subject: [PATCH v2 07/15] common/idpf: add irq map/unmap
> 
> From: Beilei Xing 
> 
> Add irq map/unmap functions in common module.

Seems patch not just adding new functions, but also move something from 
net/idpf to common/idpf.
Better to add more description, and same to other patch in this patch set.

> 
> Signed-off-by: Jingjing Wu 
> Signed-off-by: Beilei Xing 
> ---
>  drivers/common/idpf/idpf_common_device.c   |  99
> 
>  drivers/common/idpf/idpf_common_device.h   |   6 ++
>  drivers/common/idpf/idpf_common_virtchnl.c |   8 --
>  drivers/common/idpf/idpf_common_virtchnl.h |   5 +-
>  drivers/common/idpf/version.map|   3 +-
>  drivers/net/idpf/idpf_ethdev.c | 102 +++--
>  drivers/net/idpf/idpf_ethdev.h |   1 -
>  7 files changed, 121 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/common/idpf/idpf_common_device.c
> b/drivers/common/idpf/idpf_common_device.c
> index 19d638824d..7c69bdf6a8 100644
> --- a/drivers/common/idpf/idpf_common_device.c
> +++ b/drivers/common/idpf/idpf_common_device.c


...

RE: [PATCH v4] net/iavf: add lock for vf commands

[Zhang, Qi Z]

> -Original Message-
> From: Mike Pattrick 
> Sent: Thursday, December 29, 2022 7:00 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z ; tho...@monjalon.net;
> david.march...@redhat.com; Zhou, YidingX ;
> ktray...@redhat.com; Mike Pattrick ; sta...@dpdk.org
> Subject: [PATCH v4] net/iavf: add lock for vf commands
> 
> iavf admin queue commands aren't thread-safe. Bugs surrounding this issue
> can manifest in a variety of ways but frequently pend_cmd is over written.
> Simultaneously executing commands can result in a misconfigured device or
> DPDK sleeping in a thread for 2 second.
> 
> Despite this limitation, vf commands may be executed from both
> iavf_dev_alarm_handler() in a control thread and the applications main
> thread. This is trivial to simulate in the testpmd application by creating a 
> bond
> of vf's in active backup mode, and then turning the bond off and then on
> again repeatedly.
> 
> Previously [1] was proposed as a potential solution, but this commit did not
> resolve all potential issues concerning the admin queue and has been
> reverted from the stable branch. I propose adding locks until a more
> complete solution is available.
> 
> [1] commit cb5c1b91f76f ("net/iavf: add thread for event callbacks")
> 
> Fixes: 48de41ca11f0 ("net/avf: enable link status update")
> Fixes: 84108425054a ("net/iavf: support asynchronous virtual channel
> message")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Mike Pattrick 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH v3 3/7] net/e1000: fix whitespace

[Wu, Wenjun1]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Tuesday, January 17, 2023 8:15 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Su, Simei
> ; Wu, Wenjun1 ; Burakov,
> Anatoly 
> Subject: [PATCH v3 3/7] net/e1000: fix whitespace
> 
> The style standard is to use blank after keywords.
> I.e "if (" not "if("
> 
> Signed-off-by: Stephen Hemminger 
Acked-by: Wenjun Wu 

RE: [PATCH v3 3/7] net/e1000: fix whitespace

[Su, Simei]

> -Original Message-
> From: Stephen Hemminger 
> Sent: Tuesday, January 17, 2023 8:15 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger ; Su, Simei
> ; Wu, Wenjun1 ; Burakov,
> Anatoly 
> Subject: [PATCH v3 3/7] net/e1000: fix whitespace
> 
> The style standard is to use blank after keywords.
> I.e "if (" not "if("
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  drivers/net/e1000/em_ethdev.c  |  2 +-
>  drivers/net/e1000/igb_ethdev.c | 16 
>  drivers/net/e1000/igb_pf.c |  2 +-
>  drivers/net/e1000/igb_rxtx.c   |  6 +++---
>  4 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 8ee9be12ad19..3cb09538b2df 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -872,7 +872,7 @@ eth_em_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *rte_stats)
>   E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
>   int pause_frames;
> 
> - if(hw->phy.media_type == e1000_media_type_copper ||
> + if (hw->phy.media_type == e1000_media_type_copper ||
>   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
>   stats->symerrs += E1000_READ_REG(hw,E1000_SYMERRS);
>   stats->sec += E1000_READ_REG(hw, E1000_SEC); diff --git
> a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index
> 8858f975f8cc..e89bfa248fb0 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -735,7 +735,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
>   /* for secondary processes, we don't initialise any further as primary
>* has already done this work. Only check we don't need a different
>* RX function */
> - if (rte_eal_process_type() != RTE_PROC_PRIMARY){
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>   if (eth_dev->data->scattered_rx)
>   eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
>   return 0;
> @@ -927,7 +927,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
>   /* for secondary processes, we don't initialise any further as primary
>* has already done this work. Only check we don't need a different
>* RX function */
> - if (rte_eal_process_type() != RTE_PROC_PRIMARY){
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>   if (eth_dev->data->scattered_rx)
>   eth_dev->rx_pkt_burst = ð_igb_recv_scattered_pkts;
>   return 0;
> @@ -1683,7 +1683,7 @@ igb_read_stats_registers(struct e1000_hw *hw,
> struct e1000_hw_stats *stats)
>   uint64_t old_rpthc = stats->rpthc;
>   uint64_t old_hgptc = stats->hgptc;
> 
> - if(hw->phy.media_type == e1000_media_type_copper ||
> + if (hw->phy.media_type == e1000_media_type_copper ||
>   (E1000_READ_REG(hw, E1000_STATUS) & E1000_STATUS_LU)) {
>   stats->symerrs +=
>   E1000_READ_REG(hw,E1000_SYMERRS); @@ -3498,12
> +3498,12 @@ static void igbvf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>   E1000_DEV_PRIVATE_TO_VFTA(dev->data->dev_private);
>   int i = 0, j = 0, vfta = 0, mask = 1;
> 
> - for (i = 0; i < IGB_VFTA_SIZE; i++){
> + for (i = 0; i < IGB_VFTA_SIZE; i++) {
>   vfta = shadow_vfta->vfta[i];
> - if(vfta){
> + if (vfta) {
>   mask = 1;
> - for (j = 0; j < 32; j++){
> - if(vfta & mask)
> + for (j = 0; j < 32; j++) {
> + if (vfta & mask)
>   igbvf_set_vfta(hw,
>   (uint16_t)((i<<5)+j), on);
>   mask<<=1;
> @@ -3528,7 +3528,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev,
> uint16_t vlan_id, int on)
> 
>   /*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
>   ret = igbvf_set_vfta(hw, vlan_id, !!on);
> - if(ret){
> + if (ret) {
>   PMD_INIT_LOG(ERR, "Unable to set VF vlan");
>   return ret;
>   }
> diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c index
> c7588ea57eaa..b1f74eb841d2 100644
> --- a/drivers/net/e1000/igb_pf.c
> +++ b/drivers/net/e1000/igb_pf.c
> @@ -78,7 +78,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
> 
>   if (hw->mac.type == e1000_i350)
>   nb_queue = 1;
> - else if(hw->mac.type == e1000_82576)
> + else if (hw->mac.type == e1000_82576)
>   /* per datasheet, it should be 2, but 1 seems correct */
>   nb_queue = 1;
>   else
> diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c index
> f32dee46df82..2d4a1292053e 100644
> --- a/drivers/net/e1000/igb_rxtx.c
> +++ b/drivers/net/e1000/igb_rxtx.c
> @@ -196,13 +196,13 @@ struct igb_tx_queue {  #ifdef
> RTE_PMD_USE_P

RE: [PATCH v1 1/2] net/i40e: fix unsupported flow rule transfer attribute

[Zhang, Qi Z]

> -Original Message-
> From: Yang, SteveX 
> Sent: Monday, December 26, 2022 8:37 AM
> To: dev@dpdk.org
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Yang, Qiming ; Zhang, Qi
> Z ; Yang, SteveX 
> Subject: [PATCH v1 1/2] net/i40e: fix unsupported flow rule transfer attribute
> 
> i40e doesn't support transfer attribute of flow rule, ignore it when 
> validating
> rule attributes.
> 
> Fixes: 86eb05d6350b ("net/i40e: add flow validate function")

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH v1 1/2] net/i40e: fix unsupported flow rule transfer attribute

[Zhang, Qi Z]

> -Original Message-
> From: Zhang, Qi Z
> Sent: Tuesday, January 17, 2023 9:27 AM
> To: 'Yang, SteveX' ; dev@dpdk.org
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Yang, Qiming 
> Subject: RE: [PATCH v1 1/2] net/i40e: fix unsupported flow rule transfer
> attribute
> 
> 
> 
> > -Original Message-
> > From: Yang, SteveX 
> > Sent: Monday, December 26, 2022 8:37 AM
> > To: dev@dpdk.org
> > Cc: Zhang, Yuying ; Xing, Beilei
> > ; Yang, Qiming ; Zhang,
> > Qi Z ; Yang, SteveX 
> > Subject: [PATCH v1 1/2] net/i40e: fix unsupported flow rule transfer
> > attribute
> >
> > i40e doesn't support transfer attribute of flow rule, ignore it when
> > validating rule attributes.
> >
> > Fixes: 86eb05d6350b ("net/i40e: add flow validate function")

Also added Cc stable.
> 
> Acked-by: Qi Zhang 
> 
> Applied to dpdk-next-net-intel.
> 
> Thanks
> Qi

RE: [PATCH v1 2/2] net/ice: fix unsupported flow rule transfer attribute

[Zhang, Qi Z]

> -Original Message-
> From: Yang, SteveX 
> Sent: Monday, December 26, 2022 8:37 AM
> To: dev@dpdk.org
> Cc: Zhang, Yuying ; Xing, Beilei
> ; Yang, Qiming ; Zhang, Qi
> Z ; Yang, SteveX 
> Subject: [PATCH v1 2/2] net/ice: fix unsupported flow rule transfer attribute
> 
> ice doesn't support transfer attribute of flow rule, ignore it when validating
> rule attributes.
> 
> Fixes: d76116a4678f ("net/ice: add generic flow API")
> 
> Signed-off-by: Steve Yang 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH] net/af_xdp: parse numa node id from sysfs

[Du, Frank]

Hi ferruh,

Our application use rte_eth_dev_socket_id to query the socket that a NIC port 
connected, then allocate lcore/memory according to this affinity.

The remote memory access is really slow compared to local.

Thanks,
Frank

-Original Message-
From: Ferruh Yigit,  
Sent: Monday, January 16, 2023 9:15 PM
To: Du, Frank ; Loftus, Ciara 
Cc: dev@dpdk.org
Subject: Re: [PATCH] net/af_xdp: parse numa node id from sysfs

On 12/12/2022 12:48 AM, Frank Du wrote:
> Get from /sys/class/net/{if}/device/numa_node.
> 
> Signed-off-by: Frank Du 
> ---
>  drivers/net/af_xdp/rte_eth_af_xdp.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index b6ec9bf490..38b9d36ab5 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -39,6 +39,7 @@
>  #include 
>  
>  #include "compat.h"
> +#include "eal_filesystem.h"
>  
>  #ifndef SO_PREFER_BUSY_POLL
>  #define SO_PREFER_BUSY_POLL 69
> @@ -2038,9 +2039,6 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
>   return -EINVAL;
>   }
>  
> - if (dev->device.numa_node == SOCKET_ID_ANY)
> - dev->device.numa_node = rte_socket_id();
> -
>   if (parse_parameters(kvlist, if_name, &xsk_start_queue_idx,
>&xsk_queue_cnt, &shared_umem, prog_path,
>&busy_budget, &force_copy) < 0) { @@ -2053,6 
> +2051,19 @@ 
> rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
>   return -EINVAL;
>   }
>  
> + /* get numa node id from net sysfs */
> + if (dev->device.numa_node == SOCKET_ID_ANY) {
> + unsigned long numa = 0;
> + char numa_path[PATH_MAX];
> +
> + snprintf(numa_path, sizeof(numa_path), 
> "/sys/class/net/%s/device/numa_node",
> +  if_name);
> + if (eal_parse_sysfs_value(numa_path, &numa) != 0)
> + dev->device.numa_node = rte_socket_id();
> + else
> + dev->device.numa_node = numa;
> + }
> +
>   busy_budget = busy_budget == -1 ? ETH_AF_XDP_DFLT_BUSY_BUDGET :
>   busy_budget;
>  

Hi Frank,

It looks reasonable to set virtual DPDK af_xdp device socket to actual 
underlying device socket. And as I checked quickly, it works as expected.

But what is the impact and motivation of the patch? In other words why you are 
doing this patch and what output you are expecting as a result?
Did you able to do any performance testing, and are you observing any 
difference before and after this test?


Thanks,
ferruh

RE: [PATCH] Intel iavf: Return in the case of ADD/DEL ETH address

[Zhang, Qi Z]

> -Original Message-
> From: Vipin P R 
> Sent: Friday, January 13, 2023 9:19 PM
> To: Wu, Jingjing ; Xing, Beilei 
> Cc: dev@dpdk.org; Vipin P R ; sta...@dpdk.org
> Subject: [PATCH] Intel iavf: Return in the case of ADD/DEL ETH address
> 
> In case of i40vf, VIRTCHNL_OP_DEL_ETH_ADDR and
> VIRTCHNL_OP_ADD_ETH_ADDR are unsupported.
> i40evf_execute_vf_cmd is invoked with these operations as part of
> i40evf_set_mc_addr_list()
> 
> The cases are not handled in i40evf_execute_vf_cmd() thus hitting the
> default case.
> There is a retry logic of upto 200 times (2000 in iavf) with a delay of 10ms 
> (1ms
> in iavf).
> This results in a needless delay of 2s in the init phase for each VNIC.
> 

Sorry I didn't get this, why this is related with i40evf? I40evf PMD has been 
replaced by iavf PMD.
The iavf PMD works with both i40e and ice, does this will break ice's usage?

> The patch aims to rectify that delay.
> In fe2a571c70cc397f2ad4e280f8d084148fea5d62, i40e_ethdev_vf.c was
> deleted. Hence adding this in iavf_vchnl.c.
> 
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Vipin P R 
> ---
>  drivers/net/iavf/iavf_vchnl.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c 
> index
> f92daf9..e2f65f5 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -367,6 +367,14 @@ iavf_execute_vf_cmd(struct iavf_adapter *adapter,
> struct iavf_cmd_info *args,
>   }
>   _clear_cmd(vf);
>   break;
> +
> +case VIRTCHNL_OP_ADD_ETH_ADDR:
> +case VIRTCHNL_OP_DEL_ETH_ADDR:
> +PMD_DRV_LOG(WARNING, "OP_{ADD/DEL}_ETH_ADDR
> unsupported");
> +err = 0;
> +_clear_cmd(vf);
> +break;
> +
>   default:
>   /* For other virtchnl ops in running time,
>* wait for the cmd done flag.
> --
> 2.7.4

RE: [PATCH] net/iavf: protect insertion in flow list

[Zhang, Qi Z]

> -Original Message-
> From: David Marchand 
> Sent: Thursday, January 5, 2023 9:58 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Wu, Jingjing ; Xing, Beilei
> ; Zhang, Qi Z ; Yang, Qiming
> 
> Subject: [PATCH] net/iavf: protect insertion in flow list
> 
> Add missing lock acquire.
> 
> Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: David Marchand 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH] net/iavf: fix building data desc

[Zhang, Qi Z]

> -Original Message-
> From: Zhichao Zeng 
> Sent: Thursday, January 12, 2023 5:32 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Zhou, YidingX ; Zeng,
> ZhichaoX ; Wu, Jingjing ;
> Xing, Beilei ; Doherty, Declan
> ; Sinha, Abhijit ;
> Nicolau, Radu 
> Subject: [PATCH] net/iavf: fix building data desc
> 
> Build correct data desc for UFO pkt by adding UDP_SEG flag, and disable
> L4 checksum offload when TSO/UFO is enabled to prevent the MDD.
> 
> Fixes: 1e728b01120c ("net/iavf: rework Tx path")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Zhichao Zeng 

Acked-by: Qi Zhang 

Applied to dpdk-next-net-intel.

Thanks
Qi

RE: [PATCH 1/3] net/igc: code refactoring

[Zhang, Qi Z]

> -Original Message-
> From: Su, Simei 
> Sent: Tuesday, December 20, 2022 11:41 AM
> To: Zhang, Qi Z ; Guo, Junfeng
> 
> Cc: dev@dpdk.org; Wu, Wenjun1 ; Su, Simei
> 
> Subject: [PATCH 1/3] net/igc: code refactoring
> 
> Move related structures for Rx/Tx queue from igc_txrx.c to igc_txrx.h to
> make code cleaner and variables used more conveniently.

Not sure if this is necessary.
If a structure only be used internally, keep it internally should be OK.
Otherwise need to give a reason why to expose it.

> 
> Signed-off-by: Simei Su 
> ---
>  drivers/net/igc/igc_txrx.c | 118 
> -
>  drivers/net/igc/igc_txrx.h | 115
> +++
>  2 files changed, 115 insertions(+), 118 deletions(-)
> 
> diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c index
> ffd219b..c462e91 100644
> --- a/drivers/net/igc/igc_txrx.c
> +++ b/drivers/net/igc/igc_txrx.c
> @@ -93,124 +93,6 @@
> 
>  #define IGC_TX_OFFLOAD_NOTSUP_MASK
> (RTE_MBUF_F_TX_OFFLOAD_MASK ^ IGC_TX_OFFLOAD_MASK)
> 
> -/**
> - * Structure associated with each descriptor of the RX ring of a RX queue.
> - */
> -struct igc_rx_entry {
> - struct rte_mbuf *mbuf; /**< mbuf associated with RX descriptor. */
> -};
> -
> -/**
> - * Structure associated with each RX queue.
> - */
> -struct igc_rx_queue {
> - struct rte_mempool  *mb_pool;   /**< mbuf pool to populate RX ring.
> */
> - volatile union igc_adv_rx_desc *rx_ring;
> - /**< RX ring virtual address. */
> - uint64_trx_ring_phys_addr; /**< RX ring DMA address. */
> - volatile uint32_t   *rdt_reg_addr; /**< RDT register address. */
> - volatile uint32_t   *rdh_reg_addr; /**< RDH register address. */
> - struct igc_rx_entry *sw_ring;   /**< address of RX software ring. */
> - struct rte_mbuf *pkt_first_seg; /**< First segment of current packet.
> */
> - struct rte_mbuf *pkt_last_seg;  /**< Last segment of current packet.
> */
> - uint16_tnb_rx_desc; /**< number of RX descriptors. */
> - uint16_trx_tail;/**< current value of RDT register. */
> - uint16_tnb_rx_hold; /**< number of held free RX desc. */
> - uint16_trx_free_thresh; /**< max free RX desc to hold. */
> - uint16_tqueue_id;   /**< RX queue index. */
> - uint16_treg_idx;/**< RX queue register index. */
> - uint16_tport_id;/**< Device port identifier. */
> - uint8_t pthresh;/**< Prefetch threshold register. */
> - uint8_t hthresh;/**< Host threshold register. */
> - uint8_t wthresh;/**< Write-back threshold register. */
> - uint8_t crc_len;/**< 0 if CRC stripped, 4 otherwise. */
> - uint8_t drop_en;/**< If not 0, set SRRCTL.Drop_En. */
> - uint32_tflags;  /**< RX flags. */
> - uint64_toffloads;   /**< offloads of
> RTE_ETH_RX_OFFLOAD_* */
> -};
> -
> -/** Offload features */
> -union igc_tx_offload {
> - uint64_t data;
> - struct {
> - uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> - uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> - uint64_t vlan_tci:16;
> - /**< VLAN Tag Control Identifier(CPU order). */
> - uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> - uint64_t tso_segsz:16; /**< TCP TSO segment size. */
> - /* uint64_t unused:8; */
> - };
> -};
> -
> -/*
> - * Compare mask for igc_tx_offload.data,
> - * should be in sync with igc_tx_offload layout.
> - */
> -#define TX_MACIP_LEN_CMP_MASK0xULL /**< L2L3
> header mask. */
> -#define TX_VLAN_CMP_MASK 0xULL /**< Vlan
> mask. */
> -#define TX_TCP_LEN_CMP_MASK  0x00FFULL /**< TCP
> header mask. */
> -#define TX_TSO_MSS_CMP_MASK  0x0000ULL /**< TSO
> segsz mask. */
> -/** Mac + IP + TCP + Mss mask. */
> -#define TX_TSO_CMP_MASK  \
> - (TX_MACIP_LEN_CMP_MASK | TX_TCP_LEN_CMP_MASK |
> TX_TSO_MSS_CMP_MASK)
> -
> -/**
> - * Structure to check if new context need be built
> - */
> -struct igc_advctx_info {
> - uint64_t flags;   /**< ol_flags related to context build. */
> - /** tx offload: vlan, tso, l2-l3-l4 lengths. */
> - union igc_tx_offload tx_offload;
> - /** compare mask for tx offload. */
> - union igc_tx_offload tx_offload_mask;
> -};
> -
> -/**
> - * Hardware context number
> - */
> -enum {
> - IGC_CTX_0= 0, /**< CTX0*/
> - IGC_CTX_1= 1, /**< CTX1*/
> - IGC_CTX_NUM  = 2, /**< CTX_NUM */
> -};
> -
> -/**
> - * Structure associated with each descriptor of the TX ring of a TX queue.
> - */
> -struct igc_tx_entry {
> - struct rte_mbuf *mbuf; /**< mbuf associated with TX desc, if any. */
> - uint16_t next_id; /**< Index of next descriptor in ring. */
> - 

[PATCH v2] app/dma-perf: introduce dma-perf application

[Cheng Jiang]

There are many high-performance DMA devices supported in DPDK now, and
these DMA devices can also be integrated into other modules of DPDK as
accelerators, such as Vhost. Before integrating DMA into applications,
developers need to know the performance of these DMA devices in various
scenarios and the performance of CPUs in the same scenario, such as
different buffer lengths. Only in this way can we know the target
performance of the application accelerated by using them. This patch
introduces a high-performance testing tool, which supports comparing the
performance of CPU and DMA in different scenarios automatically with a
pre-set config file. Memory Copy performance test are supported for now.

Signed-off-by: Cheng Jiang 
Signed-off-by: Jiayu Hu 
Signed-off-by: Yuan Wang 
Acked-by: Morten Brørup 
---
v2: fixed some CI issues.

 app/meson.build   |   1 +
 app/test-dma-perf/benchmark.c | 539 ++
 app/test-dma-perf/benchmark.h |  12 +
 app/test-dma-perf/config.ini  |  61 
 app/test-dma-perf/main.c  | 434 +++
 app/test-dma-perf/main.h  |  53 
 app/test-dma-perf/meson.build |  22 ++
 7 files changed, 1122 insertions(+)
 create mode 100644 app/test-dma-perf/benchmark.c
 create mode 100644 app/test-dma-perf/benchmark.h
 create mode 100644 app/test-dma-perf/config.ini
 create mode 100644 app/test-dma-perf/main.c
 create mode 100644 app/test-dma-perf/main.h
 create mode 100644 app/test-dma-perf/meson.build

diff --git a/app/meson.build b/app/meson.build
index e32ea4bd5c..a060ad2725 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -28,6 +28,7 @@ apps = [
 'test-regex',
 'test-sad',
 'test-security-perf',
+'test-dma-perf',
 ]

 default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
diff --git a/app/test-dma-perf/benchmark.c b/app/test-dma-perf/benchmark.c
new file mode 100644
index 00..1cb5b0b291
--- /dev/null
+++ b/app/test-dma-perf/benchmark.c
@@ -0,0 +1,539 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "main.h"
+#include "benchmark.h"
+
+
+#define MAX_DMA_CPL_NB 255
+
+#define CSV_LINE_DMA_FMT "Scenario %u,%u,%u,%u,%u,%u,%" PRIu64 ",%.3lf,%" 
PRIu64 "\n"
+#define CSV_LINE_CPU_FMT "Scenario %u,%u,NA,%u,%u,%u,%" PRIu64 ",%.3lf,%" 
PRIu64 "\n"
+
+struct lcore_params {
+   uint16_t dev_id;
+   uint32_t nr_buf;
+   uint16_t kick_batch;
+   uint32_t buf_size;
+   uint32_t repeat_times;
+   uint16_t mpool_iter_step;
+   struct rte_mbuf **srcs;
+   struct rte_mbuf **dsts;
+   uint8_t scenario_id;
+};
+
+struct buf_info {
+   struct rte_mbuf **array;
+   uint32_t nr_buf;
+   uint32_t buf_size;
+};
+
+static struct rte_mempool *src_pool;
+static struct rte_mempool *dst_pool;
+
+uint16_t dmadev_ids[MAX_WORKER_NB];
+uint32_t nb_dmadevs;
+
+#define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__)
+
+static inline int
+__rte_format_printf(3, 4)
+print_err(const char *func, int lineno, const char *format, ...)
+{
+   va_list ap;
+   int ret;
+
+   ret = fprintf(stderr, "In %s:%d - ", func, lineno);
+   va_start(ap, format);
+   ret += vfprintf(stderr, format, ap);
+   va_end(ap);
+
+   return ret;
+}
+
+static inline void
+calc_result(struct lcore_params *p, uint64_t cp_cycle_sum, double time_sec,
+   uint32_t repeat_times, uint32_t *memory, uint64_t 
*ave_cycle,
+   float *bandwidth, uint64_t *ops)
+{
+   *memory = (p->buf_size * p->nr_buf * 2) / (1024 * 1024);
+   *ave_cycle = cp_cycle_sum / (p->repeat_times * p->nr_buf);
+   *bandwidth = p->buf_size * 8 * rte_get_timer_hz() / (*ave_cycle * 1000 
* 1000 * 1000.0);
+   *ops = (double)p->nr_buf * repeat_times / time_sec;
+}
+
+static void
+output_result(uint8_t scenario_id, uint32_t lcore_id, uint16_t dev_id, 
uint64_t ave_cycle,
+   uint32_t buf_size, uint32_t nr_buf, uint32_t memory,
+   float bandwidth, uint64_t ops, bool is_dma)
+{
+   if (is_dma)
+   printf("lcore %u, DMA %u:\n"
+   "average cycles: %" PRIu64 ","
+   " buffer size: %u, nr_buf: %u,"
+   " memory: %uMB, frequency: %" PRIu64 ".\n",
+   lcore_id,
+   dev_id,
+   ave_cycle,
+   buf_size,
+   nr_buf,
+   memory,
+   rte_get_timer_hz());
+   else
+   printf("lcore %u\n"
+   "average cycles: %" PRIu64 ","
+   " buffer size: %u, nr_buf: %u,"
+   " memory: %uMB, frequency: %" PRIu64 ".\n",
+   lcor

RE: [RFC 1/9] ethdev: add IPv6 routing extension header definition

[Rongwei Liu]

HI Ori:
Those three matching items are redundant. Has been removed already.
Do I need to send RFC v2?

BR
Rongwei

> -Original Message-
> From: Ori Kam 
> Sent: Tuesday, January 17, 2023 00:15
> To: Rongwei Liu ; Matan Azrad ;
> Slava Ovsiienko ; NBU-Contact-Thomas Monjalon
> (EXTERNAL) ; Ferruh Yigit ;
> Andrew Rybchenko ; Olivier Matz
> 
> Cc: dev@dpdk.org; Raslan Darawsheh 
> Subject: RE: [RFC 1/9] ethdev: add IPv6 routing extension header definition
> 
> Hi Rongwei,
> 
> 
> > -Original Message-
> > From: Rongwei Liu 
> > Sent: Wednesday, 21 December 2022 10:43
> >
> > Add IPv6 routing extension header definition and no TLV support for
> > now.
> > At rte_flow layer, there are new items defined for matching
> > type/nexthdr/segment_left field.
> >
> > Signed-off-by: Rongwei Liu 
> > ---
> >  doc/guides/prog_guide/rte_flow.rst |  9 ++
> >  doc/guides/rel_notes/release_22_03.rst |  5 
> >  lib/ethdev/rte_flow.c  | 15 ++
> >  lib/ethdev/rte_flow.h  | 39 ++
> >  lib/net/rte_ip.h   | 21 ++
> >  5 files changed, 89 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 3e6242803d..1ebc159893 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1544,6 +1544,15 @@ Matches Color Marker set by a Meter.
> >
> >  - ``color``: Metering color marker.
> >
> > +Item: ``IPV6_ROUTING_EXT``
> > +^
> > +
> > +Matches ipv6 routing extension header.
> > +
> > +- ``nexthdr``: Next layer header type.
> > +- ``type``: IPv6 routing extension header type.
> > +- ``segments_left``: How many IPv6 destination addresses carries on
> > +
> >  Actions
> >  ~~~
> >
> > diff --git a/doc/guides/rel_notes/release_22_03.rst
> > b/doc/guides/rel_notes/release_22_03.rst
> > index 0923707cb8..cc050ff3e7 100644
> > --- a/doc/guides/rel_notes/release_22_03.rst
> > +++ b/doc/guides/rel_notes/release_22_03.rst
> > @@ -207,6 +207,11 @@ API Changes
> >  * ethdev: Old public macros and enumeration constants without
> > ``RTE_ETH_`` prefix,
> >which are kept for backward compatibility, are marked as deprecated.
> >
> > +* ethdev: added a new structure:
> > +
> > +- IPv6 routing extension header ``rte_flow_item_ipv6_routing_ext`` and
> > +  ``rte_ipv6_routing_ext``
> > +
> >  * cryptodev: The asymmetric session handling was modified to use a single
> >mempool object. An API ``rte_cryptodev_asym_session_pool_create``
> > was added
> >to create a mempool with element size big enough to hold the
> > generic asymmetric diff --git a/lib/ethdev/rte_flow.c
> > b/lib/ethdev/rte_flow.c index 7d0c24366c..e08b690300 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -76,6 +76,19 @@ rte_flow_item_flex_conv(void *buf, const void *data)
> > return src->length;
> >  }
> >
> > +static size_t
> > +rte_flow_item_ipv6_routing_ext_conv(void *buf, const void *data) {
> > +   struct rte_flow_item_ipv6_routing_ext *dst = buf;
> > +   const struct rte_flow_item_ipv6_routing_ext *src = data;
> > +
> > +   if (buf)
> > +   rte_memcpy((void *)((uintptr_t)(dst->hdr.segments)),
> > +  src->hdr.segments,
> > +  src->hdr.segments_left << 4);
> > +   return src->hdr.segments_left << 4;
> > +}
> > +
> >  /** Generate flow_item[] entry. */
> >  #define MK_FLOW_ITEM(t, s) \
> > [RTE_FLOW_ITEM_TYPE_ ## t] = { \
> > @@ -157,6 +170,8 @@ static const struct rte_flow_desc_data
> > rte_flow_desc_item[] = {
> > MK_FLOW_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> > MK_FLOW_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> > MK_FLOW_ITEM(METER_COLOR, sizeof(struct
> rte_flow_item_meter_color)),
> > +   MK_FLOW_ITEM_FN(IPV6_ROUTING_EXT, sizeof(struct
> > rte_flow_item_ipv6_routing_ext),
> > +   rte_flow_item_ipv6_routing_ext_conv),
> >  };
> >
> >  /** Generate flow_action[] entry. */
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> > b60987db4b..f8f1d6f9dd 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -624,6 +624,33 @@ enum rte_flow_item_type {
> >  * See struct rte_flow_item_meter_color.
> >  */
> > RTE_FLOW_ITEM_TYPE_METER_COLOR,
> > +   /**
> > +* Matches the presence of IPv6 routing extension header.
> > +*
> > +* See struct rte_flow_item_ipv6_routing_ext.
> > +*/
> > +   RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT,
> > +
> > +   /**
> > +* Matches IPv6 routing extension header next header.
> > +*
> > +* See struct rte_flow_item_ipv6_routing_ext.
> > +*/
> > +   RTE_FLOW_ITEM_TYPE_IPV6_ROUTING_EXT_NEXT_HDR,
> 
> Why do we need this item? Isn't it part of the rte_ipv6_routing_ext?
Will remove in the patch.
> 
> > +
> > +   /**
> > +* Matches IPv6 routing extension header type.
> > +*
>

RE: [PATCH 1/3] net/igc: code refactoring

[Su, Simei]

Hi Qi,

> -Original Message-
> From: Zhang, Qi Z 
> Sent: Tuesday, January 17, 2023 10:25 AM
> To: Su, Simei ; Guo, Junfeng 
> Cc: dev@dpdk.org; Wu, Wenjun1 
> Subject: RE: [PATCH 1/3] net/igc: code refactoring
> 
> 
> 
> > -Original Message-
> > From: Su, Simei 
> > Sent: Tuesday, December 20, 2022 11:41 AM
> > To: Zhang, Qi Z ; Guo, Junfeng
> > 
> > Cc: dev@dpdk.org; Wu, Wenjun1 ; Su, Simei
> > 
> > Subject: [PATCH 1/3] net/igc: code refactoring
> >
> > Move related structures for Rx/Tx queue from igc_txrx.c to igc_txrx.h
> > to make code cleaner and variables used more conveniently.
> 
> Not sure if this is necessary.
> If a structure only be used internally, keep it internally should be OK.
> Otherwise need to give a reason why to expose it.

OK. I will rework commit log to give detailed reason in v2.

Thanks,
Simei

> 
> >
> > Signed-off-by: Simei Su 
> > ---
> >  drivers/net/igc/igc_txrx.c | 118
> > -
> >  drivers/net/igc/igc_txrx.h | 115
> > +++
> >  2 files changed, 115 insertions(+), 118 deletions(-)
> >
> > diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
> > index
> > ffd219b..c462e91 100644
> > --- a/drivers/net/igc/igc_txrx.c
> > +++ b/drivers/net/igc/igc_txrx.c
> > @@ -93,124 +93,6 @@
> >
> >  #define IGC_TX_OFFLOAD_NOTSUP_MASK
> > (RTE_MBUF_F_TX_OFFLOAD_MASK ^ IGC_TX_OFFLOAD_MASK)
> >
> > -/**
> > - * Structure associated with each descriptor of the RX ring of a RX queue.
> > - */
> > -struct igc_rx_entry {
> > -   struct rte_mbuf *mbuf; /**< mbuf associated with RX descriptor. */
> > -};
> > -
> > -/**
> > - * Structure associated with each RX queue.
> > - */
> > -struct igc_rx_queue {
> > -   struct rte_mempool  *mb_pool;   /**< mbuf pool to populate RX ring.
> > */
> > -   volatile union igc_adv_rx_desc *rx_ring;
> > -   /**< RX ring virtual address. */
> > -   uint64_trx_ring_phys_addr; /**< RX ring DMA address. */
> > -   volatile uint32_t   *rdt_reg_addr; /**< RDT register address. */
> > -   volatile uint32_t   *rdh_reg_addr; /**< RDH register address. */
> > -   struct igc_rx_entry *sw_ring;   /**< address of RX software ring. */
> > -   struct rte_mbuf *pkt_first_seg; /**< First segment of current packet.
> > */
> > -   struct rte_mbuf *pkt_last_seg;  /**< Last segment of current packet.
> > */
> > -   uint16_tnb_rx_desc; /**< number of RX descriptors. */
> > -   uint16_trx_tail;/**< current value of RDT register. */
> > -   uint16_tnb_rx_hold; /**< number of held free RX desc. */
> > -   uint16_trx_free_thresh; /**< max free RX desc to hold. */
> > -   uint16_tqueue_id;   /**< RX queue index. */
> > -   uint16_treg_idx;/**< RX queue register index. */
> > -   uint16_tport_id;/**< Device port identifier. */
> > -   uint8_t pthresh;/**< Prefetch threshold register. */
> > -   uint8_t hthresh;/**< Host threshold register. */
> > -   uint8_t wthresh;/**< Write-back threshold register. */
> > -   uint8_t crc_len;/**< 0 if CRC stripped, 4 otherwise. */
> > -   uint8_t drop_en;/**< If not 0, set SRRCTL.Drop_En. */
> > -   uint32_tflags;  /**< RX flags. */
> > -   uint64_toffloads;   /**< offloads of
> > RTE_ETH_RX_OFFLOAD_* */
> > -};
> > -
> > -/** Offload features */
> > -union igc_tx_offload {
> > -   uint64_t data;
> > -   struct {
> > -   uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> > -   uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> > -   uint64_t vlan_tci:16;
> > -   /**< VLAN Tag Control Identifier(CPU order). */
> > -   uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> > -   uint64_t tso_segsz:16; /**< TCP TSO segment size. */
> > -   /* uint64_t unused:8; */
> > -   };
> > -};
> > -
> > -/*
> > - * Compare mask for igc_tx_offload.data,
> > - * should be in sync with igc_tx_offload layout.
> > - */
> > -#define TX_MACIP_LEN_CMP_MASK  0xULL /**< L2L3
> > header mask. */
> > -#define TX_VLAN_CMP_MASK   0xULL /**< Vlan
> > mask. */
> > -#define TX_TCP_LEN_CMP_MASK0x00FFULL /**< TCP
> > header mask. */
> > -#define TX_TSO_MSS_CMP_MASK0x0000ULL /**< TSO
> > segsz mask. */
> > -/** Mac + IP + TCP + Mss mask. */
> > -#define TX_TSO_CMP_MASK\
> > -   (TX_MACIP_LEN_CMP_MASK | TX_TCP_LEN_CMP_MASK |
> > TX_TSO_MSS_CMP_MASK)
> > -
> > -/**
> > - * Structure to check if new context need be built
> > - */
> > -struct igc_advctx_info {
> > -   uint64_t flags;   /**< ol_flags related to context build. */
> > -   /** tx offload: vlan, tso, l2-l3-l4 lengths. */
> > -   union igc_tx_offload tx_offload;
> > -   /** compare mask for tx offload. */
> > -   union igc_tx_offload tx_offload_mask;
> > -};
> > -
> 

Re: [PATCH v4 2/4] eal: remove thread getname API

[Jerin Jacob]

On Sat, Jan 14, 2023 at 12:22 AM Tyler Retzlaff
 wrote:
>
> Remove the rte_thread_getname API.  The API is __rte_experimental and
> requires no deprecation notice.
>
> Fold the platform specific variants into the one place it is used as a
> special case to retain the functionality for linux only.
>

>  void
>  __rte_trace_mem_per_thread_alloc(void)
>  {
> @@ -356,7 +369,7 @@ rte_trace_mode rte_trace_mode_get(void)
> /* Store the thread name */
> char *name = header->stream_header.thread_name;
> memset(name, 0, __RTE_TRACE_EMIT_STRING_LEN_MAX);
> -   rte_thread_getname(pthread_self(), name,
> +   rte_thread_get_name(rte_thread_self(), name,

Since it's a local function. Please change thread_get_name or so?