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 84E5C41CE3; Sun, 19 Feb 2023 21:43:40 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2C21D42D3F; Sun, 19 Feb 2023 21:43:40 +0100 (CET) Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by mails.dpdk.org (Postfix) with ESMTP id 0DDE340698 for ; Sun, 19 Feb 2023 21:43:39 +0100 (CET) Received: by mail-ed1-f47.google.com with SMTP id k5so5515618edo.3 for ; Sun, 19 Feb 2023 12:43:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=o6lUgSe82IfSvjY5SZbmXlhiK06T7ODlx8htM+qrweg=; b=m8al/QGBAitU59ej76m6SbVlxfybxG2dAeOOMpsZeYIVyCjyB/xFSnW7v/4ZVog8/r O5yY+TOG2vtv64JucormPqRpJ62OMN5QfYN5fknl3z4eey5cHGXqebrSbTyDV00zITZq MJX7PjfOleDWz6VDPtjjcszYd/vdRnvsoQ8oB3uC8wFjwEwDU4uaPP3fyUTT6rsxKcRl FETnxCBPRt5WkkWQ1tu9ZgSIr23sRlBLyIBF3N1PMhNF68zsSfqf6SFwbA7SzPw4RrqB +qWgbwoDY78HK/xGZ+HEzdm9sQKwD9nCN4fQxglXce9qz4yaTs6eaTEBeWhWcl+5dXU3 dGcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=o6lUgSe82IfSvjY5SZbmXlhiK06T7ODlx8htM+qrweg=; b=anP2FESESFhfkqE2puiRrDia/KR7ukzoLkeYi27A2yvu740fjOuFnMktPeHU7XfHLq lPytLpkbduG2tAa9qBBiEVODRQLgbYJY1hYeRE6eJG5haJ/XkFx8kFd/0Xk3Cx3pugY4 nxt831RWATYMfEsf72GI8yz4WBvyycjs/K5T6BIYGQ9WQRl0xM5LwWwjF71L/dMkScht holHiC0XqJDKWpaJQ+ryd8h8zqpj39bbVhji7j/SRMGrqRPMRwxtg/YaKI0IBGrLiNds 6rvlnns4I8x2J77SxY42I/RYCRfKbyo9PI/r5fT8dUbPZQPlrKfz7/udt+RV9MUkuuvm Dlmg== X-Gm-Message-State: AO0yUKUdT1ZrTfIdk45YXe2S/TKvin1HsiARUABqqcMpY2zPUULe9LFk lp+rlM+wCYPS5N7GQ4GJBfI= X-Google-Smtp-Source: AK7set9V9mcxRmKoJhk+vCM1H1gADJqXwnzgs/cpQUhmAP/e1iYgch4K4izVChBzfyZxf1Oal5jXRg== X-Received: by 2002:aa7:c54d:0:b0:4ab:cb8c:932b with SMTP id s13-20020aa7c54d000000b004abcb8c932bmr2347867edr.40.1676839418528; Sun, 19 Feb 2023 12:43:38 -0800 (PST) Received: from smtpclient.apple ([176.41.28.141]) by smtp.gmail.com with ESMTPSA id q27-20020a50c35b000000b004ad601533a3sm906948edb.55.2023.02.19.12.43.34 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 19 Feb 2023 12:43:38 -0800 (PST) From: Levend Sayar Message-Id: <9B306604-8CC8-4CC6-B58F-9A5B28C3D3CB@gmail.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_EF831679-B6E4-477B-80EA-D009D3A8B2EB" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.300.101.1.3\)) Subject: Re: [PATCH] net/gve: fix Rx no mbufs stats counter update Date: Sun, 19 Feb 2023 23:43:08 +0300 In-Reply-To: <20230219093543.45ed457e@hermes.local> Cc: junfeng.guo@intel.com, dev@dpdk.org To: Stephen Hemminger References: <20230219003059.85479-1-levendsayar@gmail.com> <20230219093543.45ed457e@hermes.local> X-Mailer: Apple Mail (2.3731.300.101.1.3) 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 --Apple-Mail=_EF831679-B6E4-477B-80EA-D009D3A8B2EB Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 19 Feb 2023, at 20:35, Stephen Hemminger = wrote: >=20 > On Sun, 19 Feb 2023 03:30:59 +0300 > Levend Sayar > = wrote: >=20 >> rx no_mbufs stats counter update is added for another error case. >>=20 >> Fixes: 4f6b1dd8240c ("net/gve: support basic statistics") >> Cc: junfeng.guo@intel.com >>=20 >> Signed-off-by: Levend Sayar >> --- >> drivers/net/gve/gve_rx.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >>=20 >> diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c >> index 66fbcf3930..b0427731f8 100644 >> --- a/drivers/net/gve/gve_rx.c >> +++ b/drivers/net/gve/gve_rx.c >> @@ -24,6 +24,7 @@ gve_rx_refill(struct gve_rx_queue *rxq) >> nmb =3D rte_pktmbuf_alloc(rxq->mpool); >> if (!nmb) >> break; >> + >> rxq->sw_ring[idx + i] =3D nmb; >> } >> if (i !=3D nb_alloc) { >=20 > Looks like accidental whitespace change included in this patch. LS: Right. Let me correct. >> @@ -59,9 +60,13 @@ gve_rx_refill(struct gve_rx_queue *rxq) >> nmb =3D rte_pktmbuf_alloc(rxq->mpool); >> if (!nmb) >> break; >> + >> rxq->sw_ring[idx + i] =3D nmb; >> } >> - nb_alloc =3D i; >> + if (i !=3D nb_alloc) { >> + rxq->no_mbufs +=3D nb_alloc - i; >> + nb_alloc =3D i; >> + } >=20 > Would be better to add unlikely() here like: > if (unlikely(i < nb_alloc)) { > rxq->no_mbufs +=3D nb_alloc - i; > nb_alloc =3D i; > } >=20 > Or eliminate conditional branch in hot path completely. > rxq->no_mbufs +=3D nb_alloc - i; > nb_alloc =3D i; >=20 > Or better yet refactor code here to use rte_pktmbuf_alloc_bulk() which > does single ring operation. >=20 >> } >> rxq->nb_avail -=3D nb_alloc; >> next_avail +=3D nb_alloc; LS: =E2=80=9Cunlikely=E2=80=9D can be added. You=E2=80=99re right. Code = already tries to make a bulk allocation first. If that bulk allocation does not work, it tries to allocate one my one.=20= I will supersede this one and create v2. Thanks Stephen. Best, Levend= --Apple-Mail=_EF831679-B6E4-477B-80EA-D009D3A8B2EB Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On 19 = Feb 2023, at 20:35, Stephen Hemminger <stephen@networkplumber.org> = wrote:

