> From: Stephen Hemminger [mailto:step...@networkplumber.org] > Sent: Friday, 2 August 2024 19.33 > > On Fri, 2 Aug 2024 19:28:26 +0200 > Morten Brørup <m...@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://patchwork.dpdk.org/project/dpdk/patch/20240801160312.205281-1...@smartsharesystems.com/ [vhost-user]: https://patchwork.dpdk.org/project/dpdk/patch/20240802143259.269827-1...@smartsharesystems.com/ > > 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.
