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

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

[taozj888]

From: taozijin <taozj888@163.com>

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_EOF flag set, especially with unknown packet
type that would penetrate the hardware offload for the IP packets.

The unlimited prints here would greatly bother and delay
the system booting process unless the source stop sending
packets since they occupy the console output bandwidth and
other processes have to wait for the completion of printing those
messages.

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

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

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 8e5c034dc3a4..ca60960bec36 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));
 	}
 
-- 
2.34.1

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

[Nicolai Buchwitz]

Hi Zijin

On 30.9.2026 13:35, taozj888@163.com wrote:
> From: taozijin <taozj888@163.com>
> 
> 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
> ...

Out of interest: Is this a Phytium vendor kernel?

> 
> in gem_rx() by received a large number of packets without
> RX_EOF flag set, especially with unknown packet
> type that would penetrate the hardware offload for the IP packets.
> 
> The unlimited prints here would greatly bother and delay
> the system booting process unless the source stop sending
> packets since they occupy the console output bandwidth and
> other processes have to wait for the completion of printing those
> messages.
> 
> So rate limit the netdev error information print in the receive
> and transmit data path.
> 
> Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")

IMHO the correct tag is 4df95131ea80 ("net/macb: change RX path for 
GEM")?
At least the gem_rx() messages were introduced here.

> Cc: stable@vger.kernel.org
> 

Drop the blank line as otherwise tooling might get confused and doesn't
get all tags correctly.

> Signed-off-by: Zijin Tao <taozj888@163.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 8e5c034dc3a4..ca60960bec36 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");

This will just hide the error message, but the split/drop is still 
present.
How about limiting JML in macb_init_hw() properly?

         if ((bp->caps & MACB_CAPS_JUMBO) && bp->jumbo_max_len) {
                 u32 jml = bp->rx_buffer_size - NET_IP_ALIGN + 
ETH_FCS_LEN;
                 gem_writel(bp, JML, min(jml, bp->jumbo_max_len));
         }

The code above is untested, so probably needs further tweaking. An 
alternative could
be to handle the split frames in gem_rx() correctly.

> [...]

Thanks,
Nicolai

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

[Théo Lebrun]

Hello Nicolai,

On Fri Oct 2, 2026 at 12:10 PM CEST, Nicolai Buchwitz wrote:
> On 30.9.2026 13:35, taozj888@163.com wrote:
>> From: taozijin <taozj888@163.com>
>> 
>> 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
>> ...
>
> Out of interest: Is this a Phytium vendor kernel?

I looked in the past so I'll answer: yes!

Internet search gives one worthy result: a 2025 series to DPDK adding
MACB support. Some extracts about the exact string match and also how
they support ACPI or OF and no other platform apparently.

#define OF_PHYTIUM_GEM1P0_MAC	"cdns,phytium-gem-1.0"	/* Phytium 1.0 MAC */
#define OF_PHYTIUM_GEM2P0_MAC	"cdns,phytium-gem-2.0"	/* Phytium 2.0 MAC */
#define ACPI_PHYTIUM_GEM1P0_MAC	"PHYT0036"	/* Phytium 1.0  MAC */

static int macb_get_dev_type(struct rte_eth_dev *dev)
{
	// ...

	if (!strcmp(dev_type, OF_PHYTIUM_GEM1P0_MAC) ||
	    !strcmp(dev_type, ACPI_PHYTIUM_GEM1P0_MAC)) {
		priv->dev_type = DEV_TYPE_PHYTIUM_GEM1P0_MAC;
	} else if (!strcmp(dev_type, OF_PHYTIUM_GEM2P0_MAC)) {
		priv->dev_type = DEV_TYPE_PHYTIUM_GEM2P0_MAC;
	} else {
		MACB_LOG(ERR, "Unsupported device type: %s.", dev_type);
		ret = -EINVAL;
	}

	// ...
}

@Zijin: do you have any other Linux MACB patches for it to work?

>> in gem_rx() by received a large number of packets without
>> RX_EOF flag set, especially with unknown packet
>> type that would penetrate the hardware offload for the IP packets.
>> 
>> The unlimited prints here would greatly bother and delay
>> the system booting process unless the source stop sending
>> packets since they occupy the console output bandwidth and
>> other processes have to wait for the completion of printing those
>> messages.
>> 
>> So rate limit the netdev error information print in the receive
>> and transmit data path.
>> 
>> Fixes: 89e5785fc8a6 ("[PATCH] Atmel MACB ethernet driver")
>
> IMHO the correct tag is 4df95131ea80 ("net/macb: change RX path for 
> GEM")?
> At least the gem_rx() messages were introduced here.

The patch used to touch macb_start_xmit(), which explains why I told
Zijin to target the initial commit on previous revision.

@Zijin: where is the V2 changelog? Each new revision must list (below
the fold line for single patches) their exhaustive list of changes
compared to the previous revision.

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#respond-to-review-comments
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

>> Cc: stable@vger.kernel.org
>> 
>
> Drop the blank line as otherwise tooling might get confused and doesn't
> get all tags correctly.
>
>> Signed-off-by: Zijin Tao <taozj888@163.com>
>> ---
>>  drivers/net/ethernet/cadence/macb_main.c | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index 8e5c034dc3a4..ca60960bec36 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");
>
> This will just hide the error message, but the split/drop is still 
> present.
> How about limiting JML in macb_init_hw() properly?
>
>          if ((bp->caps & MACB_CAPS_JUMBO) && bp->jumbo_max_len) {
>                  u32 jml = bp->rx_buffer_size - NET_IP_ALIGN + 
> ETH_FCS_LEN;
>                  gem_writel(bp, JML, min(jml, bp->jumbo_max_len));
>          }
>
> The code above is untested, so probably needs further tweaking. An 
> alternative could
> be to handle the split frames in gem_rx() correctly.

I agree with you but to clarify for Zijin: if you do this then it should
be a separate patch as changes are pretty unrelated.

Thanks,

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