Re: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?

From: Joe Perches
Date: Mon Jun 07 2021 - 19:37:20 EST

On Mon, 2021-06-07 at 22:40 +0000, Saleem, Shiraz wrote:
> > Subject: Re: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
> >
> > On Mon, 2021-06-07 at 14:54 +0000, Saleem, Shiraz wrote:
> > > > Subject: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
> > > >
> > > > There are some odd mismatches in field and access index.
> > > > These may be simple cut/paste typos.
> > > >
> > > > Are these intentional?
> > >
> > > No. Accidental. Likely cut/copy mistake. I will send a fix. Thanks
> > > Joe!
> >
> > Reason is I was refactoring what I thought was rather templated and
> > overly verbose
> > code.
> >
> > Here's a possible patch (with probable corrections to these index
> > typos):
>
> Looks reasonable to me. Wondering if irdma_update_stats should also
> use your IRDMA_STAT macro.

Probably, and it could really a macro or two of its own.

And it looks like that block has it's own copy/paste defects.

Maybe:
---
drivers/infiniband/hw/irdma/ctrl.c | 219 +++++++++----------------------------
1 file changed, 51 insertions(+), 168 deletions(-)

diff --git a/drivers/infiniband/hw/irdma/ctrl.c b/drivers/infiniband/hw/irdma/ctrl.c
index 5aa112067bce8..90c6f82ea0c8d 100644
--- a/drivers/infiniband/hw/irdma/ctrl.c
+++ b/drivers/infiniband/hw/irdma/ctrl.c
@@ -5484,174 +5484,57 @@ void irdma_update_stats(struct irdma_dev_hw_stats *hw_stats,
{
u64 *stats_val = hw_stats->stats_val_32;

- stats_val[IRDMA_HW_STAT_INDEX_RXVLANERR] +=
- IRDMA_STATS_DELTA(gather_stats->rxvlanerr,
- last_gather_stats->rxvlanerr,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXDISCARD] +=
- IRDMA_STATS_DELTA(gather_stats->ip4rxdiscard,
- last_gather_stats->ip4rxdiscard,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXTRUNC] +=
- IRDMA_STATS_DELTA(gather_stats->ip4rxtrunc,
- last_gather_stats->ip4rxtrunc,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP4TXNOROUTE] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txnoroute,
- last_gather_stats->ip4txnoroute,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXDISCARD] +=
- IRDMA_STATS_DELTA(gather_stats->ip6rxdiscard,
- last_gather_stats->ip6rxdiscard,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXTRUNC] +=
- IRDMA_STATS_DELTA(gather_stats->ip6rxtrunc,
- last_gather_stats->ip6rxtrunc,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_IP6TXNOROUTE] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txnoroute,
- last_gather_stats->ip6txnoroute,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_TCPRTXSEG] +=
- IRDMA_STATS_DELTA(gather_stats->tcprtxseg,
- last_gather_stats->tcprtxseg,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_TCPRXOPTERR] +=
- IRDMA_STATS_DELTA(gather_stats->tcprxopterr,
- last_gather_stats->tcprxopterr,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_TCPRXPROTOERR] +=
- IRDMA_STATS_DELTA(gather_stats->tcprxprotoerr,
- last_gather_stats->tcprxprotoerr,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_RXRPCNPHANDLED] +=
- IRDMA_STATS_DELTA(gather_stats->rxrpcnphandled,
- last_gather_stats->rxrpcnphandled,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_RXRPCNPIGNORED] +=
- IRDMA_STATS_DELTA(gather_stats->rxrpcnpignored,
- last_gather_stats->rxrpcnpignored,
- IRDMA_MAX_STATS_32);
- stats_val[IRDMA_HW_STAT_INDEX_TXNPCNPSENT] +=
- IRDMA_STATS_DELTA(gather_stats->txnpcnpsent,
- last_gather_stats->txnpcnpsent,
- IRDMA_MAX_STATS_32);
+#define irdma_update_stat(index, var, size) \
+ stats_val[IRDMA_STAT(index)] += \
+ IRDMA_STATS_DELTA(gather_stats->var, \
+ last_gather_stats->var, \
+ IRDMA_MAX_STATS_##size)
+
+ irdma_update_stat(RXVLANERR, rxvlanerr, 32);
+ irdma_update_stat(IP4RXDISCARD, ip4rxdiscard, 32);
+ irdma_update_stat(IP4RXTRUNC, ip4rxtrunc, 32);
+ irdma_update_stat(IP4TXNOROUTE, ip4txnoroute, 32);
+ irdma_update_stat(IP6RXDISCARD, ip6rxdiscard, 32);
+ irdma_update_stat(IP6RXTRUNC, ip6rxtrunc, 32);
+ irdma_update_stat(IP6TXNOROUTE, ip6txnoroute, 32);
+ irdma_update_stat(TCPRTXSEG, tcprtxseg, 32);
+ irdma_update_stat(TCPRXOPTERR, tcprxopterr, 32);
+ irdma_update_stat(TCPRXPROTOERR, tcprxprotoerr, 32);
+ irdma_update_stat(RXRPCNPHANDLED, rxrpcnphandled, 32);
+ irdma_update_stat(RXRPCNPIGNORED, rxrpcnpignored, 32);
+ irdma_update_stat(TXNPCNPSENT, txnpcnpsent, 32);
+
stats_val = hw_stats->stats_val_64;
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXOCTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4rxocts,
- last_gather_stats->ip4rxocts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4rxpkts,
- last_gather_stats->ip4rxpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXFRAGS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txfrag,
- last_gather_stats->ip4txfrag,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4RXMCPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4rxmcpkts,
- last_gather_stats->ip4rxmcpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4TXOCTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txocts,
- last_gather_stats->ip4txocts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4TXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txpkts,
- last_gather_stats->ip4txpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4TXFRAGS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txfrag,
- last_gather_stats->ip4txfrag,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP4TXMCPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip4txmcpkts,
- last_gather_stats->ip4txmcpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXOCTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6rxocts,
- last_gather_stats->ip6rxocts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6rxpkts,
- last_gather_stats->ip6rxpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXFRAGS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txfrags,
- last_gather_stats->ip6txfrags,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6RXMCPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6rxmcpkts,
- last_gather_stats->ip6rxmcpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6TXOCTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txocts,
- last_gather_stats->ip6txocts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6TXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txpkts,
- last_gather_stats->ip6txpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6TXFRAGS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txfrags,
- last_gather_stats->ip6txfrags,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_IP6TXMCPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->ip6txmcpkts,
- last_gather_stats->ip6txmcpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_TCPRXSEGS] +=
- IRDMA_STATS_DELTA(gather_stats->tcprxsegs,
- last_gather_stats->tcprxsegs,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_TCPTXSEG] +=
- IRDMA_STATS_DELTA(gather_stats->tcptxsegs,
- last_gather_stats->tcptxsegs,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMARXRDS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmarxrds,
- last_gather_stats->rdmarxrds,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMARXSNDS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmarxsnds,
- last_gather_stats->rdmarxsnds,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMARXWRS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmarxwrs,
- last_gather_stats->rdmarxwrs,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMATXRDS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmatxrds,
- last_gather_stats->rdmatxrds,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMATXSNDS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmatxsnds,
- last_gather_stats->rdmatxsnds,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMATXWRS] +=
- IRDMA_STATS_DELTA(gather_stats->rdmatxwrs,
- last_gather_stats->rdmatxwrs,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMAVBND] +=
- IRDMA_STATS_DELTA(gather_stats->rdmavbn,
- last_gather_stats->rdmavbn,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RDMAVINV] +=
- IRDMA_STATS_DELTA(gather_stats->rdmavinv,
- last_gather_stats->rdmavinv,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_UDPRXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->udprxpkts,
- last_gather_stats->udprxpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_UDPTXPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->udptxpkts,
- last_gather_stats->udptxpkts,
- IRDMA_MAX_STATS_48);
- stats_val[IRDMA_HW_STAT_INDEX_RXNPECNMARKEDPKTS] +=
- IRDMA_STATS_DELTA(gather_stats->rxnpecnmrkpkts,
- last_gather_stats->rxnpecnmrkpkts,
- IRDMA_MAX_STATS_48);
+
+ irdma_update_stat(IP4RXOCTS, ip4rxocts, 48);
+ irdma_update_stat(IP4RXPKTS, ip4rxpkts, 48);
+ irdma_update_stat(IP4RXFRAGS, ip4rxfrags, 48);
+ irdma_update_stat(IP4RXMCPKTS, ip4rxmcpkts, 48);
+ irdma_update_stat(IP4TXOCTS, ip4txocts, 48);
+ irdma_update_stat(IP4TXPKTS, ip4txpkts, 48);
+ irdma_update_stat(IP4TXFRAGS, ip4txfrag, 48);
+ irdma_update_stat(IP4TXMCPKTS, ip4txmcpkts, 48);
+ irdma_update_stat(IP6RXOCTS, ip6rxocts, 48);
+ irdma_update_stat(IP6RXPKTS, ip6rxpkts, 48);
+ irdma_update_stat(IP6RXFRAGS, ip6rxfrags, 48);
+ irdma_update_stat(IP6RXMCPKTS, ip6rxmcpkts, 48);
+ irdma_update_stat(IP6TXOCTS, ip6txocts, 48);
+ irdma_update_stat(IP6TXPKTS, ip6txpkts, 48);
+ irdma_update_stat(IP6TXFRAGS, ip6txfrags, 48);
+ irdma_update_stat(IP6TXMCPKTS, ip6txmcpkts, 48);
+ irdma_update_stat(TCPRXSEGS, tcprxsegs, 48);
+ irdma_update_stat(TCPTXSEG, tcptxsegs, 48);
+ irdma_update_stat(RDMARXRDS, rdmarxrds, 48);
+ irdma_update_stat(RDMARXSNDS, rdmarxsnds, 48);
+ irdma_update_stat(RDMARXWRS, rdmarxwrs, 48);
+ irdma_update_stat(RDMATXRDS, rdmatxrds, 48);
+ irdma_update_stat(RDMATXSNDS, rdmatxsnds, 48);
+ irdma_update_stat(RDMATXWRS, rdmatxwrs, 48);
+ irdma_update_stat(RDMAVBND, rdmavbn, 48);
+ irdma_update_stat(RDMAVINV, rdmavinv, 48);
+ irdma_update_stat(UDPRXPKTS, udprxpkts, 48);
+ irdma_update_stat(UDPTXPKTS, udptxpkts, 48);
+ irdma_update_stat(RXNPECNMARKEDPKTS, rxnpecnmrkpkts, 48);
+
memcpy(last_gather_stats, gather_stats, sizeof(*last_gather_stats));
}