RE: [PATCH v6 net-next 13/15] net: dsa: netc: initialize buffer pool table and implement flow-control

[Wei Fang <wei.fang@nxp.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 <wei.fang@nxp.com>
> 
> 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.