From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6E6C045A58; Wed, 9 Oct 2024 02:57:46 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F3CB1402B3; Wed, 9 Oct 2024 02:57:45 +0200 (CEST) Received: from szxga06-in.huawei.com (szxga06-in.huawei.com [45.249.212.32]) by mails.dpdk.org (Postfix) with ESMTP id 91AC04025C for ; Wed, 9 Oct 2024 02:57:44 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.163.44]) by szxga06-in.huawei.com (SkyGuard) with ESMTP id 4XNZHZ4J0Nz1ymc3; Wed, 9 Oct 2024 08:57:46 +0800 (CST) Received: from dggpeml500024.china.huawei.com (unknown [7.185.36.10]) by mail.maildlp.com (Postfix) with ESMTPS id 3D553140259; Wed, 9 Oct 2024 08:57:42 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 9 Oct 2024 08:57:41 +0800 Message-ID: Date: Wed, 9 Oct 2024 08:57:41 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/3] ethdev: add traffic manager query function To: Bruce Richardson , CC: References: <20241008105350.1396216-1-bruce.richardson@intel.com> <20241008144320.1632138-1-bruce.richardson@intel.com> <20241008144320.1632138-2-bruce.richardson@intel.com> Content-Language: en-US From: fengchengwen In-Reply-To: <20241008144320.1632138-2-bruce.richardson@intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml500024.china.huawei.com (7.185.36.10) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2024/10/8 22:43, Bruce Richardson wrote: > Add function to allow querying a node in the scheduler tree. Returns > the parameters as were given to the add function. Adding this function > allows apps to just query the hierarchy rather than having to maintain > their own copies of it internally. > > Signed-off-by: Bruce Richardson > --- > lib/ethdev/ethdev_trace.h | 16 ++++++++++ > lib/ethdev/ethdev_trace_points.c | 3 ++ > lib/ethdev/rte_tm.c | 25 ++++++++++++++++ > lib/ethdev/rte_tm.h | 50 ++++++++++++++++++++++++++++++++ > lib/ethdev/rte_tm_driver.h | 12 ++++++++ > lib/ethdev/version.map | 1 + > 6 files changed, 107 insertions(+) > > diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h > index 738d9829af..70fd6214fc 100644 > --- a/lib/ethdev/ethdev_trace.h > +++ b/lib/ethdev/ethdev_trace.h > @@ -1905,6 +1905,22 @@ RTE_TRACE_POINT( > rte_trace_point_emit_int(ret); > ) > > +RTE_TRACE_POINT( > + rte_tm_trace_node_query, > + RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t node_id, > + uint32_t *parent_node_id, uint32_t *priority, > + uint32_t *weight, uint32_t *level_id, > + struct rte_tm_node_params *params, int ret), > + rte_trace_point_emit_u16(port_id); > + rte_trace_point_emit_u32(node_id); > + rte_trace_point_emit_ptr(parent_node_id); > + rte_trace_point_emit_ptr(priority); > + rte_trace_point_emit_ptr(weight); > + rte_trace_point_emit_ptr(level_id); > + rte_trace_point_emit_ptr(params); > + rte_trace_point_emit_int(ret); > +) > + > RTE_TRACE_POINT( > rte_tm_trace_node_delete, > RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t node_id, int ret), > diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace_points.c > index 902e4f7533..eee23613b8 100644 > --- a/lib/ethdev/ethdev_trace_points.c > +++ b/lib/ethdev/ethdev_trace_points.c > @@ -706,6 +706,9 @@ RTE_TRACE_POINT_REGISTER(rte_tm_trace_mark_vlan_dei, > RTE_TRACE_POINT_REGISTER(rte_tm_trace_node_add, > lib.ethdev.tm.node_add) > > +RTE_TRACE_POINT_REGISTER(rte_tm_trace_node_query, > + lib.ethdev.tm.node_query) > + > RTE_TRACE_POINT_REGISTER(rte_tm_trace_node_capabilities_get, > lib.ethdev.tm.node_capabilities_get) > > diff --git a/lib/ethdev/rte_tm.c b/lib/ethdev/rte_tm.c > index d594fe0049..9ea140df09 100644 > --- a/lib/ethdev/rte_tm.c > +++ b/lib/ethdev/rte_tm.c > @@ -301,6 +301,31 @@ int rte_tm_node_add(uint16_t port_id, > return ret; > } > > +int rte_tm_node_query(uint16_t port_id, > + uint32_t node_id, > + uint32_t *parent_node_id, > + uint32_t *priority, > + uint32_t *weight, > + uint32_t *level_id, > + struct rte_tm_node_params *params, > + struct rte_tm_error *error) > +{ > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > + int ret; > + > + if (dev == NULL) > + return -EINVAL; The RTE_TM_FUNC will check it, so no need do above judgement. > + > + ret = RTE_TM_FUNC(port_id, node_query)(dev, > + node_id, parent_node_id, priority, weight, level_id, > + params, error); > + > + rte_tm_trace_node_query(port_id, node_id, parent_node_id, priority, > + weight, level_id, params, ret); > + > + return ret; > +} > + > /* Delete node from traffic manager hierarchy */ > int rte_tm_node_delete(uint16_t port_id, > uint32_t node_id, > diff --git a/lib/ethdev/rte_tm.h b/lib/ethdev/rte_tm.h > index 07028c9b36..395e9d6d7f 100644 > --- a/lib/ethdev/rte_tm.h > +++ b/lib/ethdev/rte_tm.h > @@ -20,6 +20,7 @@ > > #include > #include > +#include > > #ifdef __cplusplus > extern "C" { > @@ -1599,6 +1600,55 @@ rte_tm_node_add(uint16_t port_id, > struct rte_tm_node_params *params, > struct rte_tm_error *error); > > +/** > + * Return information about a traffic management node > + * > + * Return information about a hierarchy node, using the same format of parameters > + * as was passed to the rte_rm_node_add() function. > + * Each of the "out" parameters pointers (except error) may be passed as NULL if the > + * information is not needed by the caller. For example, to one may check if a node id > + * is in use by: > + * > + * struct rte_tm_error error; > + * int ret = rte_tm_node_query(port, node_id, NULL, NULL, NULL, NULL, NULL, &error); > + * if (ret == ENOENT) ... > + * > + * @param[in] port_id > + * The port identifier of the Ethernet device. > + * @param[in] node_id > + * Node ID. Should be a valid node id. > + * @param[out] parent_node_id > + * Parent node ID. > + * @param[out] priority > + * Node priority. The highest node priority is zero. Used by the SP algorithm > + * running on the parent of the current node for scheduling this child node. > + * @param[out] weight > + * Node weight. The node weight is relative to the weight sum of all siblings > + * that have the same priority. The lowest weight is one. Used by the WFQ > + * algorithm running on the parent of the current node for scheduling this > + * child node. > + * @param[out] level_id > + * The node level in the scheduler hierarchy. > + * @param[out] params > + * Node parameters, as would be used when creating the node. > + * @param[out] error > + * Error details. Filled in only on error, when not NULL. This parameter should not be NULL, as said before: Each of the "out" parameters pointers (except error) may be passed as NULL if the > + * @return > + * 0 on success, non-zero error code otherwise. > + * -EINVAL - port or node id value is invalid > + * -ENOENT - no node exists with the provided id id -> port no node exists with the provided port > + */ > +__rte_experimental > +int > +rte_tm_node_query(uint16_t port_id, > + uint32_t node_id, > + uint32_t *parent_node_id, > + uint32_t *priority, > + uint32_t *weight, > + uint32_t *level_id, > + struct rte_tm_node_params *params, > + struct rte_tm_error *error); > + Suggest this new function place after node_resume in header/impl.c(e.g. source or trace), keep them consisteny > /** > * Traffic manager node delete > * > diff --git a/lib/ethdev/rte_tm_driver.h b/lib/ethdev/rte_tm_driver.h > index 45290fb3fd..1efb2caaec 100644 > --- a/lib/ethdev/rte_tm_driver.h > +++ b/lib/ethdev/rte_tm_driver.h > @@ -119,6 +119,16 @@ typedef int (*rte_tm_node_resume_t)(struct rte_eth_dev *dev, > uint32_t node_id, > struct rte_tm_error *error); > > +/** @internal Traffic manager node query */ > +typedef int (*rte_tm_node_query_t)(const struct rte_eth_dev *dev, > + uint32_t node_id, > + uint32_t *parent_node_id, > + uint32_t *priority, > + uint32_t *weight, > + uint32_t *level_id, > + struct rte_tm_node_params *params, > + struct rte_tm_error *error); > + > /** @internal Traffic manager hierarchy commit */ > typedef int (*rte_tm_hierarchy_commit_t)(struct rte_eth_dev *dev, > int clear_on_fail, > @@ -248,6 +258,8 @@ struct rte_tm_ops { > rte_tm_node_suspend_t node_suspend; > /** Traffic manager node resume */ > rte_tm_node_resume_t node_resume; > + /** Traffic manager node resume */ Should be node query > + rte_tm_node_query_t node_query; > /** Traffic manager hierarchy commit */ > rte_tm_hierarchy_commit_t hierarchy_commit; > > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > index f63dc32aa2..c1a386eddc 100644 > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -335,6 +335,7 @@ EXPERIMENTAL { > rte_eth_speed_lanes_get_capability; > rte_eth_speed_lanes_set; > rte_flow_async_create_by_index_with_pattern; > + rte_tm_node_query; > }; > > INTERNAL {