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@xxxxxxx wrote:
>> From: taozijin <taozj888@xxxxxxx>
>> 
>> 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@xxxxxxxxxxxxxxx
>> 
>
> Drop the blank line as otherwise tooling might get confused and doesn't
> get all tags correctly.
>
>> Signed-off-by: Zijin Tao <taozj888@xxxxxxx>
>> ---
>>  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