[PATCH] net: macb: rate limit netdev error info print in the data path

[PATCH] net: macb: rate limit netdev error info print in the data path

[Zijin Tao]

Now the MACB ethernet driver print the netdev error information
directly by netdev_err(), which would lead to a large number of
error information print if there was a significant number of
error or just jumbo packets exceeding the MTU received when booting.
For example, it would print a large number of:

macb PHYT0036:00 eth0: not whole frame pointed by descriptor
macb PHYT0036:00 eth0: not whole frame pointed by descriptor
...

in gem_rx() by received a large number of packets without
RX_SOF or RX_EOF flag set, especially with unknown packet
type.

The unlimited prints here would greatly bother and delay
the system booting process unless the source stop sending
packets.

So rate limit the netdev error information print in the receive
and transmit data path.

Signed-off-by: Zijin Tao <taozj888@163.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..c32d48d03008 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1617,16 +1617,16 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		count++;
 
 		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
-			netdev_err(bp->netdev,
-				   "not whole frame pointed by descriptor\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "not whole frame pointed by descriptor\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
 		}
 		skb = queue->rx_skbuff[entry];
 		if (unlikely(!skb)) {
-			netdev_err(bp->netdev,
-				   "inconsistent Rx descriptor chain\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "inconsistent Rx descriptor chain\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
@@ -1829,7 +1829,8 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
 		unsigned long flags;
 		u32 ctrl;
 
-		netdev_err(bp->netdev, "RX queue corruption: reset it\n");
+		if (net_ratelimit())
+			netdev_err(bp->netdev, "RX queue corruption: reset it\n");
 
 		spin_lock_irqsave(&bp->lock, flags);
 
@@ -2102,7 +2103,8 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
 
 	if (status & MACB_BIT(HRESP)) {
 		queue_work(system_bh_wq, &bp->hresp_err_bh_work);
-		netdev_err(netdev, "DMA bus error: HRESP not OK\n");
+		if (net_ratelimit())
+			netdev_err(netdev, "DMA bus error: HRESP not OK\n");
 		macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
 	}
 
@@ -2511,7 +2513,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb,
 		else
 			hdrlen = skb_tcp_all_headers(skb);
 		if (skb_headlen(skb) < hdrlen) {
-			netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
 			/* if this is required, would need to copy to single buffer */
 			return NETDEV_TX_BUSY;
 		}
-- 
2.34.1

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[Jakub Kicinski]

On Sun, 20 Sep 2026 18:09:20 +0800 Zijin Tao wrote:
> Now the MACB ethernet driver print the netdev error information
> directly by netdev_err(), which would lead to a large number of
> error information print if there was a significant number of
> error or just jumbo packets exceeding the MTU received when booting.
> For example, it would print a large number of:
> 
> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
> ...

Do you have the HW or you're acting based on LLM output?
If the latter, and since you can't follow the process please
don't post any more such patches.

If you do have the HW to test this - put into the commit message
what HW you have, and covering all the cases.
-- 
pw-bot: cr

Re:Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[taozj888]

Hi Jakub:

Yes, we had the actual HW for our production environment which is based on a chip
that used the Cadence MACB implementation, and it really bothered our booting process
since it received a lot of jumob pkts which are just over the current MTU of the driver
and it has to show those error msgs. 

The issue was reported by our production section. Actually the err msg here should include a timestamp
at the beginning, looks like:
[ 318.594950][ C0] macb PHYT0036:00 eth0: not whole frame pointed by descriptor
[ 318.602366][ C0] macb PHYT0036:00 eth0: not whole frame pointed by descriptor
...

but for simplicity, I just removed that part.

I will put some of the HW info into the commit msg as what you indicated.

Thanks,

Zijin Tao






At 2026-09-24 09:17:46, "Jakub Kicinski" <kuba@kernel.org> wrote:
>On Sun, 20 Sep 2026 18:09:20 +0800 Zijin Tao wrote:
>> Now the MACB ethernet driver print the netdev error information
>> directly by netdev_err(), which would lead to a large number of
>> error information print if there was a significant number of
>> error or just jumbo packets exceeding the MTU received when booting.
>> For example, it would print a large number of:
>> 
>> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
>> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
>> ...
>
>Do you have the HW or you're acting based on LLM output?
>If the latter, and since you can't follow the process please
>don't post any more such patches.
>
>If you do have the HW to test this - put into the commit message
>what HW you have, and covering all the cases.
>-- 
>pw-bot: cr

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[Théo Lebrun]

Hello Zijin,

You missed part of my recent feedback [0][1]. Copy paste:

 - Also you are missing the prefix [PATCH net] or [PATCH net-next].
   Read up about this here (and read the full page):
   https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

 - Also your To/Cc list is weird, make sure to use
   scripts/get_maintainer.pl (or use b4 for patch
   management which uses it automatically).

In addition, make sure to read the "submitting patches" guide [2].
You missed:
 - replying to all review points one by one using interleaved [4]
 - V2 in subject [3]
 - write up a changelog [4]

Also this one is less well known, but the net subsystem (and many others
nowadays) expect people to reply to Sashiko review emails to say
whether they agree or disagree. Especially if they disagree. You can
mostly skip over the pre-existing issues which don't relate to your
series.

For example Sashiko says you don't cover some log netdev_err() calls.
You can reply explaining why only the ones you touched are important to
deal with.

--

And I see just now I have in my inbox an email from you asking how to do
it properly. Good! But it doesn't show up on lore, I'm not sure why.

Replying to it here:

 - Don't send the same patch but slightly modified. Maintainers need to
   know the latest version. New version means V2/V3/etc, even if
   changes are tiny (like a typo fix in commit message).

 - Don't put V1 for the first revision. I think that's git-format-patch
   default behavior.

 - Using git-format-patch looks something like:

   ⟩ git format-patch -1 998b159fdd78 --subject-prefix="PATCH net" -v2
   v2-0001-net-macb-take-bp-lock-around-NCR-read-modify-writ.patch

   ⟩ grep ^Subject v2-0001-net-macb-take-bp-lock-around-NCR-read-modify-writ.patch
   Subject: [PATCH net v2] net: macb: take bp->lock around NCR read-modify-writes

   ⟩ scripts/get_maintainer.pl v2-0001-*.patch
   "Théo Lebrun" <theo.lebrun@bootlin.com> (maintainer:ATMEL MACB ETHERNET DRIVER)
   Conor Dooley <conor.dooley@microchip.com> (reviewer:ATMEL MACB ETHERNET DRIVER)
   Andrew Lunn <andrew+netdev@lunn.ch> (maintainer:NETWORKING DRIVERS)
   ...

   I think most people call scripts/get_maintainer.pl and write the
   git send-email --to/--cc flags by hand. I've been using b4 for a few
   years now so I don't really know the usual git format-patch workflow.

   Or you can use `git send-email --cc-cmd=scripts/get_maintainer.pl`.

[0]: https://lore.kernel.org/all/DLKVCOXTNGVZ.3CC6KFXDIFJ3X@bootlin.com/
[1]: https://lore.kernel.org/all/DLKVEXS7X14N.XLLMBDDX6ZJW@bootlin.com/
[2]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html
[3]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#subject-line
[4]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#respond-to-review-comments

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re:Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[taozj888]

Hi Theo: 

Thank you very much for your kindly reply.

For all your concerns in this mail thread and [0][1], I try to give my answers here
and hope it does not miss anything:


1. Yes, I missed the prefix [PATCH net], I will add it in my next commit; I would like to send it again as the 1st version
since I missed this needed part [PATCH net];


2. For the To/Cc list, I will use `git send-email --cc-cmd=scripts/get_maintainer.pl` as you indicated;


3. This whole patch is  NOT LLM generated, it's really a bug hit in practice, the "PHYT0036:00" 
device name is the real name that showed in our console output, I can't just imagine or made this name and it's really of out of my imagination.
 I think it may be related to a vendor with some of the PCI addr but I think it may not be polite to say it directly. 
Actually hallucinating a kernel log even a kernel patch is out of my knowledge, if you know to how and 
wish to tell me, I think it should be really interesting!


4. Why it bothered the booting process? 
In the booting process, the MACB driver received a large number of jumbo pkts that the driver can't see 
the RX_EOF  flag in the ctrl since the length of the pkt is larger than the current MTU. Those jumbo packets are 
sent by other network cards in our machine box and they are legal and used to collect information for all other cards/blades So netdev_err() 
prints a lot of error information in the console and occupied the full bandwidth of console, but other booting processes
also want to show some information in the console, and they have to wait util the whole error print of "not whole frame pointed by descriptor"
finished, so the system is delayed to be ready to the user.

5. For the severity tag and should this carry a Fixes tag and name the intended tree,  please give out a clear indication from your experiences for what I described above. I really have not too much knowledge about that.

Thanks,

Zijin Tao


At 2026-09-24 17:02:03, "Théo Lebrun" <theo.lebrun@bootlin.com> wrote:
>Hello Zijin,
>
>You missed part of my recent feedback [0][1]. Copy paste:
>
> - Also you are missing the prefix [PATCH net] or [PATCH net-next].
>   Read up about this here (and read the full page):
>   https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> - Also your To/Cc list is weird, make sure to use
>   scripts/get_maintainer.pl (or use b4 for patch
>   management which uses it automatically).
>
>In addition, make sure to read the "submitting patches" guide [2].
>You missed:
> - replying to all review points one by one using interleaved [4]
> - V2 in subject [3]
> - write up a changelog [4]
>
>Also this one is less well known, but the net subsystem (and many others
>nowadays) expect people to reply to Sashiko review emails to say
>whether they agree or disagree. Especially if they disagree. You can
>mostly skip over the pre-existing issues which don't relate to your
>series.
>
>For example Sashiko says you don't cover some log netdev_err() calls.
>You can reply explaining why only the ones you touched are important to
>deal with.
>
>--
>
>And I see just now I have in my inbox an email from you asking how to do
>it properly. Good! But it doesn't show up on lore, I'm not sure why.
>
>Replying to it here:
>
> - Don't send the same patch but slightly modified. Maintainers need to
>   know the latest version. New version means V2/V3/etc, even if
>   changes are tiny (like a typo fix in commit message).
>
> - Don't put V1 for the first revision. I think that's git-format-patch
>   default behavior.
>
> - Using git-format-patch looks something like:
>
>   ⟩ git format-patch -1 998b159fdd78 --subject-prefix="PATCH net" -v2
>   v2-0001-net-macb-take-bp-lock-around-NCR-read-modify-writ.patch
>
>   ⟩ grep ^Subject v2-0001-net-macb-take-bp-lock-around-NCR-read-modify-writ.patch
>   Subject: [PATCH net v2] net: macb: take bp->lock around NCR read-modify-writes
>
>   ⟩ scripts/get_maintainer.pl v2-0001-*.patch
>   "Théo Lebrun" <theo.lebrun@bootlin.com> (maintainer:ATMEL MACB ETHERNET DRIVER)
>   Conor Dooley <conor.dooley@microchip.com> (reviewer:ATMEL MACB ETHERNET DRIVER)
>   Andrew Lunn <andrew+netdev@lunn.ch> (maintainer:NETWORKING DRIVERS)
>   ...
>
>   I think most people call scripts/get_maintainer.pl and write the
>   git send-email --to/--cc flags by hand. I've been using b4 for a few
>   years now so I don't really know the usual git format-patch workflow.
>
>   Or you can use `git send-email --cc-cmd=scripts/get_maintainer.pl`.
>
>[0]: https://lore.kernel.org/all/DLKVCOXTNGVZ.3CC6KFXDIFJ3X@bootlin.com/
>[1]: https://lore.kernel.org/all/DLKVEXS7X14N.XLLMBDDX6ZJW@bootlin.com/
>[2]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>[3]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#subject-line
>[4]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#respond-to-review-comments
>
>Thanks,
>
>--
>Théo Lebrun, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[Théo Lebrun]

Hello Zijin,

Please use interleaved posting for replies, not top posting.

https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions

On Tue Sep 29, 2026 at 5:56 AM CEST, Zijin wrote:
> Thank you very much for your kindly reply.
>
> For all your concerns in this mail thread and [0][1], I try to give
> my answers here and hope it does not miss anything:
>
> 1. Yes, I missed the prefix [PATCH net], I will add it in my next
> commit; I would like to send it again as the 1st version
> since I missed this needed part [PATCH net];

If patch stays the same it is OK, if it changes (for example because you
add trailers) then you must iterate onto v2.

> 2. For the To/Cc list, I will use `git send-email --cc-cmd=scripts/get_maintainer.pl` as you indicated;
>
> 3. This whole patch is  NOT LLM generated, it's really a bug hit in practice, the "PHYT0036:00" 
> device name is the real name that showed in our console output, I can't just imagine or made this name and it's really of out of my imagination.
>  I think it may be related to a vendor with some of the PCI addr but I think it may not be polite to say it directly. 
> Actually hallucinating a kernel log even a kernel patch is out of my knowledge, if you know to how and 
> wish to tell me, I think it should be really interesting!

No I don't encourage you to hallucinate! It's what LLMs do sometimes
when you ask one to generate a commit message for a bug fix. They have
seen tons of commits with logs so they reproduce.

[...]

> 5. For the severity tag and should this carry a Fixes tag and name the
> intended tree,  please give out a clear indication from your
> experiences for what I described above. I really have not too much
> knowledge about that.

Yes. You put those two trailers:

   Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
   Cc: stable@vger.kernel.org

See documentation link below (or kernel git log for many examples).

In your case you want the oldest commit that introduced one of those
logs. The macb_start_xmit() one for example got introduced with the
first driver commit, so that's what you target.

https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 (1) · High: 1 · Medium: 0 · Low: 1

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).

- [High] Incomplete fix: the patch rate-limits only 4 netdev_err() sites,
  leaving the equally floodable peer prints in the very same RX/TX data…
- [Low] The commit message describes a user-visible defect (unbounded
  console flooding from gem_rx() delaying boot) but carries only a…

Pre-existing issues:
- [Critical] Use-after-free of the skb the network stack still owns:
  macb_start_xmit() calls macb_pad_and_fcs(&skb, netdev) at…

--

net: macb: rate limit netdev error info print in the data path

The patch wraps five netdev_err() calls in the macb receive and transmit
paths with net_ratelimit(), so a flood of bad descriptors or LSO errors no
longer prints unbounded messages.  The motivating case is "not whole frame
pointed by descriptor" repeating from gem_rx() during boot.

No Link: tags were present in the commit header.

[Severity: Low]
This isn't a bug in the code, but the changelog describes a user-visible
defect (boot delayed by unbounded printing from gem_rx()) and the patch
carries only:

  Signed-off-by: Zijin Tao <taozj888@163.com>

Should this carry a Fixes tag and name the intended tree?  The unconditional
print in gem_rx() goes back to commit 4df95131ea80 ("net/macb: change RX
path for GEM"), which isn't referenced, and there is no note about whether
this is aimed at net or net-next.

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index b8234ac4b602..c32d48d03008 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1617,16 +1617,16 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
>  		count++;
>  
>  		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
> -			netdev_err(bp->netdev,
> -				   "not whole frame pointed by descriptor\n");
> +			if (net_ratelimit())
> +				netdev_err(bp->netdev, "not whole frame pointed by descriptor\n");
>  			bp->netdev->stats.rx_dropped++;
>  			queue->stats.rx_dropped++;
>  			break;
>  		}
>  		skb = queue->rx_skbuff[entry];
>  		if (unlikely(!skb)) {
> -			netdev_err(bp->netdev,
> -				   "inconsistent Rx descriptor chain\n");
> +			if (net_ratelimit())
> +				netdev_err(bp->netdev, "inconsistent Rx descriptor chain\n");
>  			bp->netdev->stats.rx_dropped++;
>  			queue->stats.rx_dropped++;
>  			break;

[Severity: High]
The changelog says the change will "rate limit the netdev error information
print in the receive and transmit data path", but several equally floodable
netdev_err() calls in the same paths are left as they are.  Can the same
console flood still be produced through them?

In the receive path, gem_rx() ends with gem_rx_refill(queue), which prints
once per NAPI poll while allocations keep failing:

drivers/net/ethernet/cadence/macb_main.c:gem_rx_refill() {
	...
			skb = netdev_alloc_skb(bp->netdev, bp->rx_buffer_size);
			if (unlikely(!skb)) {
				netdev_err(bp->netdev,
					   "Unable to allocate sk_buff\n");
				break;
			}
	...
}

In the transmit path, macb_start_xmit() -> macb_tx_map() prints once per
skb, and a dma_map failure tends to persist:

drivers/net/ethernet/cadence/macb_main.c:macb_tx_map() {
	...
	/* Should never happen */
	if (unlikely(!tx_skb)) {
		netdev_err(bp->netdev, "BUG! empty skb!\n");
		return 0;
	}
	...
dma_error:
	netdev_err(bp->netdev, "TX DMA map failed\n");
	...
}

The at91ether entry points in the same file are in the same situation.
at91ether_start_xmit() returns NETDEV_TX_BUSY from the busy branch, so
sch_direct_xmit() -> dev_requeue_skb() -> __netif_schedule() retries the
same skb and the message can repeat at softirq rate:

drivers/net/ethernet/cadence/macb_main.c:at91ether_start_xmit() {
	...
		if (dma_mapping_error(dev, bp->rm9200_txq[desc].mapping)) {
			dev_kfree_skb_any(skb);
			netdev->stats.tx_dropped++;
			netdev_err(netdev, "%s: DMA mapping error\n", __func__);
			return NETDEV_TX_OK;
		}
	...
	} else {
		netdev_err(netdev, "%s called, but device is busy!\n",
			   __func__);
		return NETDEV_TX_BUSY;
	}
}

And at91ether_interrupt() prints from hard IRQ on every receive overrun,
which is the same remotely triggered error storm the changelog describes:

drivers/net/ethernet/cadence/macb_main.c:at91ether_interrupt() {
	...
	if (intstatus & MACB_BIT(ISR_ROVR))
		netdev_err(netdev, "ROVR error\n");
	...
}

Would it make sense to convert these as well, or to narrow the changelog to
the sites that are actually covered?

[ ... ]

> @@ -2511,7 +2513,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb,
>  		else
>  			hdrlen = skb_tcp_all_headers(skb);
>  		if (skb_headlen(skb) < hdrlen) {
> -			netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
> +			if (net_ratelimit())
> +				netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
>  			/* if this is required, would need to copy to single buffer */
>  			return NETDEV_TX_BUSY;
>  		}

[Severity: Critical]
This isn't a bug introduced by this patch, but while looking at the
NETDEV_TX_BUSY returns in macb_start_xmit(): can the non-LSO ring-full
return use an skb that was already freed?

macb_start_xmit() calls macb_pad_and_fcs(&skb, netdev) early on, and that
helper can free the pointer the transmit core still holds:

drivers/net/ethernet/cadence/macb_main.c:macb_pad_and_fcs() {
	...
	if (cloned || tailroom < padlen) {
		nskb = skb_copy_expand(*skb, 0, padlen, GFP_ATOMIC);
		if (!nskb)
			return -ENOMEM;

		dev_consume_skb_any(*skb);
		*skb = nskb;
	}
	...
}

Execution then continues, and the ring-full check can return
NETDEV_TX_BUSY:

drivers/net/ethernet/cadence/macb_main.c:macb_start_xmit() {
	...
	if (CIRC_SPACE(queue->tx_head, queue->tx_tail,
		       bp->tx_ring_size) < desc_cnt) {
		netif_stop_subqueue(netdev, q);
		...
		ret = NETDEV_TX_BUSY;
		goto unlock;
	}
	...
}

NETDEV_TX_BUSY is not a completion status, so the core writes into the skb
it passed in and then requeues it:

net/core/dev.c:dev_hard_start_xmit() {
	...
		rc = xmit_one(skb, dev, txq, next != NULL);
		if (unlikely(!dev_xmit_complete(rc))) {
			skb->next = next;
			goto out;
		}
	...
}

sch_direct_xmit() then hands that pointer to dev_requeue_skb(), which
stores it in q->gso_skb for a later dequeue and transmit.

Both conditions look independently satisfiable: a short or tailroom-poor,
non-CHECKSUM_PARTIAL, cloned or nonlinear frame on a NETIF_F_HW_CSUM
interface gets replaced by macb_pad_and_fcs(), and CIRC_SPACE() can be
below desc_cnt at that moment under sustained transmit load.

The LSO branch shown in this hunk avoids the sequence only because
macb_pad_and_fcs() returns 0 early when skb_shinfo(*skb)->gso_size is set.
Is the non-LSO ring-full path meant to return NETDEV_TX_BUSY after the skb
may have been replaced?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260920095839.62852-1-taozj888%40163.com

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[Théo Lebrun]

Hello Zijin,

Also your To/Cc list is weird, make sure to use
scripts/get_maintainer.pl (or use b4 for patch
management which uses it automatically).

On Mon Sep 21, 2026 at 10:58 AM CEST, Théo Lebrun wrote:
> Hello Zijin,
>
> You sent the same patch 3 times?
> Message-ID: <20260920095839.62852-1-taozj888@163.com>
> Message-ID: <20260920100658.65087-1-taozj888@163.com>
> Message-ID: <20260920100920.65773-1-taozj888@163.com>
>
> Also you are missing the prefix [PATCH net] or [PATCH net-next].
> Read up about this here (and read the full page):
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> On Sun Sep 20, 2026 at 11:58 AM CEST, Zijin Tao wrote:
>> Now the MACB ethernet driver print the netdev error information
>> directly by netdev_err(), which would lead to a large number of
>> error information print if there was a significant number of
>> error or just jumbo packets received when booting.
>> For example, it would print a large number of:
>>
>> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
>> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
>> ...
>
> This whole patch is LLM generated? I don't think this "PHYT0036:00"
> device name could ever exist. Please don't hallucinate kernel logs.
>
> In both cases, I prefer the commit message to either indicate whether it
> is a theoretical bugfix or a bug hit in practice. I know I've faced
> such a log spam on MACB in the past, but I don't remember which log
> line it was.
>
>> in gem_rx() by received a large number of packets without
>> RX_SOF or RX_EOF flag set.
>>
>> The unlimited print here would greatly bother and delay
>> the system booting process unless the source stop sending
>> packets.
>
> I don't understand the relation to the booting process here.
>
>> So rate limit the netdev error information print in the receive
>> and transmit data path.
>>
>> Signed-off-by: Zijin Tao <taozj888@163.com>

Thanks Zijin,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] net: macb: rate limit netdev error info print in the data path

