| Message ID | 20240529171946.2866274-1-vadfed@meta.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] bnxt_en: add timestamping statistics support | expand |
On Wed, May 29, 2024 at 10:19 AM 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> > --- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 18 +++++++++++++----- > .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 18 ++++++++++++++++++ > drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 18 ++++++++++++++++++ > drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 8 ++++++++ > 4 files changed, 57 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h > index 2c3415c8fc03..589e093b1608 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; I think there is no need to allocate this small stats structure separately. It can just be: struct bnxt_ptp_stats stats; The struct bnxt_ptp_cfg will only be allocated if the device supports PTP. So the stats can always be a part of struct bnxt_ptp_cfg. Other than that, it looks good to me. Thanks. > }; >
On 29/05/2024 18:48, Michael Chan wrote: > On Wed, May 29, 2024 at 10:19 AM 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> >> --- >> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 18 +++++++++++++----- >> .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 18 ++++++++++++++++++ >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 18 ++++++++++++++++++ >> drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 8 ++++++++ >> 4 files changed, 57 insertions(+), 5 deletions(-) >> > >> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h >> index 2c3415c8fc03..589e093b1608 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; > > I think there is no need to allocate this small stats structure > separately. It can just be: > > struct bnxt_ptp_stats stats; > > The struct bnxt_ptp_cfg will only be allocated if the device supports > PTP. So the stats can always be a part of struct bnxt_ptp_cfg. Yeah, I was thinking about embedding the struct into bnxt_ptp_cfg. Ok, I'll make v2 in 24hr then. Thanks! > Other than that, it looks good to me. Thanks. > >> }; >>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index c437ca1c0fd3..f3db8b0fbbe6 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..594e9967c591 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -5233,6 +5233,23 @@ 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); + } else { + ts_stats->pkts = 0; + ts_stats->err = 0; + ts_stats->lost = 0; + } +} + static void bnxt_get_link_ext_stats(struct net_device *dev, struct ethtool_link_ext_stats *stats) { @@ -5316,4 +5333,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..f75bcb8d43c0 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,18 @@ int bnxt_ptp_init(struct bnxt *bp, bool phc_cfg) rc = err; goto out; } + if (!ptp->stats) { + ptp->stats = kzalloc(sizeof(*ptp->stats), GFP_KERNEL); + if (!ptp->stats) { + rc = -ENOMEM; + goto out; + } + atomic64_set(&ptp->stats->ts_err, 0); + } else { + 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 +1027,9 @@ void bnxt_ptp_clear(struct bnxt *bp) dev_kfree_skb_any(ptp->tx_skb); ptp->tx_skb = NULL; } + + kfree(ptp->stats); + ptp->stats = 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..589e093b1608 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> --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 18 +++++++++++++----- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 18 ++++++++++++++++++ drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 18 ++++++++++++++++++ drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h | 8 ++++++++ 4 files changed, 57 insertions(+), 5 deletions(-)