[PATCH net v2 1/3] can: dev: prevent bus-off recovery when the interface is not running
Marc Kleine-Budde
mkl at pengutronix.de
Thu Oct 15 12:09:52 PDT 2026
Hello,
there was a hickup with the sashiko bot.
It seems the review for patch:
| https://lore.kernel.org/all/20261014-stmmac-rx-mb-v7-2-5d2d70f052cd@oss.qualcomm.com/
was send to as a rely to this patch:
| https://lore.kernel.org/all/tencent_196A2433801631CD2FA42998465E26316105@qq.com/
regards,
Marc
On 15.10.2026 16:37:52, netdev-bot+sashiko at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] Split-frame drop accounting in stmmac_rx_zc() is inconsistent
> per frame and per core.
>
> Pre-existing issues:
> - [High] stmmac_dispatch_skb_zc() (stmmac_main.c:5533) sets
> `skb->ip_summed = CHECKSUM_UNNECESSARY` for every IP frame when RX COE…
> - [Medium] priv->xstats.rx_dropped and priv->xstats.rx_errors are device-
> wide unsigned longs.
>
> --
>
> net: stmmac: drop split frames on AF_XDP zero-copy RX
>
> This patch fixes stmmac_rx_zc() so that split frames are drained as
> errored, and the last fragment is no longer handed to XDP as a fresh
> frame. It also changes the legacy normal and enhanced descriptor
> callbacks to report rx_not_ls together with discard_frame for non-last
> descriptors.
>
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> > index 0512536012251..297e2428a8f9b 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
> > @@ -184,7 +184,7 @@ static int enh_desc_get_rx_status(struct stmmac_extra_stats *x,
> >
> > if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
> > x->rx_length++;
> > - return discard_frame;
> > + return discard_frame | rx_not_ls;
> > }
> >
> > if (unlikely(rdes0 & RDES0_ERROR_SUMMARY)) {
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> > index c4b613564f87c..d19bf7ca4db38 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> > @@ -70,7 +70,7 @@ static int ndesc_get_rx_status(struct stmmac_extra_stats *x,
> >
> > if (unlikely(!(rdes0 & RDES0_LAST_DESCRIPTOR))) {
> > x->rx_length++;
> > - return discard_frame;
> > + return discard_frame | rx_not_ls;
> > }
> >
> > if (unlikely(rdes0 & RDES0_ERROR_SUMMARY)) {
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index f0835be2f65a7..10ea72cbf889a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -5695,18 +5695,21 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
> > if (priv->extend_desc)
> > stmmac_rx_extended_status(priv, &priv->xstats,
> > rx_q->dma_erx + entry);
> > - if (unlikely(status == discard_frame)) {
> > - xsk_buff_free(buf->xdp);
> > - buf->xdp = NULL;
> > - dirty++;
> > +
> > + if (unlikely(status & discard_frame)) {
> > error = 1;
> > if (!priv->hwts_rx_en)
> > rx_errors++;
> > }
> >
> > - if (unlikely(error && (status & rx_not_ls)))
> > - goto read_again;
> > if (unlikely(error)) {
> > + xsk_buff_free(buf->xdp);
> > + buf->xdp = NULL;
> > + dirty++;
> > +
> > + if (status & rx_not_ls)
> > + goto read_again;
> > +
> > count++;
> > continue;
> > }
> > @@ -5715,8 +5718,9 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
> > if (likely(status & rx_not_ls)) {
> > xsk_buff_free(buf->xdp);
> > buf->xdp = NULL;
> > + rx_dropped++;
> > + error = 1;
>
> [Severity: Medium]
>
> Is the split-frame drop accounting here consistent across descriptor
> formats?
>
> The legacy callbacks now return discard_frame | rx_not_ls for every
> non-last descriptor. So the discard branch above runs first for each of
> those descriptors, and there is no !error guard:
>
> if (unlikely(status & discard_frame)) {
> error = 1;
> if (!priv->hwts_rx_en)
> rx_errors++;
> }
>
> On dwmac100/dwmac1000, a frame spanning N descriptors would then add N-1
> to rx_errors. It adds 0 to rx_dropped, while count / rx_pkt_n goes up
> only once for the frame. The new rx_dropped++ in this 1:1 block can never
> be reached on those cores.
>
> On dwmac4 (and similarly xgmac), dwmac4_wrback_get_rx_status() returns a
> bare rx_not_ls for non-last descriptors:
>
> if (likely(!(rdes3 & RDES3_LAST_DESCRIPTOR)))
> return rx_not_ls;
>
> That means this block does rx_dropped++ and sets error = 1. If the last
> descriptor has RDES3_ERROR_SUMMARY set, the discard branch also does
> rx_errors++. The same dropped frame then shows up in both rx_dropped and
> rx_errors.
>
> Later in the series, stmmac_rx() is changed to count only once, using
> (!error && (status & discard_frame)). Should stmmac_rx_zc() get the same
> treatment, so each dropped frame is counted once in either rx_errors or
> rx_dropped?
>
> > dirty++;
> > - count++;
> > goto read_again;
> > }
> >
>
> [Severity: Medium]
>
> This is a pre-existing issue and was not introduced by this patch. The
> new rx_dropped++ feeds into an unsynchronized accumulation at the end of
> stmmac_rx_zc():
>
> priv->xstats.rx_dropped += rx_dropped;
> priv->xstats.rx_errors += rx_errors;
>
> These are device-wide unsigned longs. They are updated with a plain +=
> from per-queue NAPI contexts that can run at the same time on different
> CPUs. stmmac_rx() does the same thing, and stmmac_get_stats64() reads
> them without any locking.
>
> Can concurrent NAPI polls on different queues lose increments to these
> counters?
>
> [Severity: High]
>
> This isn't a bug introduced by this patch, but it is in the same receive
> path. On XDP_PASS, stmmac_rx_zc() calls stmmac_dispatch_skb_zc() without
> passing status. That function marks every IP frame as verified when RX
> COE is enabled:
>
> stmmac_dispatch_skb_zc() {
> ...
> if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
> skb_checksum_none_assert(skb);
> else
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> ...
> }
>
> On dwmac4, dwmac4_wrback_get_rx_status() reports L4 payload checksum
> failures only as csum_none, without discard_frame:
>
> if (rdes1 & RDES1_IP_PAYLOAD_ERROR) {
> x->ip_payload_err++;
> ret |= csum_none;
> }
>
> Also, dwmac4_dma_rx_chan_op_mode() sets MTL_OP_MODE_DIS_TCP_EF, so the
> hardware forwards these frames instead of dropping them.
>
> Suppose a frame with a bad TCP/UDP checksum arrives on a ZC queue and
> the XDP program passes it. Could it reach the stack as
> CHECKSUM_UNNECESSARY and hand corrupted payload to applications?
>
> The non-ZC stmmac_rx() handles this case:
>
> if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb) ||
> (status & csum_none))
> skb_checksum_none_assert(skb);
>
> This still seems to be the case at the end of the series.
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/tencent_196A2433801631CD2FA42998465E26316105%40qq.com
>
>
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20261015/e9bcf290/attachment.sig>
More information about the Linux-rockchip
mailing list