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$(echo .)org \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=arnd@arndb$(echo .)de \
--cc=bcm-kernel-feedback-list@broadcom$(echo .)com \
--cc=bhargava.marreddy@broadcom$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=florian.fainelli@broadcom$(echo .)com \
--cc=fmancera@suse$(echo .)de \
--cc=justin.chen@broadcom$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=markus@blochl$(echo .)de \
--cc=nb@tipi-net$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=opendmb@gmail$(echo .)com \
--cc=pabeni@redhat$(echo .)com \
--cc=rajashekar.hudumula@broadcom$(echo .)com \
--cc=vikas.gupta@broadcom$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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