From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: <dev@dpdk.org>
Subject: Re: [RFC PATCH 0/6] remove deprecated queue stats
Date: Tue, 23 Sep 2025 16:33:04 +0100 [thread overview]
Message-ID: <aNK9sG1ISFOBUb98@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <20250923080443.5430306c@hermes.local>
On Tue, Sep 23, 2025 at 08:04:43AM -0700, Stephen Hemminger wrote:
> On Tue, 23 Sep 2025 15:12:00 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > Since DPDK 20.11 release, the use of queue stats inside the rte_eth_stats
> > structure has been deprecated with the intention to remove them. Sadly,
> > despite 5 years passing that has still not been done. This patchset
> > finally attempts to fix that situation and remove the queue stats fields.
> >
> > The biggest complication here is the fact that, as part of the deprecation,
> > a new driver flag was added which caused ethdev to automatically add the
> > queue stats into xstats. While this was good, in that it allowed quick use
> > of xstats for getting queue statistics, it now causes lots of problems
> > because we have 35 drivers (by a rough count using grep) which rely on this
> > functionality to export their queue stats via xstats. This means that if we
> > drop the queue stats fields from the regular stats structure, then 35
> > drivers will lose *all* queue stats reporting functionality! This is not an
> > acceptable situation IMHO. [And in one patchset trying to change all 35
> > drivers to directly export via xstats is not feasible either, since adding
> > extra xstats can be very complicated at times.]
> >
> > Therefore, we need to make changes that a) removes the queue fields from
> > the regular stats structure, while also b) continuing to allow the
> > individual driver stats_get functions return those stats to ethdev for
> > filling in. I tried a number of approaches here to find one that was most
> > feasible to implement for large numbers of drivers, so that either scripts
> > or AI assistants could automate a lot of the changes. The most feasible
> > approach I found was to define a DPDK-internal queue-stats-only structure,
> > which would be passed as a 3rd parameter to the drivers' stats_get
> > functions. When calling stats_get from ethdev, this third parameter would
> > be NULL, but for gathering xstats, it would be non-NULL for drivers which
> > have the RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS flag set.
> >
> > For this set, I've separated out the driver changes from the ethdev changes
> > and the app changes, since the driver changes are so numerous, making the
> > rest hard to review. In order to ensure clean compilation and git history,
> > patches 3 through 6 should be squashed into a single one on apply.
> >
> > Beyond these patches, more cleanup should/could be done:
> > * ensure that any drivers which don't set the AUTOFILL flag have the qstats
> > parameter set as __rte_unused.
> > * For those that do use the AUTOFILL flag, we should ensure that they
> > actually do fill in the stats, and not just set them to zero. (First two
> > patches here fix that for a couple of Intel drivers)
> > * then work to reduce the number of drivers which use the flag to set the
> > xstats, with a view to completely removing the queue stats from regular
> > stats functions.
> > * Within ethdev (and testpmd), we should consider what to do with any other
> > legacy queue stats related functions, for example, do we still keep the
> > queue mappings functions.
> >
> >
> > Bruce Richardson (6):
> > net/ice: don't report empty queue xstats
> > net/ipn3ke: drop unsupported per-queue xstats
> > ethdev: remove queue stats from ethdev stats structure
> > drivers/net: update to remove queue stats from eth stats
> > app: remove queue stats from eth stats
> > doc: update docs for ethdev changes
> >
> > app/proc-info/main.c | 16 ---
> > app/test-pmd/config.c | 6 -
> > app/test/virtual_pmd.c | 3 +-
> > config/rte_config.h | 1 -
> > doc/guides/rel_notes/deprecation.rst | 7 -
> > doc/guides/rel_notes/release_25_11.rst | 6 +
> > drivers/net/af_packet/rte_eth_af_packet.c | 13 +-
> > drivers/net/af_xdp/rte_eth_af_xdp.c | 35 +++--
> > drivers/net/ark/ark_ethdev.c | 36 +++--
> > drivers/net/ark/ark_ethdev_rx.c | 14 +-
> > drivers/net/ark/ark_ethdev_rx.h | 3 +-
> > drivers/net/ark/ark_ethdev_tx.c | 12 +-
> > drivers/net/ark/ark_ethdev_tx.h | 3 +-
> > drivers/net/atlantic/atl_ethdev.c | 19 +--
> > drivers/net/atlantic/atl_types.h | 1 +
> > drivers/net/avp/avp_ethdev.c | 20 ++-
> > drivers/net/axgbe/axgbe_ethdev.c | 22 +--
> > drivers/net/axgbe/axgbe_ethdev.h | 1 +
> > drivers/net/bnx2x/bnx2x_ethdev.c | 3 +-
> > drivers/net/bnxt/bnxt_reps.c | 14 +-
> > drivers/net/bnxt/bnxt_reps.h | 2 +-
> > drivers/net/bnxt/bnxt_stats.c | 133 ++++++++++--------
> > drivers/net/bnxt/bnxt_stats.h | 2 +-
> > drivers/net/bonding/rte_eth_bond_pmd.c | 14 +-
> > drivers/net/cnxk/cnxk_ethdev.h | 3 +-
> > drivers/net/cnxk/cnxk_rep.h | 3 +-
> > drivers/net/cnxk/cnxk_rep_ops.c | 15 +-
> > drivers/net/cnxk/cnxk_stats.c | 49 ++++---
> > drivers/net/cxgbe/cxgbe_ethdev.c | 3 +-
> > drivers/net/cxgbe/cxgbevf_ethdev.c | 3 +-
> > drivers/net/dpaa/dpaa_ethdev.c | 3 +-
> > drivers/net/dpaa2/dpaa2_ethdev.c | 28 ++--
> > drivers/net/ena/ena_ethdev.c | 46 +++---
> > drivers/net/enetc/enetc_ethdev.c | 4 +-
> > drivers/net/enetfec/enet_ethdev.c | 3 +-
> > drivers/net/enic/enic.h | 3 +-
> > drivers/net/enic/enic_ethdev.c | 4 +-
> > drivers/net/enic/enic_main.c | 3 +-
> > drivers/net/enic/enic_vf_representor.c | 3 +-
> > drivers/net/failsafe/failsafe_ether.c | 9 --
> > drivers/net/failsafe/failsafe_ops.c | 3 +-
> > drivers/net/gve/gve_ethdev.c | 4 +-
> > drivers/net/hinic/hinic_pmd_ethdev.c | 64 ++++++---
> > drivers/net/hns3/hns3_stats.c | 4 +-
> > drivers/net/hns3/hns3_stats.h | 3 +-
> > drivers/net/intel/cpfl/cpfl_ethdev.c | 3 +-
> > drivers/net/intel/e1000/em_ethdev.c | 7 +-
> > drivers/net/intel/e1000/igb_ethdev.c | 14 +-
> > drivers/net/intel/e1000/igc_ethdev.c | 33 +++--
> > drivers/net/intel/fm10k/fm10k_ethdev.c | 27 ++--
> > drivers/net/intel/i40e/i40e_ethdev.c | 5 +-
> > drivers/net/intel/i40e/i40e_vf_representor.c | 2 +-
> > drivers/net/intel/iavf/iavf_ethdev.c | 5 +-
> > drivers/net/intel/ice/ice_dcf_ethdev.c | 3 +-
> > drivers/net/intel/ice/ice_ethdev.c | 7 +-
> > drivers/net/intel/idpf/idpf_ethdev.c | 3 +-
> > drivers/net/intel/ipn3ke/ipn3ke_representor.c | 14 +-
> > drivers/net/intel/ixgbe/ixgbe_ethdev.c | 29 ++--
> > drivers/net/ionic/ionic_ethdev.c | 6 +-
> > drivers/net/ionic/ionic_lif.c | 35 +++--
> > drivers/net/ionic/ionic_lif.h | 3 +-
> > drivers/net/mana/mana.c | 15 +-
> > drivers/net/memif/rte_eth_memif.c | 15 +-
> > drivers/net/mlx4/mlx4.h | 3 +-
> > drivers/net/mlx4/mlx4_ethdev.c | 17 +--
> > drivers/net/mlx5/mlx5.h | 3 +-
> > drivers/net/mlx5/mlx5_stats.c | 17 +--
> > drivers/net/mvneta/mvneta_ethdev.c | 5 +-
> > drivers/net/mvpp2/mrvl_ethdev.c | 23 +--
> > drivers/net/netvsc/hn_ethdev.c | 17 +--
> > drivers/net/netvsc/hn_var.h | 3 +-
> > drivers/net/netvsc/hn_vf.c | 3 +-
> > drivers/net/nfp/flower/nfp_flower.c | 8 +-
> > .../net/nfp/flower/nfp_flower_representor.c | 19 +--
> > .../net/nfp/flower/nfp_flower_representor.h | 3 +
> > drivers/net/nfp/nfp_net_common.c | 46 +++---
> > drivers/net/nfp/nfp_net_common.h | 4 +-
> > drivers/net/ngbe/ngbe_ethdev.c | 53 +++----
> > drivers/net/ngbe/ngbe_ethdev_vf.c | 5 +-
> > drivers/net/ntnic/ntnic_ethdev.c | 22 +--
> > drivers/net/null/rte_eth_null.c | 15 +-
> > drivers/net/octeon_ep/otx_ep_ethdev.c | 17 ++-
> > drivers/net/octeontx/octeontx_ethdev.c | 3 +-
> > drivers/net/pcap/pcap_ethdev.c | 23 +--
> > drivers/net/pfe/pfe_ethdev.c | 3 +-
> > drivers/net/qede/qede_ethdev.c | 23 ++-
> > drivers/net/r8169/r8169_ethdev.c | 6 +-
> > drivers/net/ring/rte_eth_ring.c | 13 +-
> > drivers/net/rnp/rnp_ethdev.c | 15 +-
> > drivers/net/sfc/sfc_ethdev.c | 3 +-
> > drivers/net/sfc/sfc_repr.c | 3 +-
> > drivers/net/tap/rte_eth_tap.c | 23 +--
> > drivers/net/thunderx/nicvf_ethdev.c | 27 ++--
> > drivers/net/txgbe/txgbe_ethdev.c | 53 +++----
> > drivers/net/txgbe/txgbe_ethdev_vf.c | 5 +-
> > drivers/net/vhost/rte_eth_vhost.c | 26 ++--
> > drivers/net/virtio/virtio_ethdev.c | 23 +--
> > drivers/net/vmxnet3/vmxnet3_ethdev.c | 20 +--
> > drivers/net/xsc/xsc_ethdev.c | 19 +--
> > drivers/net/zxdh/zxdh_ethdev_ops.c | 39 ++---
> > drivers/net/zxdh/zxdh_ethdev_ops.h | 3 +-
> > lib/ethdev/ethdev_driver.h | 23 ++-
> > lib/ethdev/ethdev_private.c | 26 ++++
> > lib/ethdev/ethdev_private.h | 2 +
> > lib/ethdev/rte_ethdev.c | 37 ++---
> > lib/ethdev/rte_ethdev.h | 11 --
> > lib/ethdev/rte_ethdev_telemetry.c | 20 +--
> > 107 files changed, 898 insertions(+), 706 deletions(-)
> >
> > --
> > 2.48.1
> >
>
> There is one thing xstats does not do well. Because xstats is a per-driver
> API, there is no uniformity in reporting per-queue information. And if you
> take off the auto-fill flag what little information there is will be disappear.
>
> I would propose a new API (and driver callback) to query a single queues
> statistics. This would give uniformity across drivers.
>
> But not certain it really matters, have not seen any code that references
> the queue counters directly. Maybe somewhere in VPP (doing SNMP??) could care.
> The main use of xstats is in debugging and in that case the per-driver
> naming is not a big issue.
>
Yes, I agree that xstats is not ideally and that a separate API for
querying the queue stats would indeed be best. We should add that in
future, but for now, I think a first step is to start the removal process
we flagged 5 years ago!
/Bruce
prev parent reply other threads:[~2025-09-23 15:33 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-23 14:12 Bruce Richardson
2025-09-23 14:12 ` [RFC PATCH 1/6] net/ice: don't report empty queue xstats Bruce Richardson
2025-09-23 14:12 ` [RFC PATCH 2/6] net/ipn3ke: drop unsupported per-queue xstats Bruce Richardson
2025-09-24 1:38 ` Xu, Rosen
2025-09-23 14:12 ` [RFC PATCH 3/6] ethdev: remove queue stats from ethdev stats structure Bruce Richardson
2025-09-24 7:37 ` Morten Brørup
2025-09-24 7:42 ` Bruce Richardson
2025-09-23 14:12 ` [RFC PATCH 4/6] drivers/net: update to remove queue stats from eth stats Bruce Richardson
2025-09-24 1:39 ` Xu, Rosen
2025-09-23 14:12 ` [RFC PATCH 5/6] app: " Bruce Richardson
2025-09-23 14:12 ` [RFC PATCH 6/6] doc: update docs for ethdev changes Bruce Richardson
2025-09-23 15:04 ` [RFC PATCH 0/6] remove deprecated queue stats Stephen Hemminger
2025-09-23 15:33 ` Bruce Richardson [this message]
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=aNK9sG1ISFOBUb98@bricha3-mobl1.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=stephen@networkplumber.org \
/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).