* [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 6:14 ` Xiaolei Wang
0 siblings, 0 replies; 23+ messages in thread
From: Xiaolei Wang @ 2024-05-30 6:14 UTC (permalink / raw)
To: linux, alexandre.torgue, joabreu, andrew, davem, edumazet, kuba,
pabeni, mcoquelin.stm32
Cc: netdev, linux-arm-kernel, linux-kernel
When the port is relinked, if the speed changes, the CBS parameters
should be updated, so saving the user transmission parameters so
that idle_slope and send_slope can be recalculated after the speed
changes after linking up can help reconfigure CBS after the speed
changes.
Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
v1 -> v2
- Update CBS parameters when speed changes
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 45 ++++++++++++++++++-
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 6 +++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b23b920eedb1..7a386b43f117 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -356,6 +356,10 @@ struct stmmac_priv {
unsigned int rfs_entries_total;
struct stmmac_rfs_entry *rfs_entries;
+ /* Save CBS configuration to adjust parameters when port link up speed changes */
+ s32 old_idleslope[MTL_MAX_TX_QUEUES];
+ s32 old_sendslope[MTL_MAX_TX_QUEUES];
+
/* Pulse Per Second output */
struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b3afc7cb7d72..44db35a7ca6a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -138,6 +138,7 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);
static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
u32 rxmode, u32 chan);
+static void stmmac_configure_cbs(struct stmmac_priv *priv);
#ifdef CONFIG_DEBUG_FS
static const struct net_device_ops stmmac_netdev_ops;
@@ -1075,7 +1076,11 @@ static void stmmac_mac_link_up(struct phylink_config *config,
}
}
- priv->speed = speed;
+ /* Update speed and CBS parameters when speed changes */
+ if (speed != priv->speed) {
+ priv->speed = speed;
+ stmmac_configure_cbs(priv);
+ }
if (priv->plat->fix_mac_speed)
priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
@@ -1115,6 +1120,7 @@ static void stmmac_mac_link_up(struct phylink_config *config,
if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
stmmac_hwtstamp_correct_latency(priv, priv);
+
}
static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
@@ -3182,13 +3188,42 @@ static void stmmac_set_tx_queue_weight(struct stmmac_priv *priv)
/**
* stmmac_configure_cbs - Configure CBS in TX queue
* @priv: driver private structure
- * Description: It is used for configuring CBS in AVB TX queues
+ * Description: It is used for configuring CBS in AVB TX queues,
+ * and when the speed changes, update CBS parameters to reconfigure
*/
static void stmmac_configure_cbs(struct stmmac_priv *priv)
{
u32 tx_queues_count = priv->plat->tx_queues_to_use;
u32 mode_to_use;
u32 queue;
+ u32 ptr, speed_div;
+ u64 value;
+
+ /* Port Transmit Rate and Speed Divider */
+ switch (priv->speed) {
+ case SPEED_10000:
+ ptr = 32;
+ speed_div = 10000000;
+ break;
+ case SPEED_5000:
+ ptr = 32;
+ speed_div = 5000000;
+ break;
+ case SPEED_2500:
+ ptr = 8;
+ speed_div = 2500000;
+ break;
+ case SPEED_1000:
+ ptr = 8;
+ speed_div = 1000000;
+ break;
+ case SPEED_100:
+ ptr = 4;
+ speed_div = 100000;
+ break;
+ default:
+ netdev_dbg(priv->dev, "link speed is not known\n");
+ }
/* queue 0 is reserved for legacy traffic */
for (queue = 1; queue < tx_queues_count; queue++) {
@@ -3196,6 +3231,12 @@ static void stmmac_configure_cbs(struct stmmac_priv *priv)
if (mode_to_use == MTL_QUEUE_DCB)
continue;
+ value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
+ priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
+
+ value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
+ priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
+
stmmac_config_cbs(priv, priv->hw,
priv->plat->tx_queues_cfg[queue].send_slope,
priv->plat->tx_queues_cfg[queue].idle_slope,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 222540b55480..d3526ad91aff 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -355,6 +355,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
if (!priv->dma_cap.av)
return -EOPNOTSUPP;
+ if (!netif_carrier_ok(priv->dev))
+ return -ENETDOWN;
+
/* Port Transmit Rate and Speed Divider */
switch (priv->speed) {
case SPEED_10000:
@@ -397,6 +400,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
}
+ priv->old_idleslope[queue] = qopt->idleslope;
+ priv->old_sendslope[queue] = qopt->sendslope;
+
/* Final adjustments for HW */
value = div_s64(qopt->idleslope * 1024ll * ptr, speed_div);
priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 23+ messages in thread* [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 6:14 ` Xiaolei Wang
0 siblings, 0 replies; 23+ messages in thread
From: Xiaolei Wang @ 2024-05-30 6:14 UTC (permalink / raw)
To: linux, alexandre.torgue, joabreu, andrew, davem, edumazet, kuba,
pabeni, mcoquelin.stm32
Cc: netdev, linux-arm-kernel, linux-kernel
When the port is relinked, if the speed changes, the CBS parameters
should be updated, so saving the user transmission parameters so
that idle_slope and send_slope can be recalculated after the speed
changes after linking up can help reconfigure CBS after the speed
changes.
Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
v1 -> v2
- Update CBS parameters when speed changes
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 45 ++++++++++++++++++-
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 6 +++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b23b920eedb1..7a386b43f117 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -356,6 +356,10 @@ struct stmmac_priv {
unsigned int rfs_entries_total;
struct stmmac_rfs_entry *rfs_entries;
+ /* Save CBS configuration to adjust parameters when port link up speed changes */
+ s32 old_idleslope[MTL_MAX_TX_QUEUES];
+ s32 old_sendslope[MTL_MAX_TX_QUEUES];
+
/* Pulse Per Second output */
struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b3afc7cb7d72..44db35a7ca6a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -138,6 +138,7 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);
static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
u32 rxmode, u32 chan);
+static void stmmac_configure_cbs(struct stmmac_priv *priv);
#ifdef CONFIG_DEBUG_FS
static const struct net_device_ops stmmac_netdev_ops;
@@ -1075,7 +1076,11 @@ static void stmmac_mac_link_up(struct phylink_config *config,
}
}
- priv->speed = speed;
+ /* Update speed and CBS parameters when speed changes */
+ if (speed != priv->speed) {
+ priv->speed = speed;
+ stmmac_configure_cbs(priv);
+ }
if (priv->plat->fix_mac_speed)
priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
@@ -1115,6 +1120,7 @@ static void stmmac_mac_link_up(struct phylink_config *config,
if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY)
stmmac_hwtstamp_correct_latency(priv, priv);
+
}
static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
@@ -3182,13 +3188,42 @@ static void stmmac_set_tx_queue_weight(struct stmmac_priv *priv)
/**
* stmmac_configure_cbs - Configure CBS in TX queue
* @priv: driver private structure
- * Description: It is used for configuring CBS in AVB TX queues
+ * Description: It is used for configuring CBS in AVB TX queues,
+ * and when the speed changes, update CBS parameters to reconfigure
*/
static void stmmac_configure_cbs(struct stmmac_priv *priv)
{
u32 tx_queues_count = priv->plat->tx_queues_to_use;
u32 mode_to_use;
u32 queue;
+ u32 ptr, speed_div;
+ u64 value;
+
+ /* Port Transmit Rate and Speed Divider */
+ switch (priv->speed) {
+ case SPEED_10000:
+ ptr = 32;
+ speed_div = 10000000;
+ break;
+ case SPEED_5000:
+ ptr = 32;
+ speed_div = 5000000;
+ break;
+ case SPEED_2500:
+ ptr = 8;
+ speed_div = 2500000;
+ break;
+ case SPEED_1000:
+ ptr = 8;
+ speed_div = 1000000;
+ break;
+ case SPEED_100:
+ ptr = 4;
+ speed_div = 100000;
+ break;
+ default:
+ netdev_dbg(priv->dev, "link speed is not known\n");
+ }
/* queue 0 is reserved for legacy traffic */
for (queue = 1; queue < tx_queues_count; queue++) {
@@ -3196,6 +3231,12 @@ static void stmmac_configure_cbs(struct stmmac_priv *priv)
if (mode_to_use == MTL_QUEUE_DCB)
continue;
+ value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
+ priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
+
+ value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
+ priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
+
stmmac_config_cbs(priv, priv->hw,
priv->plat->tx_queues_cfg[queue].send_slope,
priv->plat->tx_queues_cfg[queue].idle_slope,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 222540b55480..d3526ad91aff 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -355,6 +355,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
if (!priv->dma_cap.av)
return -EOPNOTSUPP;
+ if (!netif_carrier_ok(priv->dev))
+ return -ENETDOWN;
+
/* Port Transmit Rate and Speed Divider */
switch (priv->speed) {
case SPEED_10000:
@@ -397,6 +400,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
priv->plat->tx_queues_cfg[queue].mode_to_use = MTL_QUEUE_DCB;
}
+ priv->old_idleslope[queue] = qopt->idleslope;
+ priv->old_sendslope[queue] = qopt->sendslope;
+
/* Final adjustments for HW */
value = div_s64(qopt->idleslope * 1024ll * ptr, speed_div);
priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 6:14 ` Xiaolei Wang
@ 2024-05-30 12:50 ` Andrew Lunn
-1 siblings, 0 replies; 23+ messages in thread
From: Andrew Lunn @ 2024-05-30 12:50 UTC (permalink / raw)
To: Xiaolei Wang
Cc: linux, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
> static void stmmac_configure_cbs(struct stmmac_priv *priv)
> {
> u32 tx_queues_count = priv->plat->tx_queues_to_use;
> u32 mode_to_use;
> u32 queue;
> + u32 ptr, speed_div;
> + u64 value;
> +
> + /* Port Transmit Rate and Speed Divider */
> + switch (priv->speed) {
> + case SPEED_10000:
> + ptr = 32;
> + speed_div = 10000000;
> + break;
> + case SPEED_5000:
> + ptr = 32;
> + speed_div = 5000000;
> + break;
> + case SPEED_2500:
> + ptr = 8;
> + speed_div = 2500000;
> + break;
> + case SPEED_1000:
> + ptr = 8;
> + speed_div = 1000000;
> + break;
> + case SPEED_100:
> + ptr = 4;
> + speed_div = 100000;
> + break;
> + default:
No SPEED_10 ?
> + netdev_dbg(priv->dev, "link speed is not known\n");
> + }
>
> /* queue 0 is reserved for legacy traffic */
> for (queue = 1; queue < tx_queues_count; queue++) {
> @@ -3196,6 +3231,12 @@ static void stmmac_configure_cbs(struct stmmac_priv *priv)
> if (mode_to_use == MTL_QUEUE_DCB)
> continue;
>
> + value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
> + priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
Rather than masking off the top bits, shouldn't you be looking for
overflow? that indicates the configuration is not possible. You don't
have a good way to report the problem, since there is no user action
on link up, so you cannot return -EINVAL or -EOPNOTSUPP. So you
probably want to set the hardware as close as possible.
Also, what happens if the result of the div is 0? Does 0 have a
special meaning?
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 222540b55480..d3526ad91aff 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -355,6 +355,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
> if (!priv->dma_cap.av)
> return -EOPNOTSUPP;
>
> + if (!netif_carrier_ok(priv->dev))
> + return -ENETDOWN;
> +
Now that you are configuring the hardware on link up, does that
matter?
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 12:50 ` Andrew Lunn
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Lunn @ 2024-05-30 12:50 UTC (permalink / raw)
To: Xiaolei Wang
Cc: linux, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
> static void stmmac_configure_cbs(struct stmmac_priv *priv)
> {
> u32 tx_queues_count = priv->plat->tx_queues_to_use;
> u32 mode_to_use;
> u32 queue;
> + u32 ptr, speed_div;
> + u64 value;
> +
> + /* Port Transmit Rate and Speed Divider */
> + switch (priv->speed) {
> + case SPEED_10000:
> + ptr = 32;
> + speed_div = 10000000;
> + break;
> + case SPEED_5000:
> + ptr = 32;
> + speed_div = 5000000;
> + break;
> + case SPEED_2500:
> + ptr = 8;
> + speed_div = 2500000;
> + break;
> + case SPEED_1000:
> + ptr = 8;
> + speed_div = 1000000;
> + break;
> + case SPEED_100:
> + ptr = 4;
> + speed_div = 100000;
> + break;
> + default:
No SPEED_10 ?
> + netdev_dbg(priv->dev, "link speed is not known\n");
> + }
>
> /* queue 0 is reserved for legacy traffic */
> for (queue = 1; queue < tx_queues_count; queue++) {
> @@ -3196,6 +3231,12 @@ static void stmmac_configure_cbs(struct stmmac_priv *priv)
> if (mode_to_use == MTL_QUEUE_DCB)
> continue;
>
> + value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
> + priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
Rather than masking off the top bits, shouldn't you be looking for
overflow? that indicates the configuration is not possible. You don't
have a good way to report the problem, since there is no user action
on link up, so you cannot return -EINVAL or -EOPNOTSUPP. So you
probably want to set the hardware as close as possible.
Also, what happens if the result of the div is 0? Does 0 have a
special meaning?
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index 222540b55480..d3526ad91aff 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -355,6 +355,9 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
> if (!priv->dma_cap.av)
> return -EOPNOTSUPP;
>
> + if (!netif_carrier_ok(priv->dev))
> + return -ENETDOWN;
> +
Now that you are configuring the hardware on link up, does that
matter?
Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 12:50 ` Andrew Lunn
@ 2024-05-30 13:28 ` Vladimir Oltean
-1 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 13:28 UTC (permalink / raw)
To: Xiaolei Wang, Andrew Lunn
Cc: linux, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> When the port is relinked, if the speed changes, the CBS parameters
> should be updated, so saving the user transmission parameters so
> that idle_slope and send_slope can be recalculated after the speed
> changes after linking up can help reconfigure CBS after the speed
> changes.
>
> Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
> v1 -> v2
> - Update CBS parameters when speed changes
May I ask what is the point of this patch? The bandwidth fraction, as
IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
the latter of which is a runtime variant. If the link speed changes at
runtime, which is entirely possible, I see no alternative than to let
user space figure out that this happened, and decide what to do. This is
a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
as direct input, rather than something more high level like the desired
bandwidth for the stream itself, which could be dynamically computed by
the kernel.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 13:28 ` Vladimir Oltean
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 13:28 UTC (permalink / raw)
To: Xiaolei Wang, Andrew Lunn
Cc: linux, alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> When the port is relinked, if the speed changes, the CBS parameters
> should be updated, so saving the user transmission parameters so
> that idle_slope and send_slope can be recalculated after the speed
> changes after linking up can help reconfigure CBS after the speed
> changes.
>
> Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
> v1 -> v2
> - Update CBS parameters when speed changes
May I ask what is the point of this patch? The bandwidth fraction, as
IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
the latter of which is a runtime variant. If the link speed changes at
runtime, which is entirely possible, I see no alternative than to let
user space figure out that this happened, and decide what to do. This is
a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
as direct input, rather than something more high level like the desired
bandwidth for the stream itself, which could be dynamically computed by
the kernel.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 13:28 ` Vladimir Oltean
@ 2024-05-30 13:40 ` Russell King (Oracle)
-1 siblings, 0 replies; 23+ messages in thread
From: Russell King (Oracle) @ 2024-05-30 13:40 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > When the port is relinked, if the speed changes, the CBS parameters
> > should be updated, so saving the user transmission parameters so
> > that idle_slope and send_slope can be recalculated after the speed
> > changes after linking up can help reconfigure CBS after the speed
> > changes.
> >
> > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > ---
> > v1 -> v2
> > - Update CBS parameters when speed changes
>
> May I ask what is the point of this patch? The bandwidth fraction, as
> IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> the latter of which is a runtime variant. If the link speed changes at
> runtime, which is entirely possible, I see no alternative than to let
> user space figure out that this happened, and decide what to do. This is
> a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> as direct input, rather than something more high level like the desired
> bandwidth for the stream itself, which could be dynamically computed by
> the kernel.
So what should be the behaviour here? Refuse setting CBS parameters if
the link is down, and clear the hardware configuration of the CBS
parameters each and every time there is a link-down event? Isn't that
going to make the driver's in-use settings inconsistent with what the
kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
kernel just because the link went down.
I think what you're proposing leads to the hardware being effectively
"de-programmed" for CBS while "tc qdisc show" will probably report
that CBS is active on the interface - which clearly would be absurd.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 13:40 ` Russell King (Oracle)
0 siblings, 0 replies; 23+ messages in thread
From: Russell King (Oracle) @ 2024-05-30 13:40 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > When the port is relinked, if the speed changes, the CBS parameters
> > should be updated, so saving the user transmission parameters so
> > that idle_slope and send_slope can be recalculated after the speed
> > changes after linking up can help reconfigure CBS after the speed
> > changes.
> >
> > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > ---
> > v1 -> v2
> > - Update CBS parameters when speed changes
>
> May I ask what is the point of this patch? The bandwidth fraction, as
> IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> the latter of which is a runtime variant. If the link speed changes at
> runtime, which is entirely possible, I see no alternative than to let
> user space figure out that this happened, and decide what to do. This is
> a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> as direct input, rather than something more high level like the desired
> bandwidth for the stream itself, which could be dynamically computed by
> the kernel.
So what should be the behaviour here? Refuse setting CBS parameters if
the link is down, and clear the hardware configuration of the CBS
parameters each and every time there is a link-down event? Isn't that
going to make the driver's in-use settings inconsistent with what the
kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
kernel just because the link went down.
I think what you're proposing leads to the hardware being effectively
"de-programmed" for CBS while "tc qdisc show" will probably report
that CBS is active on the interface - which clearly would be absurd.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 13:40 ` Russell King (Oracle)
@ 2024-05-30 13:53 ` Vladimir Oltean
-1 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 13:53 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 02:40:30PM +0100, Russell King (Oracle) wrote:
> On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> > On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > > When the port is relinked, if the speed changes, the CBS parameters
> > > should be updated, so saving the user transmission parameters so
> > > that idle_slope and send_slope can be recalculated after the speed
> > > changes after linking up can help reconfigure CBS after the speed
> > > changes.
> > >
> > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > > ---
> > > v1 -> v2
> > > - Update CBS parameters when speed changes
> >
> > May I ask what is the point of this patch? The bandwidth fraction, as
> > IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> > the latter of which is a runtime variant. If the link speed changes at
> > runtime, which is entirely possible, I see no alternative than to let
> > user space figure out that this happened, and decide what to do. This is
> > a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> > as direct input, rather than something more high level like the desired
> > bandwidth for the stream itself, which could be dynamically computed by
> > the kernel.
>
> So what should be the behaviour here? Refuse setting CBS parameters if
> the link is down, and clear the hardware configuration of the CBS
> parameters each and every time there is a link-down event? Isn't that
> going to make the driver's in-use settings inconsistent with what the
> kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
> kernel just because the link went down.
>
> I think what you're proposing leads to the hardware being effectively
> "de-programmed" for CBS while "tc qdisc show" will probably report
> that CBS is active on the interface - which clearly would be absurd.
No, just program to hardware right away the idleSlope, sendSlope,
loCredit and hiCredit that were communicated by user space. Those were
computed for a specific link speed and it is user space's business to
monitor that this link speed is maintained for as long as the streams
are necessary (otherwise those parameters are no longer valid).
One could even recover the portTransmitRate that the parameters were
computed for (it should be idleSlope - sendSlope, in Kbps).
AKA keep the driver as it is.
I don't see why the CBS parameters would need to be de-programmed from
hardware on a link down event. Is that some stmmac specific thing?
Xiaolei may have a bone to pick with the fact that tc-cbs takes its
input the way it does, but that's an entirely different matter..
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 13:53 ` Vladimir Oltean
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 13:53 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 02:40:30PM +0100, Russell King (Oracle) wrote:
> On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> > On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > > When the port is relinked, if the speed changes, the CBS parameters
> > > should be updated, so saving the user transmission parameters so
> > > that idle_slope and send_slope can be recalculated after the speed
> > > changes after linking up can help reconfigure CBS after the speed
> > > changes.
> > >
> > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > > ---
> > > v1 -> v2
> > > - Update CBS parameters when speed changes
> >
> > May I ask what is the point of this patch? The bandwidth fraction, as
> > IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> > the latter of which is a runtime variant. If the link speed changes at
> > runtime, which is entirely possible, I see no alternative than to let
> > user space figure out that this happened, and decide what to do. This is
> > a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> > as direct input, rather than something more high level like the desired
> > bandwidth for the stream itself, which could be dynamically computed by
> > the kernel.
>
> So what should be the behaviour here? Refuse setting CBS parameters if
> the link is down, and clear the hardware configuration of the CBS
> parameters each and every time there is a link-down event? Isn't that
> going to make the driver's in-use settings inconsistent with what the
> kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
> kernel just because the link went down.
>
> I think what you're proposing leads to the hardware being effectively
> "de-programmed" for CBS while "tc qdisc show" will probably report
> that CBS is active on the interface - which clearly would be absurd.
No, just program to hardware right away the idleSlope, sendSlope,
loCredit and hiCredit that were communicated by user space. Those were
computed for a specific link speed and it is user space's business to
monitor that this link speed is maintained for as long as the streams
are necessary (otherwise those parameters are no longer valid).
One could even recover the portTransmitRate that the parameters were
computed for (it should be idleSlope - sendSlope, in Kbps).
AKA keep the driver as it is.
I don't see why the CBS parameters would need to be de-programmed from
hardware on a link down event. Is that some stmmac specific thing?
Xiaolei may have a bone to pick with the fact that tc-cbs takes its
input the way it does, but that's an entirely different matter..
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 13:53 ` Vladimir Oltean
@ 2024-05-30 14:14 ` Russell King (Oracle)
-1 siblings, 0 replies; 23+ messages in thread
From: Russell King (Oracle) @ 2024-05-30 14:14 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 04:53:35PM +0300, Vladimir Oltean wrote:
> On Thu, May 30, 2024 at 02:40:30PM +0100, Russell King (Oracle) wrote:
> > On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> > > On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > > > When the port is relinked, if the speed changes, the CBS parameters
> > > > should be updated, so saving the user transmission parameters so
> > > > that idle_slope and send_slope can be recalculated after the speed
> > > > changes after linking up can help reconfigure CBS after the speed
> > > > changes.
> > > >
> > > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > > > ---
> > > > v1 -> v2
> > > > - Update CBS parameters when speed changes
> > >
> > > May I ask what is the point of this patch? The bandwidth fraction, as
> > > IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> > > the latter of which is a runtime variant. If the link speed changes at
> > > runtime, which is entirely possible, I see no alternative than to let
> > > user space figure out that this happened, and decide what to do. This is
> > > a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> > > as direct input, rather than something more high level like the desired
> > > bandwidth for the stream itself, which could be dynamically computed by
> > > the kernel.
> >
> > So what should be the behaviour here? Refuse setting CBS parameters if
> > the link is down, and clear the hardware configuration of the CBS
> > parameters each and every time there is a link-down event? Isn't that
> > going to make the driver's in-use settings inconsistent with what the
> > kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
> > kernel just because the link went down.
> >
> > I think what you're proposing leads to the hardware being effectively
> > "de-programmed" for CBS while "tc qdisc show" will probably report
> > that CBS is active on the interface - which clearly would be absurd.
>
> No, just program to hardware right away the idleSlope, sendSlope,
> loCredit and hiCredit that were communicated by user space. Those were
> computed for a specific link speed and it is user space's business to
> monitor that this link speed is maintained for as long as the streams
> are necessary (otherwise those parameters are no longer valid).
> One could even recover the portTransmitRate that the parameters were
> computed for (it should be idleSlope - sendSlope, in Kbps).
>
> AKA keep the driver as it is.
>
> I don't see why the CBS parameters would need to be de-programmed from
> hardware on a link down event. Is that some stmmac specific thing?
If the driver is having to do computation on the parameters based on
the link speed, then when the link speed changes, the parameters
no longer match what the kernel _thinks_ those parameters were
programmed with.
What I'm trying to get over to you is that what you propose causes
an inconsistency between how the hardware is _programmed_ to behave
for CBS and what the kernel reports the CBS settings are if the
link speed changes.
For example, if the link was operating at 10G, and the idle slope
set by userspace is A, and the send slope was B, tc qdisc show
will report an idle slope of A and send slope of B.
If the link speed now changes to 5G, then, without updating the
settings in the hardware, the multiplier for the register values
will have reduced by a factor of two, meaning they're twice as
large as they should be for values of A and B.
However, tc qdisc show continues to report that values of A and B
are being used, but the hardware is actually using 2 * A and 2 * B.
It's all very well saying that userspace should basically reconstruct
the tc settings when the link changes, but not everyone is aware of
that. I'm saying it's a problem if one isn't aware of this issue with
this hardware, and one looks at tc qdisc show output, and assumes
that reflects what is actually being used when it isn't.
It's quality of implmentation - as far as I'm concerned, the kernel
should *not* mislead the user like this.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 14:14 ` Russell King (Oracle)
0 siblings, 0 replies; 23+ messages in thread
From: Russell King (Oracle) @ 2024-05-30 14:14 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 04:53:35PM +0300, Vladimir Oltean wrote:
> On Thu, May 30, 2024 at 02:40:30PM +0100, Russell King (Oracle) wrote:
> > On Thu, May 30, 2024 at 04:28:22PM +0300, Vladimir Oltean wrote:
> > > On Thu, May 30, 2024 at 02:50:52PM +0200, Xiaolei Wang wrote:
> > > > When the port is relinked, if the speed changes, the CBS parameters
> > > > should be updated, so saving the user transmission parameters so
> > > > that idle_slope and send_slope can be recalculated after the speed
> > > > changes after linking up can help reconfigure CBS after the speed
> > > > changes.
> > > >
> > > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC")
> > > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > > > ---
> > > > v1 -> v2
> > > > - Update CBS parameters when speed changes
> > >
> > > May I ask what is the point of this patch? The bandwidth fraction, as
> > > IEEE 802.1Q defines it, it a function of idleSlope / portTransmitRate,
> > > the latter of which is a runtime variant. If the link speed changes at
> > > runtime, which is entirely possible, I see no alternative than to let
> > > user space figure out that this happened, and decide what to do. This is
> > > a consequence of the fact that the tc-cbs UAPI takes the raw idleSlope
> > > as direct input, rather than something more high level like the desired
> > > bandwidth for the stream itself, which could be dynamically computed by
> > > the kernel.
> >
> > So what should be the behaviour here? Refuse setting CBS parameters if
> > the link is down, and clear the hardware configuration of the CBS
> > parameters each and every time there is a link-down event? Isn't that
> > going to make the driver's in-use settings inconsistent with what the
> > kernel thinks have been set? AFAIK, tc qdisc's don't vanish from the
> > kernel just because the link went down.
> >
> > I think what you're proposing leads to the hardware being effectively
> > "de-programmed" for CBS while "tc qdisc show" will probably report
> > that CBS is active on the interface - which clearly would be absurd.
>
> No, just program to hardware right away the idleSlope, sendSlope,
> loCredit and hiCredit that were communicated by user space. Those were
> computed for a specific link speed and it is user space's business to
> monitor that this link speed is maintained for as long as the streams
> are necessary (otherwise those parameters are no longer valid).
> One could even recover the portTransmitRate that the parameters were
> computed for (it should be idleSlope - sendSlope, in Kbps).
>
> AKA keep the driver as it is.
>
> I don't see why the CBS parameters would need to be de-programmed from
> hardware on a link down event. Is that some stmmac specific thing?
If the driver is having to do computation on the parameters based on
the link speed, then when the link speed changes, the parameters
no longer match what the kernel _thinks_ those parameters were
programmed with.
What I'm trying to get over to you is that what you propose causes
an inconsistency between how the hardware is _programmed_ to behave
for CBS and what the kernel reports the CBS settings are if the
link speed changes.
For example, if the link was operating at 10G, and the idle slope
set by userspace is A, and the send slope was B, tc qdisc show
will report an idle slope of A and send slope of B.
If the link speed now changes to 5G, then, without updating the
settings in the hardware, the multiplier for the register values
will have reduced by a factor of two, meaning they're twice as
large as they should be for values of A and B.
However, tc qdisc show continues to report that values of A and B
are being used, but the hardware is actually using 2 * A and 2 * B.
It's all very well saying that userspace should basically reconstruct
the tc settings when the link changes, but not everyone is aware of
that. I'm saying it's a problem if one isn't aware of this issue with
this hardware, and one looks at tc qdisc show output, and assumes
that reflects what is actually being used when it isn't.
It's quality of implmentation - as far as I'm concerned, the kernel
should *not* mislead the user like this.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 14:14 ` Russell King (Oracle)
@ 2024-05-30 14:35 ` Vladimir Oltean
-1 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 14:35 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 03:14:48PM +0100, Russell King (Oracle) wrote:
> > I don't see why the CBS parameters would need to be de-programmed from
> > hardware on a link down event. Is that some stmmac specific thing?
>
> If the driver is having to do computation on the parameters based on
> the link speed, then when the link speed changes, the parameters
> no longer match what the kernel _thinks_ those parameters were
> programmed with.
>
> What I'm trying to get over to you is that what you propose causes
> an inconsistency between how the hardware is _programmed_ to behave
> for CBS and what the kernel reports the CBS settings are if the
> link speed changes.
>
> It's all very well saying that userspace should basically reconstruct
> the tc settings when the link changes, but not everyone is aware of
> that. I'm saying it's a problem if one isn't aware of this issue with
> this hardware, and one looks at tc qdisc show output, and assumes
> that reflects what is actually being used when it isn't.
>
> It's quality of implmentation - as far as I'm concerned, the kernel
> should *not* mislead the user like this.
I was saying that the tc-cbs parameters input into the kernel should
already have the link speed baked into them:
portTransmitRate = idleSlope - sendSlope. In theory one could feed any
data into the kernel, but this is based on the IEEE 802.1Q formulas.
I had missed the fact that there is a calculation dependent on
priv->speed within tc_setup_cbs(), and I'm sorry for that. I thought
that the values were passed unaltered down to stmmac_config_cbs(). So
"make no change to the driver" is no longer my recommendation.
In that case, my recommendation is to do as sja1105_setup_tc_cbs() does:
replace priv->speed with the portTransmitRate recovered from the tc-cbs
parameters, and fully expect that when the link speed changes, user
space comes along and changes those parameters.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 14:35 ` Vladimir Oltean
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Oltean @ 2024-05-30 14:35 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Xiaolei Wang, Andrew Lunn, alexandre.torgue, joabreu, davem,
edumazet, kuba, pabeni, mcoquelin.stm32, netdev, linux-arm-kernel,
linux-kernel
On Thu, May 30, 2024 at 03:14:48PM +0100, Russell King (Oracle) wrote:
> > I don't see why the CBS parameters would need to be de-programmed from
> > hardware on a link down event. Is that some stmmac specific thing?
>
> If the driver is having to do computation on the parameters based on
> the link speed, then when the link speed changes, the parameters
> no longer match what the kernel _thinks_ those parameters were
> programmed with.
>
> What I'm trying to get over to you is that what you propose causes
> an inconsistency between how the hardware is _programmed_ to behave
> for CBS and what the kernel reports the CBS settings are if the
> link speed changes.
>
> It's all very well saying that userspace should basically reconstruct
> the tc settings when the link changes, but not everyone is aware of
> that. I'm saying it's a problem if one isn't aware of this issue with
> this hardware, and one looks at tc qdisc show output, and assumes
> that reflects what is actually being used when it isn't.
>
> It's quality of implmentation - as far as I'm concerned, the kernel
> should *not* mislead the user like this.
I was saying that the tc-cbs parameters input into the kernel should
already have the link speed baked into them:
portTransmitRate = idleSlope - sendSlope. In theory one could feed any
data into the kernel, but this is based on the IEEE 802.1Q formulas.
I had missed the fact that there is a calculation dependent on
priv->speed within tc_setup_cbs(), and I'm sorry for that. I thought
that the values were passed unaltered down to stmmac_config_cbs(). So
"make no change to the driver" is no longer my recommendation.
In that case, my recommendation is to do as sja1105_setup_tc_cbs() does:
replace priv->speed with the portTransmitRate recovered from the tc-cbs
parameters, and fully expect that when the link speed changes, user
space comes along and changes those parameters.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 14:35 ` Vladimir Oltean
@ 2024-05-31 8:23 ` xiaolei wang
-1 siblings, 0 replies; 23+ messages in thread
From: xiaolei wang @ 2024-05-31 8:23 UTC (permalink / raw)
To: Vladimir Oltean, Russell King (Oracle)
Cc: Andrew Lunn, alexandre.torgue, joabreu, davem, edumazet, kuba,
pabeni, mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
On 5/30/24 22:35, Vladimir Oltean wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Thu, May 30, 2024 at 03:14:48PM +0100, Russell King (Oracle) wrote:
>>> I don't see why the CBS parameters would need to be de-programmed from
>>> hardware on a link down event. Is that some stmmac specific thing?
>> If the driver is having to do computation on the parameters based on
>> the link speed, then when the link speed changes, the parameters
>> no longer match what the kernel _thinks_ those parameters were
>> programmed with.
>>
>> What I'm trying to get over to you is that what you propose causes
>> an inconsistency between how the hardware is _programmed_ to behave
>> for CBS and what the kernel reports the CBS settings are if the
>> link speed changes.
>>
>> It's all very well saying that userspace should basically reconstruct
>> the tc settings when the link changes, but not everyone is aware of
>> that. I'm saying it's a problem if one isn't aware of this issue with
>> this hardware, and one looks at tc qdisc show output, and assumes
>> that reflects what is actually being used when it isn't.
>>
>> It's quality of implmentation - as far as I'm concerned, the kernel
>> should *not* mislead the user like this.
> I was saying that the tc-cbs parameters input into the kernel should
> already have the link speed baked into them:
> portTransmitRate = idleSlope - sendSlope. In theory one could feed any
> data into the kernel, but this is based on the IEEE 802.1Q formulas.
>
> I had missed the fact that there is a calculation dependent on
> priv->speed within tc_setup_cbs(), and I'm sorry for that. I thought
> that the values were passed unaltered down to stmmac_config_cbs(). So
> "make no change to the driver" is no longer my recommendation.
>
> In that case, my recommendation is to do as sja1105_setup_tc_cbs() does:
> replace priv->speed with the portTransmitRate recovered from the tc-cbs
> parameters, and fully expect that when the link speed changes, user
> space comes along and changes those parameters.
I will try it
thanks
xiaolei
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-31 8:23 ` xiaolei wang
0 siblings, 0 replies; 23+ messages in thread
From: xiaolei wang @ 2024-05-31 8:23 UTC (permalink / raw)
To: Vladimir Oltean, Russell King (Oracle)
Cc: Andrew Lunn, alexandre.torgue, joabreu, davem, edumazet, kuba,
pabeni, mcoquelin.stm32, netdev, linux-arm-kernel, linux-kernel
On 5/30/24 22:35, Vladimir Oltean wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Thu, May 30, 2024 at 03:14:48PM +0100, Russell King (Oracle) wrote:
>>> I don't see why the CBS parameters would need to be de-programmed from
>>> hardware on a link down event. Is that some stmmac specific thing?
>> If the driver is having to do computation on the parameters based on
>> the link speed, then when the link speed changes, the parameters
>> no longer match what the kernel _thinks_ those parameters were
>> programmed with.
>>
>> What I'm trying to get over to you is that what you propose causes
>> an inconsistency between how the hardware is _programmed_ to behave
>> for CBS and what the kernel reports the CBS settings are if the
>> link speed changes.
>>
>> It's all very well saying that userspace should basically reconstruct
>> the tc settings when the link changes, but not everyone is aware of
>> that. I'm saying it's a problem if one isn't aware of this issue with
>> this hardware, and one looks at tc qdisc show output, and assumes
>> that reflects what is actually being used when it isn't.
>>
>> It's quality of implmentation - as far as I'm concerned, the kernel
>> should *not* mislead the user like this.
> I was saying that the tc-cbs parameters input into the kernel should
> already have the link speed baked into them:
> portTransmitRate = idleSlope - sendSlope. In theory one could feed any
> data into the kernel, but this is based on the IEEE 802.1Q formulas.
>
> I had missed the fact that there is a calculation dependent on
> priv->speed within tc_setup_cbs(), and I'm sorry for that. I thought
> that the values were passed unaltered down to stmmac_config_cbs(). So
> "make no change to the driver" is no longer my recommendation.
>
> In that case, my recommendation is to do as sja1105_setup_tc_cbs() does:
> replace priv->speed with the portTransmitRate recovered from the tc-cbs
> parameters, and fully expect that when the link speed changes, user
> space comes along and changes those parameters.
I will try it
thanks
xiaolei
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 13:40 ` Russell King (Oracle)
@ 2024-05-30 16:13 ` Jakub Kicinski
-1 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2024-05-30 16:13 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Vladimir Oltean, Xiaolei Wang, Andrew Lunn, alexandre.torgue,
joabreu, davem, edumazet, pabeni, mcoquelin.stm32, netdev,
linux-arm-kernel, linux-kernel
On Thu, 30 May 2024 14:40:30 +0100 Russell King (Oracle) wrote:
> I think what you're proposing leads to the hardware being effectively
> "de-programmed" for CBS while "tc qdisc show" will probably report
> that CBS is active on the interface - which clearly would be absurd.
FWIW the "switch-offloaded" qdiscs do support reporting that they got
"de-programmed" given that more complex hierarchies can easily go out
of what HW is capable of.
They call the driver from the .dump callback, nominally to get
stats (e.g. red_dump() -> red_dump_offload_stats()) but it also
refreshes the offloaded state (see qdisc_offload_dump_helper()).
For "NIC-offloaded" qdiscs (i.e. all traffic passes thru the host,
rather than being forwarded) the stats callback makes less sense.
But all this is to say that there _is_ precedent for clearing
qdisc "offloaded" bits.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-05-30 16:13 ` Jakub Kicinski
0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2024-05-30 16:13 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Vladimir Oltean, Xiaolei Wang, Andrew Lunn, alexandre.torgue,
joabreu, davem, edumazet, pabeni, mcoquelin.stm32, netdev,
linux-arm-kernel, linux-kernel
On Thu, 30 May 2024 14:40:30 +0100 Russell King (Oracle) wrote:
> I think what you're proposing leads to the hardware being effectively
> "de-programmed" for CBS while "tc qdisc show" will probably report
> that CBS is active on the interface - which clearly would be absurd.
FWIW the "switch-offloaded" qdiscs do support reporting that they got
"de-programmed" given that more complex hierarchies can easily go out
of what HW is capable of.
They call the driver from the .dump callback, nominally to get
stats (e.g. red_dump() -> red_dump_offload_stats()) but it also
refreshes the offloaded state (see qdisc_offload_dump_helper()).
For "NIC-offloaded" qdiscs (i.e. all traffic passes thru the host,
rather than being forwarded) the stats callback makes less sense.
But all this is to say that there _is_ precedent for clearing
qdisc "offloaded" bits.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-05-30 6:14 ` Xiaolei Wang
@ 2024-06-05 5:26 ` Dan Carpenter
-1 siblings, 0 replies; 23+ messages in thread
From: Dan Carpenter @ 2024-06-05 5:26 UTC (permalink / raw)
To: oe-kbuild, Xiaolei Wang, linux, alexandre.torgue, joabreu, andrew,
davem, edumazet, kuba, pabeni, mcoquelin.stm32
Cc: lkp, oe-kbuild-all, netdev, linux-arm-kernel, linux-kernel
Hi Xiaolei,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-Update-CBS-parameters-when-speed-changes-after-linking-up/20240530-141843
base: net/main
patch link: https://lore.kernel.org/r/20240530061453.561708-1-xiaolei.wang%40windriver.com
patch subject: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
config: i386-randconfig-141-20240604 (https://download.01.org/0day-ci/archive/20240605/202406050318.jsyBFsxx-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202406050318.jsyBFsxx-lkp@intel.com/
New smatch warnings:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'ptr'.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'speed_div'.
vim +/ptr +3234 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
19d9187317979c Joao Pinto 2017-03-10 3194 static void stmmac_configure_cbs(struct stmmac_priv *priv)
19d9187317979c Joao Pinto 2017-03-10 3195 {
19d9187317979c Joao Pinto 2017-03-10 3196 u32 tx_queues_count = priv->plat->tx_queues_to_use;
19d9187317979c Joao Pinto 2017-03-10 3197 u32 mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3198 u32 queue;
882212f550d669 Xiaolei Wang 2024-05-30 3199 u32 ptr, speed_div;
882212f550d669 Xiaolei Wang 2024-05-30 3200 u64 value;
882212f550d669 Xiaolei Wang 2024-05-30 3201
882212f550d669 Xiaolei Wang 2024-05-30 3202 /* Port Transmit Rate and Speed Divider */
882212f550d669 Xiaolei Wang 2024-05-30 3203 switch (priv->speed) {
882212f550d669 Xiaolei Wang 2024-05-30 3204 case SPEED_10000:
882212f550d669 Xiaolei Wang 2024-05-30 3205 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3206 speed_div = 10000000;
882212f550d669 Xiaolei Wang 2024-05-30 3207 break;
882212f550d669 Xiaolei Wang 2024-05-30 3208 case SPEED_5000:
882212f550d669 Xiaolei Wang 2024-05-30 3209 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3210 speed_div = 5000000;
882212f550d669 Xiaolei Wang 2024-05-30 3211 break;
882212f550d669 Xiaolei Wang 2024-05-30 3212 case SPEED_2500:
882212f550d669 Xiaolei Wang 2024-05-30 3213 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3214 speed_div = 2500000;
882212f550d669 Xiaolei Wang 2024-05-30 3215 break;
882212f550d669 Xiaolei Wang 2024-05-30 3216 case SPEED_1000:
882212f550d669 Xiaolei Wang 2024-05-30 3217 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3218 speed_div = 1000000;
882212f550d669 Xiaolei Wang 2024-05-30 3219 break;
882212f550d669 Xiaolei Wang 2024-05-30 3220 case SPEED_100:
882212f550d669 Xiaolei Wang 2024-05-30 3221 ptr = 4;
882212f550d669 Xiaolei Wang 2024-05-30 3222 speed_div = 100000;
882212f550d669 Xiaolei Wang 2024-05-30 3223 break;
882212f550d669 Xiaolei Wang 2024-05-30 3224 default:
882212f550d669 Xiaolei Wang 2024-05-30 3225 netdev_dbg(priv->dev, "link speed is not known\n");
return;?
882212f550d669 Xiaolei Wang 2024-05-30 3226 }
19d9187317979c Joao Pinto 2017-03-10 3227
44781fef137896 Joao Pinto 2017-03-31 3228 /* queue 0 is reserved for legacy traffic */
44781fef137896 Joao Pinto 2017-03-31 3229 for (queue = 1; queue < tx_queues_count; queue++) {
19d9187317979c Joao Pinto 2017-03-10 3230 mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3231 if (mode_to_use == MTL_QUEUE_DCB)
19d9187317979c Joao Pinto 2017-03-10 3232 continue;
19d9187317979c Joao Pinto 2017-03-10 3233
882212f550d669 Xiaolei Wang 2024-05-30 @3234 value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
^^^ ^^^^^^^^^^
Uninitialized.
882212f550d669 Xiaolei Wang 2024-05-30 3235 priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3236
882212f550d669 Xiaolei Wang 2024-05-30 3237 value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
882212f550d669 Xiaolei Wang 2024-05-30 3238 priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3239
c10d4c82a5c84c Jose Abreu 2018-04-16 3240 stmmac_config_cbs(priv, priv->hw,
19d9187317979c Joao Pinto 2017-03-10 3241 priv->plat->tx_queues_cfg[queue].send_slope,
19d9187317979c Joao Pinto 2017-03-10 3242 priv->plat->tx_queues_cfg[queue].idle_slope,
19d9187317979c Joao Pinto 2017-03-10 3243 priv->plat->tx_queues_cfg[queue].high_credit,
19d9187317979c Joao Pinto 2017-03-10 3244 priv->plat->tx_queues_cfg[queue].low_credit,
19d9187317979c Joao Pinto 2017-03-10 3245 queue);
19d9187317979c Joao Pinto 2017-03-10 3246 }
19d9187317979c Joao Pinto 2017-03-10 3247 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-06-05 5:26 ` Dan Carpenter
0 siblings, 0 replies; 23+ messages in thread
From: Dan Carpenter @ 2024-06-05 5:26 UTC (permalink / raw)
To: oe-kbuild, Xiaolei Wang, linux, alexandre.torgue, joabreu, andrew,
davem, edumazet, kuba, pabeni, mcoquelin.stm32
Cc: lkp, oe-kbuild-all, netdev, linux-arm-kernel, linux-kernel
Hi Xiaolei,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-Update-CBS-parameters-when-speed-changes-after-linking-up/20240530-141843
base: net/main
patch link: https://lore.kernel.org/r/20240530061453.561708-1-xiaolei.wang%40windriver.com
patch subject: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
config: i386-randconfig-141-20240604 (https://download.01.org/0day-ci/archive/20240605/202406050318.jsyBFsxx-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202406050318.jsyBFsxx-lkp@intel.com/
New smatch warnings:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'ptr'.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'speed_div'.
vim +/ptr +3234 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
19d9187317979c Joao Pinto 2017-03-10 3194 static void stmmac_configure_cbs(struct stmmac_priv *priv)
19d9187317979c Joao Pinto 2017-03-10 3195 {
19d9187317979c Joao Pinto 2017-03-10 3196 u32 tx_queues_count = priv->plat->tx_queues_to_use;
19d9187317979c Joao Pinto 2017-03-10 3197 u32 mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3198 u32 queue;
882212f550d669 Xiaolei Wang 2024-05-30 3199 u32 ptr, speed_div;
882212f550d669 Xiaolei Wang 2024-05-30 3200 u64 value;
882212f550d669 Xiaolei Wang 2024-05-30 3201
882212f550d669 Xiaolei Wang 2024-05-30 3202 /* Port Transmit Rate and Speed Divider */
882212f550d669 Xiaolei Wang 2024-05-30 3203 switch (priv->speed) {
882212f550d669 Xiaolei Wang 2024-05-30 3204 case SPEED_10000:
882212f550d669 Xiaolei Wang 2024-05-30 3205 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3206 speed_div = 10000000;
882212f550d669 Xiaolei Wang 2024-05-30 3207 break;
882212f550d669 Xiaolei Wang 2024-05-30 3208 case SPEED_5000:
882212f550d669 Xiaolei Wang 2024-05-30 3209 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3210 speed_div = 5000000;
882212f550d669 Xiaolei Wang 2024-05-30 3211 break;
882212f550d669 Xiaolei Wang 2024-05-30 3212 case SPEED_2500:
882212f550d669 Xiaolei Wang 2024-05-30 3213 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3214 speed_div = 2500000;
882212f550d669 Xiaolei Wang 2024-05-30 3215 break;
882212f550d669 Xiaolei Wang 2024-05-30 3216 case SPEED_1000:
882212f550d669 Xiaolei Wang 2024-05-30 3217 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3218 speed_div = 1000000;
882212f550d669 Xiaolei Wang 2024-05-30 3219 break;
882212f550d669 Xiaolei Wang 2024-05-30 3220 case SPEED_100:
882212f550d669 Xiaolei Wang 2024-05-30 3221 ptr = 4;
882212f550d669 Xiaolei Wang 2024-05-30 3222 speed_div = 100000;
882212f550d669 Xiaolei Wang 2024-05-30 3223 break;
882212f550d669 Xiaolei Wang 2024-05-30 3224 default:
882212f550d669 Xiaolei Wang 2024-05-30 3225 netdev_dbg(priv->dev, "link speed is not known\n");
return;?
882212f550d669 Xiaolei Wang 2024-05-30 3226 }
19d9187317979c Joao Pinto 2017-03-10 3227
44781fef137896 Joao Pinto 2017-03-31 3228 /* queue 0 is reserved for legacy traffic */
44781fef137896 Joao Pinto 2017-03-31 3229 for (queue = 1; queue < tx_queues_count; queue++) {
19d9187317979c Joao Pinto 2017-03-10 3230 mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3231 if (mode_to_use == MTL_QUEUE_DCB)
19d9187317979c Joao Pinto 2017-03-10 3232 continue;
19d9187317979c Joao Pinto 2017-03-10 3233
882212f550d669 Xiaolei Wang 2024-05-30 @3234 value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
^^^ ^^^^^^^^^^
Uninitialized.
882212f550d669 Xiaolei Wang 2024-05-30 3235 priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3236
882212f550d669 Xiaolei Wang 2024-05-30 3237 value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
882212f550d669 Xiaolei Wang 2024-05-30 3238 priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3239
c10d4c82a5c84c Jose Abreu 2018-04-16 3240 stmmac_config_cbs(priv, priv->hw,
19d9187317979c Joao Pinto 2017-03-10 3241 priv->plat->tx_queues_cfg[queue].send_slope,
19d9187317979c Joao Pinto 2017-03-10 3242 priv->plat->tx_queues_cfg[queue].idle_slope,
19d9187317979c Joao Pinto 2017-03-10 3243 priv->plat->tx_queues_cfg[queue].high_credit,
19d9187317979c Joao Pinto 2017-03-10 3244 priv->plat->tx_queues_cfg[queue].low_credit,
19d9187317979c Joao Pinto 2017-03-10 3245 queue);
19d9187317979c Joao Pinto 2017-03-10 3246 }
19d9187317979c Joao Pinto 2017-03-10 3247 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
2024-06-05 5:26 ` Dan Carpenter
@ 2024-06-05 5:30 ` xiaolei wang
-1 siblings, 0 replies; 23+ messages in thread
From: xiaolei wang @ 2024-06-05 5:30 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, linux, alexandre.torgue, joabreu,
andrew, davem, edumazet, kuba, pabeni, mcoquelin.stm32
Cc: lkp, oe-kbuild-all, netdev, linux-arm-kernel, linux-kernel
On 6/5/24 13:26, Dan Carpenter wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Xiaolei,
>
> kernel test robot noticed the following build warnings:
Please drop this patch as there are still some questions on this issue
thanks
xiaolei
>
> url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-Update-CBS-parameters-when-speed-changes-after-linking-up/20240530-141843
> base: net/main
> patch link: https://lore.kernel.org/r/20240530061453.561708-1-xiaolei.wang%40windriver.com
> patch subject: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
> config: i386-randconfig-141-20240604 (https://download.01.org/0day-ci/archive/20240605/202406050318.jsyBFsxx-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202406050318.jsyBFsxx-lkp@intel.com/
>
> New smatch warnings:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'ptr'.
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'speed_div'.
>
> vim +/ptr +3234 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>
> 19d9187317979c Joao Pinto 2017-03-10 3194 static void stmmac_configure_cbs(struct stmmac_priv *priv)
> 19d9187317979c Joao Pinto 2017-03-10 3195 {
> 19d9187317979c Joao Pinto 2017-03-10 3196 u32 tx_queues_count = priv->plat->tx_queues_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3197 u32 mode_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3198 u32 queue;
> 882212f550d669 Xiaolei Wang 2024-05-30 3199 u32 ptr, speed_div;
> 882212f550d669 Xiaolei Wang 2024-05-30 3200 u64 value;
> 882212f550d669 Xiaolei Wang 2024-05-30 3201
> 882212f550d669 Xiaolei Wang 2024-05-30 3202 /* Port Transmit Rate and Speed Divider */
> 882212f550d669 Xiaolei Wang 2024-05-30 3203 switch (priv->speed) {
> 882212f550d669 Xiaolei Wang 2024-05-30 3204 case SPEED_10000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3205 ptr = 32;
> 882212f550d669 Xiaolei Wang 2024-05-30 3206 speed_div = 10000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3207 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3208 case SPEED_5000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3209 ptr = 32;
> 882212f550d669 Xiaolei Wang 2024-05-30 3210 speed_div = 5000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3211 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3212 case SPEED_2500:
> 882212f550d669 Xiaolei Wang 2024-05-30 3213 ptr = 8;
> 882212f550d669 Xiaolei Wang 2024-05-30 3214 speed_div = 2500000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3215 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3216 case SPEED_1000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3217 ptr = 8;
> 882212f550d669 Xiaolei Wang 2024-05-30 3218 speed_div = 1000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3219 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3220 case SPEED_100:
> 882212f550d669 Xiaolei Wang 2024-05-30 3221 ptr = 4;
> 882212f550d669 Xiaolei Wang 2024-05-30 3222 speed_div = 100000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3223 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3224 default:
> 882212f550d669 Xiaolei Wang 2024-05-30 3225 netdev_dbg(priv->dev, "link speed is not known\n");
>
> return;?
>
> 882212f550d669 Xiaolei Wang 2024-05-30 3226 }
> 19d9187317979c Joao Pinto 2017-03-10 3227
> 44781fef137896 Joao Pinto 2017-03-31 3228 /* queue 0 is reserved for legacy traffic */
> 44781fef137896 Joao Pinto 2017-03-31 3229 for (queue = 1; queue < tx_queues_count; queue++) {
> 19d9187317979c Joao Pinto 2017-03-10 3230 mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3231 if (mode_to_use == MTL_QUEUE_DCB)
> 19d9187317979c Joao Pinto 2017-03-10 3232 continue;
> 19d9187317979c Joao Pinto 2017-03-10 3233
> 882212f550d669 Xiaolei Wang 2024-05-30 @3234 value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
> ^^^ ^^^^^^^^^^
> Uninitialized.
>
> 882212f550d669 Xiaolei Wang 2024-05-30 3235 priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
> 882212f550d669 Xiaolei Wang 2024-05-30 3236
> 882212f550d669 Xiaolei Wang 2024-05-30 3237 value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
> 882212f550d669 Xiaolei Wang 2024-05-30 3238 priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
> 882212f550d669 Xiaolei Wang 2024-05-30 3239
> c10d4c82a5c84c Jose Abreu 2018-04-16 3240 stmmac_config_cbs(priv, priv->hw,
> 19d9187317979c Joao Pinto 2017-03-10 3241 priv->plat->tx_queues_cfg[queue].send_slope,
> 19d9187317979c Joao Pinto 2017-03-10 3242 priv->plat->tx_queues_cfg[queue].idle_slope,
> 19d9187317979c Joao Pinto 2017-03-10 3243 priv->plat->tx_queues_cfg[queue].high_credit,
> 19d9187317979c Joao Pinto 2017-03-10 3244 priv->plat->tx_queues_cfg[queue].low_credit,
> 19d9187317979c Joao Pinto 2017-03-10 3245 queue);
> 19d9187317979c Joao Pinto 2017-03-10 3246 }
> 19d9187317979c Joao Pinto 2017-03-10 3247 }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-06-05 5:30 ` xiaolei wang
0 siblings, 0 replies; 23+ messages in thread
From: xiaolei wang @ 2024-06-05 5:30 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, linux, alexandre.torgue, joabreu,
andrew, davem, edumazet, kuba, pabeni, mcoquelin.stm32
Cc: lkp, oe-kbuild-all, netdev, linux-arm-kernel, linux-kernel
On 6/5/24 13:26, Dan Carpenter wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Xiaolei,
>
> kernel test robot noticed the following build warnings:
Please drop this patch as there are still some questions on this issue
thanks
xiaolei
>
> url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-Update-CBS-parameters-when-speed-changes-after-linking-up/20240530-141843
> base: net/main
> patch link: https://lore.kernel.org/r/20240530061453.561708-1-xiaolei.wang%40windriver.com
> patch subject: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
> config: i386-randconfig-141-20240604 (https://download.01.org/0day-ci/archive/20240605/202406050318.jsyBFsxx-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202406050318.jsyBFsxx-lkp@intel.com/
>
> New smatch warnings:
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'ptr'.
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'speed_div'.
>
> vim +/ptr +3234 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>
> 19d9187317979c Joao Pinto 2017-03-10 3194 static void stmmac_configure_cbs(struct stmmac_priv *priv)
> 19d9187317979c Joao Pinto 2017-03-10 3195 {
> 19d9187317979c Joao Pinto 2017-03-10 3196 u32 tx_queues_count = priv->plat->tx_queues_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3197 u32 mode_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3198 u32 queue;
> 882212f550d669 Xiaolei Wang 2024-05-30 3199 u32 ptr, speed_div;
> 882212f550d669 Xiaolei Wang 2024-05-30 3200 u64 value;
> 882212f550d669 Xiaolei Wang 2024-05-30 3201
> 882212f550d669 Xiaolei Wang 2024-05-30 3202 /* Port Transmit Rate and Speed Divider */
> 882212f550d669 Xiaolei Wang 2024-05-30 3203 switch (priv->speed) {
> 882212f550d669 Xiaolei Wang 2024-05-30 3204 case SPEED_10000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3205 ptr = 32;
> 882212f550d669 Xiaolei Wang 2024-05-30 3206 speed_div = 10000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3207 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3208 case SPEED_5000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3209 ptr = 32;
> 882212f550d669 Xiaolei Wang 2024-05-30 3210 speed_div = 5000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3211 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3212 case SPEED_2500:
> 882212f550d669 Xiaolei Wang 2024-05-30 3213 ptr = 8;
> 882212f550d669 Xiaolei Wang 2024-05-30 3214 speed_div = 2500000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3215 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3216 case SPEED_1000:
> 882212f550d669 Xiaolei Wang 2024-05-30 3217 ptr = 8;
> 882212f550d669 Xiaolei Wang 2024-05-30 3218 speed_div = 1000000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3219 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3220 case SPEED_100:
> 882212f550d669 Xiaolei Wang 2024-05-30 3221 ptr = 4;
> 882212f550d669 Xiaolei Wang 2024-05-30 3222 speed_div = 100000;
> 882212f550d669 Xiaolei Wang 2024-05-30 3223 break;
> 882212f550d669 Xiaolei Wang 2024-05-30 3224 default:
> 882212f550d669 Xiaolei Wang 2024-05-30 3225 netdev_dbg(priv->dev, "link speed is not known\n");
>
> return;?
>
> 882212f550d669 Xiaolei Wang 2024-05-30 3226 }
> 19d9187317979c Joao Pinto 2017-03-10 3227
> 44781fef137896 Joao Pinto 2017-03-31 3228 /* queue 0 is reserved for legacy traffic */
> 44781fef137896 Joao Pinto 2017-03-31 3229 for (queue = 1; queue < tx_queues_count; queue++) {
> 19d9187317979c Joao Pinto 2017-03-10 3230 mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
> 19d9187317979c Joao Pinto 2017-03-10 3231 if (mode_to_use == MTL_QUEUE_DCB)
> 19d9187317979c Joao Pinto 2017-03-10 3232 continue;
> 19d9187317979c Joao Pinto 2017-03-10 3233
> 882212f550d669 Xiaolei Wang 2024-05-30 @3234 value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
> ^^^ ^^^^^^^^^^
> Uninitialized.
>
> 882212f550d669 Xiaolei Wang 2024-05-30 3235 priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
> 882212f550d669 Xiaolei Wang 2024-05-30 3236
> 882212f550d669 Xiaolei Wang 2024-05-30 3237 value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
> 882212f550d669 Xiaolei Wang 2024-05-30 3238 priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
> 882212f550d669 Xiaolei Wang 2024-05-30 3239
> c10d4c82a5c84c Jose Abreu 2018-04-16 3240 stmmac_config_cbs(priv, priv->hw,
> 19d9187317979c Joao Pinto 2017-03-10 3241 priv->plat->tx_queues_cfg[queue].send_slope,
> 19d9187317979c Joao Pinto 2017-03-10 3242 priv->plat->tx_queues_cfg[queue].idle_slope,
> 19d9187317979c Joao Pinto 2017-03-10 3243 priv->plat->tx_queues_cfg[queue].high_credit,
> 19d9187317979c Joao Pinto 2017-03-10 3244 priv->plat->tx_queues_cfg[queue].low_credit,
> 19d9187317979c Joao Pinto 2017-03-10 3245 queue);
> 19d9187317979c Joao Pinto 2017-03-10 3246 }
> 19d9187317979c Joao Pinto 2017-03-10 3247 }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
@ 2024-06-04 20:04 kernel test robot
0 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2024-06-04 20:04 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240530061453.561708-1-xiaolei.wang@windriver.com>
References: <20240530061453.561708-1-xiaolei.wang@windriver.com>
TO: Xiaolei Wang <xiaolei.wang@windriver.com>
TO: linux@armlinux.org.uk
TO: alexandre.torgue@foss.st.com
TO: joabreu@synopsys.com
TO: andrew@lunn.ch
TO: davem@davemloft.net
TO: edumazet@google.com
TO: kuba@kernel.org
TO: pabeni@redhat.com
TO: mcoquelin.stm32@gmail.com
CC: netdev@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
Hi Xiaolei,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/net-stmmac-Update-CBS-parameters-when-speed-changes-after-linking-up/20240530-141843
base: net/main
patch link: https://lore.kernel.org/r/20240530061453.561708-1-xiaolei.wang%40windriver.com
patch subject: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up
:::::: branch date: 6 days ago
:::::: commit date: 6 days ago
config: i386-randconfig-141-20240604 (https://download.01.org/0day-ci/archive/20240605/202406050318.jsyBFsxx-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202406050318.jsyBFsxx-lkp@intel.com/
New smatch warnings:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'ptr'.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3234 stmmac_configure_cbs() error: uninitialized symbol 'speed_div'.
Old smatch warnings:
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'dev->irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'priv->lpi_irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'priv->sfty_ce_irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'priv->sfty_irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'priv->sfty_ue_irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3819 stmmac_request_irq_multi_msi() warn: 'priv->wol_irq' from request_irq() not released on lines: 3819.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3885 stmmac_request_irq_single() warn: 'dev->irq' from request_irq() not released on lines: 3885.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3885 stmmac_request_irq_single() warn: 'priv->lpi_irq' from request_irq() not released on lines: 3885.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3885 stmmac_request_irq_single() warn: 'priv->wol_irq' from request_irq() not released on lines: 3885.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5669 stmmac_rx() error: 'skb' dereferencing possible ERR_PTR()
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5681 stmmac_rx() error: 'skb' dereferencing possible ERR_PTR()
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5707 stmmac_rx() error: 'skb' dereferencing possible ERR_PTR()
vim +/ptr +3234 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
6a3a7193b28115 Joao Pinto 2017-03-10 3187
19d9187317979c Joao Pinto 2017-03-10 3188 /**
19d9187317979c Joao Pinto 2017-03-10 3189 * stmmac_configure_cbs - Configure CBS in TX queue
19d9187317979c Joao Pinto 2017-03-10 3190 * @priv: driver private structure
882212f550d669 Xiaolei Wang 2024-05-30 3191 * Description: It is used for configuring CBS in AVB TX queues,
882212f550d669 Xiaolei Wang 2024-05-30 3192 * and when the speed changes, update CBS parameters to reconfigure
19d9187317979c Joao Pinto 2017-03-10 3193 */
19d9187317979c Joao Pinto 2017-03-10 3194 static void stmmac_configure_cbs(struct stmmac_priv *priv)
19d9187317979c Joao Pinto 2017-03-10 3195 {
19d9187317979c Joao Pinto 2017-03-10 3196 u32 tx_queues_count = priv->plat->tx_queues_to_use;
19d9187317979c Joao Pinto 2017-03-10 3197 u32 mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3198 u32 queue;
882212f550d669 Xiaolei Wang 2024-05-30 3199 u32 ptr, speed_div;
882212f550d669 Xiaolei Wang 2024-05-30 3200 u64 value;
882212f550d669 Xiaolei Wang 2024-05-30 3201
882212f550d669 Xiaolei Wang 2024-05-30 3202 /* Port Transmit Rate and Speed Divider */
882212f550d669 Xiaolei Wang 2024-05-30 3203 switch (priv->speed) {
882212f550d669 Xiaolei Wang 2024-05-30 3204 case SPEED_10000:
882212f550d669 Xiaolei Wang 2024-05-30 3205 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3206 speed_div = 10000000;
882212f550d669 Xiaolei Wang 2024-05-30 3207 break;
882212f550d669 Xiaolei Wang 2024-05-30 3208 case SPEED_5000:
882212f550d669 Xiaolei Wang 2024-05-30 3209 ptr = 32;
882212f550d669 Xiaolei Wang 2024-05-30 3210 speed_div = 5000000;
882212f550d669 Xiaolei Wang 2024-05-30 3211 break;
882212f550d669 Xiaolei Wang 2024-05-30 3212 case SPEED_2500:
882212f550d669 Xiaolei Wang 2024-05-30 3213 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3214 speed_div = 2500000;
882212f550d669 Xiaolei Wang 2024-05-30 3215 break;
882212f550d669 Xiaolei Wang 2024-05-30 3216 case SPEED_1000:
882212f550d669 Xiaolei Wang 2024-05-30 3217 ptr = 8;
882212f550d669 Xiaolei Wang 2024-05-30 3218 speed_div = 1000000;
882212f550d669 Xiaolei Wang 2024-05-30 3219 break;
882212f550d669 Xiaolei Wang 2024-05-30 3220 case SPEED_100:
882212f550d669 Xiaolei Wang 2024-05-30 3221 ptr = 4;
882212f550d669 Xiaolei Wang 2024-05-30 3222 speed_div = 100000;
882212f550d669 Xiaolei Wang 2024-05-30 3223 break;
882212f550d669 Xiaolei Wang 2024-05-30 3224 default:
882212f550d669 Xiaolei Wang 2024-05-30 3225 netdev_dbg(priv->dev, "link speed is not known\n");
882212f550d669 Xiaolei Wang 2024-05-30 3226 }
19d9187317979c Joao Pinto 2017-03-10 3227
44781fef137896 Joao Pinto 2017-03-31 3228 /* queue 0 is reserved for legacy traffic */
44781fef137896 Joao Pinto 2017-03-31 3229 for (queue = 1; queue < tx_queues_count; queue++) {
19d9187317979c Joao Pinto 2017-03-10 3230 mode_to_use = priv->plat->tx_queues_cfg[queue].mode_to_use;
19d9187317979c Joao Pinto 2017-03-10 3231 if (mode_to_use == MTL_QUEUE_DCB)
19d9187317979c Joao Pinto 2017-03-10 3232 continue;
19d9187317979c Joao Pinto 2017-03-10 3233
882212f550d669 Xiaolei Wang 2024-05-30 @3234 value = div_s64(priv->old_idleslope[queue] * 1024ll * ptr, speed_div);
882212f550d669 Xiaolei Wang 2024-05-30 3235 priv->plat->tx_queues_cfg[queue].idle_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3236
882212f550d669 Xiaolei Wang 2024-05-30 3237 value = div_s64(-priv->old_sendslope[queue] * 1024ll * ptr, speed_div);
882212f550d669 Xiaolei Wang 2024-05-30 3238 priv->plat->tx_queues_cfg[queue].send_slope = value & GENMASK(31, 0);
882212f550d669 Xiaolei Wang 2024-05-30 3239
c10d4c82a5c84c Jose Abreu 2018-04-16 3240 stmmac_config_cbs(priv, priv->hw,
19d9187317979c Joao Pinto 2017-03-10 3241 priv->plat->tx_queues_cfg[queue].send_slope,
19d9187317979c Joao Pinto 2017-03-10 3242 priv->plat->tx_queues_cfg[queue].idle_slope,
19d9187317979c Joao Pinto 2017-03-10 3243 priv->plat->tx_queues_cfg[queue].high_credit,
19d9187317979c Joao Pinto 2017-03-10 3244 priv->plat->tx_queues_cfg[queue].low_credit,
19d9187317979c Joao Pinto 2017-03-10 3245 queue);
19d9187317979c Joao Pinto 2017-03-10 3246 }
19d9187317979c Joao Pinto 2017-03-10 3247 }
19d9187317979c Joao Pinto 2017-03-10 3248
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-06-05 5:31 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-30 6:14 [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up Xiaolei Wang
2024-05-30 6:14 ` Xiaolei Wang
2024-05-30 12:50 ` Andrew Lunn
2024-05-30 12:50 ` Andrew Lunn
2024-05-30 13:28 ` Vladimir Oltean
2024-05-30 13:28 ` Vladimir Oltean
2024-05-30 13:40 ` Russell King (Oracle)
2024-05-30 13:40 ` Russell King (Oracle)
2024-05-30 13:53 ` Vladimir Oltean
2024-05-30 13:53 ` Vladimir Oltean
2024-05-30 14:14 ` Russell King (Oracle)
2024-05-30 14:14 ` Russell King (Oracle)
2024-05-30 14:35 ` Vladimir Oltean
2024-05-30 14:35 ` Vladimir Oltean
2024-05-31 8:23 ` xiaolei wang
2024-05-31 8:23 ` xiaolei wang
2024-05-30 16:13 ` Jakub Kicinski
2024-05-30 16:13 ` Jakub Kicinski
2024-06-05 5:26 ` Dan Carpenter
2024-06-05 5:26 ` Dan Carpenter
2024-06-05 5:30 ` xiaolei wang
2024-06-05 5:30 ` xiaolei wang
-- strict thread matches above, loose matches on Subject: below --
2024-06-04 20:04 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.