[PATCH 00/16] small bug fixes from PVS studio bug list

[PATCH 00/16] small bug fixes from PVS studio bug list

[Stephen Hemminger]

More bug fixes from PVS studio bug reports.
And one other fix to ptpclient.

Stephen Hemminger (16):
  common/cnxk: remove duplicate condition
  net/cpfl: avoid calling log (printf) with null
  raw/cnxk_gpio: fix file descriptor leak
  net/ntnic: remove dead code
  net/i40e: remove duplicate code
  eal: fix out of bounds access in devargs
  net/qede: fix missing debug string
  examples/ptpclient: replace rte_memcpy with assignment
  examples/ptpclient: fix self memcmp
  net/octeon_ep: remove duplicate code
  net/hinic: fix flow type bitmask overflow
  crypto/dpaa2_sec: fix bitmask truncation
  crypto/dpaa_sec: fix bitmask truncation
  event/dpaa: fix bitmask truncation
  net/dpaa: fix bitmask truncation
  net/dpaa2: fix bitmask truncation

 drivers/common/cnxk/cnxk_security.c         | 16 ++++++++------
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c |  8 +++----
 drivers/crypto/dpaa_sec/dpaa_sec.c          |  7 +++---
 drivers/event/dpaa/dpaa_eventdev.c          | 10 ++++-----
 drivers/net/cpfl/cpfl_ethdev.c              |  2 +-
 drivers/net/dpaa/dpaa_rxtx.c                |  7 +++---
 drivers/net/dpaa2/dpaa2_rxtx.c              |  6 +++---
 drivers/net/hinic/hinic_pmd_flow.c          | 14 ++++++------
 drivers/net/i40e/i40e_fdir.c                | 10 ++++-----
 drivers/net/ntnic/ntnic_ethdev.c            |  8 -------
 drivers/net/octeon_ep/otx_ep_ethdev.c       |  9 ++------
 drivers/net/qede/qede_debug.c               |  3 +++
 drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c  | 24 +++++++++++++--------
 examples/ptpclient/ptpclient.c              | 10 +++------
 lib/eal/common/eal_common_devargs.c         |  2 +-
 15 files changed, 64 insertions(+), 72 deletions(-)

-- 
2.45.2

[PATCH 01/16] common/cnxk: remove duplicate condition

[Stephen Hemminger]

The same condition is checked twice in an if statement.
Harmless, but redundant.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/cnxk/cnxk_security.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/common/cnxk/cnxk_security.c b/drivers/common/cnxk/cnxk_security.c
index c2871ad2bd..9446c14ac8 100644
--- a/drivers/common/cnxk/cnxk_security.c
+++ b/drivers/common/cnxk/cnxk_security.c
@@ -174,9 +174,11 @@ ot_ipsec_sa_common_param_fill(union roc_ot_ipsec_sa_word2 *w2, uint8_t *cipher_k
 	}
 
 	/* Set AES key length */
