RE: [PATCH] netvsc: optimize stats counters performance

[Long Li <longli@microsoft.com>]

> Subject: RE: [PATCH] netvsc: optimize stats counters performance
> 
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, 2 August 2024 19.33
> >
> > On Fri, 2 Aug 2024 19:28:26 +0200
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> >
> > > > >  	ea = rte_pktmbuf_mtod(m, const struct rte_ether_addr *);
> > > > > -	if (rte_is_multicast_ether_addr(ea)) {
> > > > > -		if (rte_is_broadcast_ether_addr(ea))
> > > > > -			stats->broadcast++;
> > > > > -		else
> > > > > -			stats->multicast++;
> > > > > -	}
> > > > > +	RTE_BUILD_BUG_ON(offsetof(struct hn_stats, broadcast) !=
> > > > > +			offsetof(struct hn_stats, multicast) +
> > > > sizeof(uint64_t));
> > > > > +	if (unlikely(rte_is_multicast_ether_addr(ea)))
> > > > > +		(&stats-
> >multicast)[rte_is_broadcast_ether_addr(ea)]++;
> > > > >  }
> > > >
> > > > This makes the code a little harder to read.
> > >
> > > I agree it is somewhat convoluted.
> > > It's a tradeoff... I preferred performance at the cost of making the code
> > somewhat harder to read.
> > > The RTE_BUILD_BUG_ON() also helps showing what is going on with the
> weird
> > indexing.
> 
> Similar patches have been accepted by other drivers:
> [virtio]:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20240801160312.205281-1-
> mb%40smartsharesystems.com%2F&data=05%7C02%7Clongli%40microsoft.com
> %7C014c845ad8ec4ea25fb508dcd8b22474%7C72f988bf86f141af91ab2d7cd011
> db47%7C1%7C0%7C638623506854135960%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> %7C%7C%7C&sdata=DrVP6TuTruREAxAoSEBED11TOapSqDN5stsDJJTuux0%3D&r
> eserved=0
> [vhost-user]:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20240802143259.269827-1-
> mb%40smartsharesystems.com%2F&data=05%7C02%7Clongli%40microsoft.com
> %7C014c845ad8ec4ea25fb508dcd8b22474%7C72f988bf86f141af91ab2d7cd011
> db47%7C1%7C0%7C638623506854150865%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> %7C%7C%7C&sdata=85T9qnC4nbAwU53sxnfQ2lMayp5soX8BMEJsbB58ZBw%3D
> &reserved=0
> 
> >
> > Optimizing for multicast packets is not worth bothering.
> 
> Optimizing for multicast/broadcast comes into play in multicast environments,
> and during network broadcast storms.
> Although I don't know if any of those two scenarios are relevant for this specific
> driver.
> 
> > Keep the original code it is simpler.
> 
> Let's keep similar code similar across drivers.
> 
> @Long, @Wei, please Review/Ack, so the patch can be applied.

Reviewed-by: Long Li <longli@microsoft.com>