[Théo Lebrun]

Hello Zijin,

You sent the same patch 3 times?
Message-ID: <20260920095839.62852-1-taozj888@163.com>
Message-ID: <20260920100658.65087-1-taozj888@163.com>
Message-ID: <20260920100920.65773-1-taozj888@163.com>

Also you are missing the prefix [PATCH net] or [PATCH net-next].
Read up about this here (and read the full page):
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

On Sun Sep 20, 2026 at 11:58 AM CEST, Zijin Tao wrote:
> Now the MACB ethernet driver print the netdev error information
> directly by netdev_err(), which would lead to a large number of
> error information print if there was a significant number of
> error or just jumbo packets received when booting.
> For example, it would print a large number of:
>
> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
> macb PHYT0036:00 eth0: not whole frame pointed by descriptor
> ...

This whole patch is LLM generated? I don't think this "PHYT0036:00"
device name could ever exist. Please don't hallucinate kernel logs.

In both cases, I prefer the commit message to either indicate whether it
is a theoretical bugfix or a bug hit in practice. I know I've faced
such a log spam on MACB in the past, but I don't remember which log
line it was.

> in gem_rx() by received a large number of packets without
> RX_SOF or RX_EOF flag set.
>
> The unlimited print here would greatly bother and delay
> the system booting process unless the source stop sending
> packets.

