Re: [PATCH net v2] net: ti: icssg-prueth: Fix buffer allocation for ICSSG

["MITTAL, HIMANSHU" <h-mittal1@ti.com>]

On 7/11/2025 8:13 PM, Simon Horman wrote:
> On Thu, Jul 10, 2025 at 06:42:50PM +0530, Himanshu Mittal wrote:
>> Fixes overlapping buffer allocation for ICSSG peripheral
>> used for storing packets to be received/transmitted.
>> There are 3 buffers:
>> 1. Buffer for Locally Injected Packets
>> 2. Buffer for Forwarding Packets
>> 3. Buffer for Host Egress Packets
>>
>> In existing allocation buffers for 2. and 3. are overlapping causing packet
>> corruption.
>>
>> Packet corruption observations:
>> During tcp iperf testing, due to overlapping buffers the received ack
>> packet overwrites the packet to be transmitted. So, we see packets on wire
>> with the ack packet content inside the content of next TCP packet from
>> sender device.
>>
>> Details for AM64x switch mode:
>> -> Allocation by existing driver:
>> +---------+-------------------------------------------------------------+
>> |         |          SLICE 0             |          SLICE 1             |
>> |         +------+--------------+--------+------+--------------+--------+
>> |         | Slot | Base Address | Size   | Slot | Base Address | Size   |
>> |---------+------+--------------+--------+------+--------------+--------+
>> |         | 0    | 70000000     | 0x2000 | 0    | 70010000     | 0x2000 |
>> |         | 1    | 70002000     | 0x2000 | 1    | 70012000     | 0x2000 |
>> |         | 2    | 70004000     | 0x2000 | 2    | 70014000     | 0x2000 |
>> | FWD     | 3    | 70006000     | 0x2000 | 3    | 70016000     | 0x2000 |
>> | Buffers | 4    | 70008000     | 0x2000 | 4    | 70018000     | 0x2000 |
>> |         | 5    | 7000A000     | 0x2000 | 5    | 7001A000     | 0x2000 |
>> |         | 6    | 7000C000     | 0x2000 | 6    | 7001C000     | 0x2000 |
>> |         | 7    | 7000E000     | 0x2000 | 7    | 7001E000     | 0x2000 |
>> +---------+------+--------------+--------+------+--------------+--------+
>> |         | 8    | 70020000     | 0x1000 | 8    | 70028000     | 0x1000 |
>> |         | 9    | 70021000     | 0x1000 | 9    | 70029000     | 0x1000 |
>> |         | 10   | 70022000     | 0x1000 | 10   | 7002A000     | 0x1000 |
>> | Our     | 11   | 70023000     | 0x1000 | 11   | 7002B000     | 0x1000 |
>> | LI      | 12   | 00000000     | 0x0    | 12   | 00000000     | 0x0    |
>> | Buffers | 13   | 00000000     | 0x0    | 13   | 00000000     | 0x0    |
>> |         | 14   | 00000000     | 0x0    | 14   | 00000000     | 0x0    |
>> |         | 15   | 00000000     | 0x0    | 15   | 00000000     | 0x0    |
>> +---------+------+--------------+--------+------+--------------+--------+
>> |         | 16   | 70024000     | 0x1000 | 16   | 7002C000     | 0x1000 |
>> |         | 17   | 70025000     | 0x1000 | 17   | 7002D000     | 0x1000 |
>> |         | 18   | 70026000     | 0x1000 | 18   | 7002E000     | 0x1000 |
>> | Their   | 19   | 70027000     | 0x1000 | 19   | 7002F000     | 0x1000 |
>> | LI      | 20   | 00000000     | 0x0    | 20   | 00000000     | 0x0    |
>> | Buffers | 21   | 00000000     | 0x0    | 21   | 00000000     | 0x0    |
>> |         | 22   | 00000000     | 0x0    | 22   | 00000000     | 0x0    |
>> |         | 23   | 00000000     | 0x0    | 23   | 00000000     | 0x0    |
>> +---------+------+--------------+--------+------+--------------+--------+
>> --> here 16, 17, 18, 19 overlapping with below express buffer
>>
>> +-----+-----------------------------------------------+
>> |     |       SLICE 0       |        SLICE 1          |
>> |     +------------+----------+------------+----------+
>> |     | Start addr | End addr | Start addr | End addr |
>> +-----+------------+----------+------------+----------+
>> | EXP | 70024000   | 70028000 | 7002C000   | 70030000 | <-- Overlapping
> Thanks for the detailed explanation with these tables.
> It is very helpful. I follow both the existing and new mappings
> with their help. Except for one thing.
>
> It's not clear how EXP was set to the values on the line above.
> Probably I'm missing something very obvious.
> Could you help me out here?

The root cause for this issue is that, buffer configuration for Express 
Frames
in function: prueth_fw_offload_buffer_setup() is missing.


Details:
The driver implements two distinct buffer configuration functions that 
are invoked
based on the driver state and ICSSG firmware:- 
prueth_fw_offload_buffer_setup()
- prueth_emac_buffer_setup()

During initialization, the driver creates standard network interfaces 
(netdevs) and
configures buffers via prueth_emac_buffer_setup(). This function 
properly allocates
and configures all required memory regions including:
- LI buffers
- Express packet buffers
- Preemptible packet buffers

However, when the driver transitions to an offload mode (switch/HSR/PRP),
buffer reconfiguration is handled by prueth_fw_offload_buffer_setup().
This function does not reconfigure the buffer regions required for 
Express packets,
leading to incorrect buffer allocation.
>> | PRE | 70030000   | 70033800 | 70034000   | 70037800 |
>> +-----+------------+----------+------------+----------+
>>
>> +---------------------+----------+----------+
>> |                     | SLICE 0  |  SLICE 1 |
>> +---------------------+----------+----------+
>> | Default Drop Offset | 00000000 | 00000000 |     <-- Field not configured
>> +---------------------+----------+----------+
> ...

Thanks,
Himanshu