lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 8 Jun 2021 16:18:03 +0000
From:   "Saleem, Shiraz" <shiraz.saleem@...el.com>
To:     Joe Perches <joe@...ches.com>,
        "Ismail, Mustafa" <mustafa.ismail@...el.com>
CC:     Doug Ledford <dledford@...hat.com>, Jason Gunthorpe <jgg@...pe.ca>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?

> Subject: Re: irdma: utils.c typos in irdma_cqp_gather_stats_gen1 ?
> 
> 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)				\

Maybe var --> name. Other than that looks good.


> +	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));  }

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