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 096BE438F1; Thu, 18 Jan 2024 10:06:51 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C3186402B0; Thu, 18 Jan 2024 10:06:50 +0100 (CET) Received: from mail-oi1-f175.google.com (mail-oi1-f175.google.com [209.85.167.175]) by mails.dpdk.org (Postfix) with ESMTP id 82740400D7 for ; Thu, 18 Jan 2024 10:06:49 +0100 (CET) Received: by mail-oi1-f175.google.com with SMTP id 5614622812f47-3bd884146e9so1990286b6e.0 for ; Thu, 18 Jan 2024 01:06:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705568809; x=1706173609; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=nZDpxYfmF7dwC8S15GKyZhYNKRxrP4VGPhJH2sW8T50=; b=Arsjq9P4Il72tBiCorTC49OJEOZ9dK282A5BcrKTfpAkkvT02HbPV4w/HVHTxwauTF U2rTVb1O53DZ7FC6zMNUEqLJUEnqZaQajke1DDYeK8iDkEjDbQFHIrUcSn7NYLAMaWSD XO4hUctWaVOQoLI3zVCnnTvy0r3SiE1bO8zkSkM9hoM2BfRkv8R470fKOSrgDPAWgVDJ f96kEa0F8xEQNWcL3+j2X0NB8UOo6o+dCn93Wp+6losbG0mQmIwnf8yXTr4hHmumFiEG XVoMPdA/L++BvG/Qq0k/rlrMbBWy9S1gwS7l/Q95suZ2OAMeMpc2Z7+5PzyWFMynsms+ wwHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705568809; x=1706173609; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nZDpxYfmF7dwC8S15GKyZhYNKRxrP4VGPhJH2sW8T50=; b=KUtwq/vJFAV6HyO60Nq641RSMimokAk/REa8uBDHp7c5EQtJixl2RBj3gTejEetjEJ RBe3Ath9Qhi/VQ2eonaurMkIHbpUQTHWkyTT6puOtwPr5HEKXmS5gQbR+vjIDonxq2MB F+J0awx2RBiCUvUqg3s6HNjgfjQDPVSDjP6gUkQILTghoDkwHRRkAoGyHy60oR/bjkZN 6fwA3xUv0SGC+ziD2nrKk2aza3xsMtivd8XfoULuk6lA5arI0pNH+RDyXGsTnZtgA+QT At2WLHY22nFV45kYNhAbr9uODxr4yap7FFK65sUWZ2HkKv2bzzcNc1ZI2XoqnjjBETGw 8hlA== X-Gm-Message-State: AOJu0YwHFiIih8ubr+FIlB17lMf2i3J+SnQy0vm9ud03s2KCgCpX6Mj4 yi57NnPVjy4PBn0rhmpWeTkpeQj/5D97eTyfjnXNrJEzYTgXJEWSbayhYKWjUj7ntDyvikwZZbX 8l1EQMqJxcVfRQ1zuMHk/FjFhtHM= X-Google-Smtp-Source: AGHT+IHWY1ikipxeN5uUqNYLNLAVTahmHfYj4oF5DFY7380o6YwL8JXc1vV6Klp1JfkE7u1VCW1WsiLEsVJ4WQ3Yg78= X-Received: by 2002:a05:6808:1242:b0:3bd:3d83:9507 with SMTP id o2-20020a056808124200b003bd3d839507mr683363oiv.65.1705568808647; Thu, 18 Jan 2024 01:06:48 -0800 (PST) MIME-Version: 1.0 References: <20231219172948.3909749-1-jerinj@marvell.com> <20240111151745.3800170-1-jerinj@marvell.com> <5251f00a-a039-460d-805b-0325cfced32b@amd.com> In-Reply-To: <5251f00a-a039-460d-805b-0325cfced32b@amd.com> From: Jerin Jacob Date: Thu, 18 Jan 2024 14:36:21 +0530 Message-ID: Subject: Re: [dpdk-dev] [v1] ethdev: support Tx queue used count To: Ferruh Yigit Cc: jerinj@marvell.com, dev@dpdk.org, Thomas Monjalon , Andrew Rybchenko , ferruh.yigit@xilinx.com, ajit.khaparde@broadcom.com, aboyer@pensando.io, beilei.xing@intel.com, bruce.richardson@intel.com, chas3@att.com, chenbo.xia@intel.com, ciara.loftus@intel.com, dsinghrawat@marvell.com, ed.czeck@atomicrules.com, evgenys@amazon.com, grive@u256.net, g.singh@nxp.com, zhouguoyang@huawei.com, haiyue.wang@intel.com, hkalra@marvell.com, heinrich.kuhn@corigine.com, hemant.agrawal@nxp.com, hyonkim@cisco.com, igorch@amazon.com, irusskikh@marvell.com, jgrajcia@cisco.com, jasvinder.singh@intel.com, jianwang@trustnetic.com, jiawenwu@trustnetic.com, jingjing.wu@intel.com, johndale@cisco.com, john.miller@atomicrules.com, linville@tuxdriver.com, keith.wiles@intel.com, kirankumark@marvell.com, oulijun@huawei.com, lironh@marvell.com, longli@microsoft.com, mw@semihalf.com, spinler@cesnet.cz, matan@nvidia.com, matt.peters@windriver.com, maxime.coquelin@redhat.com, mk@semihalf.com, humin29@huawei.com, pnalla@marvell.com, ndabilpuram@marvell.com, qiming.yang@intel.com, qi.z.zhang@intel.com, radhac@marvell.com, rahul.lakkireddy@chelsio.com, rmody@marvell.com, rosen.xu@intel.com, sachin.saxena@oss.nxp.com, skoteshwar@marvell.com, shshaikh@marvell.com, shaibran@amazon.com, shepard.siegel@atomicrules.com, asomalap@amd.com, somnath.kotur@broadcom.com, sthemmin@microsoft.com, steven.webster@windriver.com, skori@marvell.com, mtetsuyah@gmail.com, vburru@marvell.com, viacheslavo@nvidia.com, xiao.w.wang@intel.com, cloud.wangxiaoyun@huawei.com, yisen.zhuang@huawei.com, yongwang@vmware.com, xuanziyang2@huawei.com, cristian.dumitrescu@intel.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Fri, Jan 12, 2024 at 5:04=E2=80=AFPM Ferruh Yigit = wrote: > > On 1/11/2024 3:17 PM, jerinj@marvell.com wrote: > > From: Jerin Jacob > > > > Introduce a new API to retrieve the number of used descriptors > > in a Tx queue. Applications can leverage this API in the fast path to > > inspect the Tx queue occupancy and take appropriate actions based on th= e > > available free descriptors. > > > > A notable use case could be implementing Random Early Discard (RED) > > in software based on Tx queue occupancy. > > > > Signed-off-by: Jerin Jacob > > --- > > > > As we are adding a new API and dev_ops, is a driver implementation and > testpmd/example implementation planned for this release? Yes. > > > > rfc..v1: > > - Updated API similar to rte_eth_rx_queue_count() where it returns > > "used" count instead of "free" count > > > > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rs= t > > index f7d9980849..0d5a8733fc 100644 > > --- a/doc/guides/nics/features.rst > > +++ b/doc/guides/nics/features.rst > > @@ -962,6 +962,16 @@ management (see :doc:`../prog_guide/power_man` for= more details). > > > > * **[implements] eth_dev_ops**: ``get_monitor_addr`` > > > > +.. _nic_features_tx_queue_used_count: > > + > > +Tx queue count > > +-------------- > > + > > +Supports to get the number of used descriptors of a Tx queue. > > + > > +* **[implements] eth_dev_ops**: ``tx_queue_count``. > > +* **[related] API**: ``rte_eth_tx_queue_count()``. > > + > > > > Can you please keep the order same with 'default.ini' file, > I recognized there is already some mismatch in order but we can fix them > later. Ack > > > .. _nic_features_other: > > > > Other dev ops not represented by a Feature > > diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/fea= tures/default.ini > > index 806cb033ff..3ef6d45c0e 100644 > > --- a/doc/guides/nics/features/default.ini > > +++ b/doc/guides/nics/features/default.ini > > @@ -59,6 +59,7 @@ Packet type parsing =3D > > Timesync =3D > > Rx descriptor status =3D > > Tx descriptor status =3D > > +Tx queue count =3D > > > > Existing Rx queue count is not documented, if we are documenting this, > can you please add "Rx queue count" in a separate patch? I will do. > > > Basic stats =3D > > Extended stats =3D > > Stats per queue =3D > > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > > index b482cd12bb..f05f68a67c 100644 > > --- a/lib/ethdev/ethdev_driver.h > > +++ b/lib/ethdev/ethdev_driver.h > > @@ -58,6 +58,8 @@ struct rte_eth_dev { > > eth_rx_queue_count_t rx_queue_count; > > /** Check the status of a Rx descriptor */ > > eth_rx_descriptor_status_t rx_descriptor_status; > > + /** Get the number of used Tx descriptors */ > > + eth_tx_queue_count_t tx_queue_count; > > /** Check the status of a Tx descriptor */ > > eth_tx_descriptor_status_t tx_descriptor_status; > > /** Pointer to PMD transmit mbufs reuse function */ > > diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c > > index a656df293c..626524558a 100644 > > --- a/lib/ethdev/ethdev_private.c > > +++ b/lib/ethdev/ethdev_private.c > > @@ -273,6 +273,7 @@ eth_dev_fp_ops_setup(struct rte_eth_fp_ops *fpo, > > fpo->tx_pkt_prepare =3D dev->tx_pkt_prepare; > > fpo->rx_queue_count =3D dev->rx_queue_count; > > fpo->rx_descriptor_status =3D dev->rx_descriptor_status; > > + fpo->tx_queue_count =3D dev->tx_queue_count; > > fpo->tx_descriptor_status =3D dev->tx_descriptor_status; > > fpo->recycle_tx_mbufs_reuse =3D dev->recycle_tx_mbufs_reuse; > > fpo->recycle_rx_descriptors_refill =3D dev->recycle_rx_descriptor= s_refill; > > diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace= _points.c > > index 91f71d868b..e618414392 100644 > > --- a/lib/ethdev/ethdev_trace_points.c > > +++ b/lib/ethdev/ethdev_trace_points.c > > @@ -481,6 +481,9 @@ RTE_TRACE_POINT_REGISTER(rte_eth_trace_count_aggr_p= orts, > > RTE_TRACE_POINT_REGISTER(rte_eth_trace_map_aggr_tx_affinity, > > lib.ethdev.map_aggr_tx_affinity) > > > > +RTE_TRACE_POINT_REGISTER(rte_eth_trace_tx_queue_count, > > + lib.ethdev.tx_queue_count) > > + > > Can you please group this with 'tx_burst' & 'call_tx_callbacks' above? Ack > > > RTE_TRACE_POINT_REGISTER(rte_flow_trace_copy, > > lib.ethdev.flow.copy) > > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > > index 21e3a21903..af59da9652 100644 > > --- a/lib/ethdev/rte_ethdev.h > > +++ b/lib/ethdev/rte_ethdev.h > > @@ -6803,6 +6803,80 @@ rte_eth_recycle_mbufs(uint16_t rx_port_id, uint1= 6_t rx_queue_id, > > __rte_experimental > > int rte_eth_buffer_split_get_supported_hdr_ptypes(uint16_t port_id, ui= nt32_t *ptypes, int num); > > > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change, or be removed, without prior = notice > > + * > > + * Get the number of used descriptors of a Tx queue > > + * > > + * This function retrieves the number of used descriptors of a transmi= t queue. > > + * Applications can use this API in the fast path to inspect Tx queue = occupancy and take > > + * appropriate actions based on the available free descriptors. > > + * An example action could be implementing the Random Early Discard (R= ED). > > > > Above is good, but do you think does it help to mention that intended > usacase is QoS, to address the risk Bruce mentioned? I will add following notes in next version. * @note There is no requirement to call this function before rte_eth_tx_burst() invocation. * @note Utilize this function exclusively when the caller needs to determine the used queue count * across all descriptors of a Tx queue. If the use case only involves checking the status of a * specific descriptor slot, opt for rte_eth_tx_descriptor_status() instead= . > > > > + * > > + * Since it's a fast-path function, no check is performed on port_id a= nd > > + * tx_queue_id. The caller must therefore ensure that the port is enab= led > > + * and the queue is configured and running. > > + * > > + * @param port_id > > + * The port identifier of the device. > > + * @param tx_queue_id > > + * The index of the transmit queue. > > + * The value must be in the range [0, nb_tx_queue - 1] previously su= pplied > > + * to rte_eth_dev_configure(). > > + * @return > > + * The number of used descriptors in the specific queue, or: > > + * - (-ENODEV) if *port_id* is invalid. Enabled only when RTE_ETHDEV= _DEBUG_TX is enabled > > + * - (-EINVAL) if *queue_id* is invalid. Enabled only when RTE_ETHDE= V_DEBUG_TX is enabled > > + * - (-ENOTSUP) if the device does not support this function. > > + * > > + * @note This function is designed for fast-path use. > > + */ > > +__rte_experimental > > +static inline int > > +rte_eth_tx_queue_count(uint16_t port_id, uint16_t tx_queue_id) > > +{ > > + struct rte_eth_fp_ops *fops; > > + void *qd; > > + int rc; > > + > > +#ifdef RTE_ETHDEV_DEBUG_TX > > + if (port_id >=3D RTE_MAX_ETHPORTS || !rte_eth_dev_is_valid_port(p= ort_id)) { > > + RTE_ETHDEV_LOG_LINE(ERR, "Invalid port_id=3D%u", port_id)= ; > > + rc =3D -ENODEV; > > + rte_eth_trace_tx_queue_count(port_id, tx_queue_id, rc); > > + return rc; > > + } > > + > > + rc =3D -EINVAL; > > + if (tx_queue_id >=3D RTE_MAX_QUEUES_PER_PORT) { > > + RTE_ETHDEV_LOG_LINE(ERR, "Invalid Tx queue_id=3D%u for po= rt_id=3D%u", > > + tx_queue_id, port_id); > > + rte_eth_trace_tx_queue_count(port_id, tx_queue_id, rc); > > + return rc; > > + } > > +#endif > > + > > + /* Fetch pointer to Tx queue data */ > > + fops =3D &rte_eth_fp_ops[port_id]; > > + qd =3D fops->txq.data[tx_queue_id]; > > + > > +#ifdef RTE_ETHDEV_DEBUG_TX > > + if (qd =3D=3D NULL) { > > + RTE_ETHDEV_LOG_LINE(ERR, "Invalid Tx queue_id=3D%u for po= rt_id=3D%u", > > + tx_queue_id, port_id); > > + rte_eth_trace_tx_queue_count(port_id, tx_queue_id, rc); > > + return rc; > > + } > > +#endif > > + if (fops->tx_queue_count =3D=3D NULL) > > + return -ENOTSUP; > > + > > + rc =3D fops->tx_queue_count(qd); > > > > Can you please put an empty line here to have a separate block for trace > code? Ack > > > + rte_eth_trace_tx_queue_count(port_id, tx_queue_id, rc); > > + > > + return rc; > > +} > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.= h > > index 4bfaf79c6c..d3f09f390d 100644 > > --- a/lib/ethdev/rte_ethdev_core.h > > +++ b/lib/ethdev/rte_ethdev_core.h > > @@ -60,6 +60,9 @@ typedef uint16_t (*eth_recycle_tx_mbufs_reuse_t)(void= *txq, > > /** @internal Refill Rx descriptors with the recycling mbufs */ > > typedef void (*eth_recycle_rx_descriptors_refill_t)(void *rxq, uint16_= t nb); > > > > +/** @internal Get number of used descriptors on a transmit queue. */ > > +typedef int (*eth_tx_queue_count_t)(void *txq); > > + > > Can you please move it above 'tx_descriptor_status', to keep same order > kept in many other locations: > rx_queue_count > rx_descriptor_status > tx_queue_count > tx_descriptor_status Ack > > > > /** > > * @internal > > * Structure used to hold opaque pointers to internal ethdev Rx/Tx > > @@ -116,7 +119,9 @@ struct rte_eth_fp_ops { > > eth_tx_descriptor_status_t tx_descriptor_status; > > /** Copy used mbufs from Tx mbuf ring into Rx. */ > > eth_recycle_tx_mbufs_reuse_t recycle_tx_mbufs_reuse; > > - uintptr_t reserved2[2]; > > + /** Get the number of used Tx descriptors. */ > > + eth_tx_queue_count_t tx_queue_count; > > > > Similarly, can you please move it above 'tx_descriptor_status'? Ack > > > > + uintptr_t reserved2[1]; > > /**@}*/ > > > > } __rte_cache_aligned; > > diff --git a/lib/ethdev/rte_ethdev_trace_fp.h b/lib/ethdev/rte_ethdev_t= race_fp.h > > index 186271c9ff..c98c488433 100644 > > --- a/lib/ethdev/rte_ethdev_trace_fp.h > > +++ b/lib/ethdev/rte_ethdev_trace_fp.h > > @@ -73,6 +73,14 @@ RTE_TRACE_POINT_FP( > > rte_trace_point_emit_u64(count); > > ) > > > > +RTE_TRACE_POINT_FP( > > + rte_eth_trace_tx_queue_count, > > + RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t tx_queue_id, int = rc), > > + rte_trace_point_emit_u16(port_id); > > + rte_trace_point_emit_u16(tx_queue_id); > > + rte_trace_point_emit_int(rc); > > +) > > + > > Existing 'rx_queue_count' is missing tracepoints, for consistency, can > you please add that too in a separate patch? I will do. > > > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > > index 5c4917c020..e03830902a 100644 > > --- a/lib/ethdev/version.map > > +++ b/lib/ethdev/version.map > > @@ -316,6 +316,9 @@ EXPERIMENTAL { > > rte_eth_recycle_rx_queue_info_get; > > rte_flow_group_set_miss_actions; > > rte_flow_calc_table_hash; > > + > > + # added in 24.03 > > + rte_eth_tx_queue_count; > > }; > > > > INTERNAL { >