[PATCH net-next] net: stmmac: fix mqprio and cbs combination

[PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Lorenzo Bianconi]

The channel-mode mqprio offload and the CBS offload can currently not be
enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
mqprio offload as soon as any TX queue is configured in AVB mode, and
tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
is active. As a result the combination is rejected whichever order the
two are configured in.

The strict-priority scheduler selected by the channel-mode mqprio
offload and the per-queue CBS shaper are independent MTL settings and do
not conflict, so drop the checks and let the combination be offloaded.
The following configuration, which was previously refused, now works:

  $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
        map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
        queues 1@0 1@1 1@2 1@3 mode channel hw 1
  $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
        idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
  $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
        idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
  $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
        idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1

Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
Tested-by: Daniel Thompson <daniel@riscstar.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  8 --------
 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 16 ----------------
 2 files changed, 24 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 56d89a696272..51d502930219 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7740,14 +7740,6 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
 	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
 		return -EINVAL;
 
-	if (priv->xmit_qdisc.prio_offload) {
-		for (i = priv->plat->tx_queues_to_use; i < tx_cnt; i++) {
-			if (priv->plat->tx_queues_cfg[i].mode_to_use ==
-			    MTL_QUEUE_AVB)
-				return -EINVAL;
-		}
-	}
-
 	if (netif_running(dev))
 		stmmac_release(dev);
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index ea1f97d6f0bd..175236ba379d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -346,9 +346,6 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
 	if (!priv->dma_cap.av)
 		return -EOPNOTSUPP;
 
-	if (qopt->enable && priv->xmit_qdisc.prio_offload)
-		return -EOPNOTSUPP;
-
 	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
 
 	if (qopt->enable) {
@@ -1340,7 +1337,6 @@ static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
 static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
 					struct tc_mqprio_qopt_offload *mqprio)
 {
-	struct plat_stmmacenet_data *pdata = priv->plat;
 	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
 	int i;
 
@@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
 		return -EOPNOTSUPP;
 	}
 
