From: David Marchand <david.marchand@redhat.com>
To: dev@dpdk.org
Cc: bruce.richardson@intel.com, rjarry@redhat.com,
ktraynor@redhat.com, stable@dpdk.org,
Thomas Monjalon <thomas@monjalon.net>,
Ferruh Yigit <ferruh.yigit@amd.com>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
Keith Wiles <keith.wiles@intel.com>,
Ciara Power <ciara.power@intel.com>
Subject: [PATCH 2/2] ethdev: fix race on ports for telemetry commands
Date: Wed, 2 Oct 2024 17:57:08 +0200 [thread overview]
Message-ID: <20241002155709.2522273-3-david.marchand@redhat.com> (raw)
In-Reply-To: <20241002155709.2522273-1-david.marchand@redhat.com>
While invoking telemetry commands (which may happen at any time,
out of control of the application), an application thread may
concurrently add/remove ports.
The telemetry callbacks may then access partially
initialised/uninitialised ethdev data.
Reuse the ethdev lock that protects port allocation/destruction.
Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
lib/ethdev/rte_ethdev_telemetry.c | 93 +++++++++++++++++++------------
1 file changed, 56 insertions(+), 37 deletions(-)
diff --git a/lib/ethdev/rte_ethdev_telemetry.c b/lib/ethdev/rte_ethdev_telemetry.c
index 8031a58595..7f9c924209 100644
--- a/lib/ethdev/rte_ethdev_telemetry.c
+++ b/lib/ethdev/rte_ethdev_telemetry.c
@@ -6,6 +6,7 @@
#include <stdlib.h>
#include <rte_kvargs.h>
+#include <rte_spinlock.h>
#include <rte_telemetry.h>
#include "rte_ethdev.h"
@@ -1403,43 +1404,61 @@ eth_dev_handle_port_tm_node_caps(const char *cmd __rte_unused,
return ret;
}
+#define ETHDEV_TELEMETRY_HANDLERS \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/list", eth_dev_handle_port_list, \
+ "Returns list of available ethdev ports. Takes no parameters") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/stats", eth_dev_handle_port_stats, \
+ "Returns the common stats for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/xstats", eth_dev_handle_port_xstats, \
+ "Returns the extended stats for a port. Parameters: int port_id,hide_zero=true|false(Optional for indicates hide zero xstats)") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/dump_priv", eth_dev_handle_port_dump_priv, \
+ "Returns dump private information for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/link_status", eth_dev_handle_port_link_status, \
+ "Returns the link status for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/info", eth_dev_handle_port_info, \
+ "Returns the device info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/module_eeprom", eth_dev_handle_port_module_eeprom, \
+ "Returns module EEPROM info with SFF specs. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/macs", eth_dev_handle_port_macs, \
+ "Returns the MAC addresses for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/flow_ctrl", eth_dev_handle_port_flow_ctrl, \
+ "Returns flow ctrl info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/rx_queue", eth_dev_handle_port_rxq, \
+ "Returns Rx queue info for a port. Parameters: int port_id, int queue_id (Optional if only one queue)") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/tx_queue", eth_dev_handle_port_txq, \
+ "Returns Tx queue info for a port. Parameters: int port_id, int queue_id (Optional if only one queue)") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/dcb", eth_dev_handle_port_dcb, \
+ "Returns DCB info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/rss_info", eth_dev_handle_port_rss_info, \
+ "Returns RSS info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/fec", eth_dev_handle_port_fec, \
+ "Returns FEC info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/vlan", eth_dev_handle_port_vlan, \
+ "Returns VLAN info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/tm_capability", eth_dev_handle_port_tm_caps, \
+ "Returns TM Capabilities info for a port. Parameters: int port_id") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/tm_level_capability", eth_dev_handle_port_tm_level_caps, \
+ "Returns TM Level Capabilities info for a port. Parameters: int port_id, int level_id (see tm_capability for the max)") \
+ ETHDEV_TELEMETRY_HANDLER("/ethdev/tm_node_capability", eth_dev_handle_port_tm_node_caps, \
+ "Returns TM Node Capabilities info for a port. Parameters: int port_id, int node_id (see tm_capability for the max)")
+
+#define ETHDEV_TELEMETRY_HANDLER(command, func, usage) \
+static int func ## _locked(const char *cmd __rte_unused, const char *params, \
+ struct rte_tel_data *d) \
+{ \
+ int ret; \
+ rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); \
+ ret = func(cmd, params, d); \
+ rte_spinlock_unlock(rte_mcfg_ethdev_get_lock()); \
+ return ret; \
+}
+ETHDEV_TELEMETRY_HANDLERS
+#undef ETHDEV_TELEMETRY_HANDLER
+
RTE_INIT(ethdev_init_telemetry)
{
- rte_telemetry_register_cmd("/ethdev/list", eth_dev_handle_port_list,
- "Returns list of available ethdev ports. Takes no parameters");
- rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
- "Returns the common stats for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
- "Returns the extended stats for a port. Parameters: int port_id,hide_zero=true|false(Optional for indicates hide zero xstats)");
- rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
- "Returns dump private information for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/link_status",
- eth_dev_handle_port_link_status,
- "Returns the link status for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/info", eth_dev_handle_port_info,
- "Returns the device info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/module_eeprom", eth_dev_handle_port_module_eeprom,
- "Returns module EEPROM info with SFF specs. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/macs", eth_dev_handle_port_macs,
- "Returns the MAC addresses for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/flow_ctrl", eth_dev_handle_port_flow_ctrl,
- "Returns flow ctrl info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/rx_queue", eth_dev_handle_port_rxq,
- "Returns Rx queue info for a port. Parameters: int port_id, int queue_id (Optional if only one queue)");
- rte_telemetry_register_cmd("/ethdev/tx_queue", eth_dev_handle_port_txq,
- "Returns Tx queue info for a port. Parameters: int port_id, int queue_id (Optional if only one queue)");
- rte_telemetry_register_cmd("/ethdev/dcb", eth_dev_handle_port_dcb,
- "Returns DCB info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/rss_info", eth_dev_handle_port_rss_info,
- "Returns RSS info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/fec", eth_dev_handle_port_fec,
- "Returns FEC info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/vlan", eth_dev_handle_port_vlan,
- "Returns VLAN info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/tm_capability", eth_dev_handle_port_tm_caps,
- "Returns TM Capabilities info for a port. Parameters: int port_id");
- rte_telemetry_register_cmd("/ethdev/tm_level_capability", eth_dev_handle_port_tm_level_caps,
- "Returns TM Level Capabilities info for a port. Parameters: int port_id, int level_id (see tm_capability for the max)");
- rte_telemetry_register_cmd("/ethdev/tm_node_capability", eth_dev_handle_port_tm_node_caps,
- "Returns TM Node Capabilities info for a port. Parameters: int port_id, int node_id (see tm_capability for the max)");
+#define ETHDEV_TELEMETRY_HANDLER(command, func, usage) \
+ rte_telemetry_register_cmd(command, func ## _locked, usage);
+ ETHDEV_TELEMETRY_HANDLERS
+#undef ETHDEV_TELEMETRY_HANDLER
}
--
2.46.2
next parent reply other threads:[~2024-10-02 15:57 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20241002155709.2522273-1-david.marchand@redhat.com>
2024-10-02 15:57 ` David Marchand [this message]
2024-10-02 16:27 ` Bruce Richardson
2024-10-02 19:06 ` David Marchand
2024-10-02 19:09 ` Robin Jarry
2024-10-02 19:18 ` David Marchand
2024-10-02 19:26 ` Robin Jarry
2024-10-03 9:46 ` Bruce Richardson
2024-10-03 9:58 ` David Marchand
2024-10-03 11:24 ` [PATCH dpdk v2 2/2] ethdev: fix potential race in telemetry endpoints Robin Jarry
2024-10-03 11:39 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241002155709.2522273-3-david.marchand@redhat.com \
--to=david.marchand@redhat.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=bruce.richardson@intel.com \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=keith.wiles@intel.com \
--cc=ktraynor@redhat.com \
--cc=rjarry@redhat.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).