[v2] drivers/net: remove queue xstats auto-fill flag
Checks
Commit Message
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(-)
Comments
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.
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.
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.
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.
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.
>
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.
>>
>
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.
@@ -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;
@@ -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) {
@@ -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;
@@ -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;
@@ -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;
@@ -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;
@@ -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) *
@@ -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;
@@ -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;
@@ -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 */
@@ -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;
@@ -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");
@@ -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;
@@ -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;
@@ -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);
@@ -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.
@@ -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);