I don't understand the relation to the booting process here.

> So rate limit the netdev error information print in the receive
> and transmit data path.
>
> Signed-off-by: Zijin Tao <taozj888@163.com>

Thanks Zijin,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH] net: macb: rate limit netdev error info print in the data path

[Zijin Tao]

Now the MACB ethernet driver print the netdev error information
directly by netdev_err(), which would lead to a large number of
error information print if there was a significant number of
error or just jumbo packets exceeding the MTU received when booting.
For example, it would print a large number of:

macb PHYT0036:00 eth0: not whole frame pointed by descriptor
macb PHYT0036:00 eth0: not whole frame pointed by descriptor
...

in gem_rx() by received a large number of packets without
RX_SOF or RX_EOF flag set, espcially with unknown packet
type.

The unlimited print here would greatly bother and delay
the system booting process unless the source stop sending
packets.

So rate limit the netdev error information print in the receive
and transmit data path.

Signed-off-by: Zijin Tao <taozj888@163.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..c32d48d03008 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1617,16 +1617,16 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		count++;
 
 		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
-			netdev_err(bp->netdev,
-				   "not whole frame pointed by descriptor\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "not whole frame pointed by descriptor\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
 		}
 		skb = queue->rx_skbuff[entry];
 		if (unlikely(!skb)) {
-			netdev_err(bp->netdev,
-				   "inconsistent Rx descriptor chain\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "inconsistent Rx descriptor chain\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
@@ -1829,7 +1829,8 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
 		unsigned long flags;
 		u32 ctrl;
 
-		netdev_err(bp->netdev, "RX queue corruption: reset it\n");
+		if (net_ratelimit())
+			netdev_err(bp->netdev, "RX queue corruption: reset it\n");
 
 		spin_lock_irqsave(&bp->lock, flags);
 
@@ -2102,7 +2103,8 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
 
 	if (status & MACB_BIT(HRESP)) {
 		queue_work(system_bh_wq, &bp->hresp_err_bh_work);
-		netdev_err(netdev, "DMA bus error: HRESP not OK\n");
+		if (net_ratelimit())
+			netdev_err(netdev, "DMA bus error: HRESP not OK\n");
 		macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
 	}
 
@@ -2511,7 +2513,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb,
 		else
 			hdrlen = skb_tcp_all_headers(skb);
 		if (skb_headlen(skb) < hdrlen) {
-			netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
 			/* if this is required, would need to copy to single buffer */
 			return NETDEV_TX_BUSY;
 		}
-- 
2.34.1

[PATCH] net: macb: rate limit netdev error info print in the data path

[Zijin Tao]

Now the MACB ethernet driver print the netdev error information
directly by netdev_err(), which would lead to a large number of
error information print if there was a significant number of
error or just jumbo packets received when booting.
For example, it would print a large number of:

macb PHYT0036:00 eth0: not whole frame pointed by descriptor
macb PHYT0036:00 eth0: not whole frame pointed by descriptor
...

in gem_rx() by received a large number of packets without
RX_SOF or RX_EOF flag set.

The unlimited print here would greatly bother and delay
the system booting process unless the source stop sending
packets.

So rate limit the netdev error information print in the receive
and transmit data path.

Signed-off-by: Zijin Tao <taozj888@163.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..c32d48d03008 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1617,16 +1617,16 @@ static int gem_rx(struct macb_queue *queue, struct napi_struct *napi,
 		count++;
 
 		if (!(ctrl & MACB_BIT(RX_SOF) && ctrl & MACB_BIT(RX_EOF))) {
-			netdev_err(bp->netdev,
-				   "not whole frame pointed by descriptor\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "not whole frame pointed by descriptor\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
 		}
 		skb = queue->rx_skbuff[entry];
 		if (unlikely(!skb)) {
-			netdev_err(bp->netdev,
-				   "inconsistent Rx descriptor chain\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "inconsistent Rx descriptor chain\n");
 			bp->netdev->stats.rx_dropped++;
 			queue->stats.rx_dropped++;
 			break;
@@ -1829,7 +1829,8 @@ static int macb_rx(struct macb_queue *queue, struct napi_struct *napi,
 		unsigned long flags;
 		u32 ctrl;
 
-		netdev_err(bp->netdev, "RX queue corruption: reset it\n");
+		if (net_ratelimit())
+			netdev_err(bp->netdev, "RX queue corruption: reset it\n");
 
 		spin_lock_irqsave(&bp->lock, flags);
 
@@ -2102,7 +2103,8 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
 
 	if (status & MACB_BIT(HRESP)) {
 		queue_work(system_bh_wq, &bp->hresp_err_bh_work);
-		netdev_err(netdev, "DMA bus error: HRESP not OK\n");
+		if (net_ratelimit())
+			netdev_err(netdev, "DMA bus error: HRESP not OK\n");
 		macb_queue_isr_clear(bp, queue, MACB_BIT(HRESP));
 	}
 
@@ -2511,7 +2513,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb,
 		else
 			hdrlen = skb_tcp_all_headers(skb);
 		if (skb_headlen(skb) < hdrlen) {
-			netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
+			if (net_ratelimit())
+				netdev_err(bp->netdev, "Error - LSO headers fragmented!!!\n");
 			/* if this is required, would need to copy to single buffer */
 			return NETDEV_TX_BUSY;
 		}
-- 
2.34.1