On Fri, Jun 14, 2024 at 04:19:16PM +0800, Xiaolei Wang wrote:
> commit be27b8965297 ("net: stmmac: replace priv->speed with
> the portTransmitRate from the tc-cbs parameters") introduced
> a problem. When deleting, it prompts "Invalid portTransmitRate
> 0 (idleSlope - sendSlope)" and exits. Add judgment on cbs.enable.
> Only when offload is enabled, speed divider needs to be calculated.
>
> Fixes: be27b8965297 ("net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters")
> Signed-off-by: Xiaolei Wang <[email protected]>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 38 ++++++++++---------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 1562fbdd0a04..b0fd2d6e525e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -358,24 +358,26 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>
> port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
>
> - /* Port Transmit Rate and Speed Divider */
> - switch (div_s64(port_transmit_rate_kbps, 1000)) {
> - case SPEED_10000:
> - case SPEED_5000:
> - ptr = 32;
> - break;
> - case SPEED_2500:
> - case SPEED_1000:
> - ptr = 8;
> - break;
> - case SPEED_100:
> - ptr = 4;
> - break;
> - default:
> - netdev_err(priv->dev,
> - "Invalid portTransmitRate %lld (idleSlope - sendSlope)\n",
> - port_transmit_rate_kbps);
> - return -EINVAL;
> + if (qopt->enable) {
> + /* Port Transmit Rate and Speed Divider */
> + switch (div_s64(port_transmit_rate_kbps, 1000)) {
> + case SPEED_10000:
> + case SPEED_5000:
> + ptr = 32;
> + break;
> + case SPEED_2500:
> + case SPEED_1000:
> + ptr = 8;
> + break;
> + case SPEED_100:
> + ptr = 4;
> + break;
> + default:
> + netdev_err(priv->dev,
> + "Invalid portTransmitRate %lld (idleSlope - sendSlope)\n",
> + port_transmit_rate_kbps);
> + return -EINVAL;
> + }
> }
> mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
Hi Xiaolei Wang,
The code following this function looks like this:
if (mode_to_use == MTL_QUEUE_DCB && qopt->enable) {
ret = stmmac_dma_qmode(priv, priv->ioaddr, queue, MTL_QUEUE_AVB);
if (ret)
return ret;
priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_AVB;
} else if (!qopt->enable) {
ret = stmmac_dma_qmode(priv, priv->ioaddr, queue,
MTL_QUEUE_DCB);
if (ret)
return ret;
priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
}
/* Final adjustments for HW */
value = div_s64(qopt->idleslope * 1024ll * ptr, port_transmit_rate_kbps);
priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
value = div_s64(-qopt->sendslope * 1024ll * ptr, port_transmit_rate_kbps);
priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
And the div_s64() lines above appear to use
ptr uninitialised in the !qopt->enable case.
Flagged by Smatch.
--
pw-bot: changes-requested