RE: [PATCH v6 net-next 13/15] net: dsa: netc: initialize buffer pool table and implement flow-control
Wei Fang <[email protected]> Mon, 11 May 2026 03:16:14 +0000
| Newsgroups | dev.linux.lists.sashiko,dev.linux.lists.imx,org.kernel.vger.linux-devicetree,org.kernel.vger.netdev |
|---|---|
| Message-ID | <DBBPR04MB75003B8627FD78829C98433688382@DBBPR04MB7500.eurprd04.prod.outlook.com> |
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to > consider: > - [Medium] The IPV-to-buffer-pool mapping algorithm fails when a port is > allocated more than 8 buffer pools, silently routing all traffic to a single pool. > -- > > commit 0b5e4aba36a49ce1de9eb84ec706f482331550cf > Author: Wei Fang <[email protected]> > > net: dsa: netc: initialize buffer pool table and implement flow-control > > This commit initializes the buffer pool table and implements flow control > for the NXP netc DSA driver. It adds logic to dynamically map the 8 internal > priority values (IPVs) to the available buffer pools per port based on > hardware capabilities. > > > diff --git a/drivers/net/dsa/netc/netc_main.c > b/drivers/net/dsa/netc/netc_main.c > > --- a/drivers/net/dsa/netc/netc_main.c > > +++ b/drivers/net/dsa/netc/netc_main.c > > [ ... ] > > > +static void netc_ipv_to_buffer_pool_mapping(struct netc_switch *priv) > > +{ > > + int bp_per_port = priv->num_bp / priv->info->num_ports; > > + int q = NETC_IPV_NUM / bp_per_port; > > + int r = NETC_IPV_NUM % bp_per_port; > > + int num = q + r; > > + > > + /* IPV-to-buffer-pool mapping per port: > > + * Each port is allocated 'bp_per_port' buffer pools and supports 8 > > + * IPVs, where a higher IPV indicates a higher frame priority. Each > > + * IPV can be mapped to only one buffer pool. > > + * > > + * The mapping rule is as follows: > > + * - The first 'num' IPVs share the port's first buffer pool (index > > + * 'base_id'). > > + * - After that, every 'q' IPVs share one buffer pool, with pool > > + * indices increasing sequentially. > > + */ > > + for (int i = 0; i < priv->info->num_ports; i++) { > > + u32 base_id = i * bp_per_port; > > + u32 bp_id = base_id; > > + u64 mapping = 0; > > + > > + for (int ipv = 0; ipv < NETC_IPV_NUM; ipv++) { > > + /* Update the buffer pool index */ > > + if (ipv >= num) > > + bp_id = base_id + ((ipv - num) / q) + 1; > > What happens if a future hardware revision allocates more than 8 buffer pools > per port? > > If bp_per_port is greater than NETC_IPV_NUM (8), q evaluates to 0 and num > evaluates to 8. > > Because ipv loops from 0 to 7, the condition ipv >= num will never be true. > As a result, all 8 traffic priorities would be mapped to a single buffer pool > (base_id), leaving the remaining pools unused instead of maximizing > utilization. > > Should bp_per_port be clamped to NETC_IPV_NUM before calculating q to > prevent this? From hardware perspective, each port has 8 IPVs, each IPV can only be mapped to one buffer pool, it is impossible that the hardware provides more than 8 buffer pools per port. This is a false positive, please ignore it.