From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1FB4043FE0; Fri, 10 May 2024 19:29:54 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 98E44402C4; Fri, 10 May 2024 19:29:53 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 33C27402BB for ; Fri, 10 May 2024 19:29:52 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 030D2209B2; Fri, 10 May 2024 19:29:52 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [RFC 0/3] generic sw counters X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Fri, 10 May 2024 19:29:48 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F437@smartserver.smartshare.dk> In-Reply-To: <20240510050507.14381-1-stephen@networkplumber.org> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [RFC 0/3] generic sw counters Thread-Index: Adqil6XPCTZt1yQ3TSqBrya//b0feAAY5XCA References: <20240425174617.2126159-1-ferruh.yigit@amd.com> <20240510050507.14381-1-stephen@networkplumber.org> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Stephen Hemminger" , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Friday, 10 May 2024 07.01 >=20 > This is my attempt to demonstrate: > - generic counters for SW drivers, the example does af_packet and = tap > but same should be applied to af_xdp, virtio, etc. > - counters are safe against 64 bit tearing on 32 bit platform +1 for this RFC >=20 > The naming and organization could be improved: > - should this be in rte_ethdev? I think the part for handling 64 bit counters should be moved out into a = generic library, so it can also be used by libraries and applications. = It can be extended with more features (feature creep) later. And if some = architectures offer (more feature creep) optimized implementations, all = 64 bit counters for those architectures will benefit from such = improvements. I like the concept of joining and generalizing the stats handling for SW = drivers, to avoid copy-paste of detailed stats handing between SW = drivers. Copy-paste is bad, common functions are good. I'm somewhat skeptical about putting the stats structure first in the = rte_eth_dev_data's tx_queues and rx_queues. These are void* because their types are private to the PMD. Putting the = stats structure first is somewhat a hack, partially removing that = privacy. Perhaps we can make it look less like a hack. After all, it is still a = private structure type, only the first part is public and must be the = same across drivers using the SW stats counters. Overlapping certain = parts of a private structure with a public structure is a common design = pattern; I've seen it used elsewhere in DPDK too. If we get used to it for SW ethdev drivers, we might not consider it a = hack anymore. ;-) > - better name for struct and vairables? > - audit and handle errors better. >=20 > Stephen Hemminger (3): > ethdev: add internal helper of SW driver statistics > net/af_packet: use SW stats helper > net/tap: use generic SW stats >=20 > drivers/net/af_packet/rte_eth_af_packet.c | 97 ++----- > drivers/net/tap/rte_eth_tap.c | 100 ++------ > drivers/net/tap/rte_eth_tap.h | 17 +- > lib/ethdev/ethdev_swstats.c | 294 = ++++++++++++++++++++++ > lib/ethdev/ethdev_swstats.h | 60 +++++ > lib/ethdev/meson.build | 2 + > lib/ethdev/version.map | 7 + > 7 files changed, 400 insertions(+), 177 deletions(-) > create mode 100644 lib/ethdev/ethdev_swstats.c > create mode 100644 lib/ethdev/ethdev_swstats.h >=20 > -- > 2.43.0