-	/* Forcing strict priority conflicts with the CBS algorithm
-	 * of AVB queues, so reject the offload when any queue is
-	 * configured as AVB.
-	 */
-	for (i = 0; i < pdata->tx_queues_to_use; i++) {
-		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
-			NL_SET_ERR_MSG_MOD(mqprio->extack,
-					   "SP conflicts with AVB queues");
-			return -EOPNOTSUPP;
-		}
-	}
-
 	for (i = 0; i < qopt->num_tc; i++) {
 		/* The offload switches the MTL scheduler to strict
 		 * priority, which only supports a 1:1 TC to TX queue

---
base-commit: d0bd617658ddfa5a115e64d1ab687bf155f6331f
change-id: 20261012-stmmac-mqprio-cbs-combo-341f12749ce2

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Daniel Thompson]

On Mon, Oct 12, 2026 at 11:14:06PM +0200, Lorenzo Bianconi wrote:
> The channel-mode mqprio offload and the CBS offload can currently not be
> enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> mqprio offload as soon as any TX queue is configured in AVB mode, and
> tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> is active. As a result the combination is rejected whichever order the
> two are configured in.
>
> The strict-priority scheduler selected by the channel-mode mqprio
> offload and the per-queue CBS shaper are independent MTL settings and do
> not conflict, so drop the checks and let the combination be offloaded.
> The following configuration, which was previously refused, now works:
>
>   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
>         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
>         queues 1@0 1@1 1@2 1@3 mode channel hw 1
>   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
>         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
>   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
>         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
>   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
>         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
>
> Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> Tested-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Also FWIW:
Reviewed-by: Daniel Thompson <daniel@riscstar.com>


Daniel.

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Maxime Chevallier]

Hi,

On 10/12/26 23:14, Lorenzo Bianconi wrote:
> The channel-mode mqprio offload and the CBS offload can currently not be
> enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> mqprio offload as soon as any TX queue is configured in AVB mode, and
> tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> is active. As a result the combination is rejected whichever order the
> two are configured in.
> 
> The strict-priority scheduler selected by the channel-mode mqprio
> offload and the per-queue CBS shaper are independent MTL settings and do
> not conflict, so drop the checks and let the combination be offloaded.
> The following configuration, which was previously refused, now works:
> 
>   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
>         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
>         queues 1@0 1@1 1@2 1@3 mode channel hw 1
>   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
>         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
>   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
>         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
>   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
>         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
> 
> Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> Tested-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Thank you Lorenzo,

Maxime

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Daniel Machon]

Hi Lorenzo,

Just a question:

> The channel-mode mqprio offload and the CBS offload can currently not be
> enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> mqprio offload as soon as any TX queue is configured in AVB mode, and
> tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> is active. As a result the combination is rejected whichever order the
> two are configured in.
> 
> The strict-priority scheduler selected by the channel-mode mqprio
> offload and the per-queue CBS shaper are independent MTL settings and do
> not conflict, so drop the checks and let the combination be offloaded.
> The following configuration, which was previously refused, now works:

Looks to me like these checks are shared by multiple cores. Are the MTL settings
independent on all of them?

> 
>   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
>         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
>         queues 1@0 1@1 1@2 1@3 mode channel hw 1
>   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
>         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
>   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
>         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
>   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
>         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
> 
> Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> Tested-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  8 --------
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 16 ----------------
>  2 files changed, 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 56d89a696272..51d502930219 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7740,14 +7740,6 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
>  	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
>  		return -EINVAL;
>  
> -	if (priv->xmit_qdisc.prio_offload) {
> -		for (i = priv->plat->tx_queues_to_use; i < tx_cnt; i++) {
> -			if (priv->plat->tx_queues_cfg[i].mode_to_use ==
> -			    MTL_QUEUE_AVB)
> -				return -EINVAL;
> -		}
> -	}
> -
>  	if (netif_running(dev))
>  		stmmac_release(dev);
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index ea1f97d6f0bd..175236ba379d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -346,9 +346,6 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
>  	if (!priv->dma_cap.av)
>  		return -EOPNOTSUPP;
>  
> -	if (qopt->enable && priv->xmit_qdisc.prio_offload)
> -		return -EOPNOTSUPP;
> -
>  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
>  
>  	if (qopt->enable) {
> @@ -1340,7 +1337,6 @@ static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
>  static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
>  					struct tc_mqprio_qopt_offload *mqprio)
>  {
> -	struct plat_stmmacenet_data *pdata = priv->plat;
>  	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
>  	int i;
>  
> @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	/* Forcing strict priority conflicts with the CBS algorithm
> -	 * of AVB queues, so reject the offload when any queue is
> -	 * configured as AVB.
> -	 */
> -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> -					   "SP conflicts with AVB queues");
> -			return -EOPNOTSUPP;
> -		}
> -	}
> -
>  	for (i = 0; i < qopt->num_tc; i++) {
>  		/* The offload switches the MTL scheduler to strict
>  		 * priority, which only supports a 1:1 TC to TX queue
> 
> ---
> base-commit: d0bd617658ddfa5a115e64d1ab687bf155f6331f
> change-id: 20261012-stmmac-mqprio-cbs-combo-341f12749ce2
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> 

/Daniel

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4973 bytes --]

On Oct 13, Daniel Machon wrote:
> Hi Lorenzo,
> 
> Just a question:
> 
> > The channel-mode mqprio offload and the CBS offload can currently not be
> > enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> > mqprio offload as soon as any TX queue is configured in AVB mode, and
> > tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> > is active. As a result the combination is rejected whichever order the
> > two are configured in.
> > 
> > The strict-priority scheduler selected by the channel-mode mqprio
> > offload and the per-queue CBS shaper are independent MTL settings and do
> > not conflict, so drop the checks and let the combination be offloaded.
> > The following configuration, which was previously refused, now works:
> 
> Looks to me like these checks are shared by multiple cores. Are the MTL settings
> independent on all of them?

Can you please explain what you mean?
What I mean with this patch is we can have a given queue (e.g. queue X)
configured in MTL_QUEUE_AVB mode and another one (e.g. queue Y) configured
in MTL_QUEUE_DCB mode.

Regards,
Lorenzo

> 
> > 
> >   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
> >         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
> >         queues 1@0 1@1 1@2 1@3 mode channel hw 1
> >   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
> >         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
> >   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
> >         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
> >   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
> >         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
> > 
> > Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> > Tested-by: Daniel Thompson <daniel@riscstar.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  8 --------
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 16 ----------------
> >  2 files changed, 24 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 56d89a696272..51d502930219 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -7740,14 +7740,6 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
> >  	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
> >  		return -EINVAL;
> >  
> > -	if (priv->xmit_qdisc.prio_offload) {
> > -		for (i = priv->plat->tx_queues_to_use; i < tx_cnt; i++) {
> > -			if (priv->plat->tx_queues_cfg[i].mode_to_use ==
> > -			    MTL_QUEUE_AVB)
> > -				return -EINVAL;
> > -		}
> > -	}
> > -
> >  	if (netif_running(dev))
> >  		stmmac_release(dev);
> >  
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > index ea1f97d6f0bd..175236ba379d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > @@ -346,9 +346,6 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
> >  	if (!priv->dma_cap.av)
> >  		return -EOPNOTSUPP;
> >  
> > -	if (qopt->enable && priv->xmit_qdisc.prio_offload)
> > -		return -EOPNOTSUPP;
> > -
> >  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> >  
> >  	if (qopt->enable) {
> > @@ -1340,7 +1337,6 @@ static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
> >  static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> >  					struct tc_mqprio_qopt_offload *mqprio)
> >  {
> > -	struct plat_stmmacenet_data *pdata = priv->plat;
> >  	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
> >  	int i;
> >  
> > @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> >  		return -EOPNOTSUPP;
> >  	}
> >  
> > -	/* Forcing strict priority conflicts with the CBS algorithm
> > -	 * of AVB queues, so reject the offload when any queue is
> > -	 * configured as AVB.
> > -	 */
> > -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> > -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> > -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> > -					   "SP conflicts with AVB queues");
> > -			return -EOPNOTSUPP;
> > -		}
> > -	}
> > -
> >  	for (i = 0; i < qopt->num_tc; i++) {
> >  		/* The offload switches the MTL scheduler to strict
> >  		 * priority, which only supports a 1:1 TC to TX queue
> > 
> > ---
> > base-commit: d0bd617658ddfa5a115e64d1ab687bf155f6331f
> > change-id: 20261012-stmmac-mqprio-cbs-combo-341f12749ce2
> > 
> > Best regards,
> > -- 
> > Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > 
> 
> /Daniel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Daniel Machon]

> On Oct 13, Daniel Machon wrote:
> > Hi Lorenzo,
> > 
> > Just a question:
> > 
> > > The channel-mode mqprio offload and the CBS offload can currently not be
> > > enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> > > mqprio offload as soon as any TX queue is configured in AVB mode, and
> > > tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> > > is active. As a result the combination is rejected whichever order the
> > > two are configured in.
> > > 
> > > The strict-priority scheduler selected by the channel-mode mqprio
> > > offload and the per-queue CBS shaper are independent MTL settings and do
> > > not conflict, so drop the checks and let the combination be offloaded.
> > > The following configuration, which was previously refused, now works:
> > 
> > Looks to me like these checks are shared by multiple cores. Are the MTL settings
> > independent on all of them?
> 
> Can you please explain what you mean?
> What I mean with this patch is we can have a given queue (e.g. queue X)
> configured in MTL_QUEUE_AVB mode and another one (e.g. queue Y) configured
> in MTL_QUEUE_DCB mode.

Sorry for being vague. I meant the MAC cores. So, dwmac510_tc_ops is being used
by GMAC, XGMAC, XLGMAC. My question was whether the claim that the commit
description gives (per-queue CBS shaper are independent MTL settings) apply to
all of these 3?

> 
> Regards,
> Lorenzo
> 
> > 
> > > 
> > >   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
> > >         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
> > >         queues 1@0 1@1 1@2 1@3 mode channel hw 1
> > >   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
> > >         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
> > >   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
> > >         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
> > >   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
> > >         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
> > > 
> > > Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> > > Tested-by: Daniel Thompson <daniel@riscstar.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > ---
> > >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  8 --------
> > >  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 16 ----------------
> > >  2 files changed, 24 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > index 56d89a696272..51d502930219 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > @@ -7740,14 +7740,6 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
> > >  	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
> > >  		return -EINVAL;
> > >  
> > > -	if (priv->xmit_qdisc.prio_offload) {
> > > -		for (i = priv->plat->tx_queues_to_use; i < tx_cnt; i++) {
> > > -			if (priv->plat->tx_queues_cfg[i].mode_to_use ==
> > > -			    MTL_QUEUE_AVB)
> > > -				return -EINVAL;
> > > -		}
> > > -	}
> > > -
> > >  	if (netif_running(dev))
> > >  		stmmac_release(dev);
> > >  
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > index ea1f97d6f0bd..175236ba379d 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > @@ -346,9 +346,6 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
> > >  	if (!priv->dma_cap.av)
> > >  		return -EOPNOTSUPP;
> > >  
> > > -	if (qopt->enable && priv->xmit_qdisc.prio_offload)
> > > -		return -EOPNOTSUPP;
> > > -
> > >  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> > >  
> > >  	if (qopt->enable) {
> > > @@ -1340,7 +1337,6 @@ static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
> > >  static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> > >  					struct tc_mqprio_qopt_offload *mqprio)
> > >  {
> > > -	struct plat_stmmacenet_data *pdata = priv->plat;
> > >  	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
> > >  	int i;
> > >  
> > > @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> > >  		return -EOPNOTSUPP;
> > >  	}
> > >  
> > > -	/* Forcing strict priority conflicts with the CBS algorithm
> > > -	 * of AVB queues, so reject the offload when any queue is
> > > -	 * configured as AVB.
> > > -	 */
> > > -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> > > -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> > > -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> > > -					   "SP conflicts with AVB queues");
> > > -			return -EOPNOTSUPP;
> > > -		}
> > > -	}
> > > -
> > >  	for (i = 0; i < qopt->num_tc; i++) {
> > >  		/* The offload switches the MTL scheduler to strict
> > >  		 * priority, which only supports a 1:1 TC to TX queue
> > > 
> > > ---
> > > base-commit: d0bd617658ddfa5a115e64d1ab687bf155f6331f
> > > change-id: 20261012-stmmac-mqprio-cbs-combo-341f12749ce2
> > > 
> > > Best regards,
> > > -- 
> > > Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > 
> > 
> > /Daniel

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 5970 bytes --]

> > On Oct 13, Daniel Machon wrote:
> > > Hi Lorenzo,
> > > 
> > > Just a question:
> > > 
> > > > The channel-mode mqprio offload and the CBS offload can currently not be
> > > > enabled at the same time. tc_mqprio_validate_chan_mode() refuses the
> > > > mqprio offload as soon as any TX queue is configured in AVB mode, and
> > > > tc_setup_cbs() refuses to enable CBS while the strict-priority scheduler
> > > > is active. As a result the combination is rejected whichever order the
> > > > two are configured in.
> > > > 
> > > > The strict-priority scheduler selected by the channel-mode mqprio
> > > > offload and the per-queue CBS shaper are independent MTL settings and do
> > > > not conflict, so drop the checks and let the combination be offloaded.
> > > > The following configuration, which was previously refused, now works:
> > > 
> > > Looks to me like these checks are shared by multiple cores. Are the MTL settings
> > > independent on all of them?
> > 
> > Can you please explain what you mean?
> > What I mean with this patch is we can have a given queue (e.g. queue X)
> > configured in MTL_QUEUE_AVB mode and another one (e.g. queue Y) configured
> > in MTL_QUEUE_DCB mode.
> 
> Sorry for being vague. I meant the MAC cores. So, dwmac510_tc_ops is being used
> by GMAC, XGMAC, XLGMAC. My question was whether the claim that the commit
> description gives (per-queue CBS shaper are independent MTL settings) apply to
> all of these 3?

Yes, according to my understanding. MTL_QUEUE_AVB and MTL_QUEUE_DCB are
mutually exclusive for a specific queue, but there is no restriction on two
different hw queues (MTL_QUEUE_AVB and MTL_QUEUE_DCB can be mixed).

Regards,
Lorenzo

> 
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > > 
> > > >   $ tc qdisc replace dev eth0 handle 100 parent root mqprio num_tc 4 \
> > > >         map  0  0  0  0  0  1  2  3  0  0  0  0  0  0  0  0 \
> > > >         queues 1@0 1@1 1@2 1@3 mode channel hw 1
> > > >   $ tc qdisc replace dev eth0 parent 100:2 handle 102: cbs \
> > > >         idleslope 800000 sendslope -1700000 hicredit 494 locredit -1049 offload 0
> > > >   $ tc qdisc replace dev eth0 parent 100:3 handle 103: cbs \
> > > >         idleslope 400000 sendslope -2100000 hicredit 247 locredit -1296 offload 1
> > > >   $ tc qdisc replace dev eth0 parent 100:4 handle 104: cbs \
> > > >         idleslope 200000 sendslope -2300000 hicredit 124 locredit -1419 offload 1
> > > > 
> > > > Fixes: 47891d501e34 ("net: stmmac: add tc mqprio hw queue priority offload")
> > > > Tested-by: Daniel Thompson <daniel@riscstar.com>
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > > ---
> > > >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  8 --------
> > > >  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 16 ----------------
> > > >  2 files changed, 24 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > index 56d89a696272..51d502930219 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > @@ -7740,14 +7740,6 @@ int stmmac_reinit_queues(struct net_device *dev, u8 rx_cnt, u8 tx_cnt)
> > > >  	    tx_cnt < priv->xmit_qdisc.num_tx_queues)
> > > >  		return -EINVAL;
> > > >  
> > > > -	if (priv->xmit_qdisc.prio_offload) {
> > > > -		for (i = priv->plat->tx_queues_to_use; i < tx_cnt; i++) {
> > > > -			if (priv->plat->tx_queues_cfg[i].mode_to_use ==
> > > > -			    MTL_QUEUE_AVB)
> > > > -				return -EINVAL;
> > > > -		}
> > > > -	}
> > > > -
> > > >  	if (netif_running(dev))
> > > >  		stmmac_release(dev);
> > > >  
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > > index ea1f97d6f0bd..175236ba379d 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > > > @@ -346,9 +346,6 @@ static int tc_setup_cbs(struct stmmac_priv *priv,
> > > >  	if (!priv->dma_cap.av)
> > > >  		return -EOPNOTSUPP;
> > > >  
> > > > -	if (qopt->enable && priv->xmit_qdisc.prio_offload)
> > > > -		return -EOPNOTSUPP;
> > > > -
> > > >  	port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope;
> > > >  
> > > >  	if (qopt->enable) {
> > > > @@ -1340,7 +1337,6 @@ static void tc_mqprio_config_queue_prio(struct stmmac_priv *priv,
> > > >  static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> > > >  					struct tc_mqprio_qopt_offload *mqprio)
> > > >  {
> > > > -	struct plat_stmmacenet_data *pdata = priv->plat;
> > > >  	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
> > > >  	int i;
> > > >  
> > > > @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> > > >  		return -EOPNOTSUPP;
> > > >  	}
> > > >  
> > > > -	/* Forcing strict priority conflicts with the CBS algorithm
> > > > -	 * of AVB queues, so reject the offload when any queue is
> > > > -	 * configured as AVB.
> > > > -	 */
> > > > -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> > > > -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> > > > -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> > > > -					   "SP conflicts with AVB queues");
> > > > -			return -EOPNOTSUPP;
> > > > -		}
> > > > -	}
> > > > -
> > > >  	for (i = 0; i < qopt->num_tc; i++) {
> > > >  		/* The offload switches the MTL scheduler to strict
> > > >  		 * priority, which only supports a 1:1 TC to TX queue
> > > > 
> > > > ---
> > > > base-commit: d0bd617658ddfa5a115e64d1ab687bf155f6331f
> > > > change-id: 20261012-stmmac-mqprio-cbs-combo-341f12749ce2
> > > > 
> > > > Best regards,
> > > > -- 
> > > > Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > > 
> > > 
> > > /Daniel
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The commit message says the strict-priority scheduler selected
  by the channel-mode mqprio offload and the per-queue CBS shaper "are…

--

net: stmmac: fix mqprio and cbs combination

This patch removes the checks that stopped the channel-mode mqprio offload
and the CBS offload from being enabled together. It drops the AVB queue
rejection in tc_mqprio_validate_chan_mode(), the prio_offload check in
tc_setup_cbs(), and the matching AVB check in stmmac_reinit_queues().

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index ea1f97d6f0bd5..175236ba379d0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

[ ... ]

> @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	/* Forcing strict priority conflicts with the CBS algorithm
> -	 * of AVB queues, so reject the offload when any queue is
> -	 * configured as AVB.
> -	 */
> -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> -					   "SP conflicts with AVB queues");
> -			return -EOPNOTSUPP;
> -		}
> -	}
> -

