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 23C3FA0C4E; Fri, 15 Oct 2021 14:16:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79203411CB; Fri, 15 Oct 2021 14:16:37 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id CD337410F1 for ; Fri, 15 Oct 2021 14:16:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634300196; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TrnuCVOgl0gpqCjnxSyfXVB1AbBvzKMzgI+2SAyIoSo=; b=Pd8pWAvONbrOVPU8pwnvLYrbUq5utltFkUGanibk3aXeZYBktxtndprM32Wvyb0ieC/GJI N1qsjKT7YZcZRa23sE36cBfGrEexPa+9hPw063jUoY15VMygAcNhpQTgtvlvABR4FSJkas 78BDybxKryPIFYLZKHsKbd71pq20oZQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-250-aOwWdtL8Pfaye1-okV5UOQ-1; Fri, 15 Oct 2021 08:16:35 -0400 X-MC-Unique: aOwWdtL8Pfaye1-okV5UOQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E945E100CCC2; Fri, 15 Oct 2021 12:16:33 +0000 (UTC) Received: from [10.39.208.20] (unknown [10.39.208.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3D1C51973B; Fri, 15 Oct 2021 12:16:32 +0000 (UTC) Message-ID: <6bd35937-2042-ad51-5c17-f75621f0e837@redhat.com> Date: Fri, 15 Oct 2021 14:16:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: Gaoxiang Liu , chenbo.xia@intel.com Cc: dev@dpdk.org, liugaoxiang@huawei.com References: <20210927013022.131-1-gaoxiangliu0@163.com> <20210928014348.1747-1-gaoxiangliu0@163.com> From: Maxime Coquelin In-Reply-To: <20210928014348.1747-1-gaoxiangliu0@163.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] net/vhost: merge vhost stats loop in vhost Tx/Rx 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 Sender: "dev" Hi, On 9/28/21 03:43, Gaoxiang Liu wrote: > To improve performance in vhost Tx/Rx, merge vhost stats loop. > eth_vhost_tx has 2 loop of send num iteraion. > It can be merge into one. > eth_vhost_rx has the same issue as Tx. > > Fixes: 4d6cf2ac93dc ("net/vhost: add extended statistics") Please remove the Fixes tag, this is an optimization, not a fix. > > Signed-off-by: Gaoxiang Liu > --- > > v2: > * Fix coding style issues. > --- > drivers/net/vhost/rte_eth_vhost.c | 62 ++++++++++++++----------------- > 1 file changed, 28 insertions(+), 34 deletions(-) > > diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c > index a202931e9a..a4129980f2 100644 > --- a/drivers/net/vhost/rte_eth_vhost.c > +++ b/drivers/net/vhost/rte_eth_vhost.c > @@ -336,38 +336,29 @@ vhost_count_xcast_packets(struct vhost_queue *vq, > } > > static void > -vhost_update_packet_xstats(struct vhost_queue *vq, struct rte_mbuf **bufs, > - uint16_t count, uint64_t nb_bytes, > - uint64_t nb_missed) > +vhost_update_single_packet_xstats(struct vhost_queue *vq, struct rte_mbuf *buf) I tried to build without and with your patch, and I think that what can explain most of the performance difference is that without your patch the function is not inlined, whereas it is implicitely inlined with your patch applied. I agree with your patch, but I think we might add __rte_always_inline to this function to make it explicit. What do you think? Other than that: Reviewed-by: Maxime Coquelin Thanks, Maxime > { > uint32_t pkt_len = 0; > - uint64_t i = 0; > uint64_t index; > struct vhost_stats *pstats = &vq->stats; > > - pstats->xstats[VHOST_BYTE] += nb_bytes; > - pstats->xstats[VHOST_MISSED_PKT] += nb_missed; > - pstats->xstats[VHOST_UNICAST_PKT] += nb_missed; > - > - for (i = 0; i < count ; i++) { > - pstats->xstats[VHOST_PKT]++; > - pkt_len = bufs[i]->pkt_len; > - if (pkt_len == 64) { > - pstats->xstats[VHOST_64_PKT]++; > - } else if (pkt_len > 64 && pkt_len < 1024) { > - index = (sizeof(pkt_len) * 8) > - - __builtin_clz(pkt_len) - 5; > - pstats->xstats[index]++; > - } else { > - if (pkt_len < 64) > - pstats->xstats[VHOST_UNDERSIZE_PKT]++; > - else if (pkt_len <= 1522) > - pstats->xstats[VHOST_1024_TO_1522_PKT]++; > - else if (pkt_len > 1522) > - pstats->xstats[VHOST_1523_TO_MAX_PKT]++; > - } > - vhost_count_xcast_packets(vq, bufs[i]); > + pstats->xstats[VHOST_PKT]++; > + pkt_len = buf->pkt_len; > + if (pkt_len == 64) { > + pstats->xstats[VHOST_64_PKT]++; > + } else if (pkt_len > 64 && pkt_len < 1024) { > + index = (sizeof(pkt_len) * 8) > + - __builtin_clz(pkt_len) - 5; > + pstats->xstats[index]++; > + } else { > + if (pkt_len < 64) > + pstats->xstats[VHOST_UNDERSIZE_PKT]++; > + else if (pkt_len <= 1522) > + pstats->xstats[VHOST_1024_TO_1522_PKT]++; > + else if (pkt_len > 1522) > + pstats->xstats[VHOST_1523_TO_MAX_PKT]++; > } > + vhost_count_xcast_packets(vq, buf); > } > > static uint16_t > @@ -376,7 +367,6 @@ eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs) > struct vhost_queue *r = q; > uint16_t i, nb_rx = 0; > uint16_t nb_receive = nb_bufs; > - uint64_t nb_bytes = 0; > > if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0)) > return 0; > @@ -411,11 +401,11 @@ eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs) > if (r->internal->vlan_strip) > rte_vlan_strip(bufs[i]); > > - nb_bytes += bufs[i]->pkt_len; > - } > + r->stats.bytes += bufs[i]->pkt_len; > + r->stats.xstats[VHOST_BYTE] += bufs[i]->pkt_len; > > - r->stats.bytes += nb_bytes; > - vhost_update_packet_xstats(r, bufs, nb_rx, nb_bytes, 0); > + vhost_update_single_packet_xstats(r, bufs[i]); > + } > > out: > rte_atomic32_set(&r->while_queuing, 0); > @@ -471,16 +461,20 @@ eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs) > break; > } > > - for (i = 0; likely(i < nb_tx); i++) > + for (i = 0; likely(i < nb_tx); i++) { > nb_bytes += bufs[i]->pkt_len; > + vhost_update_single_packet_xstats(r, bufs[i]); > + } > > nb_missed = nb_bufs - nb_tx; > > r->stats.pkts += nb_tx; > r->stats.bytes += nb_bytes; > - r->stats.missed_pkts += nb_bufs - nb_tx; > + r->stats.missed_pkts += nb_missed; > > - vhost_update_packet_xstats(r, bufs, nb_tx, nb_bytes, nb_missed); > + r->stats.xstats[VHOST_BYTE] += nb_bytes; > + r->stats.xstats[VHOST_MISSED_PKT] += nb_missed; > + r->stats.xstats[VHOST_UNICAST_PKT] += nb_missed; > > /* According to RFC2863, ifHCOutUcastPkts, ifHCOutMulticastPkts and > * ifHCOutBroadcastPkts counters are increased when packets are not >