From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9ABD64573A; Mon, 5 Aug 2024 03:14:17 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AAE1E40295; Mon, 5 Aug 2024 03:14:16 +0200 (CEST) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id AE7944026C for <dev@dpdk.org>; Mon, 5 Aug 2024 03:14:14 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.19.88.163]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4Wcdd701mFz1S72l; Mon, 5 Aug 2024 09:09:30 +0800 (CST) Received: from kwepemm600004.china.huawei.com (unknown [7.193.23.242]) by mail.maildlp.com (Postfix) with ESMTPS id 3790A18001B; Mon, 5 Aug 2024 09:14:11 +0800 (CST) Received: from [10.67.121.59] (10.67.121.59) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Mon, 5 Aug 2024 09:14:10 +0800 Message-ID: <3f325714-bcde-2ad9-6aaf-6f70c1db0771@huawei.com> Date: Mon, 5 Aug 2024 09:14:09 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v5] virtio: optimize stats counters performance To: =?UTF-8?Q?Morten_Br=c3=b8rup?= <mb@smartsharesystems.com>, <maxime.coquelin@redhat.com>, <chenbox@nvidia.com> CC: <stephen@networkplumber.org>, <dev@dpdk.org> References: <20240731131744.36448-1-mb@smartsharesystems.com> <20240801160312.205281-1-mb@smartsharesystems.com> <677a47da-c6ba-57a9-92c7-a98df739d216@huawei.com> <98CBD80474FA8B44BF855DF32C47DC35E9F5F0@smartserver.smartshare.dk> From: "lihuisong (C)" <lihuisong@huawei.com> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F5F0@smartserver.smartshare.dk> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To kwepemm600004.china.huawei.com (7.193.23.242) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org 在 2024/8/2 19:27, Morten Brørup 写道: >> From: lihuisong (C) [mailto:lihuisong@huawei.com] >> Sent: Friday, 2 August 2024 04.23 >> >> 在 2024/8/2 0:03, Morten Brørup 写道: >>> void >>> -virtio_update_packet_stats(struct virtnet_stats *stats, struct >> rte_mbuf *mbuf) >>> +virtio_update_packet_stats(struct virtnet_stats *const stats, >>> + const struct rte_mbuf *const mbuf) >> The two const is also for performace? Is there gain? > The "const struct rte_mbuf * mbuf" informs the optimizer that the function does not modify the mbuf. This is for the benefit of callers of the function; they can rely on the mbuf being unchanged when the function returns. > So, if the optimizer has cached some mbuf field before calling the function, it does not need to read the mbuf field again, but can continue using the cached value after the function call. > > The two "type *const ptr" probably make no performance difference with the compilers and function call conventions used by the CPU architectures supported by DPDK. ok > >>> + RTE_BUILD_BUG_ON(offsetof(struct virtnet_stats, broadcast) != >>> + offsetof(struct virtnet_stats, multicast) + >> sizeof(uint64_t)); >>> + if (unlikely(rte_is_multicast_ether_addr(ea))) >>> + (&stats->multicast)[rte_is_broadcast_ether_addr(ea)]++; >> The "rte_is_broadcast_ether_addr(ea) " will be calculated twice if >> packet is mulitcast. >> How about coding like: >> --> >> is_mulitcast = rte_is_multicast_ether_addr(ea); >> if (unlikely(is_mulitcast)) >> (&stats->multicast)[rte_is_broadcast_ether_addr(ea)]++; > I don't think "rte_is_broadcast_ether_addr(ea)" is calculated twice for multicast packets. > My code essentially does this: > if (mc(ea)) > stats[bc(ea)]++; > > Changing to this shouldn't make a difference: > m = mc(ea); > if (m) > stats[bc(ea)]++; Yeah，you are right. >