-	if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC || w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR || w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
-	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM || w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
+	if (w2->s.enc_type == ROC_IE_SA_ENC_AES_CBC ||
+	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CTR ||
+	    w2->s.enc_type == ROC_IE_SA_ENC_AES_GCM ||
+	    w2->s.enc_type == ROC_IE_SA_ENC_AES_CCM ||
+	    w2->s.auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
 		switch (length) {
 		case ROC_CPT_AES128_KEY_LEN:
 			w2->s.aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
@@ -879,9 +881,11 @@ on_ipsec_sa_ctl_set(struct rte_security_ipsec_xform *ipsec,
 	}
 
 	/* Set AES key length */
-	if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC || ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||
-	    ctl->enc_type == ROC_IE_SA_ENC_AES_CTR || ctl->enc_type == ROC_IE_SA_ENC_AES_GCM ||
-	    ctl->enc_type == ROC_IE_SA_ENC_AES_CCM || ctl->auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
+	if (ctl->enc_type == ROC_IE_SA_ENC_AES_CBC ||
+	    ctl->enc_type == ROC_IE_SA_ENC_AES_CTR ||
+	    ctl->enc_type == ROC_IE_SA_ENC_AES_GCM ||
+	    ctl->enc_type == ROC_IE_SA_ENC_AES_CCM ||
+	    ctl->auth_type == ROC_IE_SA_AUTH_AES_GMAC) {
 		switch (aes_key_len) {
 		case 16:
 			ctl->aes_key_len = ROC_IE_SA_AES_KEY_LEN_128;
-- 
2.45.2

[PATCH 02/16] net/cpfl: avoid calling log (printf) with null

[Stephen Hemminger]

The log message would always refer to str variable which
is NULL here. Looks like author intended to print original
parameter.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cpfl/cpfl_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
index 6f6707a0bd..1817221652 100644
--- a/drivers/net/cpfl/cpfl_ethdev.c
+++ b/drivers/net/cpfl/cpfl_ethdev.c
@@ -1580,7 +1580,7 @@ parse_repr(const char *key __rte_unused, const char *value, void *args)
 		RTE_DIM(eth_da->representor_ports));
 done:
 	if (str == NULL) {
-		PMD_DRV_LOG(ERR, "wrong representor format: %s", str);
+		PMD_DRV_LOG(ERR, "wrong representor format: %s", value);
 		return -1;
 	}
 
-- 
2.45.2

[PATCH 03/16] raw/cnxk_gpio: fix file descriptor leak

[Stephen Hemminger]

The function would leak file if fscanf failed.
There is a working version in other file, clone that.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 0e6557b448fa ("raw/cnxk_gpio: add self test")
Cc: tduszynski@marvell.com

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c b/drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c
index 2f3973a7b5..a0d9942f20 100644
--- a/drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c
+++ b/drivers/raw/cnxk_gpio/cnxk_gpio_selftest.c
@@ -34,24 +34,30 @@ cnxk_gpio_attr_exists(const char *attr)
 static int
 cnxk_gpio_read_attr(char *attr, char *val)
 {
+	int ret, ret2;
 	FILE *fp;
-	int ret;
 
 	fp = fopen(attr, "r");
 	if (!fp)
 		return -errno;
 
 	ret = fscanf(fp, "%s", val);
-	if (ret < 0)
-		return -errno;
-	if (ret != 1)
-		return -EIO;
+	if (ret < 0) {
+		ret = -errno;
+		goto out;
+	}
+	if (ret != 1) {
+		ret = -EIO;
+		goto out;
+	}
 
-	ret = fclose(fp);
-	if (ret)
-		return -errno;
+	ret = 0;
+out:
+	ret2 = fclose(fp);
+	if (!ret)
+		ret = ret2;
 
-	return 0;
+	return ret;
 }
 
 #define CNXK_GPIO_ERR_STR(err, str, ...) do {                                  \
-- 
2.45.2

[PATCH 04/16] net/ntnic: remove dead code

[Stephen Hemminger]

The loop to update speed would not work because num_port_speeds
was always zero so it did nothing. And the array of pls_mbps
was only used inside the loop but never set.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ntnic/ntnic_ethdev.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c
index 2a2643a106..467fea4bf2 100644
--- a/drivers/net/ntnic/ntnic_ethdev.c
+++ b/drivers/net/ntnic/ntnic_ethdev.c
@@ -2037,8 +2037,6 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
 	uint32_t exception_path = 0;
 	struct flow_queue_id_s queue_ids[MAX_QUEUES];
 	int n_phy_ports;
-	struct port_link_speed pls_mbps[NUM_ADAPTER_PORTS_MAX] = { 0 };
-	int num_port_speeds = 0;
 	enum flow_eth_dev_profile profile = FLOW_ETH_DEV_PROFILE_INLINE;
 
 	NT_LOG_DBGX(DBG, NTNIC, "Dev %s PF #%i Init : %02x:%02x:%i", pci_dev->name,
@@ -2178,12 +2176,6 @@ nthw_pci_dev_init(struct rte_pci_device *pci_dev)
 	p_nt_drv->b_shutdown = false;
 	p_nt_drv->adapter_info.pb_shutdown = &p_nt_drv->b_shutdown;
 
-	for (int i = 0; i < num_port_speeds; ++i) {
-		struct adapter_info_s *p_adapter_info = &p_nt_drv->adapter_info;
-		nt_link_speed_t link_speed = convert_link_speed(pls_mbps[i].link_speed);
-		port_ops->set_link_speed(p_adapter_info, i, link_speed);
-	}
-
 	/* store context */
 	store_pdrv(p_drv);
 
-- 
2.45.2

[PATCH 05/16] net/i40e: remove duplicate code

[Stephen Hemminger]

There are two branches in the cascading if/else that have same
condition and code; remove one. Update the code to follow DPDK
style where all statements in if should have brackets if any
leg requires them.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/i40e/i40e_fdir.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 47f79ecf11..6861bea99a 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -599,18 +599,16 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf *pf,
 		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
 			len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
 					len, ether_type);
-		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
-			len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
-					len, ether_type);
-		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6)
+		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6) {
 			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_ESP,
 					len, ether_type);
