* [dpdk-dev] [PATCH] net/sfc: remove queue xstats auto-fill flag
@ 2021-09-28 14:16 Andrew Rybchenko
2021-09-28 16:48 ` [dpdk-dev] [PATCH v2] drivers/net: " Andrew Rybchenko
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Rybchenko @ 2021-09-28 14:16 UTC (permalink / raw)
To: dev; +Cc: stable
Driver does not provide per-queue statistics. So, there is no point
to have these misleading zeros in xstats.
Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats")
Cc: stable@dpdk.org
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
drivers/net/sfc/sfc_ethdev.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 2db0d000c3..07b2bd4ce5 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -2252,7 +2252,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
/* Copy PCI device info to the dev->data */
rte_eth_copy_pci_info(dev, pci_dev);
- dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;
rc = sfc_kvargs_parse(sa);
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-28 14:16 [dpdk-dev] [PATCH] net/sfc: remove queue xstats auto-fill flag Andrew Rybchenko @ 2021-09-28 16:48 ` Andrew Rybchenko 2021-09-28 17:10 ` Stephen Hemminger 2021-10-14 22:19 ` [dpdk-dev] " Ferruh Yigit 0 siblings, 2 replies; 9+ messages in thread From: Andrew Rybchenko @ 2021-09-28 16:48 UTC (permalink / raw) To: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Ferruh Yigit, Xiao Wang, Thomas Monjalon Cc: dev, stable Some drivers do not provide per-queue statistics. So, there is no point to have these misleading zeros in xstats. Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") Cc: stable@dpdk.org Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> --- drivers/net/bnx2x/bnx2x_ethdev.c | 1 - drivers/net/dpaa/dpaa_ethdev.c | 2 -- drivers/net/e1000/em_ethdev.c | 1 - drivers/net/e1000/igb_ethdev.c | 2 -- drivers/net/enetc/enetc_ethdev.c | 2 -- drivers/net/enic/enic_ethdev.c | 1 - drivers/net/enic/enic_vf_representor.c | 3 +-- drivers/net/i40e/i40e_ethdev.c | 1 - drivers/net/i40e/i40e_ethdev_vf.c | 1 - drivers/net/i40e/i40e_vf_representor.c | 3 +-- drivers/net/iavf/iavf_ethdev.c | 1 - drivers/net/ice/ice_dcf_ethdev.c | 2 -- drivers/net/liquidio/lio_ethdev.c | 1 - drivers/net/mvneta/mvneta_ethdev.c | 1 - drivers/net/octeontx/octeontx_ethdev.c | 1 - drivers/net/pfe/pfe_ethdev.c | 2 -- drivers/net/sfc/sfc_ethdev.c | 1 - 17 files changed, 2 insertions(+), 24 deletions(-) diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c index 463886f17a..8b0806016f 100644 --- a/drivers/net/bnx2x/bnx2x_ethdev.c +++ b/drivers/net/bnx2x/bnx2x_ethdev.c @@ -648,7 +648,6 @@ bnx2x_common_dev_init(struct rte_eth_dev *eth_dev, int is_vf) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; sc->pcie_bus = pci_dev->addr.bus; sc->pcie_device = pci_dev->addr.devid; diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c index 36d8f9249d..c6937ec528 100644 --- a/drivers/net/dpaa/dpaa_ethdev.c +++ b/drivers/net/dpaa/dpaa_ethdev.c @@ -2232,8 +2232,6 @@ rte_dpaa_probe(struct rte_dpaa_driver *dpaa_drv, if (dpaa_drv->drv_flags & RTE_DPAA_DRV_INTR_LSC) eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; - /* Invoke PMD device initialization function */ diag = dpaa_dev_init(eth_dev); if (diag == 0) { diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index a0ca371b02..db24aee746 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -265,7 +265,6 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; hw->device_id = pci_dev->id.device_id; diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index d80fad01e3..d7c50b65fb 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -743,7 +743,6 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev) } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; hw->hw_addr= (void *)pci_dev->mem_resource[0].addr; @@ -938,7 +937,6 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev) pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; hw->device_id = pci_dev->id.device_id; hw->vendor_id = pci_dev->id.vendor_id; diff --git a/drivers/net/enetc/enetc_ethdev.c b/drivers/net/enetc/enetc_ethdev.c index b496cd4700..f26e605348 100644 --- a/drivers/net/enetc/enetc_ethdev.c +++ b/drivers/net/enetc/enetc_ethdev.c @@ -885,8 +885,6 @@ enetc_dev_init(struct rte_eth_dev *eth_dev) eth_dev->rx_pkt_burst = &enetc_recv_pkts; eth_dev->tx_pkt_burst = &enetc_xmit_pkts; - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; - /* Retrieving and storing the HW base address of device */ hw->hw.reg = (void *)pci_dev->mem_resource[0].addr; hw->device_id = pci_dev->id.device_id; diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index 8d5797523b..680d6c7a1b 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -1260,7 +1260,6 @@ static int eth_enic_dev_init(struct rte_eth_dev *eth_dev, pdev = RTE_ETH_DEV_TO_PCI(eth_dev); rte_eth_copy_pci_info(eth_dev, pdev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; enic->pdev = pdev; addr = &pdev->addr; diff --git a/drivers/net/enic/enic_vf_representor.c b/drivers/net/enic/enic_vf_representor.c index 79dd6e5640..437ef0e94b 100644 --- a/drivers/net/enic/enic_vf_representor.c +++ b/drivers/net/enic/enic_vf_representor.c @@ -659,8 +659,7 @@ int enic_vf_representor_init(struct rte_eth_dev *eth_dev, void *init_params) eth_dev->device->driver = pf->rte_dev->device->driver; eth_dev->dev_ops = &enic_vf_representor_dev_ops; - eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR | - RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; + eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; eth_dev->data->representor_id = vf->vf_id; eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr_vf", sizeof(struct rte_ether_addr) * diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index b8b921c145..a0a0ed8ec6 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -1442,7 +1442,6 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused) intr_handle = &pci_dev->intr_handle; rte_eth_copy_pci_info(dev, pci_dev); - dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private); pf->dev_data = dev->data; diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index e8dd6d1dab..5e27d5bd09 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -1588,7 +1588,6 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev) } i40e_set_default_ptype_table(eth_dev); rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; hw->vendor_id = pci_dev->id.vendor_id; hw->device_id = pci_dev->id.device_id; diff --git a/drivers/net/i40e/i40e_vf_representor.c b/drivers/net/i40e/i40e_vf_representor.c index 0481b55381..323fa85893 100644 --- a/drivers/net/i40e/i40e_vf_representor.c +++ b/drivers/net/i40e/i40e_vf_representor.c @@ -511,8 +511,7 @@ i40e_vf_representor_init(struct rte_eth_dev *ethdev, void *init_params) return -ENODEV; } - ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR | - RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; + ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; ethdev->data->representor_id = representor->vf_id; /* Setting the number queues allocated to the VF */ diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index 985ccc98ac..74786218fc 100644 --- a/drivers/net/iavf/iavf_ethdev.c +++ b/drivers/net/iavf/iavf_ethdev.c @@ -2331,7 +2331,6 @@ iavf_dev_init(struct rte_eth_dev *eth_dev) return 0; } rte_eth_copy_pci_info(eth_dev, pci_dev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; hw->vendor_id = pci_dev->id.vendor_id; hw->device_id = pci_dev->id.device_id; diff --git a/drivers/net/ice/ice_dcf_ethdev.c b/drivers/net/ice/ice_dcf_ethdev.c index f510bad381..3492bfe402 100644 --- a/drivers/net/ice/ice_dcf_ethdev.c +++ b/drivers/net/ice/ice_dcf_ethdev.c @@ -1091,8 +1091,6 @@ ice_dcf_dev_init(struct rte_eth_dev *eth_dev) if (rte_eal_process_type() != RTE_PROC_PRIMARY) return 0; - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; - adapter->real_hw.vc_event_msg_cb = ice_dcf_handle_pf_event_msg; if (ice_dcf_init_hw(eth_dev, &adapter->real_hw) != 0) { PMD_INIT_LOG(ERR, "Failed to init DCF hardware"); diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c index b72060a449..63cc46b100 100644 --- a/drivers/net/liquidio/lio_ethdev.c +++ b/drivers/net/liquidio/lio_ethdev.c @@ -2094,7 +2094,6 @@ lio_eth_dev_init(struct rte_eth_dev *eth_dev) return 0; rte_eth_copy_pci_info(eth_dev, pdev); - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; if (pdev->mem_resource[0].addr) { lio_dev->hw_addr = pdev->mem_resource[0].addr; diff --git a/drivers/net/mvneta/mvneta_ethdev.c b/drivers/net/mvneta/mvneta_ethdev.c index a3ee150204..bfb7f7a772 100644 --- a/drivers/net/mvneta/mvneta_ethdev.c +++ b/drivers/net/mvneta/mvneta_ethdev.c @@ -840,7 +840,6 @@ mvneta_eth_dev_create(struct rte_vdev_device *vdev, const char *name) eth_dev->rx_pkt_burst = mvneta_rx_pkt_burst; mvneta_set_tx_function(eth_dev); eth_dev->dev_ops = &mvneta_ops; - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; rte_eth_dev_probing_finish(eth_dev); return 0; diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c index 9f4c0503b4..768ac63e91 100644 --- a/drivers/net/octeontx/octeontx_ethdev.c +++ b/drivers/net/octeontx/octeontx_ethdev.c @@ -1374,7 +1374,6 @@ octeontx_create(struct rte_vdev_device *dev, int port, uint8_t evdev, data->promiscuous = 0; data->all_multicast = 0; data->scattered_rx = 0; - data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; /* Get maximum number of supported MAC entries */ max_entries = octeontx_bgx_port_mac_entries_get(nic->port_id); diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c index feec4d10a2..71d80cc432 100644 --- a/drivers/net/pfe/pfe_ethdev.c +++ b/drivers/net/pfe/pfe_ethdev.c @@ -850,8 +850,6 @@ pfe_eth_init(struct rte_vdev_device *vdev, struct pfe *pfe, int id) eth_dev->data->nb_rx_queues = 1; eth_dev->data->nb_tx_queues = 1; - eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; - /* For link status, open the PFE CDEV; Error from this function * is silently ignored; In case of error, the link status will not * be available. diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index 2db0d000c3..07b2bd4ce5 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -2252,7 +2252,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev) /* Copy PCI device info to the dev->data */ rte_eth_copy_pci_info(dev, pci_dev); - dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE; rc = sfc_kvargs_parse(sa); -- 2.30.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-28 16:48 ` [dpdk-dev] [PATCH v2] drivers/net: " Andrew Rybchenko @ 2021-09-28 17:10 ` Stephen Hemminger 2021-09-29 6:38 ` Andrew Rybchenko 2021-10-14 22:19 ` [dpdk-dev] " Ferruh Yigit 1 sibling, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2021-09-28 17:10 UTC (permalink / raw) To: Andrew Rybchenko Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Ferruh Yigit, Xiao Wang, Thomas Monjalon, dev, stable On Tue, 28 Sep 2021 19:48:54 +0300 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: > Some drivers do not provide per-queue statistics. So, there is no point > to have these misleading zeros in xstats. > > Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") > Cc: stable@dpdk.org > > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Really? It is useful to have zeros rather than random data there. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-28 17:10 ` Stephen Hemminger @ 2021-09-29 6:38 ` Andrew Rybchenko 2021-09-30 13:00 ` Ferruh Yigit 0 siblings, 1 reply; 9+ messages in thread From: Andrew Rybchenko @ 2021-09-29 6:38 UTC (permalink / raw) To: Stephen Hemminger Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Ferruh Yigit, Xiao Wang, Thomas Monjalon, dev, stable On 9/28/21 8:10 PM, Stephen Hemminger wrote: > On Tue, 28 Sep 2021 19:48:54 +0300 > Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: > >> Some drivers do not provide per-queue statistics. So, there is no point >> to have these misleading zeros in xstats. >> >> Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") >> Cc: stable@dpdk.org >> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > Really? > It is useful to have zeros rather than random data there. I guess there is a misunderstanding here. Auto-filling xstats is an addition of per-queue basic statistics to xstats by ethdev layer. It makes sense to do it if and only if there is some sensible data there. There is a related deprecation notice saying that per-queue stats should be removed from basic stats since per-queue stats should be provided by xstats API natively. Basically RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS means that corresponding driver is not ready vs the deprecation notice. So, I want to clean it up to see not yet ready drivers only. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-29 6:38 ` Andrew Rybchenko @ 2021-09-30 13:00 ` Ferruh Yigit 2021-09-30 13:45 ` Andrew Rybchenko 0 siblings, 1 reply; 9+ messages in thread From: Ferruh Yigit @ 2021-09-30 13:00 UTC (permalink / raw) To: Andrew Rybchenko, Stephen Hemminger Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Xiao Wang, Thomas Monjalon, dev, stable On 9/29/2021 7:38 AM, Andrew Rybchenko wrote: > On 9/28/21 8:10 PM, Stephen Hemminger wrote: >> On Tue, 28 Sep 2021 19:48:54 +0300 >> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: >> >>> Some drivers do not provide per-queue statistics. So, there is no point >>> to have these misleading zeros in xstats. >>> >>> Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >> Really? >> It is useful to have zeros rather than random data there. > > I guess there is a misunderstanding here. Auto-filling xstats is > an addition of per-queue basic statistics to xstats by ethdev > layer. It makes sense to do it if and only if there is some > sensible data there. > > There is a related deprecation notice saying that per-queue > stats should be removed from basic stats since per-queue > stats should be provided by xstats API natively. > > Basically RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS means > that corresponding driver is not ready vs the deprecation notice. > So, I want to clean it up to see not yet ready drivers only. > As you said, 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' flag set by driver means, driver is not ready on representing queue stats in xstats and ethdev layer is filling it automatically from basic stats. First we should wait for drivers to implement it, later clean queue stats from basic stats and remove the flag. I am not sure if we can remove the deprecation notice in this release, but agree to add a deadline for the drivers, which can be 22.11. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-30 13:00 ` Ferruh Yigit @ 2021-09-30 13:45 ` Andrew Rybchenko 2021-09-30 13:50 ` Andrew Rybchenko 0 siblings, 1 reply; 9+ messages in thread From: Andrew Rybchenko @ 2021-09-30 13:45 UTC (permalink / raw) To: Ferruh Yigit, Stephen Hemminger Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Xiao Wang, Thomas Monjalon, dev, stable On 9/30/21 4:00 PM, Ferruh Yigit wrote: > On 9/29/2021 7:38 AM, Andrew Rybchenko wrote: >> On 9/28/21 8:10 PM, Stephen Hemminger wrote: >>> On Tue, 28 Sep 2021 19:48:54 +0300 >>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: >>> >>>> Some drivers do not provide per-queue statistics. So, there is no point >>>> to have these misleading zeros in xstats. >>>> >>>> Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >>> Really? >>> It is useful to have zeros rather than random data there. >> >> I guess there is a misunderstanding here. Auto-filling xstats is >> an addition of per-queue basic statistics to xstats by ethdev >> layer. It makes sense to do it if and only if there is some >> sensible data there. >> >> There is a related deprecation notice saying that per-queue >> stats should be removed from basic stats since per-queue >> stats should be provided by xstats API natively. >> >> Basically RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS means >> that corresponding driver is not ready vs the deprecation notice. >> So, I want to clean it up to see not yet ready drivers only. >> > > As you said, 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' flag set by driver means, > driver is not ready on representing queue stats in xstats and ethdev layer is > filling it automatically from basic stats. > > First we should wait for drivers to implement it, later clean queue stats from > basic stats and remove the flag. > > I am not sure if we can remove the deprecation notice in this release, but agree > to add a deadline for the drivers, which can be 22.11. > I'm going to cleanup deprecation. I don't touch it in the patch. I just want to cleanup list of drivers which require attention/changes. Drivers covered here do not provide per-queue stats in basic stats. So, there is no point to set the flag to show it in xstats. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-30 13:45 ` Andrew Rybchenko @ 2021-09-30 13:50 ` Andrew Rybchenko 2021-10-14 22:16 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit 0 siblings, 1 reply; 9+ messages in thread From: Andrew Rybchenko @ 2021-09-30 13:50 UTC (permalink / raw) To: Ferruh Yigit, Stephen Hemminger Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Xiao Wang, Thomas Monjalon, dev, stable On 9/30/21 4:45 PM, Andrew Rybchenko wrote: > On 9/30/21 4:00 PM, Ferruh Yigit wrote: >> On 9/29/2021 7:38 AM, Andrew Rybchenko wrote: >>> On 9/28/21 8:10 PM, Stephen Hemminger wrote: >>>> On Tue, 28 Sep 2021 19:48:54 +0300 >>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: >>>> >>>>> Some drivers do not provide per-queue statistics. So, there is no point >>>>> to have these misleading zeros in xstats. >>>>> >>>>> Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") >>>>> Cc: stable@dpdk.org >>>>> >>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >>>> Really? >>>> It is useful to have zeros rather than random data there. >>> >>> I guess there is a misunderstanding here. Auto-filling xstats is >>> an addition of per-queue basic statistics to xstats by ethdev >>> layer. It makes sense to do it if and only if there is some >>> sensible data there. >>> >>> There is a related deprecation notice saying that per-queue >>> stats should be removed from basic stats since per-queue >>> stats should be provided by xstats API natively. >>> >>> Basically RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS means >>> that corresponding driver is not ready vs the deprecation notice. >>> So, I want to clean it up to see not yet ready drivers only. >>> >> >> As you said, 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' flag set by driver means, >> driver is not ready on representing queue stats in xstats and ethdev layer is >> filling it automatically from basic stats. >> >> First we should wait for drivers to implement it, later clean queue stats from >> basic stats and remove the flag. >> >> I am not sure if we can remove the deprecation notice in this release, but agree >> to add a deadline for the drivers, which can be 22.11. >> > > I'm going to cleanup deprecation. I don't touch it in the Sorry "I'm NOT going to ..." > patch. I just want to cleanup list of drivers which > require attention/changes. Drivers covered here do not > provide per-queue stats in basic stats. So, there is no > point to set the flag to show it in xstats. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-30 13:50 ` Andrew Rybchenko @ 2021-10-14 22:16 ` Ferruh Yigit 0 siblings, 0 replies; 9+ messages in thread From: Ferruh Yigit @ 2021-10-14 22:16 UTC (permalink / raw) To: Andrew Rybchenko, Stephen Hemminger Cc: Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Xiao Wang, Thomas Monjalon, dev, stable On 9/30/2021 2:50 PM, Andrew Rybchenko wrote: > On 9/30/21 4:45 PM, Andrew Rybchenko wrote: >> On 9/30/21 4:00 PM, Ferruh Yigit wrote: >>> On 9/29/2021 7:38 AM, Andrew Rybchenko wrote: >>>> On 9/28/21 8:10 PM, Stephen Hemminger wrote: >>>>> On Tue, 28 Sep 2021 19:48:54 +0300 >>>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> wrote: >>>>> >>>>>> Some drivers do not provide per-queue statistics. So, there is no point >>>>>> to have these misleading zeros in xstats. >>>>>> >>>>>> Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") >>>>>> Cc: stable@dpdk.org >>>>>> >>>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >>>>> Really? >>>>> It is useful to have zeros rather than random data there. >>>> >>>> I guess there is a misunderstanding here. Auto-filling xstats is >>>> an addition of per-queue basic statistics to xstats by ethdev >>>> layer. It makes sense to do it if and only if there is some >>>> sensible data there. >>>> >>>> There is a related deprecation notice saying that per-queue >>>> stats should be removed from basic stats since per-queue >>>> stats should be provided by xstats API natively. >>>> >>>> Basically RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS means >>>> that corresponding driver is not ready vs the deprecation notice. >>>> So, I want to clean it up to see not yet ready drivers only. >>>> >>> >>> As you said, 'RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS' flag set by driver means, >>> driver is not ready on representing queue stats in xstats and ethdev layer is >>> filling it automatically from basic stats. >>> >>> First we should wait for drivers to implement it, later clean queue stats from >>> basic stats and remove the flag. >>> >>> I am not sure if we can remove the deprecation notice in this release, but agree >>> to add a deadline for the drivers, which can be 22.11. >>> >> >> I'm going to cleanup deprecation. I don't touch it in the > > Sorry "I'm NOT going to ..." > OK, I see now, I thought intention was to clean the deprecation. Agree to not add queue stats to xstats when driver doesn't have the queue stats at all. >> patch. I just want to cleanup list of drivers which >> require attention/changes. Drivers covered here do not >> provide per-queue stats in basic stats. So, there is no >> point to set the flag to show it in xstats. >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] drivers/net: remove queue xstats auto-fill flag 2021-09-28 16:48 ` [dpdk-dev] [PATCH v2] drivers/net: " Andrew Rybchenko 2021-09-28 17:10 ` Stephen Hemminger @ 2021-10-14 22:19 ` Ferruh Yigit 1 sibling, 0 replies; 9+ messages in thread From: Ferruh Yigit @ 2021-10-14 22:19 UTC (permalink / raw) To: Andrew Rybchenko, Rasesh Mody, Shahed Shaikh, Hemant Agrawal, Sachin Saxena, Haiyue Wang, Gagandeep Singh, John Daley, Hyong Youb Kim, Beilei Xing, Jingjing Wu, Qiming Yang, Qi Zhang, Shijith Thotton, Srisivasubramanian Srinivasan, Zyta Szpak, Liron Himi, Harman Kalra, Xiao Wang, Thomas Monjalon Cc: dev, stable On 9/28/2021 5:48 PM, Andrew Rybchenko wrote: > Some drivers do not provide per-queue statistics. So, there is no point > to have these misleading zeros in xstats. > > Fixes: f30e69b41f94 ("ethdev: add device flag to bypass auto-filled queue xstats") > Cc: stable@dpdk.org > > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> Applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-14 22:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-28 14:16 [dpdk-dev] [PATCH] net/sfc: remove queue xstats auto-fill flag Andrew Rybchenko 2021-09-28 16:48 ` [dpdk-dev] [PATCH v2] drivers/net: " Andrew Rybchenko 2021-09-28 17:10 ` Stephen Hemminger 2021-09-29 6:38 ` Andrew Rybchenko 2021-09-30 13:00 ` Ferruh Yigit 2021-09-30 13:45 ` Andrew Rybchenko 2021-09-30 13:50 ` Andrew Rybchenko 2021-10-14 22:16 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit 2021-10-14 22:19 ` [dpdk-dev] " Ferruh Yigit
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).