DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 00/10] require checking ethdev get return value
@ 2024-10-04 16:21 Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 01/10] app/test: remove redundant call Stephen Hemminger
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Several places flagged by Coverity and Codeql are from code that
calls rte_eth_dev_info_get() but does not check the return value.
If rte_eth_dev_info_get() returns an error, the device info is garbage.

This patch series uses the function attribute to force code
to check the result or there will be a compiler warning.

The series is ordered to fix the current warnings first
before enabling it.

Stephen Hemminger (10):
  app/test: remove redundant call
  net/memif: check return value from rte_eth_dev_info_get
  graph: check return value from rte_eth_dev_info_get
  examples/ethtool: handle devices without registers
  examples/l3fwd: check return value from ethdev info
  examples/ntb: always check return value
  examples/pipeline: check return value of ethdev functions
  examples/qos_sched: check return value from rte_eth_link_get
  ethdev: check return value from rte_eth_dev_info_get
  ethdev: require checking results of info_get functions

 app/graph/ethdev.c                   | 20 ++++++++++++----
 app/test/test_link_bonding_rssconf.c |  1 -
 drivers/net/memif/rte_eth_memif.c    | 12 ++++++++--
 examples/ethtool/lib/rte_ethtool.c   |  6 ++---
 examples/l3fwd-graph/main.c          | 12 ++++++++--
 examples/l3fwd/l3fwd_fib.c           | 16 +++++++++----
 examples/l3fwd/l3fwd_lpm.c           | 14 ++++++++----
 examples/ntb/ntb_fwd.c               |  5 +++-
 examples/pipeline/cli.c              |  7 +++---
 examples/qos_sched/init.c            | 13 +++++++++--
 lib/ethdev/rte_class_eth.c           |  4 +++-
 lib/ethdev/rte_ethdev.h              | 34 +++++++++++++++++-----------
 12 files changed, 103 insertions(+), 41 deletions(-)

-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/10] app/test: remove redundant call
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get Stephen Hemminger
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Chas Williams, Min Hu (Connor),
	Ivan Ilchenko, Andrew Rybchenko, Ferruh Yigit

The patch to check return value of rte_eth_dev_info_get
added a duplicate call in one spot.

Fixes: 773392553bed ("app: check status of getting ethdev info")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_link_bonding_rssconf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test/test_link_bonding_rssconf.c b/app/test/test_link_bonding_rssconf.c
index 3c9c824335..2cb689b1de 100644
--- a/app/test/test_link_bonding_rssconf.c
+++ b/app/test/test_link_bonding_rssconf.c
@@ -616,7 +616,6 @@ test_setup(void)
 		mac_addr.addr_bytes[5] = 0x10 + port->port_id;
 		rte_eth_dev_default_mac_addr_set(port->port_id, &mac_addr);
 
-		rte_eth_dev_info_get(port->port_id, &port->dev_info);
 		retval = rte_eth_dev_info_get(port->port_id, &port->dev_info);
 		TEST_ASSERT((retval == 0),
 				"Error during getting device (port %u) info: %s\n",
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 01/10] app/test: remove redundant call Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-08  3:47   ` Ferruh Yigit
  2024-10-04 16:21 ` [PATCH 03/10] graph: " Stephen Hemminger
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Jakub Grajciar

Handle errors from rte_eth_dev_info_get in the same manner
as other places in this file.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/memif/rte_eth_memif.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index e220ffaf92..227025dd20 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -520,7 +520,10 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		return 0;
 	if (unlikely(ring == NULL)) {
 		/* Secondary process will attempt to request regions. */
-		rte_eth_link_get(mq->in_port, &link);
+		ret = rte_eth_link_get(mq->in_port, &link);
+		if (ret < 0)
+			MIF_LOG(ERR, "Failed to get port %u link info: %s",
+				mq->in_port, rte_strerror(-ret));
 		return 0;
 	}
 
