No subject


Wed Mar 25 13:00:02 AEDT 2026


will not be zero. Let me think how to improve it to avoid such false positi=
ve.

> the hardware register NETC_BPCAPR and returns 0, then num_port_bp would
> be
> 0 and this division would crash the kernel.
>=20
> The call chain is:
>=20
> netc_setup()
>   netc_switch_bpt_default_config()
>     priv->num_bp =3D netc_get_buffer_pool_num()
>     netc_ipv_to_buffer_pool_mapping()
>       num_port_bp =3D priv->num_bp / priv->info->num_ports;
>       q =3D NETC_IPV_NUM / num_port_bp;  <- division by zero
>=20
> Should netc_switch_bpt_default_config() validate that priv->num_bp is
> non-zero before proceeding?
>=20
> > +	int r =3D NETC_IPV_NUM % num_port_bp;
> > +	int num =3D q + r;
> > +
> > +	/* IPV-to-buffer-pool mapping per port:
>=20
> [ ... ]
>=20
> > +static int netc_switch_bpt_default_config(struct netc_switch *priv)
> > +{
> > +	priv->num_bp =3D netc_get_buffer_pool_num(priv);
> > +	priv->bpt_list =3D devm_kcalloc(priv->dev, priv->num_bp,
> > +				      sizeof(struct bpt_cfge_data),
> > +				      GFP_KERNEL);
> > +	if (!priv->bpt_list)
> > +		return -ENOMEM;
> > +
> > +	/* Initialize the maximum threshold of each buffer pool entry */
> > +	for (int i =3D 0; i < priv->num_bp; i++) {
> > +		struct bpt_cfge_data *cfge =3D &priv->bpt_list[i];
> > +		int err;
> > +
> > +		cfge->max_thresh =3D cpu_to_le16(NETC_BP_THRESH);
> > +		err =3D ntmp_bpt_update_entry(&priv->ntmp, i, cfge);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	netc_ipv_to_buffer_pool_mapping(priv);
> > +
> > +	return 0;
> > +}
> > +
>=20
> [ ... ]
>=20
> > @@ -1224,6 +1313,40 @@ static void netc_port_set_hd_flow_control(struct
> netc_port *np, bool en)
> >  			  en ? PM_CMD_CFG_HD_FCEN : 0);
> >  }
> >
> > +static void netc_port_set_tx_pause(struct netc_port *np, bool tx_pause=
)
> > +{
> > +	struct netc_switch *priv =3D np->switch_priv;
> > +	int port =3D np->dp->index;
> > +	int i, j, num_bp;
> > +
> > +	num_bp =3D priv->num_bp / priv->info->num_ports;
> > +	for (i =3D 0, j =3D port * num_bp; i < num_bp; i++, j++) {
> > +		struct bpt_cfge_data *cfge;
> > +
> > +		cfge =3D &priv->bpt_list[j];
> > +		if (tx_pause) {
> > +			cfge->fc_on_thresh =3D cpu_to_le16(NETC_FC_THRESH_ON);
> > +			cfge->fc_off_thresh =3D cpu_to_le16(NETC_FC_THRESH_OFF);
> > +			cfge->fccfg_sbpen =3D FIELD_PREP(BPT_FC_CFG,
> > +						       BPT_FC_CFG_EN_BPFC);
> > +			cfge->fc_ports =3D cpu_to_le32(BIT(port));
> > +		} else {
> > +			cfge->fc_on_thresh =3D cpu_to_le16(0);
> > +			cfge->fc_off_thresh =3D cpu_to_le16(0);
> > +			cfge->fccfg_sbpen =3D 0;
> > +			cfge->fc_ports =3D cpu_to_le32(0);
> > +		}
> > +
> > +		ntmp_bpt_update_entry(&priv->ntmp, j, cfge);
> >                 ^^^^
>=20
> Should the return value of ntmp_bpt_update_entry() be checked here?
>=20
> Simon Horman noted in v1 review that other NTMP table operations in the
> driver propagate errors from the update functions. While this was fixed
> in netc_switch_bpt_default_config() in v2, the same issue remains here.
>=20
> If the hardware update fails (for example, DMA allocation failure or
> hardware timeout), the driver's bpt_list will have flow control values th=
at
> do not match the actual hardware state, potentially causing incorrect flo=
w
> control behavior.

Okay, I will improve it.



More information about the Linuxppc-dev mailing list