[Severity: High]

The commit message says:

  The strict-priority scheduler selected by the channel-mode mqprio
  offload and the per-queue CBS shaper are independent MTL settings and do
  not conflict

Is that true on XGMAC/XLGMAC cores? They use dwmac510_tc_ops, so both
tc_setup_dwmac510_mqprio() and tc_setup_cbs() can be reached on them.

On XGMAC, SP and CBS look like two values of the same per-TC TSA field.
dwxgmac2_prog_mtl_tx_algorithms() rewrites that field on every TC:

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:dwxgmac2_prog_mtl_tx_algorithms() {
    ...
	for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
		value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(i));
		value &= ~XGMAC_TSA;
		if (ets)
			value |= XGMAC_ETS;
		writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(i));
	}
}

dwxgmac2_config_cbs() turns CBS on by writing the same field:

drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:dwxgmac2_config_cbs() {
    ...
	value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
	value &= ~XGMAC_TSA;
	value |= XGMAC_CC | XGMAC_CBS;
	writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
}

With this check gone, what happens if a TX queue is already in AVB mode?
One way to get there is a DT mtl-tx-config queue with snps,avb-algorithm.
In that case stmmac_open()->stmmac_mtl_configuration() programs CBS for the
queue when the device is opened.

After that, "tc qdisc replace ... mqprio ... mode channel hw 1" now
succeeds and runs:

