Re: Per-cpu stats for network interfaces

[Jason Thorpe <thorpej%me.com@localhost>]

> On Jan 27, 2020, at 1:57 AM, Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost> wrote:
> 
> I think it is better for device drivers to use new API such as
> ====================
> 	uint64_t *stats = IF_STAT_GETREF(ifp);
> 	if_statadd_ref(stats, if_collisions, CSR_READ(sc, WMREG_COLC)); // new API
> 	// some more code
> 	IF_STAT_PUTREF(ifp);
> ====================
> instead of such as
> ====================
> 	uint64_t *stats = IF_STAT_GETREF(ifp);
> 	stats[IF_STAT_COLLISIONS] += CSR_READ(sc, WMREG_COLC);
> 	// some more code
> 	IF_STAT_PUTREF(ifp);
> ====================
> code, because that can hide both 64bit counter in 32bit architecture
> atomicity issue and memory ordering issue from device drivers.
> 
> It seems "__IF_STATS_PERCPU" is not defined.  "__IF_STATS_PERCPU" is
> defined when MULTIPROCESSOR is defined, isn't it?
> 
> For other parts, looks good to me.  Thank you for your reworks!
> 
> By the way, it would be helpful to add new API which add both packets
> counter and bytes counter.  Thought?

Thanks for your feedback!  I've incorporated your suggestions.  The pattern now actually looks like:

	net_stat_ref_t nsr = IF_STAT_GETREF(ifp);
	if_statinc_ref(nsr, if_opackets);
	// ...etc.
	IF_STAT_PUTREF(ifp);

net_stat_ref_t is a new opaque type declared in <sys/net_stats.h> to prevent anything subscripting it directly.

I have also added if_statadd2() for handling the very common pattern of bytes & packets in several places, e.g. instead of this:

	net_stat_ref_t nsr = IF_STAT_GETREF(ifp);
	if_statinc_ref(nsr, if_ipackets);
	if_statadd_ref(nsr, if_ibytes, m0->m_pkthdr.len);
	IF_STAT_PUTREF(ifp);

...it's just this:

	if_statadd2(ifp, if_ipackets, 1, if_ibytes, m0->m_pkthdr.len);

I also simplified a few other bits, but other than the mechanical changes noted here, it's basically unchanged and I think ready to go.

My integration plan:

1- Check in the basic if_stats.[ch] and if.[ch] changes.  Leave percpu stats *disabled* for now.

2- Check in covered drivers a few at a time, testing as many as I can for correct behavior.  The changes are very mechanical, and so easy to audit.

3- Finish converting some of the harder drivers that play nasty tricks with the existing if_data stats.

4- Verify all ports build releases correctly.

5- Flip the switch to enable percpu counters.

-- thorpej