@@ -864,8 +867,13 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	if (unlikely((pmd->flags & ETH_MEMIF_FLAG_CONNECTED) == 0))
 		return 0;
 	if (unlikely(ring == NULL)) {
+		int ret;
+
 		/* Secondary process will attempt to request regions. */
-		rte_eth_link_get(mq->in_port, &link);
+		ret = rte_eth_link_get(mq->in_port, &link);
+		if (ret < 0)
+			MIF_LOG(ERR, "Failed to get port %u link info: %s",
+				mq->in_port, rte_strerror(-ret));
 		return 0;
 	}
 
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 03/10] graph: check return value from rte_eth_dev_info_get
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 01/10] app/test: remove redundant call Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 04/10] examples/ethtool: handle devices without registers Stephen Hemminger
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Sunil Kumar Kori, Rakesh Kudurumalla

The data in rte_eth_dev_info is undefined if rte_eth_dev_info_get
returns an error. Handle the errors the same as previous error.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/graph/ethdev.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/app/graph/ethdev.c b/app/graph/ethdev.c
index cfc1b18569..b8ddf21938 100644
--- a/app/graph/ethdev.c
+++ b/app/graph/ethdev.c
@@ -219,10 +219,22 @@ ethdev_show(const char *name)
 	if (rc < 0)
 		return rc;
 
-	rte_eth_dev_info_get(port_id, &info);
-	rte_eth_stats_get(port_id, &stats);
-	rte_eth_macaddr_get(port_id, &addr);
-	rte_eth_link_get(port_id, &link);
+	rc = rte_eth_dev_info_get(port_id, &info);
+	if (rc < 0)
+		return rc;
+
+	rc = rte_eth_link_get(port_id, &link);
+	if (rc < 0)
+		return rc;
+
+	rc = rte_eth_stats_get(port_id, &stats);
+	if (rc < 0)
+		return rc;
+
+	rc = rte_eth_macaddr_get(port_id, &addr);
+	if (rc < 0)
+		return rc;
+
 	rte_eth_dev_get_mtu(port_id, &mtu);
 
 	length = strlen(conn->msg_out);
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 04/10] examples/ethtool: handle devices without registers
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (2 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 03/10] graph: " Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 05/10] examples/l3fwd: check return value from ethdev info Stephen Hemminger
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

If device does not support reading registers then call to
rte_eth_dev_get_reg_info will return an error.
This fixes compiler warning when warn unused result is set.

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

diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index edc28d5c63..0b88a27e7d 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -53,10 +53,8 @@ rte_ethtool_get_drvinfo(uint16_t port_id, struct ethtool_drvinfo *drvinfo)
 		sizeof(drvinfo->bus_info));
 
 	memset(&reg_info, 0, sizeof(reg_info));
-	rte_eth_dev_get_reg_info(port_id, &reg_info);
-	n = reg_info.length;
-	if (n > 0)
-		drvinfo->regdump_len = n;
+	if (rte_eth_dev_get_reg_info(port_id, &reg_info) == 0)
+		drvinfo->regdump_len = reg_info.length;
 	else
 		drvinfo->regdump_len = 0;
 
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 05/10] examples/l3fwd: check return value from ethdev info
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (3 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 04/10] examples/ethtool: handle devices without registers Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 06/10] examples/ntb: always check return value Stephen Hemminger
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, Jerin Jacob, Kiran Kumar K, Nithin Dabilpuram,
	Zhirun Yan

Need to check return value from rte_eth_dev_info_get.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/l3fwd-graph/main.c | 12 ++++++++++--
 examples/l3fwd/l3fwd_fib.c  | 16 ++++++++++++----
 examples/l3fwd/l3fwd_lpm.c  | 14 ++++++++++----
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/examples/l3fwd-graph/main.c b/examples/l3fwd-graph/main.c
index a13dc01138..26b33184e5 100644
--- a/examples/l3fwd-graph/main.c
+++ b/examples/l3fwd-graph/main.c
@@ -1088,7 +1088,10 @@ main(int argc, char **argv)
 		printf("Creating queues: nb_rxq=%d nb_txq=%u... ",
 		       nb_rx_queue, n_tx_queue);
 