tc_setup_dwmac510_mqprio()
  tc_mqprio_validate_chan_mode()
  tc_mqprio_config_queue_prio()
    stmmac_prog_mtl_tx_algorithms(priv, priv->hw, MTL_TX_ALGORITHM_SP)
      dwxgmac2_prog_mtl_tx_algorithms()

Would this also clear TSA on the AVB queue and turn CBS off in hardware?
Nothing calls stmmac_configure_cbs() again afterwards. Meanwhile
tx_queues_cfg[].mode_to_use still says MTL_QUEUE_AVB, so the driver state
and the hardware would disagree until the next ndo_open.

Removing mqprio seems to cause the same problem. prio_offload can now be
true while AVB queues exist, and stmmac_reset_tc_mqprio() then does:

	stmmac_prog_mtl_tx_algorithms(priv, priv->hw,
				      priv->plat->tx_sched_algorithm);

On XGMAC this rewrites TSA on every TC again (to ETS or SP). That would
wipe CBS on any AVB queue and not restore it.

The example in the commit message (mqprio first, then the cbs children)
works because config_cbs runs after the SP write.

On dwmac4/EQoS the claim does seem to hold.
dwmac4_prog_mtl_tx_algorithms() only touches MTL_OPERATION_SCHALG_MASK, and
CBS is enabled separately through MTL_ETS_CTRL_AVALG.

