Hi Stefan,
+David Miller/Net ML
quoted
@@ -6844,6 +7023,10 @@ static int mvpp2_open(struct net_device
*dev)
mvpp2_start_dev(port);
+ /* Start hardware statistics gathering */
+ queue_delayed_work(priv->stats_queue, &priv->stats_work,
+ MVPP2_MIB_COUNTERS_STATS_DELAY);
+
return 0;
err_free_link_irq:
@@ -6888,6 +7071,9 @@ static int mvpp2_stop(struct net_device *dev)
mvpp2_cleanup_rxqs(port);
mvpp2_cleanup_txqs(port);
+ cancel_delayed_work_sync(&priv->stats_work);
+ flush_workqueue(priv->stats_queue);
+
Hi Miquel,
I think there are bug here.
priv is common for all ports on same CPN and they have same
priv->stats_work.
For example on A7K board with 3 Ports. queue_delayed_work and
cancel_delayed_work_sync called for each port stop and start
procedure. For example:
If Port0 and Port1 were started, then if only Port0 stopped, delayed
work would be canceled for both ports.
Thanks for spotting it, you are right this is a bug since I moved
starting/stopping the queues in the opening and close procedure of the
ports (to avoid using CPU time while no interface is actually up).
Maybe I should have a work per port, it would be easier to
handle.
Thank you,
Miquèl
-----Original Message-----
From: Miquel RAYNAL [mailto:miquel.raynal@free-electrons.com]
Sent: Tuesday, November 07, 2017 12:45 AM
To: Stefan Chulski <redacted>
Cc: Thomas Petazzoni <redacted>; Antoine Tenart
[off-list ref]; David S. Miller
[off-list ref]; netdev@vger.kernel.org
Subject: [EXT] Re: [PATCH net-next v2] net: mvpp2: add ethtool GOP statistics
External Email
----------------------------------------------------------------------
Hi Stefan,
+David Miller/Net ML
quoted
quoted
@@ -6844,6 +7023,10 @@ static int mvpp2_open(struct net_device
*dev)
mvpp2_start_dev(port);
+ /* Start hardware statistics gathering */
+ queue_delayed_work(priv->stats_queue, &priv->stats_work,
+ MVPP2_MIB_COUNTERS_STATS_DELAY);
+
return 0;
err_free_link_irq:
@@ -6888,6 +7071,9 @@ static int mvpp2_stop(struct net_device *dev)
mvpp2_cleanup_rxqs(port);
mvpp2_cleanup_txqs(port);
+ cancel_delayed_work_sync(&priv->stats_work);
+ flush_workqueue(priv->stats_queue);
+
Hi Miquel,
I think there are bug here.
priv is common for all ports on same CPN and they have same
priv->stats_work.
For example on A7K board with 3 Ports. queue_delayed_work and
cancel_delayed_work_sync called for each port stop and start
procedure. For example:
If Port0 and Port1 were started, then if only Port0 stopped, delayed
work would be canceled for both ports.
Thanks for spotting it, you are right this is a bug since I moved starting/stopping
the queues in the opening and close procedure of the ports (to avoid using CPU
time while no interface is actually up).
Maybe I should have a work per port, it would be easier to handle.
Thank you,
Miquèl
I agree with you.
If delayed_work enable/disabled upon port start/stop procedure, it should be
per port and gather MIB statistics for single port.
Stefan.