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

From: Simon Horman <[email-masked]>
Date: Fri, 18 Jul 2025 15:27:39 +0100

On Thu, Jul 17, 2025 at 03:12:20PM +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
> | PRE | 70030000   | 70033800 | 70034000   | 70037800 |
> +-----+------------+----------+------------+----------+
> 
> +---------------------+----------+----------+
> |                     | SLICE 0  |  SLICE 1 |
> +---------------------+----------+----------+
> | Default Drop Offset | 00000000 | 00000000 |     <-- Field not configured
> +---------------------+----------+----------+
> 
> -> Allocation this patch brings:
> +---------+-------------------------------------------------------------+
> |         |          SLICE 0             |          SLICE 1             |
> |         +------+--------------+--------+------+--------------+--------+
> |         | Slot | Base Address | Size   | Slot | Base Address | Size   |
> |---------+------+--------------+--------+------+--------------+--------+
> |         | 0    | 70000000     | 0x2000 | 0    | 70040000     | 0x2000 |
> |         | 1    | 70002000     | 0x2000 | 1    | 70042000     | 0x2000 |
> |         | 2    | 70004000     | 0x2000 | 2    | 70044000     | 0x2000 |
> | FWD     | 3    | 70006000     | 0x2000 | 3    | 70046000     | 0x2000 |
> | Buffers | 4    | 70008000     | 0x2000 | 4    | 70048000     | 0x2000 |
> |         | 5    | 7000A000     | 0x2000 | 5    | 7004A000     | 0x2000 |
> |         | 6    | 7000C000     | 0x2000 | 6    | 7004C000     | 0x2000 |
> |         | 7    | 7000E000     | 0x2000 | 7    | 7004E000     | 0x2000 |
> +---------+------+--------------+--------+------+--------------+--------+
> |         | 8    | 70010000     | 0x1000 | 8    | 70050000     | 0x1000 |
> |         | 9    | 70011000     | 0x1000 | 9    | 70051000     | 0x1000 |
> |         | 10   | 70012000     | 0x1000 | 10   | 70052000     | 0x1000 |
> | Our     | 11   | 70013000     | 0x1000 | 11   | 70053000     | 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   | 70014000     | 0x1000 | 16   | 70054000     | 0x1000 |
> |         | 17   | 70015000     | 0x1000 | 17   | 70055000     | 0x1000 |
> |         | 18   | 70016000     | 0x1000 | 18   | 70056000     | 0x1000 |
> | Their   | 19   | 70017000     | 0x1000 | 19   | 70057000     | 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    |
> +---------+------+--------------+--------+------+--------------+--------+
> 
> +-----+-----------------------------------------------+
> |     |       SLICE 0       |        SLICE 1          |
> |     +------------+----------+------------+----------+
> |     | Start addr | End addr | Start addr | End addr |
> +-----+------------+----------+------------+----------+
> | EXP | 70018000   | 7001C000 | 70058000   | 7005C000 |
> | PRE | 7001C000   | 7001F800 | 7005C000   | 7005F800 |
> +-----+------------+----------+------------+----------+
> 
> +---------------------+----------+----------+
> |                     | SLICE 0  |  SLICE 1 |
> +---------------------+----------+----------+
> | Default Drop Offset | 7001F800 | 7005F800 |
> +---------------------+----------+----------+
> 
> Rootcause: missing buffer configuration for Express frames in
> function: prueth_fw_offload_buffer_setup()
> 
> Details:
> 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, 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.
> 
> Fixes: abd5576b9c57 ("net: ti: icssg-prueth: Add support for ICSSG switch firmware")
> Signed-off-by: Himanshu Mittal <[email-masked]>

Thanks for the updated patch description.

Reviewed-by: Simon Horman <[email-masked]>

FTR, I did spend some time looking over the mappings described
above and correlating them with both the code and the "Details" above.
I agree with the analysis above and that the patchset addresses
the problem as described.

...