The commit in the Fixes: tag added these checks on purpose:

  Forcing strict priority conflicts with the CBS algorithm of AVB queues

Should the checks be kept for XGMAC? Or could the mqprio set and reset
paths call stmmac_configure_cbs() again after reprogramming the TX
algorithm?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-mqprio-cbs-combo-v1-1-831e323eb4ef%40oss.qualcomm.com

Re: [PATCH net-next] net: stmmac: fix mqprio and cbs combination

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 5354 bytes --]

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The commit message says the strict-priority scheduler selected
>   by the channel-mode mqprio offload and the per-queue CBS shaper "are…
> 
> --
> 
> net: stmmac: fix mqprio and cbs combination
> 
> This patch removes the checks that stopped the channel-mode mqprio offload
> and the CBS offload from being enabled together. It drops the AVB queue
> rejection in tc_mqprio_validate_chan_mode(), the prio_offload check in
> tc_setup_cbs(), and the matching AVB check in stmmac_reinit_queues().
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > index ea1f97d6f0bd5..175236ba379d0 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> 
> [ ... ]
> 
> > @@ -1350,18 +1346,6 @@ static int tc_mqprio_validate_chan_mode(struct stmmac_priv *priv,
> >  		return -EOPNOTSUPP;
> >  	}
> >  
> > -	/* Forcing strict priority conflicts with the CBS algorithm
> > -	 * of AVB queues, so reject the offload when any queue is
> > -	 * configured as AVB.
> > -	 */
> > -	for (i = 0; i < pdata->tx_queues_to_use; i++) {
> > -		if (pdata->tx_queues_cfg[i].mode_to_use == MTL_QUEUE_AVB) {
> > -			NL_SET_ERR_MSG_MOD(mqprio->extack,
> > -					   "SP conflicts with AVB queues");
> > -			return -EOPNOTSUPP;
> > -		}
> > -	}
> > -
> 
> [Severity: High]
> 
> The commit message says:
> 
>   The strict-priority scheduler selected by the channel-mode mqprio
>   offload and the per-queue CBS shaper are independent MTL settings and do
>   not conflict
> 
> Is that true on XGMAC/XLGMAC cores? They use dwmac510_tc_ops, so both
> tc_setup_dwmac510_mqprio() and tc_setup_cbs() can be reached on them.
> 
> On XGMAC, SP and CBS look like two values of the same per-TC TSA field.
> dwxgmac2_prog_mtl_tx_algorithms() rewrites that field on every TC:
> 
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:dwxgmac2_prog_mtl_tx_algorithms() {
>     ...
> 	for (i = 0; i < MTL_MAX_TX_QUEUES; i++) {
> 		value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(i));
> 		value &= ~XGMAC_TSA;
> 		if (ets)
> 			value |= XGMAC_ETS;
> 		writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(i));
> 	}
> }
> 
> dwxgmac2_config_cbs() turns CBS on by writing the same field:
> 
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:dwxgmac2_config_cbs() {
>     ...
> 	value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
> 	value &= ~XGMAC_TSA;
> 	value |= XGMAC_CC | XGMAC_CBS;
> 	writel(value, ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(queue));
> }
> 
> With this check gone, what happens if a TX queue is already in AVB mode?
> One way to get there is a DT mtl-tx-config queue with snps,avb-algorithm.
> In that case stmmac_open()->stmmac_mtl_configuration() programs CBS for the
> queue when the device is opened.