-		rte_eth_dev_info_get(portid, &dev_info);
+		ret = rte_eth_dev_info_get(portid, &dev_info);
+		if (ret != 0)
+			rte_exit(EXIT_FAILURE,
+				 "Unable to get info for port %u\n", portid);
 
 		ret = config_port_max_pkt_len(&local_port_conf, &dev_info);
 		if (ret != 0)
@@ -1220,7 +1223,12 @@ main(int argc, char **argv)
 			printf("rxq=%d,%d,%d ", portid, queueid, socketid);
 			fflush(stdout);
 
-			rte_eth_dev_info_get(portid, &dev_info);
+			ret = rte_eth_dev_info_get(portid, &dev_info);
+			if (ret < 0)
+				rte_exit(EXIT_FAILURE,
+					 "rte_eth_dev_info_get: err=%d, port=%u\n",
+					 ret, portid);
+
 			rxq_conf = dev_info.default_rxconf;
 			rxq_conf.offloads = port_conf.rxmode.offloads;
 			if (!per_port_pool)
diff --git a/examples/l3fwd/l3fwd_fib.c b/examples/l3fwd/l3fwd_fib.c
index f38b19af3f..2000fac168 100644
--- a/examples/l3fwd/l3fwd_fib.c
+++ b/examples/l3fwd/l3fwd_fib.c
@@ -675,8 +675,12 @@ setup_fib(const int socketid)
 				enabled_port_mask) == 0)
 			continue;
 
-		rte_eth_dev_info_get(route_base_v4[i].if_out,
-				     &dev_info);
+		ret = rte_eth_dev_info_get(route_base_v4[i].if_out, &dev_info);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE,
+				 "Unable to get device info for port %u\n",
+				 route_base_v4[i].if_out);
+
 		ret = rte_fib_add(ipv4_l3fwd_fib_lookup_struct[socketid],
 			route_base_v4[i].ip,
 			route_base_v4[i].depth,
@@ -729,8 +733,12 @@ setup_fib(const int socketid)
 				enabled_port_mask) == 0)
 			continue;
 
-		rte_eth_dev_info_get(route_base_v6[i].if_out,
-				     &dev_info);
+		ret = rte_eth_dev_info_get(route_base_v6[i].if_out, &dev_info);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE,
+				 "Unable to get device info for port %u\n",
+				 route_base_v6[i].if_out);
+
 		ret = rte_fib6_add(ipv6_l3fwd_fib_lookup_struct[socketid],
 			route_base_v6[i].ip_8,
 			route_base_v6[i].depth,
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index e8fd95aae9..39e5d7607b 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -588,8 +588,11 @@ setup_lpm(const int socketid)
 				enabled_port_mask) == 0)
 			continue;
 
-		rte_eth_dev_info_get(route_base_v4[i].if_out,
-				     &dev_info);
+		ret = rte_eth_dev_info_get(route_base_v4[i].if_out, &dev_info);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE, "Unable to get device info for port %u\n",
+				 route_base_v4[i].if_out);
+
 		ret = rte_lpm_add(ipv4_l3fwd_lpm_lookup_struct[socketid],
 			route_base_v4[i].ip,
 			route_base_v4[i].depth,
@@ -632,8 +635,11 @@ setup_lpm(const int socketid)
 				enabled_port_mask) == 0)
 			continue;
 
