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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.