From: Simon Horman <horms@kernel.org>
To: nb@tipi-net.de
Cc: netdev@vger.kernel.org, justin.chen@broadcom.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, opendmb@gmail.com,
florian.fainelli@broadcom.com,
bcm-kernel-feedback-list@broadcom.com, vikas.gupta@broadcom.com,
rajashekar.hudumula@broadcom.com, bhargava.marreddy@broadcom.com,
fmancera@suse.de, markus@blochl.de, arnd@arndb.de,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net: bcmgenet: convert RX path to page_pool
Date: Thu, 4 Jun 2026 19:54:53 +0100 [thread overview]
Message-ID: <20260604185453.GF3920875@horms.kernel.org> (raw)
In-Reply-To: <20260604173205.3989025-3-horms@kernel.org>
On Thu, Jun 04, 2026 at 06:32:06PM +0100, Simon Horman wrote:
> From: 'Simon Horman' <horms@kernel.org>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/
> ---
> Now I have the exact diff. Let me produce the report.
>
> commit ff8ce4b2334ded9490897cdd2cff5a25a4a0a438
> Author: Nicolai Buchwitz <nb@tipi-net.de>
>
> net: bcmgenet: convert RX path to page_pool
>
> This patch converts the bcmgenet RX path from per-packet
> __netdev_alloc_skb() + dma_map_single() to page_pool, builds SKBs from
> pool pages via napi_build_skb() with skb_mark_for_recycle(), and removes
> the now-unused priv->rx_buf_len field and rx_dma_failed soft MIB
> counter.
>
> > diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > index 7c11cf916762..80dbfba9fa88 100644
> > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>
> [ ... ]
>
> > @@ -1153,7 +1159,6 @@ static const struct bcmgenet_stats bcmgenet_gstrings_stats[] = {
> > UMAC_RBUF_ERR_CNT_V1),
> > STAT_GENET_MISC("mdf_err_cnt", mib.mdf_err_cnt, UMAC_MDF_ERR_CNT),
> > STAT_GENET_SOFT_MIB("alloc_rx_buff_failed", mib.alloc_rx_buff_failed),
> > - STAT_GENET_SOFT_MIB("rx_dma_failed", mib.rx_dma_failed),
> > STAT_GENET_SOFT_MIB("tx_dma_failed", mib.tx_dma_failed),
>
> [Medium]
> The string "rx_dma_failed" exposed via ethtool -S is user-visible: monitoring
> agents (collectd, telegraf, Prometheus exporters, ad-hoc scripts) may key off
> that name, and the strings list is ordered, so removing one shifts the index
> of every counter that follows for tools that index by offset rather than name.
>
> Should the changelog mention this user-visible change explicitly, or could
> the entry be retained reporting 0 to preserve the existing string set?
>
> Worth noting that the sibling driver drivers/net/ethernet/broadcom/bcmsysport.c
> still exposes rx_dma_failed, so the two drivers in the same Broadcom directory
> will diverge after this patch.
FWIIW, I feel that mentioning the change in the patch description
would be sufficient. And I don't buy the index argument, in my view these
are key value pairs.
...
next prev parent reply other threads:[~2026-06-04 18:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 9:42 [PATCH net-next] net: bcmgenet: convert RX path to page_pool Nicolai Buchwitz
2026-06-04 17:32 ` Simon Horman
2026-06-04 18:54 ` Simon Horman [this message]
2026-06-04 19:39 ` Nicolai Buchwitz
2026-06-04 21:31 ` Jacob Keller
2026-06-04 20:05 ` Nicolai Buchwitz
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=20260604185453.GF3920875@horms.kernel.org \
--to=horms@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=arnd@arndb.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=bhargava.marreddy@broadcom.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=florian.fainelli@broadcom.com \
--cc=fmancera@suse.de \
--cc=justin.chen@broadcom.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=markus@blochl.de \
--cc=nb@tipi-net.de \
--cc=netdev@vger.kernel.org \
--cc=opendmb@gmail.com \
--cc=pabeni@redhat.com \
--cc=rajashekar.hudumula@broadcom.com \
--cc=vikas.gupta@broadcom.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