-		else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6_UDP)
+		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6_UDP) {
 			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_UDP,
 					len, ether_type);
-		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3)
+		} else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3) {
 			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_L2TP,
 					len, ether_type);
+		}
 	} else {
 		PMD_DRV_LOG(ERR, "unknown pctype %u.", fdir_input->pctype);
 		return -1;
-- 
2.45.2

[PATCH 06/16] eal: fix out of bounds access in devargs

[Stephen Hemminger]

The code for parsing layers in devargs could reference past
the end of layers[] array on stack.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 9a1a9e4a2ddd ("devargs: support path value with global device syntax")
Cc: xuemingl@nvidia.com
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_devargs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c
index a64805b268..dd857fc839 100644
--- a/lib/eal/common/eal_common_devargs.c
+++ b/lib/eal/common/eal_common_devargs.c
@@ -88,7 +88,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
 	s = devargs->data;
 
 	while (s != NULL) {
-		if (nblayer > RTE_DIM(layers)) {
+		if (nblayer >= RTE_DIM(layers)) {
 			ret = -E2BIG;
 			goto get_out;
 		}
-- 
2.45.2

[PATCH 07/16] net/qede: fix missing debug string

[Stephen Hemminger]

The array of debug status strings did not match possible enum
values. Add the missing element and a static assert to make sure
the table has all possible values.

For more complete description see.
Link: https://pvs-studio.com/en/blog/posts/cpp/1176/

Fixes: ec55c118792b ("net/qede: add infrastructure for debug data collection")
Cc: rmody@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/qede/qede_debug.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/qede/qede_debug.c b/drivers/net/qede/qede_debug.c
index 18f2d988fb..4997f98a5d 100644
--- a/drivers/net/qede/qede_debug.c
+++ b/drivers/net/qede/qede_debug.c
@@ -5614,6 +5614,8 @@ static const char * const s_status_str[] = {
 	/* DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS */
 	"The filter/trigger constraint dword offsets are not enabled for recording",
 
+	/* DBG_STATUS_NO_MATCHING_FRAMING_MODE */
+	"No matching frame mode",
 
 	/* DBG_STATUS_VFC_READ_ERROR */
 	"Error reading from VFC",
@@ -5759,6 +5761,7 @@ static const char * const s_status_str[] = {
 	/* DBG_STATUS_MISSING_TRIGGER_STATE_STORM */
 	"When triggering on Storm data, the Storm to trigger on must be specified"
 };
+static_assert(MAX_DBG_STATUS == RTE_DIM(s_status_str), "status string table mismatch");
 
 /* Idle check severity names array */
 static const char * const s_idle_chk_severity_str[] = {
-- 
2.45.2

[PATCH 08/16] examples/ptpclient: replace rte_memcpy with assignment

[Stephen Hemminger]

Don't use rte_memcpy() when not necessary. Structure assignment
is as fast and type safe.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/ptpclient/ptpclient.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 23fa487081..2ec532d058 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -415,9 +415,7 @@ parse_sync(struct ptpv2_time_receiver_ordinary *ptp_data, uint16_t rx_tstamp_idx
 	ptp_data->seqID_SYNC = rte_be_to_cpu_16(ptp_hdr->seq_id);
 
 	if (ptp_data->ptpset == 0) {
-		rte_memcpy(&ptp_data->transmitter_clock_id,
-				&ptp_hdr->source_port_id.clock_id,
-				sizeof(struct clock_id));
+		ptp_data->transmitter_clock_id = ptp_hdr->source_port_id.clock_id;
 		ptp_data->ptpset = 1;
 	}
 
@@ -522,9 +520,7 @@ parse_fup(struct ptpv2_time_receiver_ordinary *ptp_data)
 		client_clkid->id[6] = eth_hdr->src_addr.addr_bytes[4];
 		client_clkid->id[7] = eth_hdr->src_addr.addr_bytes[5];
 
-		rte_memcpy(&ptp_data->client_clock_id,
-			   client_clkid,
-			   sizeof(struct clock_id));
+		ptp_data->client_clock_id = *client_clkid;
 
 		/* Enable flag for hardware timestamping. */
 		created_pkt->ol_flags |= RTE_MBUF_F_TX_IEEE1588_TMST;
-- 
2.45.2

[PATCH 09/16] examples/ptpclient: fix self memcmp

[Stephen Hemminger]

Calling memcmp on same structure will always be true.
Replace with same conditional used elsewhere.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
Cc: danielx.t.mrzyglod@intel.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/ptpclient/ptpclient.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
index 2ec532d058..d6dff2eb7e 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -419,7 +419,7 @@ parse_sync(struct ptpv2_time_receiver_ordinary *ptp_data, uint16_t rx_tstamp_idx
 		ptp_data->ptpset = 1;
 	}
 
-	if (memcmp(&ptp_hdr->source_port_id.clock_id,
+	if (memcmp(&ptp_data->transmitter_clock_id,
 			&ptp_hdr->source_port_id.clock_id,
 			sizeof(struct clock_id)) == 0) {
 
-- 
2.45.2

[PATCH 10/16] net/octeon_ep: remove duplicate code

[Stephen Hemminger]

Both sides of the if in uninit are using same code.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/octeon_ep/otx_ep_ethdev.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/octeon_ep/otx_ep_ethdev.c b/drivers/net/octeon_ep/otx_ep_ethdev.c
index b4f8baf3b3..8b14734b0c 100644
--- a/drivers/net/octeon_ep/otx_ep_ethdev.c
+++ b/drivers/net/octeon_ep/otx_ep_ethdev.c
@@ -721,14 +721,9 @@ static const struct eth_dev_ops otx_ep_eth_dev_ops = {
 static int
 otx_ep_eth_dev_uninit(struct rte_eth_dev *eth_dev)
 {
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		eth_dev->dev_ops = NULL;
-		eth_dev->rx_pkt_burst = NULL;
-		eth_dev->tx_pkt_burst = NULL;
-		return 0;
-	}
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		otx_ep_mbox_uninit(eth_dev);
 
-	otx_ep_mbox_uninit(eth_dev);
 	eth_dev->dev_ops = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
-- 
2.45.2

[PATCH 11/16] net/hinic: fix flow type bitmask overflow

[Stephen Hemminger]

The type mask is 64 bit value, doing a shift of literal 1 (32 bit)
will result in int type (32 bit) and cause truncation.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/
Fixes: f4ca3fd54c4d ("net/hinic: create and destroy flow director filter")
Cc: cloud.wangxiaoyun@huawei.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/hinic/hinic_pmd_flow.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/hinic/hinic_pmd_flow.c b/drivers/net/hinic/hinic_pmd_flow.c
index 8fdd5a35be..6b1ca6ff88 100644
--- a/drivers/net/hinic/hinic_pmd_flow.c
+++ b/drivers/net/hinic/hinic_pmd_flow.c
@@ -1979,8 +1979,8 @@ static int hinic_lookup_new_filter(struct hinic_5tuple_filter *filter,
 		return -EINVAL;
 	}
 
-	if (!(filter_info->type_mask & (1 << type_id))) {
-		filter_info->type_mask |= 1 << type_id;
+	if (!(filter_info->type_mask & (UINT64_C(1) << type_id))) {
+		filter_info->type_mask |= UINT64_C(1) << type_id;
 		filter->index = type_id;
 		filter_info->pkt_filters[type_id].enable = true;
 		filter_info->pkt_filters[type_id].pkt_proto =
@@ -2138,7 +2138,7 @@ static void hinic_remove_5tuple_filter(struct rte_eth_dev *dev,
 	filter_info->pkt_type = 0;
 	filter_info->qid = 0;
 	filter_info->pkt_filters[filter->index].qid = 0;
-	filter_info->type_mask &= ~(1 <<  (filter->index));
+	filter_info->type_mask &= ~(UINT64_C(1) << filter->index);
 	TAILQ_REMOVE(&filter_info->fivetuple_list, filter, entries);
 
 	rte_free(filter);
@@ -2268,8 +2268,8 @@ hinic_ethertype_filter_insert(struct hinic_filter_info *filter_info,
 	if (id < 0)
 		return -EINVAL;
 
-	if (!(filter_info->type_mask & (1 << id))) {
-		filter_info->type_mask |= 1 << id;
+	if (!(filter_info->type_mask & (UINT64_C(1) << id))) {
+		filter_info->type_mask |= UINT64_C(1) << id;
 		filter_info->pkt_filters[id].pkt_proto =
 			ethertype_filter->pkt_proto;
 		filter_info->pkt_filters[id].enable = ethertype_filter->enable;
@@ -2289,7 +2289,7 @@ hinic_ethertype_filter_remove(struct hinic_filter_info *filter_info,
 		return;
 
 	filter_info->pkt_type = 0;
-	filter_info->type_mask &= ~(1 << idx);
+	filter_info->type_mask &= ~(UINT64_C(1) << idx);
 	filter_info->pkt_filters[idx].pkt_proto = (uint16_t)0;
 	filter_info->pkt_filters[idx].enable = FALSE;
 	filter_info->pkt_filters[idx].qid = 0;
@@ -2355,7 +2355,7 @@ hinic_add_del_ethertype_filter(struct rte_eth_dev *dev,
 		if (i < 0)
 			return -EINVAL;
 
-		if ((filter_info->type_mask & (1 << i))) {
+		if ((filter_info->type_mask & (UINT64_C(1) << i))) {
 			filter_info->pkt_filters[i].enable = FALSE;
 			(void)hinic_set_fdir_filter(nic_dev->hwdev,
 					filter_info->pkt_type,
-- 
2.45.2

[PATCH 12/16] crypto/dpaa2_sec: fix bitmask truncation

[Stephen Hemminger]

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: a77db24643b7 ("crypto/dpaa2_sec: support atomic queues")
Cc: ashish.jain@nxp.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index ec6577f64c..7ad8fd47dd 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -1491,8 +1491,8 @@ dpaa2_sec_enqueue_burst(void *qp, struct rte_crypto_op **ops,
 			if (*dpaa2_seqn((*ops)->sym->m_src)) {
 				if (*dpaa2_seqn((*ops)->sym->m_src) & QBMAN_ENQUEUE_FLAG_DCA) {
 					DPAA2_PER_LCORE_DQRR_SIZE--;
-					DPAA2_PER_LCORE_DQRR_HELD &= ~(1 <<
-					*dpaa2_seqn((*ops)->sym->m_src) &
+					DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) <<
+						*dpaa2_seqn((*ops)->sym->m_src) &
 					QBMAN_EQCR_DCA_IDXMASK);
 				}
 				flags[loop] = *dpaa2_seqn((*ops)->sym->m_src);
@@ -1772,7 +1772,7 @@ dpaa2_sec_set_enqueue_descriptor(struct dpaa2_queue *dpaa2_q,
 		dq_idx = *dpaa2_seqn(m) - 1;
 		qbman_eq_desc_set_dca(eqdesc, 1, dq_idx, 0);
 		DPAA2_PER_LCORE_DQRR_SIZE--;
-		DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dq_idx);
+		DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dq_idx);
 	}
 	*dpaa2_seqn(m) = DPAA2_INVALID_MBUF_SEQN;
 }
@@ -4055,7 +4055,7 @@ dpaa2_sec_process_atomic_event(struct qbman_swp *swp __rte_unused,
 	dqrr_index = qbman_get_dqrr_idx(dq);
 	*dpaa2_seqn(crypto_op->sym->m_src) = QBMAN_ENQUEUE_FLAG_DCA | dqrr_index;
 	DPAA2_PER_LCORE_DQRR_SIZE++;
-	DPAA2_PER_LCORE_DQRR_HELD |= 1 << dqrr_index;
+	DPAA2_PER_LCORE_DQRR_HELD |= UINT64_C(1) << dqrr_index;
 	DPAA2_PER_LCORE_DQRR_MBUF(dqrr_index) = crypto_op->sym->m_src;
 	ev->event_ptr = crypto_op;
 }
-- 
2.45.2

[PATCH 13/16] crypto/dpaa_sec: fix bitmask truncation

[Stephen Hemminger]

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: fe3688ba7950 ("crypto/dpaa_sec: support event crypto adapter")
Cc: akhil.goyal@nxp.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/dpaa_sec/dpaa_sec.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c b/drivers/crypto/dpaa_sec/dpaa_sec.c
index 3fa88ca968..e117cd77a6 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -1907,13 +1907,12 @@ dpaa_sec_enqueue_burst(void *qp, struct rte_crypto_op **ops,
 			op = *(ops++);
 			if (*dpaa_seqn(op->sym->m_src) != 0) {
 				index = *dpaa_seqn(op->sym->m_src) - 1;
-				if (DPAA_PER_LCORE_DQRR_HELD & (1 << index)) {
+				if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << index)) {
 					/* QM_EQCR_DCA_IDXMASK = 0x0f */
 					flags[loop] = ((index & 0x0f) << 8);
 					flags[loop] |= QMAN_ENQUEUE_FLAG_DCA;
 					DPAA_PER_LCORE_DQRR_SIZE--;
-					DPAA_PER_LCORE_DQRR_HELD &=
-								~(1 << index);
+					DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << index);
 				}
 			}
 
@@ -3500,7 +3499,7 @@ dpaa_sec_process_atomic_event(void *event,
 	/* Save active dqrr entries */
 	index = ((uintptr_t)dqrr >> 6) & (16/*QM_DQRR_SIZE*/ - 1);
 	DPAA_PER_LCORE_DQRR_SIZE++;
-	DPAA_PER_LCORE_DQRR_HELD |= 1 << index;
+	DPAA_PER_LCORE_DQRR_HELD |= UINT64_C(1) << index;
 	DPAA_PER_LCORE_DQRR_MBUF(index) = ctx->op->sym->m_src;
 	ev->impl_opaque = index + 1;
 	*dpaa_seqn(ctx->op->sym->m_src) = (uint32_t)index + 1;
-- 
2.45.2

[PATCH 14/16] event/dpaa: fix bitmask truncation

[Stephen Hemminger]

More bitmask truncation from mask computation.

Fixes: 0ee17f79ebd0 ("event/dpaa: add enqueue/dequeue")
Cc: sunil.kori@nxp.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/event/dpaa/dpaa_eventdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/event/dpaa/dpaa_eventdev.c b/drivers/event/dpaa/dpaa_eventdev.c
index 853cc1ecf9..400e0ecd1c 100644
--- a/drivers/event/dpaa/dpaa_eventdev.c
+++ b/drivers/event/dpaa/dpaa_eventdev.c
@@ -102,7 +102,7 @@ dpaa_event_enqueue_burst(void *port, const struct rte_event ev[],
 			qman_dca_index(ev[i].impl_opaque, 0);
 			mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
 			*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-			DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+			DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
 			DPAA_PER_LCORE_DQRR_SIZE--;
 			break;
 		default:
@@ -199,11 +199,11 @@ dpaa_event_dequeue_burst(void *port, struct rte_event ev[],
 	/* Check if there are atomic contexts to be released */
 	i = 0;
 	while (DPAA_PER_LCORE_DQRR_SIZE) {
-		if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+		if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
 			qman_dca_index(i, 0);
 			mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
 			*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-			DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+			DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
 			DPAA_PER_LCORE_DQRR_SIZE--;
 		}
 		i++;
@@ -263,11 +263,11 @@ dpaa_event_dequeue_burst_intr(void *port, struct rte_event ev[],
 	/* Check if there are atomic contexts to be released */
 	i = 0;
 	while (DPAA_PER_LCORE_DQRR_SIZE) {
-		if (DPAA_PER_LCORE_DQRR_HELD & (1 << i)) {
+		if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << i)) {
 			qman_dca_index(i, 0);
 			mbuf = DPAA_PER_LCORE_DQRR_MBUF(i);
 			*dpaa_seqn(mbuf) = DPAA_INVALID_MBUF_SEQN;
-			DPAA_PER_LCORE_DQRR_HELD &= ~(1 << i);
+			DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << i);
 			DPAA_PER_LCORE_DQRR_SIZE--;
 		}
 		i++;
-- 
2.45.2

[PATCH 15/16] net/dpaa: fix bitmask truncation

[Stephen Hemminger]

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 5e7455931442 ("net/dpaa: support Rx queue configurations with eventdev")
Cc: sunil.kori@nxp.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa/dpaa_rxtx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dpaa/dpaa_rxtx.c b/drivers/net/dpaa/dpaa_rxtx.c
index 247e7b92ba..05bd73becf 100644
--- a/drivers/net/dpaa/dpaa_rxtx.c
+++ b/drivers/net/dpaa/dpaa_rxtx.c
@@ -842,7 +842,7 @@ dpaa_rx_cb_atomic(void *event,
 	/* Save active dqrr entries */
 	index = DQRR_PTR2IDX(dqrr);
 	DPAA_PER_LCORE_DQRR_SIZE++;
-	DPAA_PER_LCORE_DQRR_HELD |= 1 << index;
+	DPAA_PER_LCORE_DQRR_HELD |= UINT64_C(1) << index;
 	DPAA_PER_LCORE_DQRR_MBUF(index) = mbuf;
 	ev->impl_opaque = index + 1;
 	*dpaa_seqn(mbuf) = (uint32_t)index + 1;
@@ -1338,13 +1338,12 @@ dpaa_eth_queue_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 			seqn = *dpaa_seqn(mbuf);
 			if (seqn != DPAA_INVALID_MBUF_SEQN) {
 				index = seqn - 1;
-				if (DPAA_PER_LCORE_DQRR_HELD & (1 << index)) {
+				if (DPAA_PER_LCORE_DQRR_HELD & (UINT64_C(1) << index)) {
 					flags[loop] =
 					   ((index & QM_EQCR_DCA_IDXMASK) << 8);
 					flags[loop] |= QMAN_ENQUEUE_FLAG_DCA;
 					DPAA_PER_LCORE_DQRR_SIZE--;
-					DPAA_PER_LCORE_DQRR_HELD &=
-								~(1 << index);
+					DPAA_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << index);
 				}
 			}
 
-- 
2.45.2

[PATCH 16/16] net/dpaa2: fix bitmask truncation

[Stephen Hemminger]

The dqrr_held mask is 64 bit but updates were getting truncated
because 1 is of type int (32 bit) and the result shift of int is of
type int (32 bit); therefore any value >= 32 would get truncated.

Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 2d3788631862 ("net/dpaa2: support atomic queues")
Cc: nipun.gupta@nxp.com
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/dpaa2/dpaa2_rxtx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_rxtx.c b/drivers/net/dpaa2/dpaa2_rxtx.c
index e3b6c7e460..e253bccecd 100644
--- a/drivers/net/dpaa2/dpaa2_rxtx.c
+++ b/drivers/net/dpaa2/dpaa2_rxtx.c
@@ -933,7 +933,7 @@ dpaa2_dev_process_atomic_event(struct qbman_swp *swp __rte_unused,
 	dqrr_index = qbman_get_dqrr_idx(dq);
 	*dpaa2_seqn(ev->mbuf) = dqrr_index + 1;
 	DPAA2_PER_LCORE_DQRR_SIZE++;
-	DPAA2_PER_LCORE_DQRR_HELD |= 1 << dqrr_index;
+	DPAA2_PER_LCORE_DQRR_HELD |= UINT64_C(1) << dqrr_index;
 	DPAA2_PER_LCORE_DQRR_MBUF(dqrr_index) = ev->mbuf;
 }
 
@@ -1317,7 +1317,7 @@ dpaa2_dev_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 				flags[loop] = QBMAN_ENQUEUE_FLAG_DCA |
 						dqrr_index;
 				DPAA2_PER_LCORE_DQRR_SIZE--;
-				DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dqrr_index);
+				DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dqrr_index);
 				*dpaa2_seqn(*bufs) = DPAA2_INVALID_MBUF_SEQN;
 			}
 
@@ -1575,7 +1575,7 @@ dpaa2_set_enqueue_descriptor(struct dpaa2_queue *dpaa2_q,
 		dq_idx = *dpaa2_seqn(m) - 1;
 		qbman_eq_desc_set_dca(eqdesc, 1, dq_idx, 0);
 		DPAA2_PER_LCORE_DQRR_SIZE--;
-		DPAA2_PER_LCORE_DQRR_HELD &= ~(1 << dq_idx);
+		DPAA2_PER_LCORE_DQRR_HELD &= ~(UINT64_C(1) << dq_idx);
 	}
 	*dpaa2_seqn(m) = DPAA2_INVALID_MBUF_SEQN;
 }
-- 
2.45.2

RE: [EXTERNAL] [PATCH 01/16] common/cnxk: remove duplicate condition

[Anoob Joseph]

> The same condition is checked twice in an if statement.
> Harmless, but redundant.
>
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__pvs-2Dstudio.com_en_blog_posts_cpp_1183_&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRSxyLWs2n6B-
> WYLn1v9SyTMrT5EQqh2TU&m=tCMuFtN3iZvm6NW4QbOzKChuntNLulIgTuVpxYI9t8tfV9TfaTSFRx49kikIS84j&s=2cGhW06MxChL2e5aNV_DQOLM1lQUuNBQKHQeLsS3IdE&e=
>
> Signed-off-by: Stephen Hemminger <mailto:stephen@networkplumber.org>

Acked-by: Anoob Joseph <anoobj@marvell.com>

Re: [PATCH 05/16] net/i40e: remove duplicate code

[Bruce Richardson]

On Thu, Nov 14, 2024 at 10:05:42PM -0800, Stephen Hemminger wrote:
> There are two branches in the cascading if/else that have same
> condition and code; remove one. Update the code to follow DPDK
> style where all statements in if should have brackets if any
> leg requires them.
> 

Not actually DPDK style, that is just something that checkpatch recommends
because it is kernel style. DPDK style guide says[1] "Braces that are not
necessary should be left out."
That said, most legs of this if-else block have it so ok to have that
change included for consistency.

[1] https://doc.dpdk.org/guides/contributing/coding_style.html#control-statements-and-loops

> Link: https://pvs-studio.com/en/blog/posts/cpp/1183/

Fixes: 2ab5c84605f0 ("net/i40e: fix ESP flow creation")

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/i40e/i40e_fdir.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 47f79ecf11..6861bea99a 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -599,18 +599,16 @@ i40e_flow_fdir_fill_eth_ip_head(struct i40e_pf *pf,
>  		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
>  			len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
>  					len, ether_type);
> -		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
> -			len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
> -					len, ether_type);
> -		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6)
> +		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6) {
>  			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_ESP,
>  					len, ether_type);
> -		else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6_UDP)
> +		} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6_UDP) {
>  			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_UDP,
>  					len, ether_type);
> -		else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3)
> +		} else if (cus_pctype->index == I40E_CUSTOMIZED_IPV6_L2TPV3) {
>  			len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_L2TP,
>  					len, ether_type);
> +		}
>  	} else {
>  		PMD_DRV_LOG(ERR, "unknown pctype %u.", fdir_input->pctype);
>  		return -1;
> -- 
> 2.45.2
>