* [PATCH v2 0/2] net: dsa: add stats64 support @ 2020-12-02 12:07 Oleksij Rempel 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel 0 siblings, 2 replies; 14+ messages in thread From: Oleksij Rempel @ 2020-12-02 12:07 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King Cc: Oleksij Rempel, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips changes v2: - use stats64 instead of get_ethtool_stats - add worked to poll for the stats Oleksij Rempel (2): net: dsa: add optional stats64 support net: dsa: qca: ar9331: export stats64 drivers/net/dsa/qca/ar9331.c | 242 ++++++++++++++++++++++++++++++++++- include/net/dsa.h | 3 + net/dsa/slave.c | 14 +- 3 files changed, 257 insertions(+), 2 deletions(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] net: dsa: add optional stats64 support 2020-12-02 12:07 [PATCH v2 0/2] net: dsa: add stats64 support Oleksij Rempel @ 2020-12-02 12:07 ` Oleksij Rempel 2020-12-02 12:59 ` Vladimir Oltean ` (2 more replies) 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel 1 sibling, 3 replies; 14+ messages in thread From: Oleksij Rempel @ 2020-12-02 12:07 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King Cc: Oleksij Rempel, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips Allow DSA drivers to export stats64 Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- include/net/dsa.h | 3 +++ net/dsa/slave.c | 14 +++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 4e60d2610f20..457b89143875 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -655,6 +655,9 @@ struct dsa_switch_ops { int (*port_change_mtu)(struct dsa_switch *ds, int port, int new_mtu); int (*port_max_mtu)(struct dsa_switch *ds, int port); + + void (*get_stats64)(struct dsa_switch *ds, int port, + struct rtnl_link_stats64 *s); }; #define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes) \ diff --git a/net/dsa/slave.c b/net/dsa/slave.c index ff2266d2b998..6e1a4dc18a97 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -1602,6 +1602,18 @@ static struct devlink_port *dsa_slave_get_devlink_port(struct net_device *dev) return dp->ds->devlink ? &dp->devlink_port : NULL; } +static void dsa_slave_get_stats64(struct net_device *dev, + struct rtnl_link_stats64 *s) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_switch *ds = dp->ds; + + if (!ds->ops->get_stats64) + return dev_get_tstats64(dev, s); + + return ds->ops->get_stats64(ds, dp->index, s); +} + static const struct net_device_ops dsa_slave_netdev_ops = { .ndo_open = dsa_slave_open, .ndo_stop = dsa_slave_close, @@ -1621,7 +1633,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = { #endif .ndo_get_phys_port_name = dsa_slave_get_phys_port_name, .ndo_setup_tc = dsa_slave_setup_tc, - .ndo_get_stats64 = dev_get_tstats64, + .ndo_get_stats64 = dsa_slave_get_stats64, .ndo_get_port_parent_id = dsa_slave_get_port_parent_id, .ndo_vlan_rx_add_vid = dsa_slave_vlan_rx_add_vid, .ndo_vlan_rx_kill_vid = dsa_slave_vlan_rx_kill_vid, -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] net: dsa: add optional stats64 support 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel @ 2020-12-02 12:59 ` Vladimir Oltean 2020-12-02 15:03 ` Andrew Lunn 2020-12-02 17:12 ` Florian Fainelli 2 siblings, 0 replies; 14+ messages in thread From: Vladimir Oltean @ 2020-12-02 12:59 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On Wed, Dec 02, 2020 at 01:07:11PM +0100, Oleksij Rempel wrote: > Allow DSA drivers to export stats64 > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] net: dsa: add optional stats64 support 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel 2020-12-02 12:59 ` Vladimir Oltean @ 2020-12-02 15:03 ` Andrew Lunn 2020-12-02 17:12 ` Florian Fainelli 2 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2020-12-02 15:03 UTC (permalink / raw) To: Oleksij Rempel Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On Wed, Dec 02, 2020 at 01:07:11PM +0100, Oleksij Rempel wrote: > Allow DSA drivers to export stats64 > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] net: dsa: add optional stats64 support 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel 2020-12-02 12:59 ` Vladimir Oltean 2020-12-02 15:03 ` Andrew Lunn @ 2020-12-02 17:12 ` Florian Fainelli 2 siblings, 0 replies; 14+ messages in thread From: Florian Fainelli @ 2020-12-02 17:12 UTC (permalink / raw) To: Oleksij Rempel, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King Cc: Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On 12/2/2020 4:07 AM, Oleksij Rempel wrote: > Allow DSA drivers to export stats64 > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 [PATCH v2 0/2] net: dsa: add stats64 support Oleksij Rempel 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel @ 2020-12-02 12:07 ` Oleksij Rempel 2020-12-02 12:15 ` Marc Kleine-Budde ` (4 more replies) 1 sibling, 5 replies; 14+ messages in thread From: Oleksij Rempel @ 2020-12-02 12:07 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King Cc: Oleksij Rempel, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips Add stats support for the ar9331 switch. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/qca/ar9331.c | 242 ++++++++++++++++++++++++++++++++++- 1 file changed, 241 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c index e24a99031b80..1a8027bc9561 100644 --- a/drivers/net/dsa/qca/ar9331.c +++ b/drivers/net/dsa/qca/ar9331.c @@ -101,6 +101,57 @@ AR9331_SW_PORT_STATUS_RX_FLOW_EN | AR9331_SW_PORT_STATUS_TX_FLOW_EN | \ AR9331_SW_PORT_STATUS_SPEED_M) +/* MIB registers */ +#define AR9331_MIB_COUNTER(x) (0x20000 + ((x) * 0x100)) + +#define AR9331_PORT_MIB_rxbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x00) +#define AR9331_PORT_MIB_rxpause(_port) (AR9331_MIB_COUNTER(_port) + 0x04) +#define AR9331_PORT_MIB_rxmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x08) +#define AR9331_PORT_MIB_rxfcserr(_port) (AR9331_MIB_COUNTER(_port) + 0x0c) +#define AR9331_PORT_MIB_rxalignerr(_port) (AR9331_MIB_COUNTER(_port) + 0x10) +#define AR9331_PORT_MIB_rxrunt(_port) (AR9331_MIB_COUNTER(_port) + 0x14) +#define AR9331_PORT_MIB_rxfragment(_port) (AR9331_MIB_COUNTER(_port) + 0x18) +#define AR9331_PORT_MIB_rx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x1c) +#define AR9331_PORT_MIB_rx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x20) +#define AR9331_PORT_MIB_rx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x24) +#define AR9331_PORT_MIB_rx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x28) +#define AR9331_PORT_MIB_rx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x2c) +#define AR9331_PORT_MIB_rx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x30) +#define AR9331_PORT_MIB_rxmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x34) +#define AR9331_PORT_MIB_rxtoolong(_port) (AR9331_MIB_COUNTER(_port) + 0x38) + +/* 64 bit counter */ +#define AR9331_PORT_MIB_rxgoodbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x3c) + +/* 64 bit counter */ +#define AR9331_PORT_MIB_rxbadbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x44) + +#define AR9331_PORT_MIB_rxoverflow(_port) (AR9331_MIB_COUNTER(_port) + 0x4c) +#define AR9331_PORT_MIB_filtered(_port) (AR9331_MIB_COUNTER(_port) + 0x50) +#define AR9331_PORT_MIB_txbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x54) +#define AR9331_PORT_MIB_txpause(_port) (AR9331_MIB_COUNTER(_port) + 0x58) +#define AR9331_PORT_MIB_txmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x5c) +#define AR9331_PORT_MIB_txunderrun(_port) (AR9331_MIB_COUNTER(_port) + 0x60) +#define AR9331_PORT_MIB_tx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x64) +#define AR9331_PORT_MIB_tx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x68) +#define AR9331_PORT_MIB_tx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x6c) +#define AR9331_PORT_MIB_tx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x70) +#define AR9331_PORT_MIB_tx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x74) +#define AR9331_PORT_MIB_tx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x78) +#define AR9331_PORT_MIB_txmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x7c) +#define AR9331_PORT_MIB_txoversize(_port) (AR9331_MIB_COUNTER(_port) + 0x80) + +/* 64 bit counter */ +#define AR9331_PORT_MIB_txbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x84) + +#define AR9331_PORT_MIB_txcollision(_port) (AR9331_MIB_COUNTER(_port) + 0x8c) +#define AR9331_PORT_MIB_txabortcol(_port) (AR9331_MIB_COUNTER(_port) + 0x90) +#define AR9331_PORT_MIB_txmulticol(_port) (AR9331_MIB_COUNTER(_port) + 0x94) +#define AR9331_PORT_MIB_txsinglecol(_port) (AR9331_MIB_COUNTER(_port) + 0x98) +#define AR9331_PORT_MIB_txexcdefer(_port) (AR9331_MIB_COUNTER(_port) + 0x9c) +#define AR9331_PORT_MIB_txdefer(_port) (AR9331_MIB_COUNTER(_port) + 0xa0) +#define AR9331_PORT_MIB_txlatecol(_port) (AR9331_MIB_COUNTER(_port) + 0xa4) + /* Phy bypass mode * ------------------------------------------------------------------------ * Bit: | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |10 |11 |12 |13 |14 |15 | @@ -154,6 +205,59 @@ #define AR9331_SW_MDIO_POLL_SLEEP_US 1 #define AR9331_SW_MDIO_POLL_TIMEOUT_US 20 +#define STATS_INTERVAL_JIFFIES (100 * HZ) + +struct ar9331_sw_stats { + u64 rxbroad; + u64 rxpause; + u64 rxmulti; + u64 rxfcserr; + u64 rxalignerr; + u64 rxrunt; + u64 rxfragment; + u64 rx64byte; + u64 rx128byte; + u64 rx256byte; + u64 rx512byte; + u64 rx1024byte; + u64 rx1518byte; + u64 rxmaxbyte; + u64 rxtoolong; + u64 rxgoodbyte; + u64 rxbadbyte; + u64 rxoverflow; + u64 filtered; + u64 txbroad; + u64 txpause; + u64 txmulti; + u64 txunderrun; + u64 tx64byte; + u64 tx128byte; + u64 tx256byte; + u64 tx512byte; + u64 tx1024byte; + u64 tx1518byte; + u64 txmaxbyte; + u64 txoversize; + u64 txbyte; + u64 txcollision; + u64 txabortcol; + u64 txmulticol; + u64 txsinglecol; + u64 txexcdefer; + u64 txdefer; + u64 txlatecol; +}; + +struct ar9331_sw_priv; +struct ar9331_sw_port { + int idx; + struct ar9331_sw_priv *priv; + struct delayed_work mib_read; + struct ar9331_sw_stats stats; + struct mutex lock; /* stats access */ +}; + struct ar9331_sw_priv { struct device *dev; struct dsa_switch ds; @@ -163,6 +267,7 @@ struct ar9331_sw_priv { struct mii_bus *sbus; /* mdio slave */ struct regmap *regmap; struct reset_control *sw_reset; + struct ar9331_sw_port port[AR9331_SW_PORTS]; }; /* Warning: switch reset will reset last AR9331_SW_MDIO_PHY_MODE_PAGE request @@ -422,6 +527,7 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, phy_interface_t interface) { struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; + struct ar9331_sw_port *p = &priv->port[port]; struct regmap *regmap = priv->regmap; int ret; @@ -429,6 +535,8 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, AR9331_SW_PORT_STATUS_MAC_MASK, 0); if (ret) dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); + + cancel_delayed_work_sync(&p->mib_read); } static void ar9331_sw_phylink_mac_link_up(struct dsa_switch *ds, int port, @@ -439,10 +547,13 @@ static void ar9331_sw_phylink_mac_link_up(struct dsa_switch *ds, int port, bool tx_pause, bool rx_pause) { struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; + struct ar9331_sw_port *p = &priv->port[port]; struct regmap *regmap = priv->regmap; u32 val; int ret; + schedule_delayed_work(&p->mib_read, 0); + val = AR9331_SW_PORT_STATUS_MAC_MASK; switch (speed) { case SPEED_1000: @@ -475,6 +586,125 @@ static void ar9331_sw_phylink_mac_link_up(struct dsa_switch *ds, int port, dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); } +static u32 ar9331_stat_get_val(const struct ar9331_sw_priv *priv, u32 reg) +{ + u32 val; + int ret; + + ret = regmap_read(priv->regmap, reg, &val); + if (ret) { + dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); + return 0; + } + + return val; +} + +#define AR9331_READ_STATS(_port, _reg) \ +{ \ + const struct ar9331_sw_priv *priv = _port->priv; \ + struct ar9331_sw_stats *s = &_port->stats; \ + int idx = _port->idx; \ +\ + s->_reg += ar9331_stat_get_val(priv, AR9331_PORT_MIB_##_reg(idx)); \ +} + +static void ar9331_read_stats(struct ar9331_sw_port *port) +{ + mutex_lock(&port->lock); + + AR9331_READ_STATS(port, rxbroad); + AR9331_READ_STATS(port, rxpause); + AR9331_READ_STATS(port, rxmulti); + AR9331_READ_STATS(port, rxfcserr); + AR9331_READ_STATS(port, rxalignerr); + AR9331_READ_STATS(port, rxrunt); + AR9331_READ_STATS(port, rxfragment); + AR9331_READ_STATS(port, rx64byte); + AR9331_READ_STATS(port, rx128byte); + AR9331_READ_STATS(port, rx256byte); + AR9331_READ_STATS(port, rx512byte); + AR9331_READ_STATS(port, rx1024byte); + AR9331_READ_STATS(port, rx1518byte); + AR9331_READ_STATS(port, rxmaxbyte); + AR9331_READ_STATS(port, rxtoolong); + AR9331_READ_STATS(port, rxgoodbyte); + AR9331_READ_STATS(port, rxbadbyte); + AR9331_READ_STATS(port, rxoverflow); + AR9331_READ_STATS(port, filtered); + AR9331_READ_STATS(port, txbroad); + AR9331_READ_STATS(port, txpause); + AR9331_READ_STATS(port, txmulti); + AR9331_READ_STATS(port, txunderrun); + AR9331_READ_STATS(port, tx64byte); + AR9331_READ_STATS(port, tx128byte); + AR9331_READ_STATS(port, tx256byte); + AR9331_READ_STATS(port, tx512byte); + AR9331_READ_STATS(port, tx1024byte); + AR9331_READ_STATS(port, tx1518byte); + AR9331_READ_STATS(port, txmaxbyte); + AR9331_READ_STATS(port, txoversize); + AR9331_READ_STATS(port, txbyte); + AR9331_READ_STATS(port, txcollision); + AR9331_READ_STATS(port, txabortcol); + AR9331_READ_STATS(port, txmulticol); + AR9331_READ_STATS(port, txsinglecol); + AR9331_READ_STATS(port, txexcdefer); + AR9331_READ_STATS(port, txdefer); + AR9331_READ_STATS(port, txlatecol); + + mutex_unlock(&port->lock); +} + +static void ar9331_stats_update(struct ar9331_sw_port *port, + struct rtnl_link_stats64 *stats) +{ + struct ar9331_sw_stats *s = &port->stats; + + stats->rx_packets = s->rxbroad + s->rxmulti + s->rx64byte + + s->rx128byte + s->rx256byte + s->rx512byte + s->rx1024byte + + s->rx1518byte + s->rxmaxbyte; + stats->tx_packets = s->txbroad + s->txmulti + s->tx64byte + + s->tx128byte + s->tx256byte + s->tx512byte + s->tx1024byte + + s->tx1518byte + s->txmaxbyte; + stats->rx_bytes = s->rxgoodbyte; + stats->tx_bytes = s->txbyte; + stats->rx_errors = s->rxfcserr + s->rxalignerr + s->rxrunt + + s->rxfragment + s->rxoverflow; + stats->tx_errors = s->txoversize; + stats->multicast = s->rxmulti; + stats->collisions = s->txcollision; + stats->rx_length_errors = s->rxrunt * s->rxfragment + s->rxtoolong; + stats->rx_crc_errors = s->rxfcserr + s->rxalignerr + s->rxfragment; + stats->rx_frame_errors = s->rxalignerr; + stats->rx_missed_errors = s->rxoverflow; + stats->tx_aborted_errors = s->txabortcol; + stats->tx_fifo_errors = s->txunderrun; + stats->tx_window_errors = s->txlatecol; + stats->rx_nohandler = s->filtered; +} + +static void ar9331_do_stats_poll(struct work_struct *work) +{ + + struct ar9331_sw_port *port = container_of(work, struct ar9331_sw_port, + mib_read.work); + + ar9331_read_stats(port); + + schedule_delayed_work(&port->mib_read, STATS_INTERVAL_JIFFIES); +} + +static void ar9331_get_stats64(struct dsa_switch *ds, int port, + struct rtnl_link_stats64 *s) +{ + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; + struct ar9331_sw_port *p = &priv->port[port]; + + ar9331_read_stats(p); + ar9331_stats_update(p, s); +} + static const struct dsa_switch_ops ar9331_sw_ops = { .get_tag_protocol = ar9331_sw_get_tag_protocol, .setup = ar9331_sw_setup, @@ -483,6 +713,7 @@ static const struct dsa_switch_ops ar9331_sw_ops = { .phylink_mac_config = ar9331_sw_phylink_mac_config, .phylink_mac_link_down = ar9331_sw_phylink_mac_link_down, .phylink_mac_link_up = ar9331_sw_phylink_mac_link_up, + .get_stats64 = ar9331_get_stats64, }; static irqreturn_t ar9331_sw_irq(int irq, void *data) @@ -781,7 +1012,7 @@ static int ar9331_sw_probe(struct mdio_device *mdiodev) { struct ar9331_sw_priv *priv; struct dsa_switch *ds; - int ret; + int ret, i; priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -816,6 +1047,15 @@ static int ar9331_sw_probe(struct mdio_device *mdiodev) ds->ops = &priv->ops; dev_set_drvdata(&mdiodev->dev, priv); + for (i = 0; i < ARRAY_SIZE(priv->port); i++) { + struct ar9331_sw_port *port = &priv->port[i]; + + port->idx = i; + port->priv = priv; + mutex_init(&port->lock); + INIT_DELAYED_WORK(&port->mib_read, ar9331_do_stats_poll); + } + ret = dsa_register_switch(ds); if (ret) goto err_remove_irq; -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel @ 2020-12-02 12:15 ` Marc Kleine-Budde 2020-12-02 12:43 ` Oleksij Rempel 2020-12-02 13:04 ` Vladimir Oltean ` (3 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Marc Kleine-Budde @ 2020-12-02 12:15 UTC (permalink / raw) To: Oleksij Rempel, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King Cc: linux-mips, linux-kernel, Pengutronix Kernel Team, netdev [-- Attachment #1.1: Type: text/plain, Size: 5679 bytes --] On 12/2/20 1:07 PM, Oleksij Rempel wrote: > Add stats support for the ar9331 switch. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > drivers/net/dsa/qca/ar9331.c | 242 ++++++++++++++++++++++++++++++++++- > 1 file changed, 241 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c > index e24a99031b80..1a8027bc9561 100644 > --- a/drivers/net/dsa/qca/ar9331.c > +++ b/drivers/net/dsa/qca/ar9331.c > @@ -101,6 +101,57 @@ > AR9331_SW_PORT_STATUS_RX_FLOW_EN | AR9331_SW_PORT_STATUS_TX_FLOW_EN | \ > AR9331_SW_PORT_STATUS_SPEED_M) > > +/* MIB registers */ > +#define AR9331_MIB_COUNTER(x) (0x20000 + ((x) * 0x100)) > + > +#define AR9331_PORT_MIB_rxbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x00) > +#define AR9331_PORT_MIB_rxpause(_port) (AR9331_MIB_COUNTER(_port) + 0x04) > +#define AR9331_PORT_MIB_rxmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x08) > +#define AR9331_PORT_MIB_rxfcserr(_port) (AR9331_MIB_COUNTER(_port) + 0x0c) > +#define AR9331_PORT_MIB_rxalignerr(_port) (AR9331_MIB_COUNTER(_port) + 0x10) > +#define AR9331_PORT_MIB_rxrunt(_port) (AR9331_MIB_COUNTER(_port) + 0x14) > +#define AR9331_PORT_MIB_rxfragment(_port) (AR9331_MIB_COUNTER(_port) + 0x18) > +#define AR9331_PORT_MIB_rx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x1c) > +#define AR9331_PORT_MIB_rx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x20) > +#define AR9331_PORT_MIB_rx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x24) > +#define AR9331_PORT_MIB_rx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x28) > +#define AR9331_PORT_MIB_rx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x2c) > +#define AR9331_PORT_MIB_rx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x30) > +#define AR9331_PORT_MIB_rxmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x34) > +#define AR9331_PORT_MIB_rxtoolong(_port) (AR9331_MIB_COUNTER(_port) + 0x38) > + > +/* 64 bit counter */ > +#define AR9331_PORT_MIB_rxgoodbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x3c) > + > +/* 64 bit counter */ > +#define AR9331_PORT_MIB_rxbadbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x44) > + > +#define AR9331_PORT_MIB_rxoverflow(_port) (AR9331_MIB_COUNTER(_port) + 0x4c) > +#define AR9331_PORT_MIB_filtered(_port) (AR9331_MIB_COUNTER(_port) + 0x50) > +#define AR9331_PORT_MIB_txbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x54) > +#define AR9331_PORT_MIB_txpause(_port) (AR9331_MIB_COUNTER(_port) + 0x58) > +#define AR9331_PORT_MIB_txmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x5c) > +#define AR9331_PORT_MIB_txunderrun(_port) (AR9331_MIB_COUNTER(_port) + 0x60) > +#define AR9331_PORT_MIB_tx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x64) > +#define AR9331_PORT_MIB_tx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x68) > +#define AR9331_PORT_MIB_tx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x6c) > +#define AR9331_PORT_MIB_tx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x70) > +#define AR9331_PORT_MIB_tx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x74) > +#define AR9331_PORT_MIB_tx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x78) > +#define AR9331_PORT_MIB_txmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x7c) > +#define AR9331_PORT_MIB_txoversize(_port) (AR9331_MIB_COUNTER(_port) + 0x80) > + > +/* 64 bit counter */ > +#define AR9331_PORT_MIB_txbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x84) > + > +#define AR9331_PORT_MIB_txcollision(_port) (AR9331_MIB_COUNTER(_port) + 0x8c) > +#define AR9331_PORT_MIB_txabortcol(_port) (AR9331_MIB_COUNTER(_port) + 0x90) > +#define AR9331_PORT_MIB_txmulticol(_port) (AR9331_MIB_COUNTER(_port) + 0x94) > +#define AR9331_PORT_MIB_txsinglecol(_port) (AR9331_MIB_COUNTER(_port) + 0x98) > +#define AR9331_PORT_MIB_txexcdefer(_port) (AR9331_MIB_COUNTER(_port) + 0x9c) > +#define AR9331_PORT_MIB_txdefer(_port) (AR9331_MIB_COUNTER(_port) + 0xa0) > +#define AR9331_PORT_MIB_txlatecol(_port) (AR9331_MIB_COUNTER(_port) + 0xa4) > + > /* Phy bypass mode > * ------------------------------------------------------------------------ > * Bit: | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |10 |11 |12 |13 |14 |15 | > @@ -154,6 +205,59 @@ > #define AR9331_SW_MDIO_POLL_SLEEP_US 1 > #define AR9331_SW_MDIO_POLL_TIMEOUT_US 20 > > +#define STATS_INTERVAL_JIFFIES (100 * HZ) > + > +struct ar9331_sw_stats { > + u64 rxbroad; > + u64 rxpause; > + u64 rxmulti; > + u64 rxfcserr; > + u64 rxalignerr; > + u64 rxrunt; > + u64 rxfragment; > + u64 rx64byte; > + u64 rx128byte; > + u64 rx256byte; > + u64 rx512byte; > + u64 rx1024byte; > + u64 rx1518byte; > + u64 rxmaxbyte; > + u64 rxtoolong; > + u64 rxgoodbyte; > + u64 rxbadbyte; > + u64 rxoverflow; > + u64 filtered; > + u64 txbroad; > + u64 txpause; > + u64 txmulti; > + u64 txunderrun; > + u64 tx64byte; > + u64 tx128byte; > + u64 tx256byte; > + u64 tx512byte; > + u64 tx1024byte; > + u64 tx1518byte; > + u64 txmaxbyte; > + u64 txoversize; > + u64 txbyte; > + u64 txcollision; > + u64 txabortcol; > + u64 txmulticol; > + u64 txsinglecol; > + u64 txexcdefer; > + u64 txdefer; > + u64 txlatecol; > +}; > + > +struct ar9331_sw_priv; > +struct ar9331_sw_port { > + int idx; > + struct ar9331_sw_priv *priv; > + struct delayed_work mib_read; > + struct ar9331_sw_stats stats; > + struct mutex lock; /* stats access */ What does the lock protect? It's only used a single time. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:15 ` Marc Kleine-Budde @ 2020-12-02 12:43 ` Oleksij Rempel 2020-12-02 12:47 ` Marc Kleine-Budde 0 siblings, 1 reply; 14+ messages in thread From: Oleksij Rempel @ 2020-12-02 12:43 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, linux-mips, linux-kernel, Pengutronix Kernel Team, netdev On Wed, Dec 02, 2020 at 01:15:58PM +0100, Marc Kleine-Budde wrote: > On 12/2/20 1:07 PM, Oleksij Rempel wrote: > > Add stats support for the ar9331 switch. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > drivers/net/dsa/qca/ar9331.c | 242 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 241 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c > > index e24a99031b80..1a8027bc9561 100644 > > --- a/drivers/net/dsa/qca/ar9331.c > > +++ b/drivers/net/dsa/qca/ar9331.c > > @@ -101,6 +101,57 @@ > > AR9331_SW_PORT_STATUS_RX_FLOW_EN | AR9331_SW_PORT_STATUS_TX_FLOW_EN | \ > > AR9331_SW_PORT_STATUS_SPEED_M) > > > > +/* MIB registers */ > > +#define AR9331_MIB_COUNTER(x) (0x20000 + ((x) * 0x100)) > > + > > +#define AR9331_PORT_MIB_rxbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x00) > > +#define AR9331_PORT_MIB_rxpause(_port) (AR9331_MIB_COUNTER(_port) + 0x04) > > +#define AR9331_PORT_MIB_rxmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x08) > > +#define AR9331_PORT_MIB_rxfcserr(_port) (AR9331_MIB_COUNTER(_port) + 0x0c) > > +#define AR9331_PORT_MIB_rxalignerr(_port) (AR9331_MIB_COUNTER(_port) + 0x10) > > +#define AR9331_PORT_MIB_rxrunt(_port) (AR9331_MIB_COUNTER(_port) + 0x14) > > +#define AR9331_PORT_MIB_rxfragment(_port) (AR9331_MIB_COUNTER(_port) + 0x18) > > +#define AR9331_PORT_MIB_rx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x1c) > > +#define AR9331_PORT_MIB_rx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x20) > > +#define AR9331_PORT_MIB_rx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x24) > > +#define AR9331_PORT_MIB_rx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x28) > > +#define AR9331_PORT_MIB_rx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x2c) > > +#define AR9331_PORT_MIB_rx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x30) > > +#define AR9331_PORT_MIB_rxmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x34) > > +#define AR9331_PORT_MIB_rxtoolong(_port) (AR9331_MIB_COUNTER(_port) + 0x38) > > + > > +/* 64 bit counter */ > > +#define AR9331_PORT_MIB_rxgoodbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x3c) > > + > > +/* 64 bit counter */ > > +#define AR9331_PORT_MIB_rxbadbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x44) > > + > > +#define AR9331_PORT_MIB_rxoverflow(_port) (AR9331_MIB_COUNTER(_port) + 0x4c) > > +#define AR9331_PORT_MIB_filtered(_port) (AR9331_MIB_COUNTER(_port) + 0x50) > > +#define AR9331_PORT_MIB_txbroad(_port) (AR9331_MIB_COUNTER(_port) + 0x54) > > +#define AR9331_PORT_MIB_txpause(_port) (AR9331_MIB_COUNTER(_port) + 0x58) > > +#define AR9331_PORT_MIB_txmulti(_port) (AR9331_MIB_COUNTER(_port) + 0x5c) > > +#define AR9331_PORT_MIB_txunderrun(_port) (AR9331_MIB_COUNTER(_port) + 0x60) > > +#define AR9331_PORT_MIB_tx64byte(_port) (AR9331_MIB_COUNTER(_port) + 0x64) > > +#define AR9331_PORT_MIB_tx128byte(_port) (AR9331_MIB_COUNTER(_port) + 0x68) > > +#define AR9331_PORT_MIB_tx256byte(_port) (AR9331_MIB_COUNTER(_port) + 0x6c) > > +#define AR9331_PORT_MIB_tx512byte(_port) (AR9331_MIB_COUNTER(_port) + 0x70) > > +#define AR9331_PORT_MIB_tx1024byte(_port) (AR9331_MIB_COUNTER(_port) + 0x74) > > +#define AR9331_PORT_MIB_tx1518byte(_port) (AR9331_MIB_COUNTER(_port) + 0x78) > > +#define AR9331_PORT_MIB_txmaxbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x7c) > > +#define AR9331_PORT_MIB_txoversize(_port) (AR9331_MIB_COUNTER(_port) + 0x80) > > + > > +/* 64 bit counter */ > > +#define AR9331_PORT_MIB_txbyte(_port) (AR9331_MIB_COUNTER(_port) + 0x84) > > + > > +#define AR9331_PORT_MIB_txcollision(_port) (AR9331_MIB_COUNTER(_port) + 0x8c) > > +#define AR9331_PORT_MIB_txabortcol(_port) (AR9331_MIB_COUNTER(_port) + 0x90) > > +#define AR9331_PORT_MIB_txmulticol(_port) (AR9331_MIB_COUNTER(_port) + 0x94) > > +#define AR9331_PORT_MIB_txsinglecol(_port) (AR9331_MIB_COUNTER(_port) + 0x98) > > +#define AR9331_PORT_MIB_txexcdefer(_port) (AR9331_MIB_COUNTER(_port) + 0x9c) > > +#define AR9331_PORT_MIB_txdefer(_port) (AR9331_MIB_COUNTER(_port) + 0xa0) > > +#define AR9331_PORT_MIB_txlatecol(_port) (AR9331_MIB_COUNTER(_port) + 0xa4) > > + > > /* Phy bypass mode > > * ------------------------------------------------------------------------ > > * Bit: | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |10 |11 |12 |13 |14 |15 | > > @@ -154,6 +205,59 @@ > > #define AR9331_SW_MDIO_POLL_SLEEP_US 1 > > #define AR9331_SW_MDIO_POLL_TIMEOUT_US 20 > > > > +#define STATS_INTERVAL_JIFFIES (100 * HZ) > > + > > +struct ar9331_sw_stats { > > + u64 rxbroad; > > + u64 rxpause; > > + u64 rxmulti; > > + u64 rxfcserr; > > + u64 rxalignerr; > > + u64 rxrunt; > > + u64 rxfragment; > > + u64 rx64byte; > > + u64 rx128byte; > > + u64 rx256byte; > > + u64 rx512byte; > > + u64 rx1024byte; > > + u64 rx1518byte; > > + u64 rxmaxbyte; > > + u64 rxtoolong; > > + u64 rxgoodbyte; > > + u64 rxbadbyte; > > + u64 rxoverflow; > > + u64 filtered; > > + u64 txbroad; > > + u64 txpause; > > + u64 txmulti; > > + u64 txunderrun; > > + u64 tx64byte; > > + u64 tx128byte; > > + u64 tx256byte; > > + u64 tx512byte; > > + u64 tx1024byte; > > + u64 tx1518byte; > > + u64 txmaxbyte; > > + u64 txoversize; > > + u64 txbyte; > > + u64 txcollision; > > + u64 txabortcol; > > + u64 txmulticol; > > + u64 txsinglecol; > > + u64 txexcdefer; > > + u64 txdefer; > > + u64 txlatecol; > > +}; > > + > > +struct ar9331_sw_priv; > > +struct ar9331_sw_port { > > + int idx; > > + struct ar9331_sw_priv *priv; > > + struct delayed_work mib_read; > > + struct ar9331_sw_stats stats; > > + struct mutex lock; /* stats access */ > > What does the lock protect? It's only used a single time. The ar9331_read_stats() function is called from two different contests: from worker over ar9331_do_stats_poll() and from user space over ar9331_get_stats64(). The mutex lock should prevent a race in the read modify write operations for in the stats->* Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:43 ` Oleksij Rempel @ 2020-12-02 12:47 ` Marc Kleine-Budde 0 siblings, 0 replies; 14+ messages in thread From: Marc Kleine-Budde @ 2020-12-02 12:47 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Florian Fainelli, linux-mips, Russell King, David S. Miller, Pengutronix Kernel Team, netdev, Jakub Kicinski, Vladimir Oltean, Vivien Didelot, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 912 bytes --] On 12/2/20 1:43 PM, Oleksij Rempel wrote: >>> +struct ar9331_sw_priv; >>> +struct ar9331_sw_port { >>> + int idx; >>> + struct ar9331_sw_priv *priv; >>> + struct delayed_work mib_read; >>> + struct ar9331_sw_stats stats; >>> + struct mutex lock; /* stats access */ >> >> What does the lock protect? It's only used a single time. > > The ar9331_read_stats() function is called from two different contests: > from worker over ar9331_do_stats_poll() and from user space over > ar9331_get_stats64(). > > The mutex lock should prevent a race in the read modify write operations > for in the stats->* Makes sense! Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel 2020-12-02 12:15 ` Marc Kleine-Budde @ 2020-12-02 13:04 ` Vladimir Oltean 2020-12-02 13:51 ` Oleksij Rempel 2020-12-02 15:11 ` Andrew Lunn ` (2 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Vladimir Oltean @ 2020-12-02 13:04 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On Wed, Dec 02, 2020 at 01:07:12PM +0100, Oleksij Rempel wrote: > Add stats support for the ar9331 switch. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > /* Warning: switch reset will reset last AR9331_SW_MDIO_PHY_MODE_PAGE request > @@ -422,6 +527,7 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > phy_interface_t interface) > { > struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; > + struct ar9331_sw_port *p = &priv->port[port]; > struct regmap *regmap = priv->regmap; > int ret; > > @@ -429,6 +535,8 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > AR9331_SW_PORT_STATUS_MAC_MASK, 0); > if (ret) > dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); > + > + cancel_delayed_work_sync(&p->mib_read); Is this sufficient? Do you get a guaranteed .phylink_mac_link_down event on unbind? How do you ensure you don't race with the stats worker there? > +static void ar9331_stats_update(struct ar9331_sw_port *port, > + struct rtnl_link_stats64 *stats) > +{ > + struct ar9331_sw_stats *s = &port->stats; > + > + stats->rx_packets = s->rxbroad + s->rxmulti + s->rx64byte + > + s->rx128byte + s->rx256byte + s->rx512byte + s->rx1024byte + > + s->rx1518byte + s->rxmaxbyte; > + stats->tx_packets = s->txbroad + s->txmulti + s->tx64byte + > + s->tx128byte + s->tx256byte + s->tx512byte + s->tx1024byte + > + s->tx1518byte + s->txmaxbyte; > + stats->rx_bytes = s->rxgoodbyte; > + stats->tx_bytes = s->txbyte; > + stats->rx_errors = s->rxfcserr + s->rxalignerr + s->rxrunt + > + s->rxfragment + s->rxoverflow; > + stats->tx_errors = s->txoversize; > + stats->multicast = s->rxmulti; > + stats->collisions = s->txcollision; > + stats->rx_length_errors = s->rxrunt * s->rxfragment + s->rxtoolong; Multiplication? Is this right? > + stats->rx_crc_errors = s->rxfcserr + s->rxalignerr + s->rxfragment; > + stats->rx_frame_errors = s->rxalignerr; > + stats->rx_missed_errors = s->rxoverflow; > + stats->tx_aborted_errors = s->txabortcol; > + stats->tx_fifo_errors = s->txunderrun; > + stats->tx_window_errors = s->txlatecol; > + stats->rx_nohandler = s->filtered; > +} > + > +static void ar9331_do_stats_poll(struct work_struct *work) > +{ > + Could you remove this empty line. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 13:04 ` Vladimir Oltean @ 2020-12-02 13:51 ` Oleksij Rempel 0 siblings, 0 replies; 14+ messages in thread From: Oleksij Rempel @ 2020-12-02 13:51 UTC (permalink / raw) To: Vladimir Oltean Cc: Andrew Lunn, Florian Fainelli, netdev, Russell King, David S. Miller, Pengutronix Kernel Team, Jakub Kicinski, linux-mips, Vivien Didelot, linux-kernel On Wed, Dec 02, 2020 at 03:04:38PM +0200, Vladimir Oltean wrote: > On Wed, Dec 02, 2020 at 01:07:12PM +0100, Oleksij Rempel wrote: > > Add stats support for the ar9331 switch. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > /* Warning: switch reset will reset last AR9331_SW_MDIO_PHY_MODE_PAGE request > > @@ -422,6 +527,7 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > > phy_interface_t interface) > > { > > struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; > > + struct ar9331_sw_port *p = &priv->port[port]; > > struct regmap *regmap = priv->regmap; > > int ret; > > > > @@ -429,6 +535,8 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > > AR9331_SW_PORT_STATUS_MAC_MASK, 0); > > if (ret) > > dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); > > + > > + cancel_delayed_work_sync(&p->mib_read); > > Is this sufficient? Do you get a guaranteed .phylink_mac_link_down event > on unbind? How do you ensure you don't race with the stats worker there? Currently it works, but you are right, i'll better do this on remove as well. > > +static void ar9331_stats_update(struct ar9331_sw_port *port, > > + struct rtnl_link_stats64 *stats) > > +{ > > + struct ar9331_sw_stats *s = &port->stats; > > + > > + stats->rx_packets = s->rxbroad + s->rxmulti + s->rx64byte + > > + s->rx128byte + s->rx256byte + s->rx512byte + s->rx1024byte + > > + s->rx1518byte + s->rxmaxbyte; > > + stats->tx_packets = s->txbroad + s->txmulti + s->tx64byte + > > + s->tx128byte + s->tx256byte + s->tx512byte + s->tx1024byte + > > + s->tx1518byte + s->txmaxbyte; > > + stats->rx_bytes = s->rxgoodbyte; > > + stats->tx_bytes = s->txbyte; > > + stats->rx_errors = s->rxfcserr + s->rxalignerr + s->rxrunt + > > + s->rxfragment + s->rxoverflow; > > + stats->tx_errors = s->txoversize; > > + stats->multicast = s->rxmulti; > > + stats->collisions = s->txcollision; > > + stats->rx_length_errors = s->rxrunt * s->rxfragment + s->rxtoolong; > > Multiplication? Is this right? no. fixed. > > + stats->rx_crc_errors = s->rxfcserr + s->rxalignerr + s->rxfragment; > > + stats->rx_frame_errors = s->rxalignerr; > > + stats->rx_missed_errors = s->rxoverflow; > > + stats->tx_aborted_errors = s->txabortcol; > > + stats->tx_fifo_errors = s->txunderrun; > > + stats->tx_window_errors = s->txlatecol; > > + stats->rx_nohandler = s->filtered; > > +} > > + > > +static void ar9331_do_stats_poll(struct work_struct *work) > > +{ > > + > > Could you remove this empty line. fixed Thank you! Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel 2020-12-02 12:15 ` Marc Kleine-Budde 2020-12-02 13:04 ` Vladimir Oltean @ 2020-12-02 15:11 ` Andrew Lunn 2020-12-02 15:37 ` Vladimir Oltean 2020-12-02 18:39 ` Jakub Kicinski 4 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2020-12-02 15:11 UTC (permalink / raw) To: Oleksij Rempel Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips > @@ -422,6 +527,7 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > phy_interface_t interface) > { > struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; > + struct ar9331_sw_port *p = &priv->port[port]; > struct regmap *regmap = priv->regmap; > int ret; > > @@ -429,6 +535,8 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > AR9331_SW_PORT_STATUS_MAC_MASK, 0); > if (ret) > dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); > + > + cancel_delayed_work_sync(&p->mib_read); > } You could update the stats here, after the interface is down. You then know the stats are actually up to date and correct! Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel ` (2 preceding siblings ...) 2020-12-02 15:11 ` Andrew Lunn @ 2020-12-02 15:37 ` Vladimir Oltean 2020-12-02 18:39 ` Jakub Kicinski 4 siblings, 0 replies; 14+ messages in thread From: Vladimir Oltean @ 2020-12-02 15:37 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller, Jakub Kicinski, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On Wed, Dec 02, 2020 at 01:07:12PM +0100, Oleksij Rempel wrote: > Add stats support for the ar9331 switch. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- With Andrew's feedback applied: Reviewed-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel ` (3 preceding siblings ...) 2020-12-02 15:37 ` Vladimir Oltean @ 2020-12-02 18:39 ` Jakub Kicinski 4 siblings, 0 replies; 14+ messages in thread From: Jakub Kicinski @ 2020-12-02 18:39 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Russell King, Pengutronix Kernel Team, netdev, linux-kernel, linux-mips On Wed, 2 Dec 2020 13:07:12 +0100 Oleksij Rempel wrote: > +static void ar9331_read_stats(struct ar9331_sw_port *port) > +{ > + mutex_lock(&port->lock); > + mutex_unlock(&port->lock); > +} > +static void ar9331_get_stats64(struct dsa_switch *ds, int port, > + struct rtnl_link_stats64 *s) > +{ > + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; > + struct ar9331_sw_port *p = &priv->port[port]; > + > + ar9331_read_stats(p); > +} > + > static const struct dsa_switch_ops ar9331_sw_ops = { > + .get_stats64 = ar9331_get_stats64, > }; You can't take sleeping locks from .ndo_get_stats64. Also regmap may sleep? + ret = regmap_read(priv->regmap, reg, &val); Am I missing something? ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-12-02 18:40 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-02 12:07 [PATCH v2 0/2] net: dsa: add stats64 support Oleksij Rempel 2020-12-02 12:07 ` [PATCH v2 1/2] net: dsa: add optional " Oleksij Rempel 2020-12-02 12:59 ` Vladimir Oltean 2020-12-02 15:03 ` Andrew Lunn 2020-12-02 17:12 ` Florian Fainelli 2020-12-02 12:07 ` [PATCH v2 2/2] net: dsa: qca: ar9331: export stats64 Oleksij Rempel 2020-12-02 12:15 ` Marc Kleine-Budde 2020-12-02 12:43 ` Oleksij Rempel 2020-12-02 12:47 ` Marc Kleine-Budde 2020-12-02 13:04 ` Vladimir Oltean 2020-12-02 13:51 ` Oleksij Rempel 2020-12-02 15:11 ` Andrew Lunn 2020-12-02 15:37 ` Vladimir Oltean 2020-12-02 18:39 ` Jakub Kicinski
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).