-		rte_eth_dev_info_get(route_base_v6[i].if_out,
-				     &dev_info);
+		ret = rte_eth_dev_info_get(route_base_v6[i].if_out, &dev_info);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE, "Unable to get device info for port %u\n",
+				 route_base_v6[i].if_out);
+
 		ret = rte_lpm6_add(ipv6_l3fwd_lpm_lookup_struct[socketid],
 			route_base_v6[i].ip_8,
 			route_base_v6[i].depth,
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 06/10] examples/ntb: always check return value
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (4 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 05/10] examples/l3fwd: check return value from ethdev info Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 07/10] examples/pipeline: check return value of ethdev functions Stephen Hemminger
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, xiaoyun.li, Jingjing Wu

The ethdev_info is only valid if rte_eth_dev_info_get returns success.

Fixes: 5194299d6ef5 ("examples/ntb: support more functions")
Cc: xiaoyun.li@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/ntb/ntb_fwd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/examples/ntb/ntb_fwd.c b/examples/ntb/ntb_fwd.c
index 56c7672392..37d60208e3 100644
--- a/examples/ntb/ntb_fwd.c
+++ b/examples/ntb/ntb_fwd.c
@@ -1285,7 +1285,10 @@ main(int argc, char **argv)
 	eth_port_id = rte_eth_find_next(0);
 
 	if (eth_port_id < RTE_MAX_ETHPORTS) {
-		rte_eth_dev_info_get(eth_port_id, &ethdev_info);
+		ret = rte_eth_dev_info_get(eth_port_id, &ethdev_info);
+		if (ret)
+			rte_exit(EXIT_FAILURE, "Can't get info for port %u\n", eth_port_id);
+
 		eth_pconf.rx_adv_conf.rss_conf.rss_hf &=
 				ethdev_info.flow_type_rss_offloads;
 		ret = rte_eth_dev_configure(eth_port_id, num_queues,
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 07/10] examples/pipeline: check return value of ethdev functions
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (5 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 06/10] examples/ntb: always check return value Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 08/10] examples/qos_sched: check return value from rte_eth_link_get Stephen Hemminger
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Cristian Dumitrescu

The rte_eth_dev_info_get and rte_eth_link_get functions may return
an error, and in that case the information structure is undefined.
The port is valid check is not needed, that can be handled by
rte_eth_dev_info_get checks.

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

diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
index 015717cb39..215b4061d5 100644
--- a/examples/pipeline/cli.c
+++ b/examples/pipeline/cli.c
@@ -390,14 +390,15 @@ ethdev_show(uint16_t port_id, char **out, size_t *out_size)
 	uint32_t length;
 	uint16_t mtu = 0;
 
-	if (!rte_eth_dev_is_valid_port(port_id))
+	if (rte_eth_dev_info_get(port_id, &info) != 0)
+		return;
+
+	if (rte_eth_link_get(port_id, &link) != 0)
 		return;
 
 	rte_eth_dev_get_name_by_port(port_id, name);
-	rte_eth_dev_info_get(port_id, &info);
 	rte_eth_stats_get(port_id, &stats);
 	rte_eth_macaddr_get(port_id, &addr);
