* [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks
[not found] <20191001125315.6191-1-ktraynor@redhat.com>
@ 2019-10-01 12:53 ` Kevin Traynor
2019-10-04 10:57 ` Ferriter, Cian
2019-10-30 7:52 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
2 siblings, 2 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 12:53 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, cian.ferriter, stable
Previously rx/tx_queues were passed into eth_from_pcaps_common()
as ptrs and were checked for being NULL.
In commit da6ba28f0540 ("net/pcap: use a struct to pass user options")
that changed to pass in a ptr to a pmd_devargs_all which contains
the rx/tx_queues.
The parameter checking was not updated as part of that commit and
coverity caught that there was still a check if rx/tx_queues were
NULL, apparently after they had been dereferenced.
Fix that by checking the pmd_devargs_all ptr and removing the NULL
checks on rx/tx_queues.
1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
deref_ptr: Directly dereferencing pointer tx_queues.
1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
1235 unsigned int i;
1236
1237 /* do some parameter checking */
CID 345004: Dereference before null check (REVERSE_INULL)
[select issue]
1238 if (rx_queues == NULL && nb_rx_queues > 0)
1239 return -1;
CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking tx_queues suggests that it may be
null, but it has already been dereferenced on all paths leading to
the check.
1240 if (tx_queues == NULL && nb_tx_queues > 0)
1241 return -1;
Coverity issue: 345029
Coverity issue: 345044
Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
Cc: cian.ferriter@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/pcap/rte_eth_pcap.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 5489010b6..7cf00306e 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1229,16 +1229,19 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
{
struct pmd_process_private *pp;
- struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
- struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
- const unsigned int nb_rx_queues = rx_queues->num_of_queue;
- const unsigned int nb_tx_queues = tx_queues->num_of_queue;
+ struct pmd_devargs *rx_queues;
+ struct pmd_devargs *tx_queues;
+ unsigned int nb_rx_queues;
+ unsigned int nb_tx_queues;
unsigned int i;
- /* do some parameter checking */
- if (rx_queues == NULL && nb_rx_queues > 0)
- return -1;
- if (tx_queues == NULL && nb_tx_queues > 0)
+ if (devargs_all == NULL)
return -1;
+ rx_queues = &devargs_all->rx_queues;
+ tx_queues = &devargs_all->tx_queues;
+
+ nb_rx_queues = rx_queues->num_of_queue;
+ nb_tx_queues = tx_queues->num_of_queue;
+
if (pmd_init_internals(vdev, nb_rx_queues, nb_tx_queues, internals,
eth_dev) < 0)
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference
[not found] <20191001125315.6191-1-ktraynor@redhat.com>
2019-10-01 12:53 ` [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks Kevin Traynor
@ 2019-10-01 13:03 ` Kevin Traynor
2019-10-01 13:03 ` [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code Kevin Traynor
` (7 more replies)
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
2 siblings, 8 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:03 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, ndabilpuram, stable
Coverity complains that ctrl_flags is set to NULL at the start
of the function and it may not have been set before there is a
jump to fc_success and it is dereferenced.
Check for NULL before dereference.
312fc_success:
CID 344983 (#1 of 1): Explicit null dereferenced
(FORWARD_NULL)7. var_deref_op: Dereferencing null pointer ctrl_flags.
313 *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
Coverity issue: 344983
Fixes: 6cc54096520d ("crypto/octeontx: add supported sessions")
Cc: ndabilpuram@marvell.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
There may be further rework needed to set it to the correct value,
but for now at least prevent the NULL dereference.
---
drivers/common/cpt/cpt_ucode.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/common/cpt/cpt_ucode.h b/drivers/common/cpt/cpt_ucode.h
index 7d9c31e17..fad978c6e 100644
--- a/drivers/common/cpt/cpt_ucode.h
+++ b/drivers/common/cpt/cpt_ucode.h
@@ -311,5 +311,6 @@ cpt_fc_ciph_set_key(void *ctx, cipher_type_t type, const uint8_t *key,
fc_success:
- *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
+ if (ctrl_flags != NULL)
+ *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
success:
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
@ 2019-10-01 13:03 ` Kevin Traynor
2019-10-02 1:28 ` [dpdk-stable] [dpdk-dev] " Ajit Khaparde
2019-10-01 13:04 ` [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
` (6 subsequent siblings)
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:03 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, lance.richardson, stable
If rc contains a non-zero return code from bnxt_hwrm_send_message(),
HWRM_CHECK_RESULT_SILENT() will return.
Just after that code, there is an 'if (!rc) {...} else {...}'.
Coverity is complaining that this if else statement is dead code as
rc will always be 0 if that code is reached.
4309 rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
BNXT_USE_CHIMP_MB);
cond_const: Condition rc, taking false branch.
Now the value of rc is equal to 0.
4310 HWRM_CHECK_RESULT_SILENT();
4311
const: At condition rc, the value of rc must be equal to 0.
dead_error_condition: The condition !rc must be true.
4312 if (!rc) {
[snip]
4373 } else {
CID 343450 (#1 of 1): Logically dead code
(DEADCODE)dead_error_line: Execution cannot
reach this statement: rc = 0;.
4374 rc = 0;
4375 }
Coverity issue: 343450
Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
Cc: lance.richardson@broadcom.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/bnxt/bnxt_hwrm.c | 118 +++++++++++++++++------------------
1 file changed, 56 insertions(+), 62 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 9883fb506..5378e3e9c 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -4298,5 +4298,7 @@ int bnxt_hwrm_func_backing_store_qcaps(struct bnxt *bp)
struct hwrm_func_backing_store_qcaps_output *resp =
bp->hwrm_cmd_resp_addr;
- int rc;
+ struct bnxt_ctx_pg_info *ctx_pg;
+ struct bnxt_ctx_mem_info *ctx;
+ int rc, total_alloc_len, i;
if (!BNXT_CHIP_THOR(bp) ||
@@ -4310,68 +4312,60 @@ int bnxt_hwrm_func_backing_store_qcaps(struct bnxt *bp)
HWRM_CHECK_RESULT_SILENT();
- if (!rc) {
- struct bnxt_ctx_pg_info *ctx_pg;
- struct bnxt_ctx_mem_info *ctx;
- int total_alloc_len;
- int i;
+ total_alloc_len = sizeof(*ctx);
+ ctx = rte_malloc("bnxt_ctx_mem", total_alloc_len,
+ RTE_CACHE_LINE_SIZE);
+ if (!ctx) {
+ rc = -ENOMEM;
+ goto ctx_err;
+ }
+ memset(ctx, 0, total_alloc_len);
- total_alloc_len = sizeof(*ctx);
- ctx = rte_malloc("bnxt_ctx_mem", total_alloc_len,
- RTE_CACHE_LINE_SIZE);
- if (!ctx) {
- rc = -ENOMEM;
- goto ctx_err;
- }
- memset(ctx, 0, total_alloc_len);
+ ctx_pg = rte_malloc("bnxt_ctx_pg_mem",
+ sizeof(*ctx_pg) * BNXT_MAX_Q,
+ RTE_CACHE_LINE_SIZE);
+ if (!ctx_pg) {
+ rc = -ENOMEM;
+ goto ctx_err;
+ }
+ for (i = 0; i < BNXT_MAX_Q; i++, ctx_pg++)
+ ctx->tqm_mem[i] = ctx_pg;
- ctx_pg = rte_malloc("bnxt_ctx_pg_mem",
- sizeof(*ctx_pg) * BNXT_MAX_Q,
- RTE_CACHE_LINE_SIZE);
- if (!ctx_pg) {
- rc = -ENOMEM;
- goto ctx_err;
- }
- for (i = 0; i < BNXT_MAX_Q; i++, ctx_pg++)
- ctx->tqm_mem[i] = ctx_pg;
+ bp->ctx = ctx;
+ ctx->qp_max_entries = rte_le_to_cpu_32(resp->qp_max_entries);
+ ctx->qp_min_qp1_entries =
+ rte_le_to_cpu_16(resp->qp_min_qp1_entries);
+ ctx->qp_max_l2_entries =
+ rte_le_to_cpu_16(resp->qp_max_l2_entries);
+ ctx->qp_entry_size = rte_le_to_cpu_16(resp->qp_entry_size);
+ ctx->srq_max_l2_entries =
+ rte_le_to_cpu_16(resp->srq_max_l2_entries);
+ ctx->srq_max_entries = rte_le_to_cpu_32(resp->srq_max_entries);
+ ctx->srq_entry_size = rte_le_to_cpu_16(resp->srq_entry_size);
+ ctx->cq_max_l2_entries =
+ rte_le_to_cpu_16(resp->cq_max_l2_entries);
+ ctx->cq_max_entries = rte_le_to_cpu_32(resp->cq_max_entries);
+ ctx->cq_entry_size = rte_le_to_cpu_16(resp->cq_entry_size);
+ ctx->vnic_max_vnic_entries =
+ rte_le_to_cpu_16(resp->vnic_max_vnic_entries);
+ ctx->vnic_max_ring_table_entries =
+ rte_le_to_cpu_16(resp->vnic_max_ring_table_entries);
+ ctx->vnic_entry_size = rte_le_to_cpu_16(resp->vnic_entry_size);
+ ctx->stat_max_entries =
+ rte_le_to_cpu_32(resp->stat_max_entries);
+ ctx->stat_entry_size = rte_le_to_cpu_16(resp->stat_entry_size);
+ ctx->tqm_entry_size = rte_le_to_cpu_16(resp->tqm_entry_size);
+ ctx->tqm_min_entries_per_ring =
+ rte_le_to_cpu_32(resp->tqm_min_entries_per_ring);
+ ctx->tqm_max_entries_per_ring =
+ rte_le_to_cpu_32(resp->tqm_max_entries_per_ring);
+ ctx->tqm_entries_multiple = resp->tqm_entries_multiple;
+ if (!ctx->tqm_entries_multiple)
+ ctx->tqm_entries_multiple = 1;
+ ctx->mrav_max_entries =
+ rte_le_to_cpu_32(resp->mrav_max_entries);
+ ctx->mrav_entry_size = rte_le_to_cpu_16(resp->mrav_entry_size);
+ ctx->tim_entry_size = rte_le_to_cpu_16(resp->tim_entry_size);
+ ctx->tim_max_entries = rte_le_to_cpu_32(resp->tim_max_entries);
- bp->ctx = ctx;
- ctx->qp_max_entries = rte_le_to_cpu_32(resp->qp_max_entries);
- ctx->qp_min_qp1_entries =
- rte_le_to_cpu_16(resp->qp_min_qp1_entries);
- ctx->qp_max_l2_entries =
- rte_le_to_cpu_16(resp->qp_max_l2_entries);
- ctx->qp_entry_size = rte_le_to_cpu_16(resp->qp_entry_size);
- ctx->srq_max_l2_entries =
- rte_le_to_cpu_16(resp->srq_max_l2_entries);
- ctx->srq_max_entries = rte_le_to_cpu_32(resp->srq_max_entries);
- ctx->srq_entry_size = rte_le_to_cpu_16(resp->srq_entry_size);
- ctx->cq_max_l2_entries =
- rte_le_to_cpu_16(resp->cq_max_l2_entries);
- ctx->cq_max_entries = rte_le_to_cpu_32(resp->cq_max_entries);
- ctx->cq_entry_size = rte_le_to_cpu_16(resp->cq_entry_size);
- ctx->vnic_max_vnic_entries =
- rte_le_to_cpu_16(resp->vnic_max_vnic_entries);
- ctx->vnic_max_ring_table_entries =
- rte_le_to_cpu_16(resp->vnic_max_ring_table_entries);
- ctx->vnic_entry_size = rte_le_to_cpu_16(resp->vnic_entry_size);
- ctx->stat_max_entries =
- rte_le_to_cpu_32(resp->stat_max_entries);
- ctx->stat_entry_size = rte_le_to_cpu_16(resp->stat_entry_size);
- ctx->tqm_entry_size = rte_le_to_cpu_16(resp->tqm_entry_size);
- ctx->tqm_min_entries_per_ring =
- rte_le_to_cpu_32(resp->tqm_min_entries_per_ring);
- ctx->tqm_max_entries_per_ring =
- rte_le_to_cpu_32(resp->tqm_max_entries_per_ring);
- ctx->tqm_entries_multiple = resp->tqm_entries_multiple;
- if (!ctx->tqm_entries_multiple)
- ctx->tqm_entries_multiple = 1;
- ctx->mrav_max_entries =
- rte_le_to_cpu_32(resp->mrav_max_entries);
- ctx->mrav_entry_size = rte_le_to_cpu_16(resp->mrav_entry_size);
- ctx->tim_entry_size = rte_le_to_cpu_16(resp->tim_entry_size);
- ctx->tim_max_entries = rte_le_to_cpu_32(resp->tim_max_entries);
- } else {
- rc = 0;
- }
ctx_err:
HWRM_UNLOCK();
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
2019-10-01 13:03 ` [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-30 7:59 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-11-08 14:52 ` [dpdk-stable] " Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement Kevin Traynor
` (5 subsequent siblings)
7 siblings, 2 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, rosen.xu, stable
Coverity is complaining about identical code regardless of which branch
of the if else is taken. Functionally it means an error will always be
returned if this if else is hit. Remove the else branch.
CID 337928 (#1 of 1): Identical code for different branches
(IDENTICAL_BRANCHES)identical_branches: The same code is executed
regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
n->n_children != 0U is true, because the 'then' and 'else' branches
are identical. Should one of the branches be modified, or the entire
'if' statement replaced?
1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
1507 n->n_children != 0) {
1508 return -rte_tm_error_set(error,
1509 EINVAL,
1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
1511 NULL,
1512 rte_strerror(EINVAL));
else_branch: The else branch, identical to the then branch.
1513 } else {
1514 return -rte_tm_error_set(error,
1515 EINVAL,
1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
1517 NULL,
1518 rte_strerror(EINVAL));
1519 }
Coverity issue: 337928
Fixes: c820468ac99c ("net/ipn3ke: support TM")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
index adf02c157..a93145d59 100644
--- a/drivers/net/ipn3ke/ipn3ke_tm.c
+++ b/drivers/net/ipn3ke/ipn3ke_tm.c
@@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct rte_eth_dev *dev,
NULL,
rte_strerror(EINVAL));
- } else {
- return -rte_tm_error_set(error,
- EINVAL,
- RTE_TM_ERROR_TYPE_UNSPECIFIED,
- NULL,
- rte_strerror(EINVAL));
}
}
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
2019-10-01 13:03 ` [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code Kevin Traynor
2019-10-01 13:04 ` [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-30 8:01 ` David Marchand
2019-11-08 14:52 ` Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code Kevin Traynor
` (4 subsequent siblings)
7 siblings, 2 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, rosen.xu, stable
Coverity complains that this statement is not needed as the goto
label is on the next line anyway. Remove the if statement.
653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
CID 337930 (#1 of 1): Identical code for different branches
(IDENTICAL_BRANCHES)identical_branches: The same code is executed
when the condition ret is true or false, because the code in the
if-then branch and after the if statement is identical. Should
the if statement be removed?
654 if (ret)
655 goto end;
implicit_else: The code from the above if-then branch is identical
to the code after the if statement.
656end:
Coverity issue: 337930
Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c
index c226d6313..282295f49 100644
--- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
@@ -652,6 +652,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
- if (ret)
- goto end;
+
end:
if (kvlist)
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
` (2 preceding siblings ...)
2019-10-01 13:04 ` [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-30 8:04 ` David Marchand
2019-11-08 14:51 ` Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 7/9] compress/octeontx: " Kevin Traynor
` (3 subsequent siblings)
7 siblings, 2 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, rosen.xu, stable
These struct members and variable were commented out. Remove them.
Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
Fixes: c820468ac99c ("net/ipn3ke: support TM")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h b/drivers/net/ipn3ke/ipn3ke_ethdev.h
index c7b336bbd..0d71dcd64 100644
--- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
@@ -97,18 +97,11 @@ struct ipn3ke_tm_node {
struct ipn3ke_tm_hierarchy {
struct ipn3ke_tm_node *port_node;
- /*struct ipn3ke_tm_node_list vt_node_list;*/
- /*struct ipn3ke_tm_node_list cos_node_list;*/
-
uint32_t n_shaper_profiles;
- /*uint32_t n_shared_shapers;*/
uint32_t n_tdrop_profiles;
uint32_t n_vt_nodes;
uint32_t n_cos_nodes;
-
struct ipn3ke_tm_node *port_commit_node;
struct ipn3ke_tm_node_list vt_commit_node_list;
struct ipn3ke_tm_node_list cos_commit_node_list;
-
- /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
};
diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
index a93145d59..5a16c5f96 100644
--- a/drivers/net/ipn3ke/ipn3ke_tm.c
+++ b/drivers/net/ipn3ke/ipn3ke_tm.c
@@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t tm_id,
struct rte_tm_error *error)
{
- /*struct ipn3ke_tm_internals *tm = IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
uint32_t node_index;
uint32_t parent_index;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 7/9] compress/octeontx: remove commented out code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
` (3 preceding siblings ...)
2019-10-01 13:04 ` [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-30 8:06 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-10-01 13:04 ` [dpdk-stable] [PATCH 8/9] event/opdl: " Kevin Traynor
` (2 subsequent siblings)
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, ssahu, stable
This code is commented out. Remove it.
Fixes: 43e610bb8565 ("compress/octeontx: introduce octeontx zip PMD")
Cc: ssahu@marvell.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/compress/octeontx/include/zip_regs.h | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/compress/octeontx/include/zip_regs.h b/drivers/compress/octeontx/include/zip_regs.h
index 04c3d75e9..96e538bb7 100644
--- a/drivers/compress/octeontx/include/zip_regs.h
+++ b/drivers/compress/octeontx/include/zip_regs.h
@@ -37,5 +37,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_vqx_ena_s cn; */
} zip_vqx_ena_t;
@@ -65,5 +64,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_vqx_sbuf_addr_s cn; */
} zip_vqx_sbuf_addr_t;
@@ -85,5 +83,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_quex_doorbell_s cn; */
} zip_quex_doorbell_t;
@@ -105,5 +102,4 @@ union zip_nptr_s {
#endif /* Word 0 - End */
} s;
- /* struct zip_nptr_s_s cn83xx; */
};
@@ -198,5 +194,4 @@ union zip_inst_s {
/** Beginning of file */
uint64_t bf : 1;
- // uint64_t reserved_3_4 : 2;
/** Comp/decomp operation */
uint64_t op : 2;
@@ -211,5 +206,4 @@ union zip_inst_s {
uint64_t dg : 1;
uint64_t ds : 1;
- //uint64_t reserved_3_4 : 2;
uint64_t op : 2;
uint64_t bf : 1;
@@ -616,6 +610,4 @@ union zip_zres_s {
#endif /* Word 7 - End */
} /** ZIP Result Structure */s;
-
- /* struct zip_zres_s_s cn83xx; */
};
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 8/9] event/opdl: remove commented out code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
` (4 preceding siblings ...)
2019-10-01 13:04 ` [dpdk-stable] [PATCH 7/9] compress/octeontx: " Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-03 10:50 ` Liang, Ma
2019-10-01 13:04 ` [dpdk-stable] [PATCH 9/9] net/bnxt: " Kevin Traynor
2019-10-30 7:56 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference David Marchand
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, liang.j.ma, stable
Some variables are commented out. Remove them.
Fixes: d548ef513cd7 ("event/opdl: add unit tests")
Cc: liang.j.ma@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/event/opdl/opdl_test.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/event/opdl/opdl_test.c b/drivers/event/opdl/opdl_test.c
index 5868ec1be..e7a32fbd3 100644
--- a/drivers/event/opdl/opdl_test.c
+++ b/drivers/event/opdl/opdl_test.c
@@ -696,7 +696,4 @@ static int
single_link(struct test *t)
{
- /* const uint8_t rx_port = 0; */
- /* const uint8_t w1_port = 1; */
- /* const uint8_t w3_port = 3; */
const uint8_t tx_port = 2;
int err;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [PATCH 9/9] net/bnxt: remove commented out code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
` (5 preceding siblings ...)
2019-10-01 13:04 ` [dpdk-stable] [PATCH 8/9] event/opdl: " Kevin Traynor
@ 2019-10-01 13:04 ` Kevin Traynor
2019-10-01 15:42 ` Ajit Khaparde
2019-10-30 7:56 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference David Marchand
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-10-01 13:04 UTC (permalink / raw)
To: dev; +Cc: Kevin Traynor, ajit.khaparde, stable
This commented out todo and code is old. Remove it.
Fixes: b7435d660a8c ("net/bnxt: add ntuple filtering support")
Cc: ajit.khaparde@broadcom.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/bnxt/bnxt_ethdev.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6685ee7d9..318d49857 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2239,7 +2239,4 @@ parse_ntuple_filter(struct bnxt *bp,
}
- //TODO Priority
- //nfilter->priority = (uint8_t)filter->priority;
-
bfilter->enables = en;
return 0;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 9/9] net/bnxt: remove commented out code
2019-10-01 13:04 ` [dpdk-stable] [PATCH 9/9] net/bnxt: " Kevin Traynor
@ 2019-10-01 15:42 ` Ajit Khaparde
0 siblings, 0 replies; 45+ messages in thread
From: Ajit Khaparde @ 2019-10-01 15:42 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, dpdk stable
On Tue, Oct 1, 2019 at 6:04 AM Kevin Traynor <ktraynor@redhat.com> wrote:
> This commented out todo and code is old. Remove it.
>
> Fixes: b7435d660a8c ("net/bnxt: add ntuple filtering support")
> Cc: ajit.khaparde@broadcom.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
> drivers/net/bnxt/bnxt_ethdev.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> index 6685ee7d9..318d49857 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -2239,7 +2239,4 @@ parse_ntuple_filter(struct bnxt *bp,
> }
>
> - //TODO Priority
> - //nfilter->priority = (uint8_t)filter->priority;
> -
> bfilter->enables = en;
> return 0;
> --
> 2.21.0
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 3/9] net/bnxt: remove logically dead code
2019-10-01 13:03 ` [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code Kevin Traynor
@ 2019-10-02 1:28 ` Ajit Khaparde
2019-10-30 7:43 ` David Marchand
0 siblings, 1 reply; 45+ messages in thread
From: Ajit Khaparde @ 2019-10-02 1:28 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, Lance Richardson, dpdk stable
On Tue, Oct 1, 2019 at 6:04 AM Kevin Traynor <ktraynor@redhat.com> wrote:
> If rc contains a non-zero return code from bnxt_hwrm_send_message(),
> HWRM_CHECK_RESULT_SILENT() will return.
>
> Just after that code, there is an 'if (!rc) {...} else {...}'.
> Coverity is complaining that this if else statement is dead code as
> rc will always be 0 if that code is reached.
>
> 4309 rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
> BNXT_USE_CHIMP_MB);
> cond_const: Condition rc, taking false branch.
> Now the value of rc is equal to 0.
> 4310 HWRM_CHECK_RESULT_SILENT();
> 4311
> const: At condition rc, the value of rc must be equal to 0.
> dead_error_condition: The condition !rc must be true.
> 4312 if (!rc) {
>
> [snip]
>
> 4373 } else {
> CID 343450 (#1 of 1): Logically dead code
> (DEADCODE)dead_error_line: Execution cannot
> reach this statement: rc = 0;.
> 4374 rc = 0;
> 4375 }
>
> Coverity issue: 343450
> Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
> Cc: lance.richardson@broadcom.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> drivers/net/bnxt/bnxt_hwrm.c | 118 +++++++++++++++++------------------
> 1 file changed, 56 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 9883fb506..5378e3e9c 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -4298,5 +4298,7 @@ int bnxt_hwrm_func_backing_store_qcaps(struct bnxt
> *bp)
> struct hwrm_func_backing_store_qcaps_output *resp =
> bp->hwrm_cmd_resp_addr;
> - int rc;
> + struct bnxt_ctx_pg_info *ctx_pg;
> + struct bnxt_ctx_mem_info *ctx;
> + int rc, total_alloc_len, i;
>
> if (!BNXT_CHIP_THOR(bp) ||
> @@ -4310,68 +4312,60 @@ int bnxt_hwrm_func_backing_store_qcaps(struct bnxt
> *bp)
> HWRM_CHECK_RESULT_SILENT();
>
> - if (!rc) {
> - struct bnxt_ctx_pg_info *ctx_pg;
> - struct bnxt_ctx_mem_info *ctx;
> - int total_alloc_len;
> - int i;
> + total_alloc_len = sizeof(*ctx);
> + ctx = rte_malloc("bnxt_ctx_mem", total_alloc_len,
> + RTE_CACHE_LINE_SIZE);
> + if (!ctx) {
> + rc = -ENOMEM;
> + goto ctx_err;
> + }
> + memset(ctx, 0, total_alloc_len);
>
> - total_alloc_len = sizeof(*ctx);
> - ctx = rte_malloc("bnxt_ctx_mem", total_alloc_len,
> - RTE_CACHE_LINE_SIZE);
> - if (!ctx) {
> - rc = -ENOMEM;
> - goto ctx_err;
> - }
> - memset(ctx, 0, total_alloc_len);
> + ctx_pg = rte_malloc("bnxt_ctx_pg_mem",
> + sizeof(*ctx_pg) * BNXT_MAX_Q,
> + RTE_CACHE_LINE_SIZE);
> + if (!ctx_pg) {
> + rc = -ENOMEM;
> + goto ctx_err;
> + }
> + for (i = 0; i < BNXT_MAX_Q; i++, ctx_pg++)
> + ctx->tqm_mem[i] = ctx_pg;
>
> - ctx_pg = rte_malloc("bnxt_ctx_pg_mem",
> - sizeof(*ctx_pg) * BNXT_MAX_Q,
> - RTE_CACHE_LINE_SIZE);
> - if (!ctx_pg) {
> - rc = -ENOMEM;
> - goto ctx_err;
> - }
> - for (i = 0; i < BNXT_MAX_Q; i++, ctx_pg++)
> - ctx->tqm_mem[i] = ctx_pg;
> + bp->ctx = ctx;
> + ctx->qp_max_entries = rte_le_to_cpu_32(resp->qp_max_entries);
> + ctx->qp_min_qp1_entries =
> + rte_le_to_cpu_16(resp->qp_min_qp1_entries);
> + ctx->qp_max_l2_entries =
> + rte_le_to_cpu_16(resp->qp_max_l2_entries);
> + ctx->qp_entry_size = rte_le_to_cpu_16(resp->qp_entry_size);
> + ctx->srq_max_l2_entries =
> + rte_le_to_cpu_16(resp->srq_max_l2_entries);
> + ctx->srq_max_entries = rte_le_to_cpu_32(resp->srq_max_entries);
> + ctx->srq_entry_size = rte_le_to_cpu_16(resp->srq_entry_size);
> + ctx->cq_max_l2_entries =
> + rte_le_to_cpu_16(resp->cq_max_l2_entries);
> + ctx->cq_max_entries = rte_le_to_cpu_32(resp->cq_max_entries);
> + ctx->cq_entry_size = rte_le_to_cpu_16(resp->cq_entry_size);
> + ctx->vnic_max_vnic_entries =
> + rte_le_to_cpu_16(resp->vnic_max_vnic_entries);
> + ctx->vnic_max_ring_table_entries =
> + rte_le_to_cpu_16(resp->vnic_max_ring_table_entries);
> + ctx->vnic_entry_size = rte_le_to_cpu_16(resp->vnic_entry_size);
> + ctx->stat_max_entries =
> + rte_le_to_cpu_32(resp->stat_max_entries);
> + ctx->stat_entry_size = rte_le_to_cpu_16(resp->stat_entry_size);
> + ctx->tqm_entry_size = rte_le_to_cpu_16(resp->tqm_entry_size);
> + ctx->tqm_min_entries_per_ring =
> + rte_le_to_cpu_32(resp->tqm_min_entries_per_ring);
> + ctx->tqm_max_entries_per_ring =
> + rte_le_to_cpu_32(resp->tqm_max_entries_per_ring);
> + ctx->tqm_entries_multiple = resp->tqm_entries_multiple;
> + if (!ctx->tqm_entries_multiple)
> + ctx->tqm_entries_multiple = 1;
> + ctx->mrav_max_entries =
> + rte_le_to_cpu_32(resp->mrav_max_entries);
> + ctx->mrav_entry_size = rte_le_to_cpu_16(resp->mrav_entry_size);
> + ctx->tim_entry_size = rte_le_to_cpu_16(resp->tim_entry_size);
> + ctx->tim_max_entries = rte_le_to_cpu_32(resp->tim_max_entries);
>
> - bp->ctx = ctx;
> - ctx->qp_max_entries =
> rte_le_to_cpu_32(resp->qp_max_entries);
> - ctx->qp_min_qp1_entries =
> - rte_le_to_cpu_16(resp->qp_min_qp1_entries);
> - ctx->qp_max_l2_entries =
> - rte_le_to_cpu_16(resp->qp_max_l2_entries);
> - ctx->qp_entry_size = rte_le_to_cpu_16(resp->qp_entry_size);
> - ctx->srq_max_l2_entries =
> - rte_le_to_cpu_16(resp->srq_max_l2_entries);
> - ctx->srq_max_entries =
> rte_le_to_cpu_32(resp->srq_max_entries);
> - ctx->srq_entry_size =
> rte_le_to_cpu_16(resp->srq_entry_size);
> - ctx->cq_max_l2_entries =
> - rte_le_to_cpu_16(resp->cq_max_l2_entries);
> - ctx->cq_max_entries =
> rte_le_to_cpu_32(resp->cq_max_entries);
> - ctx->cq_entry_size = rte_le_to_cpu_16(resp->cq_entry_size);
> - ctx->vnic_max_vnic_entries =
> - rte_le_to_cpu_16(resp->vnic_max_vnic_entries);
> - ctx->vnic_max_ring_table_entries =
> -
> rte_le_to_cpu_16(resp->vnic_max_ring_table_entries);
> - ctx->vnic_entry_size =
> rte_le_to_cpu_16(resp->vnic_entry_size);
> - ctx->stat_max_entries =
> - rte_le_to_cpu_32(resp->stat_max_entries);
> - ctx->stat_entry_size =
> rte_le_to_cpu_16(resp->stat_entry_size);
> - ctx->tqm_entry_size =
> rte_le_to_cpu_16(resp->tqm_entry_size);
> - ctx->tqm_min_entries_per_ring =
> - rte_le_to_cpu_32(resp->tqm_min_entries_per_ring);
> - ctx->tqm_max_entries_per_ring =
> - rte_le_to_cpu_32(resp->tqm_max_entries_per_ring);
> - ctx->tqm_entries_multiple = resp->tqm_entries_multiple;
> - if (!ctx->tqm_entries_multiple)
> - ctx->tqm_entries_multiple = 1;
> - ctx->mrav_max_entries =
> - rte_le_to_cpu_32(resp->mrav_max_entries);
> - ctx->mrav_entry_size =
> rte_le_to_cpu_16(resp->mrav_entry_size);
> - ctx->tim_entry_size =
> rte_le_to_cpu_16(resp->tim_entry_size);
> - ctx->tim_max_entries =
> rte_le_to_cpu_32(resp->tim_max_entries);
> - } else {
> - rc = 0;
> - }
> ctx_err:
> HWRM_UNLOCK();
> --
> 2.21.0
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 8/9] event/opdl: remove commented out code
2019-10-01 13:04 ` [dpdk-stable] [PATCH 8/9] event/opdl: " Kevin Traynor
@ 2019-10-03 10:50 ` Liang, Ma
0 siblings, 0 replies; 45+ messages in thread
From: Liang, Ma @ 2019-10-03 10:50 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, stable
On 01 Oct 14:04, Kevin Traynor wrote:
> Some variables are commented out. Remove them.
>
> Fixes: d548ef513cd7 ("event/opdl: add unit tests")
> Cc: liang.j.ma@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/event/opdl/opdl_test.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/event/opdl/opdl_test.c b/drivers/event/opdl/opdl_test.c
> index 5868ec1be..e7a32fbd3 100644
> --- a/drivers/event/opdl/opdl_test.c
> +++ b/drivers/event/opdl/opdl_test.c
> @@ -696,7 +696,4 @@ static int
> single_link(struct test *t)
> {
> - /* const uint8_t rx_port = 0; */
> - /* const uint8_t w1_port = 1; */
> - /* const uint8_t w3_port = 3; */
> const uint8_t tx_port = 2;
> int err;
> --
> 2.21.0
>
Acked-by: Liang Ma <liang.j.ma@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks
2019-10-01 12:53 ` [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks Kevin Traynor
@ 2019-10-04 10:57 ` Ferriter, Cian
2019-10-30 7:52 ` [dpdk-stable] [dpdk-dev] " David Marchand
1 sibling, 0 replies; 45+ messages in thread
From: Ferriter, Cian @ 2019-10-04 10:57 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: stable
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday 1 October 2019 13:53
> To: dev@dpdk.org
> Cc: Kevin Traynor <ktraynor@redhat.com>; Ferriter, Cian
> <cian.ferriter@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/9] net/pcap: fix argument checks
>
> Previously rx/tx_queues were passed into eth_from_pcaps_common() as
> ptrs and were checked for being NULL.
>
> In commit da6ba28f0540 ("net/pcap: use a struct to pass user options") that
> changed to pass in a ptr to a pmd_devargs_all which contains the
> rx/tx_queues.
>
> The parameter checking was not updated as part of that commit and coverity
> caught that there was still a check if rx/tx_queues were NULL, apparently
> after they had been dereferenced.
>
> Fix that by checking the pmd_devargs_all ptr and removing the NULL checks
> on rx/tx_queues.
>
> 1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
> 1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
> 1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
> deref_ptr: Directly dereferencing pointer tx_queues.
> 1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
> 1235 unsigned int i;
> 1236
> 1237 /* do some parameter checking */
> CID 345004: Dereference before null check (REVERSE_INULL)
> [select issue]
> 1238 if (rx_queues == NULL && nb_rx_queues > 0)
> 1239 return -1;
> CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
> check_after_deref: Null-checking tx_queues suggests that it may be
> null, but it has already been dereferenced on all paths leading to
> the check.
> 1240 if (tx_queues == NULL && nb_tx_queues > 0)
> 1241 return -1;
>
> Coverity issue: 345029
> Coverity issue: 345044
> Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
> Cc: cian.ferriter@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
> ---
> drivers/net/pcap/rte_eth_pcap.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c
> b/drivers/net/pcap/rte_eth_pcap.c index 5489010b6..7cf00306e 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -1229,16 +1229,19 @@ eth_from_pcaps_common(struct
> rte_vdev_device *vdev, {
> struct pmd_process_private *pp;
> - struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
> - struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
> - const unsigned int nb_rx_queues = rx_queues->num_of_queue;
> - const unsigned int nb_tx_queues = tx_queues->num_of_queue;
> + struct pmd_devargs *rx_queues;
> + struct pmd_devargs *tx_queues;
> + unsigned int nb_rx_queues;
> + unsigned int nb_tx_queues;
> unsigned int i;
>
> - /* do some parameter checking */
> - if (rx_queues == NULL && nb_rx_queues > 0)
> - return -1;
> - if (tx_queues == NULL && nb_tx_queues > 0)
> + if (devargs_all == NULL)
> return -1;
>
> + rx_queues = &devargs_all->rx_queues;
> + tx_queues = &devargs_all->tx_queues;
> +
> + nb_rx_queues = rx_queues->num_of_queue;
> + nb_tx_queues = tx_queues->num_of_queue;
> +
> if (pmd_init_internals(vdev, nb_rx_queues, nb_tx_queues,
> internals,
> eth_dev) < 0)
> --
> 2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 3/9] net/bnxt: remove logically dead code
2019-10-02 1:28 ` [dpdk-stable] [dpdk-dev] " Ajit Khaparde
@ 2019-10-30 7:43 ` David Marchand
2019-10-30 16:27 ` Ajit Khaparde
0 siblings, 1 reply; 45+ messages in thread
From: David Marchand @ 2019-10-30 7:43 UTC (permalink / raw)
To: Ajit Khaparde, Kevin Traynor; +Cc: dev, Lance Richardson, dpdk stable
Hello Ajit, Kevin,
On Wed, Oct 2, 2019 at 3:29 AM Ajit Khaparde <ajit.khaparde@broadcom.com> wrote:
>
> On Tue, Oct 1, 2019 at 6:04 AM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> > If rc contains a non-zero return code from bnxt_hwrm_send_message(),
> > HWRM_CHECK_RESULT_SILENT() will return.
> >
> > Just after that code, there is an 'if (!rc) {...} else {...}'.
> > Coverity is complaining that this if else statement is dead code as
> > rc will always be 0 if that code is reached.
> >
> > 4309 rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
> > BNXT_USE_CHIMP_MB);
> > cond_const: Condition rc, taking false branch.
> > Now the value of rc is equal to 0.
> > 4310 HWRM_CHECK_RESULT_SILENT();
> > 4311
> > const: At condition rc, the value of rc must be equal to 0.
> > dead_error_condition: The condition !rc must be true.
> > 4312 if (!rc) {
> >
> > [snip]
> >
> > 4373 } else {
> > CID 343450 (#1 of 1): Logically dead code
> > (DEADCODE)dead_error_line: Execution cannot
> > reach this statement: rc = 0;.
> > 4374 rc = 0;
> > 4375 }
> >
> > Coverity issue: 343450
> > Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
> > Cc: lance.richardson@broadcom.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> >
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
I can see a *really* close patch has been submitted the day after.
http://patchwork.dpdk.org/patch/58352/
And merged after a v3:
https://git.dpdk.org/dpdk/commit/?id=b4f74051165560aa81814433dea7e6eb0bdb32b9
So I suppose this patch can be dropped.
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
2019-10-01 12:53 ` [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks Kevin Traynor
2019-10-04 10:57 ` Ferriter, Cian
@ 2019-10-30 7:52 ` David Marchand
2019-11-05 16:40 ` Kevin Traynor
1 sibling, 1 reply; 45+ messages in thread
From: David Marchand @ 2019-10-30 7:52 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, cian.ferriter, dpdk stable
On Tue, Oct 1, 2019 at 2:54 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> Previously rx/tx_queues were passed into eth_from_pcaps_common()
> as ptrs and were checked for being NULL.
>
> In commit da6ba28f0540 ("net/pcap: use a struct to pass user options")
> that changed to pass in a ptr to a pmd_devargs_all which contains
> the rx/tx_queues.
>
> The parameter checking was not updated as part of that commit and
> coverity caught that there was still a check if rx/tx_queues were
> NULL, apparently after they had been dereferenced.
>
> Fix that by checking the pmd_devargs_all ptr and removing the NULL
> checks on rx/tx_queues.
>
> 1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
> 1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
> 1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
> deref_ptr: Directly dereferencing pointer tx_queues.
> 1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
> 1235 unsigned int i;
> 1236
> 1237 /* do some parameter checking */
> CID 345004: Dereference before null check (REVERSE_INULL)
> [select issue]
> 1238 if (rx_queues == NULL && nb_rx_queues > 0)
> 1239 return -1;
> CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
> check_after_deref: Null-checking tx_queues suggests that it may be
> null, but it has already been dereferenced on all paths leading to
> the check.
> 1240 if (tx_queues == NULL && nb_tx_queues > 0)
> 1241 return -1;
>
> Coverity issue: 345029
> Coverity issue: 345044
> Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
> Cc: cian.ferriter@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
This patch hides the coverity warning.
But can't we just remove those checks?
Iiuc, the checks on NULL pointers are unnecessary since
https://git.dpdk.org/dpdk/commit/?id=a0fce1193ce13a50c00624cb36605ebef7a3e60b.
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
` (6 preceding siblings ...)
2019-10-01 13:04 ` [dpdk-stable] [PATCH 9/9] net/bnxt: " Kevin Traynor
@ 2019-10-30 7:56 ` David Marchand
7 siblings, 0 replies; 45+ messages in thread
From: David Marchand @ 2019-10-30 7:56 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, Nithin Dabilpuram, dpdk stable, anoobj
On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> Coverity complains that ctrl_flags is set to NULL at the start
> of the function and it may not have been set before there is a
> jump to fc_success and it is dereferenced.
>
> Check for NULL before dereference.
>
> 312fc_success:
> CID 344983 (#1 of 1): Explicit null dereferenced
> (FORWARD_NULL)7. var_deref_op: Dereferencing null pointer ctrl_flags.
> 313 *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
>
> Coverity issue: 344983
> Fixes: 6cc54096520d ("crypto/octeontx: add supported sessions")
> Cc: ndabilpuram@marvell.com
> Cc: stable@dpdk.org
Cc: maintainer
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>
> ---
>
> There may be further rework needed to set it to the correct value,
> but for now at least prevent the NULL dereference.
> ---
> drivers/common/cpt/cpt_ucode.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/common/cpt/cpt_ucode.h b/drivers/common/cpt/cpt_ucode.h
> index 7d9c31e17..fad978c6e 100644
> --- a/drivers/common/cpt/cpt_ucode.h
> +++ b/drivers/common/cpt/cpt_ucode.h
> @@ -311,5 +311,6 @@ cpt_fc_ciph_set_key(void *ctx, cipher_type_t type, const uint8_t *key,
>
> fc_success:
> - *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
> + if (ctrl_flags != NULL)
> + *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
>
> success:
> --
> 2.21.0
>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-10-01 13:04 ` [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
@ 2019-10-30 7:59 ` David Marchand
2019-11-05 15:41 ` Kevin Traynor
2019-11-08 14:35 ` Xu, Rosen
2019-11-08 14:52 ` [dpdk-stable] " Xu, Rosen
1 sibling, 2 replies; 45+ messages in thread
From: David Marchand @ 2019-10-30 7:59 UTC (permalink / raw)
To: Rosen Xu; +Cc: dev, Kevin Traynor, dpdk stable, Xiaolong Ye
Hello Rosen,
Review please.
On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> Coverity is complaining about identical code regardless of which branch
> of the if else is taken. Functionally it means an error will always be
> returned if this if else is hit. Remove the else branch.
>
> CID 337928 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> n->n_children != 0U is true, because the 'then' and 'else' branches
> are identical. Should one of the branches be modified, or the entire
> 'if' statement replaced?
> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> 1507 n->n_children != 0) {
> 1508 return -rte_tm_error_set(error,
> 1509 EINVAL,
> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1511 NULL,
> 1512 rte_strerror(EINVAL));
> else_branch: The else branch, identical to the then branch.
> 1513 } else {
> 1514 return -rte_tm_error_set(error,
> 1515 EINVAL,
> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1517 NULL,
> 1518 rte_strerror(EINVAL));
> 1519 }
>
> Coverity issue: 337928
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
> index adf02c157..a93145d59 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct rte_eth_dev *dev,
> NULL,
> rte_strerror(EINVAL));
> - } else {
> - return -rte_tm_error_set(error,
> - EINVAL,
> - RTE_TM_ERROR_TYPE_UNSPECIFIED,
> - NULL,
> - rte_strerror(EINVAL));
> }
> }
> --
> 2.21.0
>
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement
2019-10-01 13:04 ` [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement Kevin Traynor
@ 2019-10-30 8:01 ` David Marchand
2019-11-08 14:35 ` Xu, Rosen
2019-11-08 14:52 ` Xu, Rosen
1 sibling, 1 reply; 45+ messages in thread
From: David Marchand @ 2019-10-30 8:01 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, Rosen Xu, dpdk stable
On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> Coverity complains that this statement is not needed as the goto
> label is on the next line anyway. Remove the if statement.
>
> 653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> CID 337930 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> when the condition ret is true or false, because the code in the
> if-then branch and after the if statement is identical. Should
> the if statement be removed?
> 654 if (ret)
> 655 goto end;
> implicit_else: The code from the above if-then branch is identical
> to the code after the if statement.
> 656end:
>
> Coverity issue: 337930
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> index c226d6313..282295f49 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> @@ -652,6 +652,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
>
> ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> - if (ret)
> - goto end;
> +
> end:
> if (kvlist)
> --
> 2.21.0
>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code
2019-10-01 13:04 ` [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code Kevin Traynor
@ 2019-10-30 8:04 ` David Marchand
2019-11-08 14:40 ` Xu, Rosen
2019-11-08 14:51 ` Xu, Rosen
1 sibling, 1 reply; 45+ messages in thread
From: David Marchand @ 2019-10-30 8:04 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, Rosen Xu, dpdk stable
On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> These struct members and variable were commented out. Remove them.
>
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
> drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> index c7b336bbd..0d71dcd64 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> @@ -97,18 +97,11 @@ struct ipn3ke_tm_node {
> struct ipn3ke_tm_hierarchy {
> struct ipn3ke_tm_node *port_node;
> - /*struct ipn3ke_tm_node_list vt_node_list;*/
> - /*struct ipn3ke_tm_node_list cos_node_list;*/
> -
> uint32_t n_shaper_profiles;
> - /*uint32_t n_shared_shapers;*/
> uint32_t n_tdrop_profiles;
> uint32_t n_vt_nodes;
> uint32_t n_cos_nodes;
> -
> struct ipn3ke_tm_node *port_commit_node;
> struct ipn3ke_tm_node_list vt_commit_node_list;
> struct ipn3ke_tm_node_list cos_commit_node_list;
> -
> - /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
> };
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
> index a93145d59..5a16c5f96 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t tm_id,
> struct rte_tm_error *error)
> {
> - /*struct ipn3ke_tm_internals *tm = IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
> uint32_t node_index;
> uint32_t parent_index;
> --
> 2.21.0
>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 7/9] compress/octeontx: remove commented out code
2019-10-01 13:04 ` [dpdk-stable] [PATCH 7/9] compress/octeontx: " Kevin Traynor
@ 2019-10-30 8:06 ` David Marchand
0 siblings, 0 replies; 45+ messages in thread
From: David Marchand @ 2019-10-30 8:06 UTC (permalink / raw)
To: Kevin Traynor; +Cc: dev, ssahu, dpdk stable, Ashish Gupta
On Tue, Oct 1, 2019 at 3:05 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>
> This code is commented out. Remove it.
>
> Fixes: 43e610bb8565 ("compress/octeontx: introduce octeontx zip PMD")
> Cc: ssahu@marvell.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/compress/octeontx/include/zip_regs.h | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/compress/octeontx/include/zip_regs.h b/drivers/compress/octeontx/include/zip_regs.h
> index 04c3d75e9..96e538bb7 100644
> --- a/drivers/compress/octeontx/include/zip_regs.h
> +++ b/drivers/compress/octeontx/include/zip_regs.h
> @@ -37,5 +37,4 @@ typedef union {
> #endif /* Word 0 - End */
> } s;
> - /* struct zip_vqx_ena_s cn; */
> } zip_vqx_ena_t;
>
> @@ -65,5 +64,4 @@ typedef union {
> #endif /* Word 0 - End */
> } s;
> - /* struct zip_vqx_sbuf_addr_s cn; */
> } zip_vqx_sbuf_addr_t;
>
> @@ -85,5 +83,4 @@ typedef union {
> #endif /* Word 0 - End */
> } s;
> - /* struct zip_quex_doorbell_s cn; */
> } zip_quex_doorbell_t;
>
> @@ -105,5 +102,4 @@ union zip_nptr_s {
> #endif /* Word 0 - End */
> } s;
> - /* struct zip_nptr_s_s cn83xx; */
> };
>
> @@ -198,5 +194,4 @@ union zip_inst_s {
> /** Beginning of file */
> uint64_t bf : 1;
> - // uint64_t reserved_3_4 : 2;
> /** Comp/decomp operation */
> uint64_t op : 2;
> @@ -211,5 +206,4 @@ union zip_inst_s {
> uint64_t dg : 1;
> uint64_t ds : 1;
> - //uint64_t reserved_3_4 : 2;
> uint64_t op : 2;
> uint64_t bf : 1;
> @@ -616,6 +610,4 @@ union zip_zres_s {
> #endif /* Word 7 - End */
> } /** ZIP Result Structure */s;
> -
> - /* struct zip_zres_s_s cn83xx; */
> };
>
> --
> 2.21.0
>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 3/9] net/bnxt: remove logically dead code
2019-10-30 7:43 ` David Marchand
@ 2019-10-30 16:27 ` Ajit Khaparde
2019-11-05 15:39 ` Kevin Traynor
0 siblings, 1 reply; 45+ messages in thread
From: Ajit Khaparde @ 2019-10-30 16:27 UTC (permalink / raw)
To: David Marchand; +Cc: Kevin Traynor, dev, Lance Richardson, dpdk stable
On Wed, Oct 30, 2019 at 12:43 AM David Marchand <david.marchand@redhat.com>
wrote:
> Hello Ajit, Kevin,
>
> On Wed, Oct 2, 2019 at 3:29 AM Ajit Khaparde <ajit.khaparde@broadcom.com>
> wrote:
> >
> > On Tue, Oct 1, 2019 at 6:04 AM Kevin Traynor <ktraynor@redhat.com>
> wrote:
> >
> > > If rc contains a non-zero return code from bnxt_hwrm_send_message(),
> > > HWRM_CHECK_RESULT_SILENT() will return.
> > >
> > > Just after that code, there is an 'if (!rc) {...} else {...}'.
> > > Coverity is complaining that this if else statement is dead code as
> > > rc will always be 0 if that code is reached.
> > >
> > > 4309 rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
> > > BNXT_USE_CHIMP_MB);
> > > cond_const: Condition rc, taking false branch.
> > > Now the value of rc is equal to 0.
> > > 4310 HWRM_CHECK_RESULT_SILENT();
> > > 4311
> > > const: At condition rc, the value of rc must be equal to 0.
> > > dead_error_condition: The condition !rc must be true.
> > > 4312 if (!rc) {
> > >
> > > [snip]
> > >
> > > 4373 } else {
> > > CID 343450 (#1 of 1): Logically dead code
> > > (DEADCODE)dead_error_line: Execution cannot
> > > reach this statement: rc = 0;.
> > > 4374 rc = 0;
> > > 4375 }
> > >
> > > Coverity issue: 343450
> > > Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
> > > Cc: lance.richardson@broadcom.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > >
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>
> I can see a *really* close patch has been submitted the day after.
> http://patchwork.dpdk.org/patch/58352/
>
> And merged after a v3:
>
> https://git.dpdk.org/dpdk/commit/?id=b4f74051165560aa81814433dea7e6eb0bdb32b9
>
> So I suppose this patch can be dropped.
>
> Yes. That makes sense.
Thanks for checking.
Thanks
Ajit
>
> --
> David Marchand
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 3/9] net/bnxt: remove logically dead code
2019-10-30 16:27 ` Ajit Khaparde
@ 2019-11-05 15:39 ` Kevin Traynor
0 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-05 15:39 UTC (permalink / raw)
To: Ajit Khaparde, David Marchand; +Cc: dev, Lance Richardson, dpdk stable
On 30/10/2019 16:27, Ajit Khaparde wrote:
> On Wed, Oct 30, 2019 at 12:43 AM David Marchand <david.marchand@redhat.com>
> wrote:
>
>> Hello Ajit, Kevin,
>>
>> On Wed, Oct 2, 2019 at 3:29 AM Ajit Khaparde <ajit.khaparde@broadcom.com>
>> wrote:
>>>
>>> On Tue, Oct 1, 2019 at 6:04 AM Kevin Traynor <ktraynor@redhat.com>
>> wrote:
>>>
>>>> If rc contains a non-zero return code from bnxt_hwrm_send_message(),
>>>> HWRM_CHECK_RESULT_SILENT() will return.
>>>>
>>>> Just after that code, there is an 'if (!rc) {...} else {...}'.
>>>> Coverity is complaining that this if else statement is dead code as
>>>> rc will always be 0 if that code is reached.
>>>>
>>>> 4309 rc = bnxt_hwrm_send_message(bp, &req, sizeof(req),
>>>> BNXT_USE_CHIMP_MB);
>>>> cond_const: Condition rc, taking false branch.
>>>> Now the value of rc is equal to 0.
>>>> 4310 HWRM_CHECK_RESULT_SILENT();
>>>> 4311
>>>> const: At condition rc, the value of rc must be equal to 0.
>>>> dead_error_condition: The condition !rc must be true.
>>>> 4312 if (!rc) {
>>>>
>>>> [snip]
>>>>
>>>> 4373 } else {
>>>> CID 343450 (#1 of 1): Logically dead code
>>>> (DEADCODE)dead_error_line: Execution cannot
>>>> reach this statement: rc = 0;.
>>>> 4374 rc = 0;
>>>> 4375 }
>>>>
>>>> Coverity issue: 343450
>>>> Fixes: f8168ca0e690 ("net/bnxt: support thor controller")
>>>> Cc: lance.richardson@broadcom.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>>>
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>
>> I can see a *really* close patch has been submitted the day after.
>> http://patchwork.dpdk.org/patch/58352/
>>
>> And merged after a v3:
>>
>> https://git.dpdk.org/dpdk/commit/?id=b4f74051165560aa81814433dea7e6eb0bdb32b9
>>
>> So I suppose this patch can be dropped.
>>
>> Yes. That makes sense.
> Thanks for checking.
>
> Thanks
> Ajit
>
Thanks for catching it David.
>>
>> --
>> David Marchand
>>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-10-30 7:59 ` [dpdk-stable] [dpdk-dev] " David Marchand
@ 2019-11-05 15:41 ` Kevin Traynor
2019-11-08 14:45 ` Xu, Rosen
2019-11-08 14:35 ` Xu, Rosen
1 sibling, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-11-05 15:41 UTC (permalink / raw)
To: David Marchand, Rosen Xu; +Cc: dev, dpdk stable, Xiaolong Ye
On 30/10/2019 07:59, David Marchand wrote:
> Hello Rosen,
>
> Review please.
>
Ping Rosen.
> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>>
>> Coverity is complaining about identical code regardless of which branch
>> of the if else is taken. Functionally it means an error will always be
>> returned if this if else is hit. Remove the else branch.
>>
>> CID 337928 (#1 of 1): Identical code for different branches
>> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
>> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
>> n->n_children != 0U is true, because the 'then' and 'else' branches
>> are identical. Should one of the branches be modified, or the entire
>> 'if' statement replaced?
>> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
>> 1507 n->n_children != 0) {
>> 1508 return -rte_tm_error_set(error,
>> 1509 EINVAL,
>> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
>> 1511 NULL,
>> 1512 rte_strerror(EINVAL));
>> else_branch: The else branch, identical to the then branch.
>> 1513 } else {
>> 1514 return -rte_tm_error_set(error,
>> 1515 EINVAL,
>> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
>> 1517 NULL,
>> 1518 rte_strerror(EINVAL));
>> 1519 }
>>
>> Coverity issue: 337928
>> Fixes: c820468ac99c ("net/ipn3ke: support TM")
>> Cc: rosen.xu@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>> ---
>> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
>> index adf02c157..a93145d59 100644
>> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
>> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
>> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct rte_eth_dev *dev,
>> NULL,
>> rte_strerror(EINVAL));
>> - } else {
>> - return -rte_tm_error_set(error,
>> - EINVAL,
>> - RTE_TM_ERROR_TYPE_UNSPECIFIED,
>> - NULL,
>> - rte_strerror(EINVAL));
>> }
>> }
>> --
>> 2.21.0
>>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
2019-10-30 7:52 ` [dpdk-stable] [dpdk-dev] " David Marchand
@ 2019-11-05 16:40 ` Kevin Traynor
2019-11-05 17:10 ` Ferriter, Cian
0 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-11-05 16:40 UTC (permalink / raw)
To: David Marchand; +Cc: dev, cian.ferriter, dpdk stable, Yigit, Ferruh
On 30/10/2019 07:52, David Marchand wrote:
> On Tue, Oct 1, 2019 at 2:54 PM Kevin Traynor <ktraynor@redhat.com> wrote:
>>
>> Previously rx/tx_queues were passed into eth_from_pcaps_common()
>> as ptrs and were checked for being NULL.
>>
>> In commit da6ba28f0540 ("net/pcap: use a struct to pass user options")
>> that changed to pass in a ptr to a pmd_devargs_all which contains
>> the rx/tx_queues.
>>
>> The parameter checking was not updated as part of that commit and
>> coverity caught that there was still a check if rx/tx_queues were
>> NULL, apparently after they had been dereferenced.
>>
>> Fix that by checking the pmd_devargs_all ptr and removing the NULL
>> checks on rx/tx_queues.
>>
>> 1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
>> 1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
>> 1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
>> deref_ptr: Directly dereferencing pointer tx_queues.
>> 1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
>> 1235 unsigned int i;
>> 1236
>> 1237 /* do some parameter checking */
>> CID 345004: Dereference before null check (REVERSE_INULL)
>> [select issue]
>> 1238 if (rx_queues == NULL && nb_rx_queues > 0)
>> 1239 return -1;
>> CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
>> check_after_deref: Null-checking tx_queues suggests that it may be
>> null, but it has already been dereferenced on all paths leading to
>> the check.
>> 1240 if (tx_queues == NULL && nb_tx_queues > 0)
>> 1241 return -1;
>>
>> Coverity issue: 345029
>> Coverity issue: 345044
>> Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
>> Cc: cian.ferriter@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>
> This patch hides the coverity warning.
> But can't we just remove those checks?
>
> Iiuc, the checks on NULL pointers are unnecessary since
> https://git.dpdk.org/dpdk/commit/?id=a0fce1193ce13a50c00624cb36605ebef7a3e60b.
>
>
Aside from the Coverity complaint about rxq/txq NULL check after use,
the checks didn't seem to make sense anymore as rxq/txq are part of
devargs_all struct now, so they were removed.
I added the NULL check on devargs_all to avoid a NULL deference while
getting them instead. The check isn't doing any harm, but looking at the
current paths to this fn. can see devargs_all would not be NULL, so I'm
ok to remove it too. Cian/Ferruh, wdyt?
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
2019-11-05 16:40 ` Kevin Traynor
@ 2019-11-05 17:10 ` Ferriter, Cian
2019-11-06 19:03 ` Kevin Traynor
0 siblings, 1 reply; 45+ messages in thread
From: Ferriter, Cian @ 2019-11-05 17:10 UTC (permalink / raw)
To: Kevin Traynor, David Marchand; +Cc: dev, dpdk stable, Yigit, Ferruh
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday 5 November 2019 16:41
> To: David Marchand <david.marchand@redhat.com>
> Cc: dev <dev@dpdk.org>; Ferriter, Cian <cian.ferriter@intel.com>; dpdk
> stable <stable@dpdk.org>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
>
> On 30/10/2019 07:52, David Marchand wrote:
> > On Tue, Oct 1, 2019 at 2:54 PM Kevin Traynor <ktraynor@redhat.com>
> wrote:
> >>
> >> Previously rx/tx_queues were passed into eth_from_pcaps_common() as
> >> ptrs and were checked for being NULL.
> >>
> >> In commit da6ba28f0540 ("net/pcap: use a struct to pass user
> >> options") that changed to pass in a ptr to a pmd_devargs_all which
> >> contains the rx/tx_queues.
> >>
> >> The parameter checking was not updated as part of that commit and
> >> coverity caught that there was still a check if rx/tx_queues were
> >> NULL, apparently after they had been dereferenced.
> >>
> >> Fix that by checking the pmd_devargs_all ptr and removing the NULL
> >> checks on rx/tx_queues.
> >>
> >> 1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
> >> 1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
> >> 1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
> >> deref_ptr: Directly dereferencing pointer tx_queues.
> >> 1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
> >> 1235 unsigned int i;
> >> 1236
> >> 1237 /* do some parameter checking */
> >> CID 345004: Dereference before null check (REVERSE_INULL)
> >> [select issue]
> >> 1238 if (rx_queues == NULL && nb_rx_queues > 0)
> >> 1239 return -1;
> >> CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
> >> check_after_deref: Null-checking tx_queues suggests that it may be
> >> null, but it has already been dereferenced on all paths leading to
> >> the check.
> >> 1240 if (tx_queues == NULL && nb_tx_queues > 0)
> >> 1241 return -1;
> >>
> >> Coverity issue: 345029
> >> Coverity issue: 345044
> >> Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
> >> Cc: cian.ferriter@intel.com
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> >
> > This patch hides the coverity warning.
> > But can't we just remove those checks?
> >
> > Iiuc, the checks on NULL pointers are unnecessary since
> >
> https://git.dpdk.org/dpdk/commit/?id=a0fce1193ce13a50c00624cb36605ebe
> f7a3e60b.
> >
> >
>
> Aside from the Coverity complaint about rxq/txq NULL check after use, the
> checks didn't seem to make sense anymore as rxq/txq are part of devargs_all
> struct now, so they were removed.
>
> I added the NULL check on devargs_all to avoid a NULL deference while
> getting them instead. The check isn't doing any harm, but looking at the
> current paths to this fn. can see devargs_all would not be NULL, so I'm ok to
> remove it too. Cian/Ferruh, wdyt?
I'm OK to remove this. Looks like it's no longer necessary. Good find David!
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 1/8] net/pcap: fix argument checks
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
@ 2019-11-06 19:01 ` Kevin Traynor
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 2/8] common/cpt: fix possible NULL deference Kevin Traynor
` (6 subsequent siblings)
7 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:01 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, stable, Cian Ferriter
Previously rx/tx_queues were passed into eth_from_pcaps_common()
as ptrs and were checked for being NULL.
In commit da6ba28f0540 ("net/pcap: use a struct to pass user options")
that changed to pass in a ptr to a pmd_devargs_all which contains
the rx/tx_queues.
The parameter checking was not updated as part of that commit and
coverity caught that there was still a check if rx/tx_queues were
NULL, apparently after they had been dereferenced.
In fact as they are a members of the devargs_all struct, they will
not be NULL so remove those checks.
1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
deref_ptr: Directly dereferencing pointer tx_queues.
1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
1235 unsigned int i;
1236
1237 /* do some parameter checking */
CID 345004: Dereference before null check (REVERSE_INULL)
[select issue]
1238 if (rx_queues == NULL && nb_rx_queues > 0)
1239 return -1;
CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking tx_queues suggests that it may be
null, but it has already been dereferenced on all paths leading to
the check.
1240 if (tx_queues == NULL && nb_tx_queues > 0)
1241 return -1;
Coverity issue: 345029
Coverity issue: 345044
Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
---
drivers/net/pcap/rte_eth_pcap.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 5186d8fe5..aa7ef6fdb 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -1241,10 +1241,4 @@ eth_from_pcaps_common(struct rte_vdev_device *vdev,
unsigned int i;
- /* do some parameter checking */
- if (rx_queues == NULL && nb_rx_queues > 0)
- return -1;
- if (tx_queues == NULL && nb_tx_queues > 0)
- return -1;
-
if (pmd_init_internals(vdev, nb_rx_queues, nb_tx_queues, internals,
eth_dev) < 0)
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 2/8] common/cpt: fix possible NULL deference
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 1/8] net/pcap: fix argument checks Kevin Traynor
@ 2019-11-06 19:01 ` Kevin Traynor
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
` (5 subsequent siblings)
7 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:01 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, ndabilpuram, stable
Coverity complains that ctrl_flags is set to NULL at the start
of the function and it may not have been set before there is a
jump to fc_success and it is dereferenced.
Check for NULL before dereference.
312fc_success:
CID 344983 (#1 of 1): Explicit null dereferenced
(FORWARD_NULL)7. var_deref_op: Dereferencing null pointer ctrl_flags.
313 *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
Coverity issue: 344983
Fixes: 6cc54096520d ("crypto/octeontx: add supported sessions")
Cc: ndabilpuram@marvell.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
There may be further rework needed to set it to the correct value,
but for now at least prevent the NULL dereference.
---
drivers/common/cpt/cpt_ucode.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/common/cpt/cpt_ucode.h b/drivers/common/cpt/cpt_ucode.h
index 0dac12ee3..d5a0135d7 100644
--- a/drivers/common/cpt/cpt_ucode.h
+++ b/drivers/common/cpt/cpt_ucode.h
@@ -311,5 +311,6 @@ cpt_fc_ciph_set_key(void *ctx, cipher_type_t type, const uint8_t *key,
fc_success:
- *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
+ if (ctrl_flags != NULL)
+ *ctrl_flags = rte_cpu_to_be_64(*ctrl_flags);
success:
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 1/8] net/pcap: fix argument checks Kevin Traynor
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 2/8] common/cpt: fix possible NULL deference Kevin Traynor
@ 2019-11-06 19:01 ` Kevin Traynor
2019-11-08 14:50 ` Xu, Rosen
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement Kevin Traynor
` (4 subsequent siblings)
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:01 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, rosen.xu, stable
Coverity is complaining about identical code regardless of which branch
of the if else is taken. Functionally it means an error will always be
returned if this if else is hit. Remove the else branch.
CID 337928 (#1 of 1): Identical code for different branches
(IDENTICAL_BRANCHES)identical_branches: The same code is executed
regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
n->n_children != 0U is true, because the 'then' and 'else' branches
are identical. Should one of the branches be modified, or the entire
'if' statement replaced?
1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
1507 n->n_children != 0) {
1508 return -rte_tm_error_set(error,
1509 EINVAL,
1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
1511 NULL,
1512 rte_strerror(EINVAL));
else_branch: The else branch, identical to the then branch.
1513 } else {
1514 return -rte_tm_error_set(error,
1515 EINVAL,
1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
1517 NULL,
1518 rte_strerror(EINVAL));
1519 }
Coverity issue: 337928
Fixes: c820468ac99c ("net/ipn3ke: support TM")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
index adf02c157..a93145d59 100644
--- a/drivers/net/ipn3ke/ipn3ke_tm.c
+++ b/drivers/net/ipn3ke/ipn3ke_tm.c
@@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct rte_eth_dev *dev,
NULL,
rte_strerror(EINVAL));
- } else {
- return -rte_tm_error_set(error,
- EINVAL,
- RTE_TM_ERROR_TYPE_UNSPECIFIED,
- NULL,
- rte_strerror(EINVAL));
}
}
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
` (2 preceding siblings ...)
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
@ 2019-11-06 19:01 ` Kevin Traynor
2019-11-08 14:49 ` Xu, Rosen
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code Kevin Traynor
` (3 subsequent siblings)
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:01 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, rosen.xu, stable
Coverity complains that this statement is not needed as the goto
label is on the next line anyway. Remove the if statement.
653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
CID 337930 (#1 of 1): Identical code for different branches
(IDENTICAL_BRANCHES)identical_branches: The same code is executed
when the condition ret is true or false, because the code in the
if-then branch and after the if statement is identical. Should
the if statement be removed?
654 if (ret)
655 goto end;
implicit_else: The code from the above if-then branch is identical
to the code after the if statement.
656end:
Coverity issue: 337930
Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c b/drivers/net/ipn3ke/ipn3ke_ethdev.c
index 28d8aaf2d..af87fda6b 100644
--- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
@@ -738,6 +738,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
- if (ret)
- goto end;
+
end:
if (kvlist)
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
` (3 preceding siblings ...)
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement Kevin Traynor
@ 2019-11-06 19:02 ` Kevin Traynor
2019-11-08 14:50 ` Xu, Rosen
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 6/8] compress/octeontx: " Kevin Traynor
` (2 subsequent siblings)
7 siblings, 1 reply; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:02 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, rosen.xu, stable
These struct members and variable were commented out. Remove them.
Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
Fixes: c820468ac99c ("net/ipn3ke: support TM")
Cc: rosen.xu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
2 files changed, 8 deletions(-)
diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h b/drivers/net/ipn3ke/ipn3ke_ethdev.h
index 985f20352..9b0cf309c 100644
--- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
@@ -97,18 +97,11 @@ struct ipn3ke_tm_node {
struct ipn3ke_tm_hierarchy {
struct ipn3ke_tm_node *port_node;
- /*struct ipn3ke_tm_node_list vt_node_list;*/
- /*struct ipn3ke_tm_node_list cos_node_list;*/
-
uint32_t n_shaper_profiles;
- /*uint32_t n_shared_shapers;*/
uint32_t n_tdrop_profiles;
uint32_t n_vt_nodes;
uint32_t n_cos_nodes;
-
struct ipn3ke_tm_node *port_commit_node;
struct ipn3ke_tm_node_list vt_commit_node_list;
struct ipn3ke_tm_node_list cos_commit_node_list;
-
- /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
};
diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c b/drivers/net/ipn3ke/ipn3ke_tm.c
index a93145d59..5a16c5f96 100644
--- a/drivers/net/ipn3ke/ipn3ke_tm.c
+++ b/drivers/net/ipn3ke/ipn3ke_tm.c
@@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t tm_id,
struct rte_tm_error *error)
{
- /*struct ipn3ke_tm_internals *tm = IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
uint32_t node_index;
uint32_t parent_index;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 6/8] compress/octeontx: remove commented out code
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
` (4 preceding siblings ...)
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code Kevin Traynor
@ 2019-11-06 19:02 ` Kevin Traynor
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 7/8] event/opdl: " Kevin Traynor
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 8/8] net/bnxt: " Kevin Traynor
7 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:02 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, ssahu, stable
This code is commented out. Remove it.
Fixes: 43e610bb8565 ("compress/octeontx: introduce octeontx zip PMD")
Cc: ssahu@marvell.com
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
drivers/compress/octeontx/include/zip_regs.h | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/compress/octeontx/include/zip_regs.h b/drivers/compress/octeontx/include/zip_regs.h
index 04c3d75e9..96e538bb7 100644
--- a/drivers/compress/octeontx/include/zip_regs.h
+++ b/drivers/compress/octeontx/include/zip_regs.h
@@ -37,5 +37,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_vqx_ena_s cn; */
} zip_vqx_ena_t;
@@ -65,5 +64,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_vqx_sbuf_addr_s cn; */
} zip_vqx_sbuf_addr_t;
@@ -85,5 +83,4 @@ typedef union {
#endif /* Word 0 - End */
} s;
- /* struct zip_quex_doorbell_s cn; */
} zip_quex_doorbell_t;
@@ -105,5 +102,4 @@ union zip_nptr_s {
#endif /* Word 0 - End */
} s;
- /* struct zip_nptr_s_s cn83xx; */
};
@@ -198,5 +194,4 @@ union zip_inst_s {
/** Beginning of file */
uint64_t bf : 1;
- // uint64_t reserved_3_4 : 2;
/** Comp/decomp operation */
uint64_t op : 2;
@@ -211,5 +206,4 @@ union zip_inst_s {
uint64_t dg : 1;
uint64_t ds : 1;
- //uint64_t reserved_3_4 : 2;
uint64_t op : 2;
uint64_t bf : 1;
@@ -616,6 +610,4 @@ union zip_zres_s {
#endif /* Word 7 - End */
} /** ZIP Result Structure */s;
-
- /* struct zip_zres_s_s cn83xx; */
};
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 7/8] event/opdl: remove commented out code
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
` (5 preceding siblings ...)
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 6/8] compress/octeontx: " Kevin Traynor
@ 2019-11-06 19:02 ` Kevin Traynor
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 8/8] net/bnxt: " Kevin Traynor
7 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:02 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, stable, Liang Ma
Some variables are commented out. Remove them.
Fixes: d548ef513cd7 ("event/opdl: add unit tests")
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Liang Ma <liang.j.ma@intel.com>
---
drivers/event/opdl/opdl_test.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/event/opdl/opdl_test.c b/drivers/event/opdl/opdl_test.c
index 5868ec1be..e7a32fbd3 100644
--- a/drivers/event/opdl/opdl_test.c
+++ b/drivers/event/opdl/opdl_test.c
@@ -696,7 +696,4 @@ static int
single_link(struct test *t)
{
- /* const uint8_t rx_port = 0; */
- /* const uint8_t w1_port = 1; */
- /* const uint8_t w3_port = 3; */
const uint8_t tx_port = 2;
int err;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* [dpdk-stable] [v2 PATCH 8/8] net/bnxt: remove commented out code
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
` (6 preceding siblings ...)
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 7/8] event/opdl: " Kevin Traynor
@ 2019-11-06 19:02 ` Kevin Traynor
7 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:02 UTC (permalink / raw)
To: dev; +Cc: david.marchand, Kevin Traynor, stable, Ajit Khaparde
This commented out todo and code is old. Remove it.
Fixes: b7435d660a8c ("net/bnxt: add ntuple filtering support")
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
drivers/net/bnxt/bnxt_ethdev.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7d9459f0a..58a4f98c9 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2617,7 +2617,4 @@ parse_ntuple_filter(struct bnxt *bp,
}
- //TODO Priority
- //nfilter->priority = (uint8_t)filter->priority;
-
bfilter->enables = en;
return 0;
--
2.21.0
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
2019-11-05 17:10 ` Ferriter, Cian
@ 2019-11-06 19:03 ` Kevin Traynor
0 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-06 19:03 UTC (permalink / raw)
To: Ferriter, Cian, David Marchand; +Cc: dev, dpdk stable, Yigit, Ferruh
On 05/11/2019 17:10, Ferriter, Cian wrote:
>
>
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Tuesday 5 November 2019 16:41
>> To: David Marchand <david.marchand@redhat.com>
>> Cc: dev <dev@dpdk.org>; Ferriter, Cian <cian.ferriter@intel.com>; dpdk
>> stable <stable@dpdk.org>; Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 1/9] net/pcap: fix argument checks
>>
>> On 30/10/2019 07:52, David Marchand wrote:
>>> On Tue, Oct 1, 2019 at 2:54 PM Kevin Traynor <ktraynor@redhat.com>
>> wrote:
>>>>
>>>> Previously rx/tx_queues were passed into eth_from_pcaps_common() as
>>>> ptrs and were checked for being NULL.
>>>>
>>>> In commit da6ba28f0540 ("net/pcap: use a struct to pass user
>>>> options") that changed to pass in a ptr to a pmd_devargs_all which
>>>> contains the rx/tx_queues.
>>>>
>>>> The parameter checking was not updated as part of that commit and
>>>> coverity caught that there was still a check if rx/tx_queues were
>>>> NULL, apparently after they had been dereferenced.
>>>>
>>>> Fix that by checking the pmd_devargs_all ptr and removing the NULL
>>>> checks on rx/tx_queues.
>>>>
>>>> 1231 struct pmd_devargs *rx_queues = &devargs_all->rx_queues;
>>>> 1232 struct pmd_devargs *tx_queues = &devargs_all->tx_queues;
>>>> 1233 const unsigned int nb_rx_queues = rx_queues->num_of_queue;
>>>> deref_ptr: Directly dereferencing pointer tx_queues.
>>>> 1234 const unsigned int nb_tx_queues = tx_queues->num_of_queue;
>>>> 1235 unsigned int i;
>>>> 1236
>>>> 1237 /* do some parameter checking */
>>>> CID 345004: Dereference before null check (REVERSE_INULL)
>>>> [select issue]
>>>> 1238 if (rx_queues == NULL && nb_rx_queues > 0)
>>>> 1239 return -1;
>>>> CID 345029 (#1 of 1): Dereference before null check (REVERSE_INULL)
>>>> check_after_deref: Null-checking tx_queues suggests that it may be
>>>> null, but it has already been dereferenced on all paths leading to
>>>> the check.
>>>> 1240 if (tx_queues == NULL && nb_tx_queues > 0)
>>>> 1241 return -1;
>>>>
>>>> Coverity issue: 345029
>>>> Coverity issue: 345044
>>>> Fixes: da6ba28f0540 ("net/pcap: use a struct to pass user options")
>>>> Cc: cian.ferriter@intel.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>>
>>> This patch hides the coverity warning.
>>> But can't we just remove those checks?
>>>
>>> Iiuc, the checks on NULL pointers are unnecessary since
>>>
>> https://git.dpdk.org/dpdk/commit/?id=a0fce1193ce13a50c00624cb36605ebe
>> f7a3e60b.
>>>
>>>
>>
>> Aside from the Coverity complaint about rxq/txq NULL check after use, the
>> checks didn't seem to make sense anymore as rxq/txq are part of devargs_all
>> struct now, so they were removed.
>>
>> I added the NULL check on devargs_all to avoid a NULL deference while
>> getting them instead. The check isn't doing any harm, but looking at the
>> current paths to this fn. can see devargs_all would not be NULL, so I'm ok to
>> remove it too. Cian/Ferruh, wdyt?
>
> I'm OK to remove this. Looks like it's no longer necessary. Good find David!
>
Thanks Cian, I kept your Ack for v2.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-10-30 7:59 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-11-05 15:41 ` Kevin Traynor
@ 2019-11-08 14:35 ` Xu, Rosen
1 sibling, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:35 UTC (permalink / raw)
To: David Marchand; +Cc: dev, Kevin Traynor, dpdk stable, Ye, Xiaolong
Hi David,
Too many things in these days. I have reviewed it. Thanks a lot.
> -----Original Message-----
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, October 30, 2019 16:00
> To: Xu, Rosen <rosen.xu@intel.com>
> Cc: dev <dev@dpdk.org>; Kevin Traynor <ktraynor@redhat.com>; dpdk
> stable <stable@dpdk.org>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check
> logic
>
> Hello Rosen,
>
> Review please.
>
> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
> >
> > Coverity is complaining about identical code regardless of which
> > branch of the if else is taken. Functionally it means an error will
> > always be returned if this if else is hit. Remove the else branch.
> >
> > CID 337928 (#1 of 1): Identical code for different branches
> > (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> > regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> > n->n_children != 0U is true, because the 'then' and 'else' branches
> > are identical. Should one of the branches be modified, or the entire
> > 'if' statement replaced?
> > 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> > 1507 n->n_children != 0) {
> > 1508 return -rte_tm_error_set(error,
> > 1509 EINVAL,
> > 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> > 1511 NULL,
> > 1512 rte_strerror(EINVAL));
> > else_branch: The else branch, identical to the then branch.
> > 1513 } else {
> > 1514 return -rte_tm_error_set(error,
> > 1515 EINVAL,
> > 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> > 1517 NULL,
> > 1518 rte_strerror(EINVAL));
> > 1519 }
> >
> > Coverity issue: 337928
> > Fixes: c820468ac99c ("net/ipn3ke: support TM")
> > Cc: rosen.xu@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > ---
> > drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> > b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644
> > --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> > +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> > @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct
> rte_eth_dev *dev,
> > NULL,
> > rte_strerror(EINVAL));
> > - } else {
> > - return -rte_tm_error_set(error,
> > - EINVAL,
> > - RTE_TM_ERROR_TYPE_UNSPECIFIED,
> > - NULL,
> > - rte_strerror(EINVAL));
> > }
> > }
> > --
> > 2.21.0
> >
>
> --
> David Marchand
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement
2019-10-30 8:01 ` David Marchand
@ 2019-11-08 14:35 ` Xu, Rosen
0 siblings, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:35 UTC (permalink / raw)
To: David Marchand, Kevin Traynor; +Cc: dev, dpdk stable
> -----Original Message-----
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, October 30, 2019 16:01
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: dev <dev@dpdk.org>; Xu, Rosen <rosen.xu@intel.com>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if
> statement
>
> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
> >
> > Coverity complains that this statement is not needed as the goto label
> > is on the next line anyway. Remove the if statement.
> >
> > 653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> > CID 337930 (#1 of 1): Identical code for different branches
> > (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> > when the condition ret is true or false, because the code in the
> > if-then branch and after the if statement is identical. Should
> > the if statement be removed?
> > 654 if (ret)
> > 655 goto end;
> > implicit_else: The code from the above if-then branch is identical
> > to the code after the if statement.
> > 656end:
> >
> > Coverity issue: 337930
> > Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> > Cc: rosen.xu@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > ---
> > drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> > b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> > index c226d6313..282295f49 100644
> > --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> > +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> > @@ -652,6 +652,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
> >
> > ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> > - if (ret)
> > - goto end;
> > +
> > end:
> > if (kvlist)
> > --
> > 2.21.0
> >
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
>
> --
> David Marchand
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code
2019-10-30 8:04 ` David Marchand
@ 2019-11-08 14:40 ` Xu, Rosen
0 siblings, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:40 UTC (permalink / raw)
To: David Marchand, Kevin Traynor; +Cc: dev, dpdk stable
Hi,
Too busy these days, sorry for late reply.
> -----Original Message-----
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, October 30, 2019 16:04
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: dev <dev@dpdk.org>; Xu, Rosen <rosen.xu@intel.com>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out
> code
>
> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com> wrote:
> >
> > These struct members and variable were commented out. Remove them.
> >
> > Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> > Fixes: c820468ac99c ("net/ipn3ke: support TM")
> > Cc: rosen.xu@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > ---
> > drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
> > drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
> > 2 files changed, 8 deletions(-)
> >
> > diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> > b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> > index c7b336bbd..0d71dcd64 100644
> > --- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> > +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> > @@ -97,18 +97,11 @@ struct ipn3ke_tm_node { struct
> > ipn3ke_tm_hierarchy {
> > struct ipn3ke_tm_node *port_node;
> > - /*struct ipn3ke_tm_node_list vt_node_list;*/
> > - /*struct ipn3ke_tm_node_list cos_node_list;*/
> > -
> > uint32_t n_shaper_profiles;
> > - /*uint32_t n_shared_shapers;*/
> > uint32_t n_tdrop_profiles;
> > uint32_t n_vt_nodes;
> > uint32_t n_cos_nodes;
> > -
> > struct ipn3ke_tm_node *port_commit_node;
> > struct ipn3ke_tm_node_list vt_commit_node_list;
> > struct ipn3ke_tm_node_list cos_commit_node_list;
> > -
> > - /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
> > };
> >
> > diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> > b/drivers/net/ipn3ke/ipn3ke_tm.c index a93145d59..5a16c5f96 100644
> > --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> > +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> > @@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t
> tm_id,
> > struct rte_tm_error *error)
> > {
> > - /*struct ipn3ke_tm_internals *tm =
> IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
> > uint32_t node_index;
> > uint32_t parent_index;
> > --
> > 2.21.0
> >
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
>
>
> --
> David Marchand
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-11-05 15:41 ` Kevin Traynor
@ 2019-11-08 14:45 ` Xu, Rosen
2019-11-08 14:47 ` Kevin Traynor
0 siblings, 1 reply; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:45 UTC (permalink / raw)
To: Kevin Traynor, David Marchand; +Cc: dev, dpdk stable, Ye, Xiaolong
Hi Kevin,
Too many things in these days, sorry for late reply.
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday, November 05, 2019 23:42
> To: David Marchand <david.marchand@redhat.com>; Xu, Rosen
> <rosen.xu@intel.com>
> Cc: dev <dev@dpdk.org>; dpdk stable <stable@dpdk.org>; Ye, Xiaolong
> <xiaolong.ye@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check
> logic
>
> On 30/10/2019 07:59, David Marchand wrote:
> > Hello Rosen,
> >
> > Review please.
> >
>
> Ping Rosen.
>
> > On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com>
> wrote:
> >>
> >> Coverity is complaining about identical code regardless of which
> >> branch of the if else is taken. Functionally it means an error will
> >> always be returned if this if else is hit. Remove the else branch.
> >>
> >> CID 337928 (#1 of 1): Identical code for different branches
> >> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> >> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> >> n->n_children != 0U is true, because the 'then' and 'else' branches
> >> are identical. Should one of the branches be modified, or the entire
> >> 'if' statement replaced?
Yes, you are right.
> >> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> >> 1507 n->n_children != 0) {
> >> 1508 return -rte_tm_error_set(error,
> >> 1509 EINVAL,
> >> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> >> 1511 NULL,
> >> 1512 rte_strerror(EINVAL));
> >> else_branch: The else branch, identical to the then branch.
> >> 1513 } else {
> >> 1514 return -rte_tm_error_set(error,
> >> 1515 EINVAL,
> >> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> >> 1517 NULL,
> >> 1518 rte_strerror(EINVAL));
> >> 1519 }
> >>
> >> Coverity issue: 337928
> >> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> >> Cc: rosen.xu@intel.com
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> >> ---
> >> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
> >> 1 file changed, 6 deletions(-)
> >>
> >> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> >> b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644
> >> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> >> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> >> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct
> rte_eth_dev *dev,
> >> NULL,
> >> rte_strerror(EINVAL));
> >> - } else {
> >> - return -rte_tm_error_set(error,
> >> - EINVAL,
> >> - RTE_TM_ERROR_TYPE_UNSPECIFIED,
> >> - NULL,
> >> - rte_strerror(EINVAL));
> >> }
> >> }
> >> --
> >> 2.21.0
> >>
> >
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-11-08 14:45 ` Xu, Rosen
@ 2019-11-08 14:47 ` Kevin Traynor
0 siblings, 0 replies; 45+ messages in thread
From: Kevin Traynor @ 2019-11-08 14:47 UTC (permalink / raw)
To: Xu, Rosen, David Marchand; +Cc: dev, dpdk stable, Ye, Xiaolong
On 08/11/2019 14:45, Xu, Rosen wrote:
> Hi Kevin,
>
> Too many things in these days, sorry for late reply.
>
Hi Rosen, no problem, thanks for the Ack.
Kevin.
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Tuesday, November 05, 2019 23:42
>> To: David Marchand <david.marchand@redhat.com>; Xu, Rosen
>> <rosen.xu@intel.com>
>> Cc: dev <dev@dpdk.org>; dpdk stable <stable@dpdk.org>; Ye, Xiaolong
>> <xiaolong.ye@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 4/9] net/ipn3ke: fix incorrect commit check
>> logic
>>
>> On 30/10/2019 07:59, David Marchand wrote:
>>> Hello Rosen,
>>>
>>> Review please.
>>>
>>
>> Ping Rosen.
>>
>>> On Tue, Oct 1, 2019 at 3:04 PM Kevin Traynor <ktraynor@redhat.com>
>> wrote:
>>>>
>>>> Coverity is complaining about identical code regardless of which
>>>> branch of the if else is taken. Functionally it means an error will
>>>> always be returned if this if else is hit. Remove the else branch.
>>>>
>>>> CID 337928 (#1 of 1): Identical code for different branches
>>>> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
>>>> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
>>>> n->n_children != 0U is true, because the 'then' and 'else' branches
>>>> are identical. Should one of the branches be modified, or the entire
>>>> 'if' statement replaced?
>
> Yes, you are right.
>
>>>> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
>>>> 1507 n->n_children != 0) {
>>>> 1508 return -rte_tm_error_set(error,
>>>> 1509 EINVAL,
>>>> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
>>>> 1511 NULL,
>>>> 1512 rte_strerror(EINVAL));
>>>> else_branch: The else branch, identical to the then branch.
>>>> 1513 } else {
>>>> 1514 return -rte_tm_error_set(error,
>>>> 1515 EINVAL,
>>>> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
>>>> 1517 NULL,
>>>> 1518 rte_strerror(EINVAL));
>>>> 1519 }
>>>>
>>>> Coverity issue: 337928
>>>> Fixes: c820468ac99c ("net/ipn3ke: support TM")
>>>> Cc: rosen.xu@intel.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>>> ---
>>>> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
>>>> 1 file changed, 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
>>>> b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644
>>>> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
>>>> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
>>>> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct
>> rte_eth_dev *dev,
>>>> NULL,
>>>> rte_strerror(EINVAL));
>>>> - } else {
>>>> - return -rte_tm_error_set(error,
>>>> - EINVAL,
>>>> - RTE_TM_ERROR_TYPE_UNSPECIFIED,
>>>> - NULL,
>>>> - rte_strerror(EINVAL));
>>>> }
>>>> }
>>>> --
>>>> 2.21.0
>>>>
>>>
>
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement Kevin Traynor
@ 2019-11-08 14:49 ` Xu, Rosen
0 siblings, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:49 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: david.marchand, stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Thursday, November 07, 2019 3:02
> To: dev@dpdk.org
> Cc: david.marchand@redhat.com; Kevin Traynor <ktraynor@redhat.com>;
> Xu, Rosen <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [v2 PATCH 4/8] net/ipn3ke: remove useless if statement
>
> Coverity complains that this statement is not needed as the goto label is on
> the next line anyway. Remove the if statement.
>
> 653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> CID 337930 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> when the condition ret is true or false, because the code in the
> if-then branch and after the if statement is identical. Should
> the if statement be removed?
> 654 if (ret)
> 655 goto end;
> implicit_else: The code from the above if-then branch is identical
> to the code after the if statement.
> 656end:
>
> Coverity issue: 337930
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> index 28d8aaf2d..af87fda6b 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> @@ -738,6 +738,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
>
> ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> - if (ret)
> - goto end;
> +
> end:
> if (kvlist)
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code Kevin Traynor
@ 2019-11-08 14:50 ` Xu, Rosen
0 siblings, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:50 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: david.marchand, stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Thursday, November 07, 2019 3:02
> To: dev@dpdk.org
> Cc: david.marchand@redhat.com; Kevin Traynor <ktraynor@redhat.com>;
> Xu, Rosen <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [v2 PATCH 5/8] net/ipn3ke: remove commented out code
>
> These struct members and variable were commented out. Remove them.
>
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
> drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> index 985f20352..9b0cf309c 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> @@ -97,18 +97,11 @@ struct ipn3ke_tm_node { struct ipn3ke_tm_hierarchy
> {
> struct ipn3ke_tm_node *port_node;
> - /*struct ipn3ke_tm_node_list vt_node_list;*/
> - /*struct ipn3ke_tm_node_list cos_node_list;*/
> -
> uint32_t n_shaper_profiles;
> - /*uint32_t n_shared_shapers;*/
> uint32_t n_tdrop_profiles;
> uint32_t n_vt_nodes;
> uint32_t n_cos_nodes;
> -
> struct ipn3ke_tm_node *port_commit_node;
> struct ipn3ke_tm_node_list vt_commit_node_list;
> struct ipn3ke_tm_node_list cos_commit_node_list;
> -
> - /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
> };
No problem.
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> b/drivers/net/ipn3ke/ipn3ke_tm.c index a93145d59..5a16c5f96 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t
> tm_id,
> struct rte_tm_error *error)
> {
> - /*struct ipn3ke_tm_internals *tm =
> IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
> uint32_t node_index;
> uint32_t parent_index;
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
@ 2019-11-08 14:50 ` Xu, Rosen
0 siblings, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:50 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: david.marchand, stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Thursday, November 07, 2019 3:02
> To: dev@dpdk.org
> Cc: david.marchand@redhat.com; Kevin Traynor <ktraynor@redhat.com>;
> Xu, Rosen <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic
>
> Coverity is complaining about identical code regardless of which branch of
> the if else is taken. Functionally it means an error will always be returned if
> this if else is hit. Remove the else branch.
>
> CID 337928 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> n->n_children != 0U is true, because the 'then' and 'else' branches
> are identical. Should one of the branches be modified, or the entire
> 'if' statement replaced?
> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> 1507 n->n_children != 0) {
> 1508 return -rte_tm_error_set(error,
> 1509 EINVAL,
> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1511 NULL,
> 1512 rte_strerror(EINVAL));
> else_branch: The else branch, identical to the then branch.
> 1513 } else {
> 1514 return -rte_tm_error_set(error,
> 1515 EINVAL,
> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1517 NULL,
> 1518 rte_strerror(EINVAL));
> 1519 }
>
> Coverity issue: 337928
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct
> rte_eth_dev *dev,
> NULL,
> rte_strerror(EINVAL));
> - } else {
> - return -rte_tm_error_set(error,
> - EINVAL,
> -
> RTE_TM_ERROR_TYPE_UNSPECIFIED,
> - NULL,
> - rte_strerror(EINVAL));
> }
> }
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code
2019-10-01 13:04 ` [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code Kevin Traynor
2019-10-30 8:04 ` David Marchand
@ 2019-11-08 14:51 ` Xu, Rosen
1 sibling, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:51 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday, October 01, 2019 21:04
> To: dev@dpdk.org
> Cc: Kevin Traynor <ktraynor@redhat.com>; Xu, Rosen
> <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [PATCH 6/9] net/ipn3ke: remove commented out code
>
> These struct members and variable were commented out. Remove them.
>
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.h | 7 -------
> drivers/net/ipn3ke/ipn3ke_tm.c | 1 -
> 2 files changed, 8 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> index c7b336bbd..0d71dcd64 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
> @@ -97,18 +97,11 @@ struct ipn3ke_tm_node { struct ipn3ke_tm_hierarchy
> {
> struct ipn3ke_tm_node *port_node;
> - /*struct ipn3ke_tm_node_list vt_node_list;*/
> - /*struct ipn3ke_tm_node_list cos_node_list;*/
> -
> uint32_t n_shaper_profiles;
> - /*uint32_t n_shared_shapers;*/
> uint32_t n_tdrop_profiles;
> uint32_t n_vt_nodes;
> uint32_t n_cos_nodes;
> -
> struct ipn3ke_tm_node *port_commit_node;
> struct ipn3ke_tm_node_list vt_commit_node_list;
> struct ipn3ke_tm_node_list cos_commit_node_list;
> -
> - /*uint32_t n_tm_nodes[IPN3KE_TM_NODE_LEVEL_MAX];*/
> };
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> b/drivers/net/ipn3ke/ipn3ke_tm.c index a93145d59..5a16c5f96 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1088,5 +1088,4 @@ ipn3ke_tm_node_add_check_mount(uint32_t
> tm_id,
> struct rte_tm_error *error)
> {
> - /*struct ipn3ke_tm_internals *tm =
> IPN3KE_DEV_PRIVATE_TO_TM(dev);*/
> uint32_t node_index;
> uint32_t parent_index;
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement
2019-10-01 13:04 ` [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement Kevin Traynor
2019-10-30 8:01 ` David Marchand
@ 2019-11-08 14:52 ` Xu, Rosen
1 sibling, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:52 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday, October 01, 2019 21:04
> To: dev@dpdk.org
> Cc: Kevin Traynor <ktraynor@redhat.com>; Xu, Rosen
> <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [PATCH 5/9] net/ipn3ke: remove useless if statement
>
> Coverity complains that this statement is not needed as the goto label is on
> the next line anyway. Remove the if statement.
>
> 653 ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> CID 337930 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> when the condition ret is true or false, because the code in the
> if-then branch and after the if statement is identical. Should
> the if statement be removed?
> 654 if (ret)
> 655 goto end;
> implicit_else: The code from the above if-then branch is identical
> to the code after the if statement.
> 656end:
>
> Coverity issue: 337930
> Fixes: c01c748e4ae6 ("net/ipn3ke: add new driver")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_ethdev.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> index c226d6313..282295f49 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> @@ -652,6 +652,5 @@ ipn3ke_cfg_probe(struct rte_vdev_device *dev)
>
> ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> - if (ret)
> - goto end;
> +
> end:
> if (kvlist)
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
2019-10-01 13:04 ` [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
2019-10-30 7:59 ` [dpdk-stable] [dpdk-dev] " David Marchand
@ 2019-11-08 14:52 ` Xu, Rosen
1 sibling, 0 replies; 45+ messages in thread
From: Xu, Rosen @ 2019-11-08 14:52 UTC (permalink / raw)
To: Kevin Traynor, dev; +Cc: stable
Hi,
> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Tuesday, October 01, 2019 21:04
> To: dev@dpdk.org
> Cc: Kevin Traynor <ktraynor@redhat.com>; Xu, Rosen
> <rosen.xu@intel.com>; stable@dpdk.org
> Subject: [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic
>
> Coverity is complaining about identical code regardless of which branch of
> the if else is taken. Functionally it means an error will always be returned if
> this if else is hit. Remove the else branch.
>
> CID 337928 (#1 of 1): Identical code for different branches
> (IDENTICAL_BRANCHES)identical_branches: The same code is executed
> regardless of whether n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> n->n_children != 0U is true, because the 'then' and 'else' branches
> are identical. Should one of the branches be modified, or the entire
> 'if' statement replaced?
Okay
> 1506 if (n->level != IPN3KE_TM_NODE_LEVEL_COS ||
> 1507 n->n_children != 0) {
> 1508 return -rte_tm_error_set(error,
> 1509 EINVAL,
> 1510 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1511 NULL,
> 1512 rte_strerror(EINVAL));
> else_branch: The else branch, identical to the then branch.
> 1513 } else {
> 1514 return -rte_tm_error_set(error,
> 1515 EINVAL,
> 1516 RTE_TM_ERROR_TYPE_UNSPECIFIED,
> 1517 NULL,
> 1518 rte_strerror(EINVAL));
> 1519 }
>
> Coverity issue: 337928
> Fixes: c820468ac99c ("net/ipn3ke: support TM")
> Cc: rosen.xu@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> drivers/net/ipn3ke/ipn3ke_tm.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/ipn3ke_tm.c
> b/drivers/net/ipn3ke/ipn3ke_tm.c index adf02c157..a93145d59 100644
> --- a/drivers/net/ipn3ke/ipn3ke_tm.c
> +++ b/drivers/net/ipn3ke/ipn3ke_tm.c
> @@ -1511,10 +1511,4 @@ ipn3ke_tm_hierarchy_commit_check(struct
> rte_eth_dev *dev,
> NULL,
> rte_strerror(EINVAL));
> - } else {
> - return -rte_tm_error_set(error,
> - EINVAL,
> -
> RTE_TM_ERROR_TYPE_UNSPECIFIED,
> - NULL,
> - rte_strerror(EINVAL));
> }
> }
> --
> 2.21.0
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2019-11-08 14:52 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20191001125315.6191-1-ktraynor@redhat.com>
2019-10-01 12:53 ` [dpdk-stable] [PATCH 1/9] net/pcap: fix argument checks Kevin Traynor
2019-10-04 10:57 ` Ferriter, Cian
2019-10-30 7:52 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-11-05 16:40 ` Kevin Traynor
2019-11-05 17:10 ` Ferriter, Cian
2019-11-06 19:03 ` Kevin Traynor
2019-10-01 13:03 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference Kevin Traynor
2019-10-01 13:03 ` [dpdk-stable] [PATCH 3/9] net/bnxt: remove logically dead code Kevin Traynor
2019-10-02 1:28 ` [dpdk-stable] [dpdk-dev] " Ajit Khaparde
2019-10-30 7:43 ` David Marchand
2019-10-30 16:27 ` Ajit Khaparde
2019-11-05 15:39 ` Kevin Traynor
2019-10-01 13:04 ` [dpdk-stable] [PATCH 4/9] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
2019-10-30 7:59 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-11-05 15:41 ` Kevin Traynor
2019-11-08 14:45 ` Xu, Rosen
2019-11-08 14:47 ` Kevin Traynor
2019-11-08 14:35 ` Xu, Rosen
2019-11-08 14:52 ` [dpdk-stable] " Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 5/9] net/ipn3ke: remove useless if statement Kevin Traynor
2019-10-30 8:01 ` David Marchand
2019-11-08 14:35 ` Xu, Rosen
2019-11-08 14:52 ` Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 6/9] net/ipn3ke: remove commented out code Kevin Traynor
2019-10-30 8:04 ` David Marchand
2019-11-08 14:40 ` Xu, Rosen
2019-11-08 14:51 ` Xu, Rosen
2019-10-01 13:04 ` [dpdk-stable] [PATCH 7/9] compress/octeontx: " Kevin Traynor
2019-10-30 8:06 ` [dpdk-stable] [dpdk-dev] " David Marchand
2019-10-01 13:04 ` [dpdk-stable] [PATCH 8/9] event/opdl: " Kevin Traynor
2019-10-03 10:50 ` Liang, Ma
2019-10-01 13:04 ` [dpdk-stable] [PATCH 9/9] net/bnxt: " Kevin Traynor
2019-10-01 15:42 ` Ajit Khaparde
2019-10-30 7:56 ` [dpdk-stable] [PATCH 2/9] crypto/octeontx: fix possible NULL deference David Marchand
[not found] ` <20191106190203.10750-1-ktraynor@redhat.com>
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 1/8] net/pcap: fix argument checks Kevin Traynor
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 2/8] common/cpt: fix possible NULL deference Kevin Traynor
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 3/8] net/ipn3ke: fix incorrect commit check logic Kevin Traynor
2019-11-08 14:50 ` Xu, Rosen
2019-11-06 19:01 ` [dpdk-stable] [v2 PATCH 4/8] net/ipn3ke: remove useless if statement Kevin Traynor
2019-11-08 14:49 ` Xu, Rosen
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 5/8] net/ipn3ke: remove commented out code Kevin Traynor
2019-11-08 14:50 ` Xu, Rosen
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 6/8] compress/octeontx: " Kevin Traynor
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 7/8] event/opdl: " Kevin Traynor
2019-11-06 19:02 ` [dpdk-stable] [v2 PATCH 8/8] net/bnxt: " Kevin Traynor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).