| Message ID | 20200911195258.1048468-6-kuba@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ethtool: add pause frame stats | expand |
On Fri, 11 Sep 2020 15:43:43 -0700 Jakub Kicinski wrote: > > > + struct bnxt *bp = netdev_priv(dev); > > > + struct rx_port_stats *rx_stats; > > > + struct tx_port_stats *tx_stats; > > > + > > > + if (BNXT_VF(bp) || !(bp->flags & BNXT_FLAG_PORT_STATS)) > > > + return; > > > + > > > + rx_stats = (void *)bp->port_stats.sw_stats; > > > + tx_stats = (void *)((unsigned long)bp->port_stats.sw_stats + > > > + BNXT_TX_PORT_STATS_BYTE_OFFSET); > > > + > > > + epstat->rx_pause_frames = rx_stats->rx_pause_frames; > > > + epstat->tx_pause_frames = tx_stats->tx_pause_frames; > > > > This will work, but the types on the 2 sides don't match. On the > > right hand side, since you are casting to the hardware struct > > rx_port_stats and tx_port_stats, the types are __le64. > > > > If rx_stats and tx_stats are *u64 and you use these macros: > > > > BNXT_GET_RX_PORT_STATS64(rx_stats, rx_pause_frames) > > BNXT_GET_TX_PORT_STATS64(tx_stats, tx_pause_frames) > > > > the results will be the same with native CPU u64 types. > > Thanks! My build bot just poked me about this as well. > > I don't see any byte swaps in bnxt_get_ethtool_stats() - > are they not needed there? I'm slightly confused. Oof those macros don't byte swap either, even more confused now :)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index d0928334bdc8..b5de242766e3 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -1778,6 +1778,24 @@ static void bnxt_get_pauseparam(struct net_device *dev, epause->tx_pause = !!(link_info->req_flow_ctrl & BNXT_LINK_PAUSE_TX); } +static void bnxt_get_pause_stats(struct net_device *dev, + struct ethtool_pause_stats *epstat) +{ + struct bnxt *bp = netdev_priv(dev); + struct rx_port_stats *rx_stats; + struct tx_port_stats *tx_stats; + + if (BNXT_VF(bp) || !(bp->flags & BNXT_FLAG_PORT_STATS)) + return; + + rx_stats = (void *)bp->port_stats.sw_stats; + tx_stats = (void *)((unsigned long)bp->port_stats.sw_stats + + BNXT_TX_PORT_STATS_BYTE_OFFSET); + + epstat->rx_pause_frames = rx_stats->rx_pause_frames; + epstat->tx_pause_frames = tx_stats->tx_pause_frames; +} + static int bnxt_set_pauseparam(struct net_device *dev, struct ethtool_pauseparam *epause) { @@ -3645,6 +3663,7 @@ const struct ethtool_ops bnxt_ethtool_ops = { ETHTOOL_COALESCE_USE_ADAPTIVE_RX, .get_link_ksettings = bnxt_get_link_ksettings, .set_link_ksettings = bnxt_set_link_ksettings, + .get_pause_stats = bnxt_get_pause_stats, .get_pauseparam = bnxt_get_pauseparam, .set_pauseparam = bnxt_set_pauseparam, .get_drvinfo = bnxt_get_drvinfo,
These stats are already reported in ethtool -S. Hopefully they are equivalent to standard stats? Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)