-	rte_eth_link_get(port_id, &link);
 	rte_eth_dev_get_mtu(port_id, &mtu);
 
 	snprintf(*out, *out_size,
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 08/10] examples/qos_sched: check return value from rte_eth_link_get
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (6 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 07/10] examples/pipeline: check return value of ethdev functions Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-04 16:21 ` [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get Stephen Hemminger
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Cristian Dumitrescu

The return status of link is undefined if call to rte_eth_link_get
returns an error.

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

diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
index 32964fd57e..ace7279c67 100644
--- a/examples/qos_sched/init.c
+++ b/examples/qos_sched/init.c
@@ -323,6 +323,7 @@ int app_init(void)
 	uint32_t i;
 	char ring_name[MAX_NAME_LEN];
 	char pool_name[MAX_NAME_LEN];
+	int ret;
 
 	if (rte_eth_dev_count_avail() == 0)
 		rte_exit(EXIT_FAILURE, "No Ethernet port - bye\n");
@@ -368,12 +369,20 @@ int app_init(void)
 		app_init_port(qos_conf[i].tx_port, qos_conf[i].mbuf_pool);
 
 		memset(&link, 0, sizeof(link));
-		rte_eth_link_get(qos_conf[i].tx_port, &link);
+		ret = rte_eth_link_get(qos_conf[i].tx_port, &link);
+		if (ret < 0)
+			rte_exit(EXIT_FAILURE,
+				 "rte_eth_link_get: err=%d, port=%u: %s\n",
+				 ret, qos_conf[i].tx_port, rte_strerror(-ret));
 		if (link.link_status == 0)
 			printf("Waiting for link on port %u\n", qos_conf[i].tx_port);
+
 		while (link.link_status == 0 && retry_count--) {
 			rte_delay_ms(retry_delay);
-			rte_eth_link_get(qos_conf[i].tx_port, &link);
+			ret = rte_eth_link_get(qos_conf[i].tx_port, &link);
+			rte_exit(EXIT_FAILURE,
+				 "rte_eth_link_get: err=%d, port=%u: %s\n",
+				 ret, qos_conf[i].tx_port, rte_strerror(-ret));
 		}
 
 		qos_conf[i].sched_port = app_init_sched_port(qos_conf[i].tx_port, socket);
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (7 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 08/10] examples/qos_sched: check return value from rte_eth_link_get Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-08  3:47   ` Ferruh Yigit
  2024-10-04 16:21 ` [PATCH 10/10] ethdev: require checking results of info_get functions Stephen Hemminger
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko

The mac compare must check return value from rte_eth_dev_info_get
before using the dev_info information.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ethdev/rte_class_eth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c
index b52f1dd9f2..43e13e02f8 100644
--- a/lib/ethdev/rte_class_eth.c
+++ b/lib/ethdev/rte_class_eth.c
@@ -50,8 +50,10 @@ eth_mac_cmp(const char *key __rte_unused,
 	if (rte_ether_unformat_addr(value, &mac) < 0)
 		return -1; /* invalid devargs value */
 
+	if (rte_eth_dev_info_get(data->port_id, &dev_info) != 0)
+		return -1; /* device mac address unavailable */
+
 	/* Return 0 if devargs MAC is matching one of the device MACs. */
-	rte_eth_dev_info_get(data->port_id, &dev_info);
 	for (index = 0; index < dev_info.max_mac_addrs; index++)
 		if (rte_is_same_ether_addr(&mac, &data->mac_addrs[index]))
 			return 0;
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 10/10] ethdev: require checking results of info_get functions
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (8 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get Stephen Hemminger
@ 2024-10-04 16:21 ` Stephen Hemminger
  2024-10-08  3:47   ` Ferruh Yigit
  2024-10-08  9:33 ` [PATCH 00/10] require checking ethdev get return value Morten Brørup
  2024-11-11 15:20 ` Thomas Monjalon
  11 siblings, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2024-10-04 16:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko

With functions that return a data structure, the application must
check the return value since the data structure contents will
be undefined in case of error.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ethdev/rte_ethdev.h | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 548fada1c7..3df8b73175 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3058,7 +3058,8 @@ int rte_eth_allmulticast_get(uint16_t port_id);
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EINVAL) if bad parameter.
  */
-int rte_eth_link_get(uint16_t port_id, struct rte_eth_link *link);
+int rte_eth_link_get(uint16_t port_id, struct rte_eth_link *link)
+	__rte_warn_unused_result;
 
 /**
  * Retrieve the link status (up/down), the duplex mode (half/full),
@@ -3074,7 +3075,8 @@ int rte_eth_link_get(uint16_t port_id, struct rte_eth_link *link);
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EINVAL) if bad parameter.
  */
-int rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *link);
+int rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *link)
+	__rte_warn_unused_result;
 
 /**
  * @warning
@@ -3408,7 +3410,8 @@ int rte_eth_macaddrs_get(uint16_t port_id, struct rte_ether_addr *ma,
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EINVAL) if bad parameter.
  */
-int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
+int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
+	__rte_warn_unused_result;
 
 /**
  * @warning
@@ -3426,7 +3429,8 @@ int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
  *   - (-EINVAL) if bad parameter.
  */
 __rte_experimental
-int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
+int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
+	__rte_warn_unused_result;
 
 /**
  * Retrieve the firmware version of a device.
@@ -3448,8 +3452,8 @@ int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
  *   - (>0) if *fw_size* is not enough to store firmware version, return
  *          the size of the non truncated string.
  */
-int rte_eth_dev_fw_version_get(uint16_t port_id,
-			       char *fw_version, size_t fw_size);
+int rte_eth_dev_fw_version_get(uint16_t port_id, char *fw_version, size_t fw_size)
+	__rte_warn_unused_result;
 
 /**
  * Retrieve the supported packet types of an Ethernet device.
@@ -3491,7 +3495,9 @@ int rte_eth_dev_fw_version_get(uint16_t port_id,
  *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
-				     uint32_t *ptypes, int num);
+				     uint32_t *ptypes, int num)
+	__rte_warn_unused_result;
+
 /**
  * Inform Ethernet device about reduced range of packet types to handle.
  *
@@ -5089,7 +5095,8 @@ int rte_eth_get_monitor_addr(uint16_t port_id, uint16_t queue_id,
  *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
-int rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info);
+int rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
+	__rte_warn_unused_result;
 
 /**
  * Retrieve size of device EEPROM
@@ -5161,8 +5168,8 @@ int rte_eth_dev_set_eeprom(uint16_t port_id, struct rte_dev_eeprom_info *info);
  */
 __rte_experimental
 int
-rte_eth_dev_get_module_info(uint16_t port_id,
-			    struct rte_eth_dev_module_info *modinfo);
+rte_eth_dev_get_module_info(uint16_t port_id, struct rte_eth_dev_module_info *modinfo)
+	__rte_warn_unused_result;
 
 /**
  * @warning
@@ -5185,8 +5192,8 @@ rte_eth_dev_get_module_info(uint16_t port_id,
  */
 __rte_experimental
 int
-rte_eth_dev_get_module_eeprom(uint16_t port_id,
-			      struct rte_dev_eeprom_info *info);
+rte_eth_dev_get_module_eeprom(uint16_t port_id, struct rte_dev_eeprom_info *info)
+	__rte_warn_unused_result;
 
 /**
  * Set the list of multicast addresses to filter on an Ethernet device.
@@ -6839,7 +6846,8 @@ rte_eth_recycle_mbufs(uint16_t rx_port_id, uint16_t rx_queue_id,
  *   - (-EINVAL) if bad parameter.
  */
 __rte_experimental
-int rte_eth_buffer_split_get_supported_hdr_ptypes(uint16_t port_id, uint32_t *ptypes, int num);
+int rte_eth_buffer_split_get_supported_hdr_ptypes(uint16_t port_id, uint32_t *ptypes, int num)
+	__rte_warn_unused_result;
 
 /**
  * @warning
-- 
2.45.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get
  2024-10-04 16:21 ` [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get Stephen Hemminger
@ 2024-10-08  3:47   ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-10-08  3:47 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Jakub Grajciar

On 10/4/2024 5:21 PM, Stephen Hemminger wrote:
> Handle errors from rte_eth_dev_info_get in the same manner
> as other places in this file.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get
  2024-10-04 16:21 ` [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get Stephen Hemminger
@ 2024-10-08  3:47   ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-10-08  3:47 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Thomas Monjalon, Andrew Rybchenko

On 10/4/2024 5:21 PM, Stephen Hemminger wrote:
> The mac compare must check return value from rte_eth_dev_info_get
> before using the dev_info information.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 10/10] ethdev: require checking results of info_get functions
  2024-10-04 16:21 ` [PATCH 10/10] ethdev: require checking results of info_get functions Stephen Hemminger
@ 2024-10-08  3:47   ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-10-08  3:47 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Thomas Monjalon, Andrew Rybchenko

On 10/4/2024 5:21 PM, Stephen Hemminger wrote:
> With functions that return a data structure, the application must
> check the return value since the data structure contents will
> be undefined in case of error.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 00/10] require checking ethdev get return value
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (9 preceding siblings ...)
  2024-10-04 16:21 ` [PATCH 10/10] ethdev: require checking results of info_get functions Stephen Hemminger
@ 2024-10-08  9:33 ` Morten Brørup
  2024-11-11 15:20 ` Thomas Monjalon
  11 siblings, 0 replies; 16+ messages in thread
From: Morten Brørup @ 2024-10-08  9:33 UTC (permalink / raw)
  To: Stephen Hemminger, dev

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 4 October 2024 18.22
> 
> Several places flagged by Coverity and Codeql are from code that
> calls rte_eth_dev_info_get() but does not check the return value.
> If rte_eth_dev_info_get() returns an error, the device info is garbage.
> 
> This patch series uses the function attribute to force code
> to check the result or there will be a compiler warning.

This is slow path, so no performance issues.

For the series,
Acked-by: Morten Brørup <mb@smartsharesystems.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/10] require checking ethdev get return value
  2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
                   ` (10 preceding siblings ...)
  2024-10-08  9:33 ` [PATCH 00/10] require checking ethdev get return value Morten Brørup
@ 2024-11-11 15:20 ` Thomas Monjalon
  11 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2024-11-11 15:20 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

