On Tue, Oct 8, 2024 at 4:09 AM lihuisong (C) <lihuis...@huawei.com> wrote:
>
>
> 在 2024/10/2 23:57, David Marchand 写道:
> > 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.
> Agree to lock for telemetry.
> >
> > Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: David Marchand <david.march...@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) \
> In addition, the "command" and "usage" are unuseful in this definition.
> This is confused to me.
> Can the first macro definition just use 'func' as input parameter?
> The name of this macro define is the same as the below one.
> I prefer to use two different macros to do this.
This patch has been withdrawn.
Robin is working on another approach, see:
https://patchwork.dpdk.org/project/dpdk/patch/20241003112438.902397-6-rja...@redhat.com/
https://patchwork.dpdk.org/project/dpdk/patch/20241003112438.902397-7-rja...@redhat.com/
--
David Marchand
