| Message ID | 20240228140413.1862310-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: ti: am65-cpsw-qos: fix non-bql configs | expand |
but why do you want to disable BQL? On Wed, Feb 28, 2024 at 9:04 AM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > 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 <arnd@arndb.de> > --- > 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 > >
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 <leitao@debian.org> 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 <leitao@debian.org> Signed-off-by: David S. Miller <davem@davemloft.net> 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
On Wed, Feb 28, 2024 at 03:03:10PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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 <arnd@arndb.de> Reviwed-by: Breno Leitao <leitao@debian.org>
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)
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)) {