> Stephen Hemminger (10):
>   app/test: remove redundant call
>   net/memif: check return value from rte_eth_dev_info_get
>   graph: check return value from rte_eth_dev_info_get
>   examples/ethtool: handle devices without registers
>   examples/l3fwd: check return value from ethdev info
>   examples/ntb: always check return value
>   examples/pipeline: check return value of ethdev functions
>   examples/qos_sched: check return value from rte_eth_link_get
>   ethdev: check return value from rte_eth_dev_info_get
>   ethdev: require checking results of info_get functions

Applied, thanks.



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-11-11 15:20 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-04 16:21 [PATCH 00/10] require checking ethdev get return value Stephen Hemminger
2024-10-04 16:21 ` [PATCH 01/10] app/test: remove redundant call Stephen Hemminger
2024-10-04 16:21 ` [PATCH 02/10] net/memif: check return value from rte_eth_dev_info_get Stephen Hemminger
2024-10-08  3:47   ` Ferruh Yigit
2024-10-04 16:21 ` [PATCH 03/10] graph: " Stephen Hemminger
2024-10-04 16:21 ` [PATCH 04/10] examples/ethtool: handle devices without registers Stephen Hemminger
2024-10-04 16:21 ` [PATCH 05/10] examples/l3fwd: check return value from ethdev info Stephen Hemminger
2024-10-04 16:21 ` [PATCH 06/10] examples/ntb: always check return value Stephen Hemminger
2024-10-04 16:21 ` [PATCH 07/10] examples/pipeline: check return value of ethdev functions Stephen Hemminger
2024-10-04 16:21 ` [PATCH 08/10] examples/qos_sched: check return value from rte_eth_link_get Stephen Hemminger
2024-10-04 16:21 ` [PATCH 09/10] ethdev: check return value from rte_eth_dev_info_get Stephen Hemminger
2024-10-08  3:47   ` Ferruh Yigit
2024-10-04 16:21 ` [PATCH 10/10] ethdev: require checking results of info_get functions Stephen Hemminger
2024-10-08  3:47   ` Ferruh Yigit
2024-10-08  9:33 ` [PATCH 00/10] require checking ethdev get return value Morten Brørup
2024-11-11 15:20 ` Thomas Monjalon

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).