On Sun, 19 Feb 2023 03:30:59 = +0300
Levend Sayar = <levendsayar@gmail.com> wrote:

rx no_mbufs = stats counter update is added for another error case.

Fixes: = 4f6b1dd8240c ("net/gve: support basic statistics")
Cc: = junfeng.guo@intel.com

Signed-off-by: Levend Sayar = <levendsayar@gmail.com>
---
drivers/net/gve/gve_rx.c | 7 = ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff = --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c
index = 66fbcf3930..b0427731f8 100644
--- a/drivers/net/gve/gve_rx.c
+++ = b/drivers/net/gve/gve_rx.c
@@ -24,6 +24,7 @@ gve_rx_refill(struct = gve_rx_queue *rxq)
nmb =3D rte_pktmbuf_alloc(rxq->mpool);
if = (!nmb)
= = = = = break;
+
rxq->sw_ring[idx + i] =3D nmb;
= }
= = = if (i !=3D nb_alloc) {

Looks = like accidental whitespace change included in this patch.

LS: Right. Let me = correct.

@@ -59,9 +60,13 = @@ gve_rx_refill(struct gve_rx_queue *rxq)
nmb =3D = rte_pktmbuf_alloc(rxq->mpool);
if (!nmb)
= break;
+
rxq->sw_ring[idx + i] =3D nmb;
= }
- = = = nb_alloc =3D i;
+ if (i !=3D nb_alloc) {
+ = rxq->no_mbufs +=3D nb_alloc - i;
+ nb_alloc = =3D i;
+ = = = }

Would = be better to add unlikely() here like:
= if = (unlikely(i < nb_alloc)) {
= = rxq->no_mbufs +=3D = nb_alloc - i;
= nb_alloc =3D = i;
= }

Or eliminate conditional branch in hot path = completely.
= rxq->no_mbufs +=3D = nb_alloc - i;
= nb_alloc =3D = i;

Or better yet refactor = code here to use rte_pktmbuf_alloc_bulk() which
does single ring operation.

= }
= = rxq->nb_avail -=3D nb_alloc;
next_avail +=3D = nb_alloc;

LS: = =E2=80=9Cunlikely=E2=80=9D can be added. You=E2=80=99re right. Code = already tries to make a bulk allocation first.
If that bulk = allocation does not work, it tries to allocate one my = one. 

I will supersede this one and create = v2.
Thanks = Stephen.

Best,
Levend
= --Apple-Mail=_EF831679-B6E4-477B-80EA-D009D3A8B2EB--