I guess the only case that is not covered yet is if the user configures CBS
via the device-tree and then applies mqprio via tc on top of it.
I will fix in v2.

Regards,
Lorenzo

> 
> After that, "tc qdisc replace ... mqprio ... mode channel hw 1" now
> succeeds and runs:
> 
> tc_setup_dwmac510_mqprio()
>   tc_mqprio_validate_chan_mode()
>   tc_mqprio_config_queue_prio()
>     stmmac_prog_mtl_tx_algorithms(priv, priv->hw, MTL_TX_ALGORITHM_SP)
>       dwxgmac2_prog_mtl_tx_algorithms()
> 
> Would this also clear TSA on the AVB queue and turn CBS off in hardware?
> Nothing calls stmmac_configure_cbs() again afterwards. Meanwhile
> tx_queues_cfg[].mode_to_use still says MTL_QUEUE_AVB, so the driver state
> and the hardware would disagree until the next ndo_open.
> 
> Removing mqprio seems to cause the same problem. prio_offload can now be
> true while AVB queues exist, and stmmac_reset_tc_mqprio() then does:
> 
> 	stmmac_prog_mtl_tx_algorithms(priv, priv->hw,
> 				      priv->plat->tx_sched_algorithm);
> 
> On XGMAC this rewrites TSA on every TC again (to ETS or SP). That would
> wipe CBS on any AVB queue and not restore it.
> 
> The example in the commit message (mqprio first, then the cbs children)
> works because config_cbs runs after the SP write.
> 
> On dwmac4/EQoS the claim does seem to hold.
> dwmac4_prog_mtl_tx_algorithms() only touches MTL_OPERATION_SCHALG_MASK, and
> CBS is enabled separately through MTL_ETS_CTRL_AVALG.
> 
> The commit in the Fixes: tag added these checks on purpose:
> 
>   Forcing strict priority conflicts with the CBS algorithm of AVB queues
> 
> Should the checks be kept for XGMAC? Or could the mqprio set and reset
> paths call stmmac_configure_cbs() again after reprogramming the TX
> algorithm?
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-stmmac-mqprio-cbs-combo-v1-1-831e323eb4ef%40oss.qualcomm.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]