From: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
To: Paul Barker <paul.barker.ct@bp.renesas.com>
Cc: Sergey Shtylyov <s.shtylyov@omp.ru>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>,
Biju Das <biju.das.jz@bp.renesas.com>,
YueHaibing <yuehaibing@huawei.com>,
netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [net PATCH v2 1/4] net: ravb: Count packets instead of descriptors in R-Car RX path
Date: Sun, 14 Apr 2024 14:21:23 +0200 [thread overview]
Message-ID: <20240414122123.GC2860391@ragnatech.se> (raw)
In-Reply-To: <20240412100024.2296-2-paul.barker.ct@bp.renesas.com>
Hi Paul,
Sorry I just posted my review comments on v1 of this series instead of
v2. They where intended and are still valid for v2, sorry for the noise,
let me know if you want me to repost them here.
On 2024-04-12 11:00:21 +0100, Paul Barker wrote:
> The units of "work done" in the RX path should be packets instead of
> descriptors.
>
> Descriptors which are used by the hardware to record error conditions or
> are empty in the case of a DMA mapping error should not count towards
> our RX work budget.
>
> Fixes: c156633f1353 ("Renesas Ethernet AVB driver proper")
> Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com>
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> ---
> drivers/net/ethernet/renesas/ravb_main.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index ba01c8cc3c90..70f2900648d4 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -892,29 +892,25 @@ static bool ravb_rx_rcar(struct net_device *ndev, int *quota, int q)
> struct ravb_private *priv = netdev_priv(ndev);
> const struct ravb_hw_info *info = priv->info;
> int entry = priv->cur_rx[q] % priv->num_rx_ring[q];
> - int boguscnt = (priv->dirty_rx[q] + priv->num_rx_ring[q]) -
> - priv->cur_rx[q];
> struct net_device_stats *stats = &priv->stats[q];
> struct ravb_ex_rx_desc *desc;
> struct sk_buff *skb;
> dma_addr_t dma_addr;
> struct timespec64 ts;
> + int rx_packets = 0;
> u8 desc_status;
> u16 pkt_len;
> int limit;
> + int i;
>
> - boguscnt = min(boguscnt, *quota);
> - limit = boguscnt;
> + limit = priv->dirty_rx[q] + priv->num_rx_ring[q] - priv->cur_rx[q];
> desc = &priv->rx_ring[q].ex_desc[entry];
> - while (desc->die_dt != DT_FEMPTY) {
> + for (i = 0; i < limit && rx_packets < *quota && desc->die_dt != DT_FEMPTY; i++) {
> /* Descriptor type must be checked before all other reads */
> dma_rmb();
> desc_status = desc->msc;
> pkt_len = le16_to_cpu(desc->ds_cc) & RX_DS;
>
> - if (--boguscnt < 0)
> - break;
> -
> /* We use 0-byte descriptors to mark the DMA mapping errors */
> if (!pkt_len)
> continue;
> @@ -960,7 +956,7 @@ static bool ravb_rx_rcar(struct net_device *ndev, int *quota, int q)
> if (ndev->features & NETIF_F_RXCSUM)
> ravb_rx_csum(skb);
> napi_gro_receive(&priv->napi[q], skb);
> - stats->rx_packets++;
> + rx_packets++;
> stats->rx_bytes += pkt_len;
> }
>
> @@ -995,9 +991,9 @@ static bool ravb_rx_rcar(struct net_device *ndev, int *quota, int q)
> desc->die_dt = DT_FEMPTY;
> }
>
> - *quota -= limit - (++boguscnt);
> -
> - return boguscnt <= 0;
> + stats->rx_packets += rx_packets;
> + *quota -= rx_packets;
> + return *quota == 0;
> }
>
> /* Packet receive function for Ethernet AVB */
> --
> 2.39.2
>
--
Kind Regards,
Niklas Söderlund
next prev parent reply other threads:[~2024-04-14 12:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 10:00 [net PATCH v2 0/4] ravb Ethernet driver bugfixes Paul Barker
2024-04-12 10:00 ` [net PATCH v2 1/4] net: ravb: Count packets instead of descriptors in R-Car RX path Paul Barker
2024-04-14 12:21 ` Niklas Söderlund [this message]
2024-04-12 10:00 ` [net PATCH v2 2/4] net: ravb: Allow RX loop to move past DMA mapping errors Paul Barker
2024-04-12 10:00 ` [net PATCH v2 3/4] net: ravb: Fix GbEth jumbo packet RX checksum handling Paul Barker
2024-04-12 10:00 ` [net PATCH v2 4/4] net: ravb: Fix RX byte accounting for jumbo packets Paul Barker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240414122123.GC2860391@ragnatech.se \
--to=niklas.soderlund+renesas@ragnatech.se \
--cc=biju.das.jz@bp.renesas.com \
--cc=claudiu.beznea.uj@bp.renesas.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=paul.barker.ct@bp.renesas.com \
--cc=s.shtylyov@omp.ru \
--cc=yuehaibing@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).