From: Simon Horman <horms@kernel.org>
To: Himanshu Mittal <h-mittal1@ti.com>
Cc: pabeni@redhat.com, kuba@kernel.org, edumazet@google.com,
davem@davemloft.net, andrew+netdev@lunn.ch,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, srk@ti.com,
Vignesh Raghavendra <vigneshr@ti.com>,
Roger Quadros <rogerq@kernel.org>,
danishanwar@ti.com, m-malladi@ti.com, pratheesh@ti.com,
prajith@ti.com
Subject: Re: [PATCH net v3] net: ti: icssg-prueth: Fix buffer allocation for ICSSG
Date: Fri, 18 Jul 2025 15:27:39 +0100 [thread overview]
Message-ID: <20250718142739.GD2459@horms.kernel.org> (raw)
In-Reply-To: <20250717094220.546388-1-h-mittal1@ti.com>
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 <h-mittal1@ti.com>
Thanks for the updated patch description.
Reviewed-by: Simon Horman <horms@kernel.org>
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.
...
next prev parent reply other threads:[~2025-07-18 14:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-17 9:42 [PATCH net v3] net: ti: icssg-prueth: Fix buffer allocation for ICSSG Himanshu Mittal
2025-07-17 12:56 ` Markus Elfring
2025-07-18 14:32 ` Simon Horman
2025-07-18 18:30 ` [v3] " Markus Elfring
2025-07-18 14:27 ` Simon Horman [this message]
2025-07-19 0:30 ` [PATCH net v3] " patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250718142739.GD2459@horms.kernel.org \
--to=horms@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=danishanwar@ti.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=h-mittal1@ti.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m-malladi@ti.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=prajith@ti.com \
--cc=pratheesh@ti.com \
--cc=rogerq@kernel.org \
--cc=srk@ti.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).