2024-02-28 14:04:28

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] net: ethernet: ti: am65-cpsw-qos: fix non-bql configs

From: Arnd Bergmann <[email protected]>

It is now possible to disable BQL, but that causes cpsw to break:

drivers/net/ethernet/ti/am65-cpsw-nuss.c:297:28: error: no member named 'dql' in 'struct netdev_queue'
297 | dql_avail(&netif_txq->dql),

Add an #ifdef check for CONFIG_BQL around this usage.

Fixes: ea7f3cfaa588 ("net: bql: allow the config to be disabled")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 9d2f4ac783e4..3a3ec9959ee2 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -294,7 +294,11 @@ static void am65_cpsw_nuss_ndo_host_tx_timeout(struct net_device *ndev,
txqueue,
netif_tx_queue_stopped(netif_txq),
jiffies_to_msecs(jiffies - trans_start),
+#ifdef CONFIG_BQL
dql_avail(&netif_txq->dql),
+#else
+ 0,
+#endif
k3_cppi_desc_pool_avail(tx_chn->desc_pool));

if (netif_tx_queue_stopped(netif_txq)) {
--
2.39.2



2024-02-28 14:45:43

by Dave Taht

[permalink] [raw]
Subject: Re: [PATCH] net: ethernet: ti: am65-cpsw-qos: fix non-bql configs

but why do you want to disable BQL?

On Wed, Feb 28, 2024 at 9:04 AM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> It is now possible to disable BQL, but that causes cpsw to break:
>
> drivers/net/ethernet/ti/am65-cpsw-nuss.c:297:28: error: no member named 'dql' in 'struct netdev_queue'
> 297 | dql_avail(&netif_txq->dql),
>
> Add an #ifdef check for CONFIG_BQL around this usage.
>
> Fixes: ea7f3cfaa588 ("net: bql: allow the config to be disabled")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 9d2f4ac783e4..3a3ec9959ee2 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -294,7 +294,11 @@ static void am65_cpsw_nuss_ndo_host_tx_timeout(struct net_device *ndev,
> txqueue,
> netif_tx_queue_stopped(netif_txq),
> jiffies_to_msecs(jiffies - trans_start),
> +#ifdef CONFIG_BQL
> dql_avail(&netif_txq->dql),
> +#else
> + 0,
> +#endif
> k3_cppi_desc_pool_avail(tx_chn->desc_pool));
>
> if (netif_tx_queue_stopped(netif_txq)) {
> --
> 2.39.2
>
>


--
https://blog.cerowrt.org/post/2024_predictions/
Dave Täht CSO, LibreQos

2024-02-28 14:54:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] net: ethernet: ti: am65-cpsw-qos: fix non-bql configs

On Wed, Feb 28, 2024, at 15:44, Dave Taht wrote:
> but why do you want to disable BQL?

I have no idea, I'm just doing randconfig build tests.

I assume Breno has an answer for that, at least he
sent the patch that triggered the regression, see below.

Arnd

commit ea7f3cfaa58873bbe271577efa800647e30f18bd
Author: Breno Leitao <[email protected]>
Date: Thu Feb 15 09:05:07 2024 -0800

net: bql: allow the config to be disabled

It is impossible to disable BQL individually today, since there is no
prompt for the Kconfig entry, so, the BQL is always enabled if SYSFS is
enabled.

Create a prompt entry for BQL, so, it could be enabled or disabled at
build time independently of SYSFS.

Signed-off-by: Breno Leitao <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

diff --git a/net/Kconfig b/net/Kconfig
index 4adc47d0c9c2..3e57ccf0da27 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -331,6 +331,7 @@ config NET_RX_BUSY_POLL

config BQL
bool
+ prompt "Enable Byte Queue Limits"
depends on SYSFS
select DQL
default y

2024-02-28 14:57:07

by Breno Leitao

[permalink] [raw]
Subject: Re: [PATCH] net: ethernet: ti: am65-cpsw-qos: fix non-bql configs

On Wed, Feb 28, 2024 at 03:03:10PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> It is now possible to disable BQL, but that causes cpsw to break:
>
> drivers/net/ethernet/ti/am65-cpsw-nuss.c:297:28: error: no member named 'dql' in 'struct netdev_queue'
> 297 | dql_avail(&netif_txq->dql),
>
> Add an #ifdef check for CONFIG_BQL around this usage.

I confirmed that all other cases where queue->dql is accessed, it is
inside a #ifdef CONFIG_BQL block, so, seems appropriate doing the same
here.

> Fixes: ea7f3cfaa588 ("net: bql: allow the config to be disabled")
> Signed-off-by: Arnd Bergmann <[email protected]>

Reviwed-by: Breno Leitao <[email protected]>

2024-02-28 15:52:38

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH] net: ethernet: ti: am65-cpsw-qos: fix non-bql configs

On Wed, 28 Feb 2024 15:03:10 +0100 Arnd Bergmann wrote:
> +#ifdef CONFIG_BQL
> dql_avail(&netif_txq->dql),
> +#else
> + 0,
> +#endif

Same exact code exists in qdisc_avail_bulklimit()
How about we add static inline in netdevice.h wrapping
this, like we do for other BQL related functionality?
(taking in txq as the argument)