| Message ID | 20240530204751.99636-1-vadfed@meta.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 165f87691a898aea113923224c269bae14749ebc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] bnxt_en: add timestamping statistics support | expand |
On Thu, May 30, 2024 at 1:48 PM Vadim Fedorenko <vadfed@meta.com> wrote: > > The ethtool_ts_stats structure was introduced earlier this year. Now > it's time to support this group of counters in more drivers. > This patch adds support to bnxt driver. > > Signed-off-by: Vadim Fedorenko <vadfed@meta.com> > --- > v1 -> v2: > - embed stats structure into ptp_cfg structure > - keep ethtool stats untouched if ptp is not enabled > --- Thanks. Reviewed-by: Michael Chan <michael.chan@broadcom.com>
On Thu, May 30, 2024 at 01:47:51PM -0700, Vadim Fedorenko wrote: > The ethtool_ts_stats structure was introduced earlier this year. Now > it's time to support this group of counters in more drivers. > This patch adds support to bnxt driver. > > Signed-off-by: Vadim Fedorenko <vadfed@meta.com> ... > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > index 8763f8a01457..bf157f6cc042 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c > @@ -5233,6 +5233,19 @@ static void bnxt_get_rmon_stats(struct net_device *dev, > *ranges = bnxt_rmon_ranges; > } > > +static void bnxt_get_ptp_stats(struct net_device *dev, > + struct ethtool_ts_stats *ts_stats) > +{ > + struct bnxt *bp = netdev_priv(dev); > + struct bnxt_ptp_cfg *ptp = bp->ptp_cfg; Hi Vadim, If you need to update this patch for some other reason, please consider arranging these local variables in reverse xmas tree order - longest line to shortest. In this case I think that would mean separating the declaration and assignment of ptp, like this (completely untested!): struct bnxt *bp = netdev_priv(dev); struct bnxt_ptp_cfg *ptp; ptp = bp->ptp_cfg; Edward Cree's tool can be helpful here: https://github.com/ecree-solarflare/xmastree ...
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 30 May 2024 13:47:51 -0700 you wrote: > The ethtool_ts_stats structure was introduced earlier this year. Now > it's time to support this group of counters in more drivers. > This patch adds support to bnxt driver. > > Signed-off-by: Vadim Fedorenko <vadfed@meta.com> > --- > v1 -> v2: > - embed stats structure into ptp_cfg structure > - keep ethtool stats untouched if ptp is not enabled > > [...] Here is the summary with links: - [net-next,v2] bnxt_en: add timestamping statistics support https://git.kernel.org/netdev/net-next/c/165f87691a89 You are awesome, thank you!
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index c437ca1c0fd3..6d9faa78e391 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -512,8 +512,11 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) { struct bnxt_ptp_cfg *ptp = bp->ptp_cfg; - if (ptp && ptp->tx_tstamp_en && !skb_is_gso(skb) && - atomic_dec_if_positive(&ptp->tx_avail) >= 0) { + if (ptp && ptp->tx_tstamp_en && !skb_is_gso(skb)) { + if (!atomic_dec_if_positive(&ptp->tx_avail)) { + atomic64_inc(&ptp->stats.ts_err); + goto tx_no_ts; + } if (!bnxt_ptp_parse(skb, &ptp->tx_seqid, &ptp->tx_hdr_off)) { if (vlan_tag_flags) @@ -526,6 +529,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) } } +tx_no_ts: if (unlikely(skb->no_fcs)) lflags |= cpu_to_le32(TX_BD_FLAGS_NO_CRC); @@ -732,8 +736,10 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_OK; tx_dma_error: - if (BNXT_TX_PTP_IS_SET(lflags)) + if (BNXT_TX_PTP_IS_SET(lflags)) { + atomic64_inc(&bp->ptp_cfg->stats.ts_err); atomic_inc(&bp->ptp_cfg->tx_avail); + } last_frag = i; @@ -812,10 +818,12 @@ static void __bnxt_tx_int(struct bnxt *bp, struct bnxt_tx_ring_info *txr, if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) { if (BNXT_CHIP_P5(bp)) { /* PTP worker takes ownership of the skb */ - if (!bnxt_get_tx_ts_p5(bp, skb)) + if (!bnxt_get_tx_ts_p5(bp, skb)) { skb = NULL; - else + } else { + atomic64_inc(&bp->ptp_cfg->stats.ts_err); atomic_inc(&bp->ptp_cfg->tx_avail); + } } } diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index 8763f8a01457..bf157f6cc042 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -5233,6 +5233,19 @@ static void bnxt_get_rmon_stats(struct net_device *dev, *ranges = bnxt_rmon_ranges; } +static void bnxt_get_ptp_stats(struct net_device *dev, + struct ethtool_ts_stats *ts_stats) +{ + struct bnxt *bp = netdev_priv(dev); + struct bnxt_ptp_cfg *ptp = bp->ptp_cfg; + + if (ptp) { + ts_stats->pkts = ptp->stats.ts_pkts; + ts_stats->lost = ptp->stats.ts_lost; + ts_stats->err = atomic64_read(&ptp->stats.ts_err); + } +} + static void bnxt_get_link_ext_stats(struct net_device *dev, struct ethtool_link_ext_stats *stats) { @@ -5316,4 +5329,5 @@ const struct ethtool_ops bnxt_ethtool_ops = { .get_eth_mac_stats = bnxt_get_eth_mac_stats, .get_eth_ctrl_stats = bnxt_get_eth_ctrl_stats, .get_rmon_stats = bnxt_get_rmon_stats, + .get_ts_stats = bnxt_get_ptp_stats, }; diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c index e661ab154d6b..a14d46b9bfdf 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c @@ -696,11 +696,13 @@ static void bnxt_stamp_tx_skb(struct bnxt *bp, struct sk_buff *skb) spin_unlock_bh(&ptp->ptp_lock); timestamp.hwtstamp = ns_to_ktime(ns); skb_tstamp_tx(ptp->tx_skb, ×tamp); + ptp->stats.ts_pkts++; } else { if (!time_after_eq(jiffies, ptp->abs_txts_tmo)) { ptp->txts_pending = true; return; } + ptp->stats.ts_lost++; netdev_warn_once(bp->dev, "TS query for TX timer failed rc = %x\n", rc); } @@ -979,6 +981,11 @@ int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg) rc = err; goto out; } + + ptp->stats.ts_pkts = 0; + ptp->stats.ts_lost = 0; + atomic64_set(&ptp->stats.ts_err, 0); + if (BNXT_CHIP_P5(bp)) { spin_lock_bh(&ptp->ptp_lock); bnxt_refclk_read(bp, NULL, &ptp->current_time); @@ -1013,5 +1020,6 @@ void bnxt_ptp_clear(struct bnxt *bp) dev_kfree_skb_any(ptp->tx_skb); ptp->tx_skb = NULL; } + bnxt_unmap_ptp_regs(bp); } diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h index 2c3415c8fc03..8c30b428a428 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h @@ -79,6 +79,12 @@ struct bnxt_pps { struct pps_pin pins[BNXT_MAX_TSIO_PINS]; }; +struct bnxt_ptp_stats { + u64 ts_pkts; + u64 ts_lost; + atomic64_t ts_err; +}; + struct bnxt_ptp_cfg { struct ptp_clock_info ptp_info; struct ptp_clock *ptp_clock; @@ -125,6 +131,8 @@ struct bnxt_ptp_cfg { u32 refclk_mapped_regs[2]; u32 txts_tmo; unsigned long abs_txts_tmo; + + struct bnxt_ptp_stats stats; }; #if BITS_PER_LONG == 32
The ethtool_ts_stats structure was introduced earlier this year. Now it's time to support this group of counters in more drivers. This patch adds support to bnxt driver. Signed-off-by: Vadim Fedorenko <vadfed@meta.com> --- v1 -> v2: - embed stats structure into ptp_cfg structure - keep ethtool stats untouched if ptp is not enabled --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 18 +++++++++++++----- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 14 ++++++++++++++ drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 8 ++++++++ drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 8 ++++++++ 4 files changed, 43 insertions(+), 5 deletions(-)