[PATCH net-next 0/2] net: dsa: mv88e6xxx: add support for credit based shaper

[PATCH net-next 0/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Cedric Jehasse via B4 Relay]

Several of the switch families in this driver have switches with AVB
support. The switches with AVB support have support for Credit based
shaping. This series adds support for the 6352, 6390 and 6393 families.
The difference between the families is:
- total number of queues
- which queues support credit based shaping
- shaping granularity

Eg. setting up 20mbps credit based shaper on a 1GBit link:
tc qdisc add dev p8 parent root handle 100: mqprio \
    num_tc 8 \
    map 0 0 6 7 0 5 0 0 0 0 0 0 0 0 0 0 \
    queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \
    hw 0

tc qdisc replace dev p8 parent 100:8 cbs locredit -1470 hicredit 30 \
    sendslope -980000 idleslope 20000 offload 1

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
Cedric Jehasse (2):
      net: dsa: mv88e6xxx: use the hw tx queues
      net: dsa: mv88e6xxx: add support for credit based shaper

 drivers/net/dsa/mv88e6xxx/chip.c        | 137 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h        |  22 +++++
 drivers/net/dsa/mv88e6xxx/global2_avb.c |  21 +++++
 drivers/net/dsa/mv88e6xxx/port.c        |  38 +++++++++
 drivers/net/dsa/mv88e6xxx/port.h        |  16 ++++
 net/dsa/tag_dsa.c                       |   3 +-
 6 files changed, 236 insertions(+), 1 deletion(-)
---
base-commit: 022bdd9c0d036863c4bacd1688b73c6be3001cee
change-id: 20260430-net-next-mv88e6xxx-cbs-2121169caa68

Best regards,
-- 
Cedric Jehasse <cedric.jehasse@luminex.be>

[PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues

[Cedric Jehasse via B4 Relay]

From: Cedric Jehasse <cedric.jehasse@luminex.be>

From the datasheets i've looked at these switches have 4 or 8 transmit
queues per port.
There's a PRI field in the dsa tag used to indicate which egress queue
the frame is to be sent to.
This isn't done for vlan tagged frames because this would overwrite the
PCP value in the vlan tag (The PRI field in the dsa
tag is used as the PCP value in the vlan tag).

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 14 ++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h |  1 +
 net/dsa/tag_dsa.c                |  3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8ca5fd40df92..277efe24edf4 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3979,6 +3979,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 
 	chip->ds = ds;
 	ds->user_mii_bus = mv88e6xxx_default_mdio_bus(chip);
+	ds->num_tx_queues = chip->info->num_tx_queues;
 
 	/* Since virtual bridges are mapped in the PVT, the number we support
 	 * depends on the physical switch topology. We need to let DSA figure
@@ -6051,6 +6052,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6076,6 +6078,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6125,6 +6128,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 8,
 		.internal_phys_offset = 1,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6151,6 +6155,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 8,
 		.num_tcam_entries = 256,
 		.internal_phys_offset = 1,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6181,6 +6186,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 7,
 		.num_internal_phys = 2,
 		.invalid_port_mask = BIT(2) | BIT(3) | BIT(4),
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.port_base_addr = 0x08,
 		.phy_base_addr = 0x00,
@@ -6205,6 +6211,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.max_sid = 63,
 		.port_base_addr = 0x10,
@@ -6230,6 +6237,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_databases = 64,
 		.num_ports = 7,
 		.num_internal_phys = 5,
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.port_base_addr = 0x08,
 		.phy_base_addr = 0x00,
@@ -6282,6 +6290,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 2,
 		.internal_phys_offset = 3,
 		.num_gpio = 15,
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.max_sid = 63,
 		.port_base_addr = 0x10,
@@ -6310,6 +6319,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 2,
 		.internal_phys_offset = 3,
 		.num_gpio = 15,
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.max_sid = 63,
 		.port_base_addr = 0x10,
@@ -6414,6 +6424,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 7,
 		.num_internal_phys = 5,
 		.num_gpio = 15,
+		.num_tx_queues = 4,
 		.max_vid = 4095,
 		.max_sid = 63,
 		.port_base_addr = 0x10,
@@ -6468,6 +6479,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_internal_phys = 9,
 		.num_gpio = 16,
 		.num_tcam_entries = 256,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6495,6 +6507,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
+		.num_tx_queues = 8,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
@@ -6521,6 +6534,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 8,
 		.num_tcam_entries = 256,
+		.num_tx_queues = 8,
 		.internal_phys_offset = 1,
 		.max_vid = 8191,
 		.max_sid = 63,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index cde71828e9d9..19d8eda19b78 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -136,6 +136,7 @@ struct mv88e6xxx_info {
 	unsigned int num_internal_phys;
 	unsigned int num_gpio;
 	unsigned int num_tcam_entries;
+	unsigned int num_tx_queues;
 	unsigned int max_vid;
 	unsigned int max_sid;
 	unsigned int port_base_addr;
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 2a2c4fb61a65..96c8fbedbd3b 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -179,6 +179,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 			dsa_header[2] &= ~0x10;
 		}
 	} else {
+		u16 queue = skb_get_queue_mapping(skb) & 0x7;
 		u16 vid;
 
 		vid = br_dev ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
@@ -191,7 +192,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
 
 		dsa_header[0] = (cmd << 6) | tag_dev;
 		dsa_header[1] = tag_port << 3;
-		dsa_header[2] = vid >> 8;
+		dsa_header[2] = (queue << 5) | vid >> 8;
 		dsa_header[3] = vid & 0xff;
 	}
 

-- 
2.43.0

Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues

[Marek Behún]

Num tx queues for all devices:

88E6020		4
88E6071		4
88E6085		4
88E6095		4
88E6097		4
88E6123		4
88E6131		4
88E6141		4
88E6161		4
88E6165		4
88E6171		4
88E6172		4
88E6175		4
88E6176		4
88E6185		4
88E6190		8 from you
88E6190X	8 from you
88E6191		n/a can't find datasheet, does this even exist?
88E6191X	8 from you
88E6193X	8 from you
88E6220		4 from you
88E6240		4 from you
88E6250		4 from you
88E6290		8
88E6320		4 from you
88E6321		4 from you
88E6341		4
88E6350		4
88E6351		4
88E6352		4 from you
88E6361		8
88E6390		8 from you
88E6390X	8 from you
88E6393X	8 from you

Marek

On Fri, May 22, 2026 at 12:56:22PM +0200, Cedric Jehasse via B4 Relay wrote:
> From: Cedric Jehasse <cedric.jehasse@luminex.be>
> 
> From the datasheets i've looked at these switches have 4 or 8 transmit
> queues per port.
> There's a PRI field in the dsa tag used to indicate which egress queue
> the frame is to be sent to.
> This isn't done for vlan tagged frames because this would overwrite the
> PCP value in the vlan tag (The PRI field in the dsa
> tag is used as the PCP value in the vlan tag).
> 
> Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 14 ++++++++++++++
>  drivers/net/dsa/mv88e6xxx/chip.h |  1 +
>  net/dsa/tag_dsa.c                |  3 ++-
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 8ca5fd40df92..277efe24edf4 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -3979,6 +3979,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>  
>  	chip->ds = ds;
>  	ds->user_mii_bus = mv88e6xxx_default_mdio_bus(chip);
> +	ds->num_tx_queues = chip->info->num_tx_queues;
>  
>  	/* Since virtual bridges are mapped in the PVT, the number we support
>  	 * depends on the physical switch topology. We need to let DSA figure
> @@ -6051,6 +6052,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 11,	/* 10 + Z80 */
>  		.num_internal_phys = 9,
>  		.num_gpio = 16,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6076,6 +6078,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 11,	/* 10 + Z80 */
>  		.num_internal_phys = 9,
>  		.num_gpio = 16,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6125,6 +6128,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 11,	/* 10 + Z80 */
>  		.num_internal_phys = 8,
>  		.internal_phys_offset = 1,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6151,6 +6155,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_internal_phys = 8,
>  		.num_tcam_entries = 256,
>  		.internal_phys_offset = 1,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6181,6 +6186,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 7,
>  		.num_internal_phys = 2,
>  		.invalid_port_mask = BIT(2) | BIT(3) | BIT(4),
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.port_base_addr = 0x08,
>  		.phy_base_addr = 0x00,
> @@ -6205,6 +6211,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 7,
>  		.num_internal_phys = 5,
>  		.num_gpio = 15,
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.max_sid = 63,
>  		.port_base_addr = 0x10,
> @@ -6230,6 +6237,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_databases = 64,
>  		.num_ports = 7,
>  		.num_internal_phys = 5,
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.port_base_addr = 0x08,
>  		.phy_base_addr = 0x00,
> @@ -6282,6 +6290,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_internal_phys = 2,
>  		.internal_phys_offset = 3,
>  		.num_gpio = 15,
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.max_sid = 63,
>  		.port_base_addr = 0x10,
> @@ -6310,6 +6319,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_internal_phys = 2,
>  		.internal_phys_offset = 3,
>  		.num_gpio = 15,
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.max_sid = 63,
>  		.port_base_addr = 0x10,
> @@ -6414,6 +6424,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 7,
>  		.num_internal_phys = 5,
>  		.num_gpio = 15,
> +		.num_tx_queues = 4,
>  		.max_vid = 4095,
>  		.max_sid = 63,
>  		.port_base_addr = 0x10,
> @@ -6468,6 +6479,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_internal_phys = 9,
>  		.num_gpio = 16,
>  		.num_tcam_entries = 256,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6495,6 +6507,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 11,	/* 10 + Z80 */
>  		.num_internal_phys = 9,
>  		.num_gpio = 16,
> +		.num_tx_queues = 8,
>  		.max_vid = 8191,
>  		.max_sid = 63,
>  		.port_base_addr = 0x0,
> @@ -6521,6 +6534,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  		.num_ports = 11,	/* 10 + Z80 */
>  		.num_internal_phys = 8,
>  		.num_tcam_entries = 256,
> +		.num_tx_queues = 8,
>  		.internal_phys_offset = 1,
>  		.max_vid = 8191,
>  		.max_sid = 63,
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index cde71828e9d9..19d8eda19b78 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -136,6 +136,7 @@ struct mv88e6xxx_info {
>  	unsigned int num_internal_phys;
>  	unsigned int num_gpio;
>  	unsigned int num_tcam_entries;
> +	unsigned int num_tx_queues;
>  	unsigned int max_vid;
>  	unsigned int max_sid;
>  	unsigned int port_base_addr;
> diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
> index 2a2c4fb61a65..96c8fbedbd3b 100644
> --- a/net/dsa/tag_dsa.c
> +++ b/net/dsa/tag_dsa.c
> @@ -179,6 +179,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>  			dsa_header[2] &= ~0x10;
>  		}
>  	} else {
> +		u16 queue = skb_get_queue_mapping(skb) & 0x7;
>  		u16 vid;
>  
>  		vid = br_dev ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
> @@ -191,7 +192,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>  
>  		dsa_header[0] = (cmd << 6) | tag_dev;
>  		dsa_header[1] = tag_port << 3;
> -		dsa_header[2] = vid >> 8;
> +		dsa_header[2] = (queue << 5) | vid >> 8;
>  		dsa_header[3] = vid & 0xff;
>  	}
>  
> 
> -- 
> 2.43.0
> 
> 
> 

Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues

[Cedric Jehasse]

> From: Marek Behún <kabel@kernel.org>
> Sent: Monday, May 25, 2026 11:26 AM
> To: Cedric Jehasse
> Cc: Andrew Lunn; Vladimir Oltean; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Simon Horman; Russell King; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues
>
> Num tx queues for all devices:
>
> 88E6020         4
> 88E6071         4
> 88E6085         4
> 88E6095         4
> 88E6097         4
> 88E6123         4
> 88E6131         4
> 88E6141         4
> 88E6161         4
> 88E6165         4
> 88E6171         4
> 88E6172         4
> 88E6175         4
> 88E6176         4
> 88E6185         4
> 88E6190         8 from you
> 88E6190X        8 from you
> 88E6191         n/a can't find datasheet, does this even exist?
> 88E6191X        8 from you
> 88E6193X        8 from you
> 88E6220         4 from you
> 88E6240         4 from you
> 88E6250         4 from you
> 88E6290         8
> 88E6320         4 from you
> 88E6321         4 from you
> 88E6341         4
> 88E6350         4
> 88E6351         4
> 88E6352         4 from you
> 88E6361         8
> 88E6390         8 from you
> 88E6390X        8 from you
> 88E6393X        8 from you
>
> Marek

These will be added in the next patch revision.

Thanks,
Cedric

Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues

[Andrew Lunn]

On Mon, May 25, 2026 at 11:26:10AM +0200, Marek Behún wrote:
> Num tx queues for all devices:
> 
> 88E6020		4
> 88E6071		4
> 88E6085		4
> 88E6095		4
> 88E6097		4
> 88E6123		4
> 88E6131		4
> 88E6141		4
> 88E6161		4
> 88E6165		4
> 88E6171		4
> 88E6172		4
> 88E6175		4
> 88E6176		4
> 88E6185		4
> 88E6190		8 from you
> 88E6190X	8 from you
> 88E6191		n/a can't find datasheet, does this even exist?

I'm not sure it does. The version of UMSD i have does not list it. If
it does exist, it will have 8 queues, given the family it belongs to.

	Andrew

Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use the hw tx queues

[Paolo Abeni]

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
    net: dsa: mv88e6xxx: use the hw tx queues

This patch adds support for transmitting packets to specific hardware queues
on Marvell 88E6xxx switches. It exposes the hardware transmit queues to the
networking stack and modifies the DSA tagger to embed the queue mapping into
the priority field of the DSA tag.

[ ... ]

> diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
> index 2a2c4fb61a65c..96c8fbedbd3b4 100644
> --- a/net/dsa/tag_dsa.c
> +++ b/net/dsa/tag_dsa.c
> @@ -179,6 +179,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>  			dsa_header[2] &= ~0x10;
>  		}
>  	} else {
> +		u16 queue = skb_get_queue_mapping(skb) & 0x7;
>  		u16 vid;
>  
>  		vid = br_dev ? MV88E6XXX_VID_BRIDGED : MV88E6XXX_VID_STANDALONE;
> @@ -191,7 +192,7 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
>  
>  		dsa_header[0] = (cmd << 6) | tag_dev;
>  		dsa_header[1] = tag_port << 3;
> -		dsa_header[2] = vid >> 8;
> +		dsa_header[2] = (queue << 5) | vid >> 8;

[Severity: High]
Does this code map the Linux transmit queue index directly to the 802.1p
Priority (PRI) field, potentially breaking hardware queue utilization?

The PRI field in the Marvell DSA tag corresponds to the 802.1p Priority
Code Point (PCP), rather than a direct hardware queue index. By default,
Marvell switches use a Priority-to-Queue table to map the 8 possible PCP
values (0-7) to physical egress queues. 

For a 4-queue switch, PCP 0-1 map to Q0, 2-3 to Q1, 4-5 to Q2, and 6-7 to Q3.
Since Linux creates 4 TX queues for such a device, skb_get_queue_mapping(skb)
will only return values from 0 to 3. Placing 0-3 directly into the PRI field
means the switch will see PCPs 0-3 and map them strictly to physical Q0 and
Q1. Hardware queues 2 and 3 would become completely unreachable for untagged
traffic.

Should the tagger instead derive the PRI field from skb->priority (capped
to 0-7), as the priority needs to represent the PCP rather than the
compacted Linux queue index?

>  		dsa_header[3] = vid & 0xff;
>  	}
-- 
This is an AI-generated review.

[PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Cedric Jehasse via B4 Relay]

From: Cedric Jehasse <cedric.jehasse@luminex.be>

Some of the chips supported by this driver have credit based shaper
support. Support is added for the 6352, 6390 and 6393 families.
This is configured using the Qav registers in the AVB register block.
There are small differences in the Qav registers between the chip
families (eg. the unit used for the rate and number of bits in the
registers). mv88e6xxx_qav_info is introduced to configure this per chip.

Eg. setting up 20mbps credit based shaper on a 1GBit link:
tc qdisc add dev p8 parent root handle 100: mqprio \
                num_tc 8 \
                map 0 0 6 7 0 5 0 0 0 0 0 0 0 0 0 0 \
                queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \
                hw 0

tc qdisc replace dev p8 parent 100:8 cbs locredit -1470 hicredit 30 \
    sendslope -980000 idleslope 20000 offload 1

Note: only idleslope and hicredit can be programmed in the switch
registers, other parameters won't affect settings.

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
 drivers/net/dsa/mv88e6xxx/chip.c        | 123 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h        |  21 ++++++
 drivers/net/dsa/mv88e6xxx/global2_avb.c |  21 ++++++
 drivers/net/dsa/mv88e6xxx/port.c        |  38 ++++++++++
 drivers/net/dsa/mv88e6xxx/port.h        |  16 +++++
 5 files changed, 219 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 277efe24edf4..63e2f13a4900 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -32,6 +32,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/phylink.h>
 #include <net/dsa.h>
+#include <net/pkt_sched.h>
 
 #include "chip.h"
 #include "devlink.h"
@@ -5015,6 +5016,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.port_set_ether_type = mv88e6351_port_set_ether_type,
 	.port_set_jumbo_size = mv88e6165_port_set_jumbo_size,
 	.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
+	.port_set_scheduling_mode = mv88e6352_port_set_scheduling_mode,
 	.port_pause_limit = mv88e6097_port_pause_limit,
 	.port_disable_learn_limit = mv88e6xxx_port_disable_learn_limit,
 	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
@@ -5446,6 +5448,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.port_set_ether_type = mv88e6351_port_set_ether_type,
 	.port_set_jumbo_size = mv88e6165_port_set_jumbo_size,
 	.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
+	.port_set_scheduling_mode = mv88e6352_port_set_scheduling_mode,
 	.port_pause_limit = mv88e6097_port_pause_limit,
 	.port_disable_learn_limit = mv88e6xxx_port_disable_learn_limit,
 	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
@@ -5515,6 +5518,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.port_get_cmode = mv88e6352_port_get_cmode,
 	.port_set_cmode = mv88e6390_port_set_cmode,
 	.port_setup_message_port = mv88e6xxx_setup_message_port,
+	.port_set_scheduling_mode = mv88e6390_port_set_scheduling_mode,
 	.stats_snapshot = mv88e6390_g1_stats_snapshot,
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
@@ -5580,6 +5584,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.port_get_cmode = mv88e6352_port_get_cmode,
 	.port_set_cmode = mv88e6390x_port_set_cmode,
 	.port_setup_message_port = mv88e6xxx_setup_message_port,
+	.port_set_scheduling_mode = mv88e6390_port_set_scheduling_mode,
 	.stats_snapshot = mv88e6390_g1_stats_snapshot,
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
@@ -5637,6 +5642,7 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.port_set_ether_type = mv88e6393x_port_set_ether_type,
 	.port_set_jumbo_size = mv88e6165_port_set_jumbo_size,
 	.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
+	.port_set_scheduling_mode = mv88e6390_port_set_scheduling_mode,
 	.port_pause_limit = mv88e6390_port_pause_limit,
 	.port_disable_learn_limit = mv88e6xxx_port_disable_learn_limit,
 	.port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
@@ -5679,6 +5685,22 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.tcam_ops = &mv88e6393_tcam_ops,
 };
 
+static const struct mv88e6xxx_qav_info mv88e6352_qav_info = {
+	.max_rate = 1000000,
+	.rate_unit = 32,
+	.rate_mask = GENMASK(14, 0),
+	.hi_limit_mask = GENMASK(14, 0),
+	.queue_mask = GENMASK(3, 0),
+};
+
+static const struct mv88e6xxx_qav_info mv88e6390_qav_info = {
+	.max_rate = 4000000,
+	.rate_unit = 64,
+	.rate_mask = GENMASK(15, 0),
+	.hi_limit_mask = GENMASK(13, 0),
+	.queue_mask = GENMASK(7, 0),
+};
+
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 	[MV88E6020] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6020,
@@ -6227,6 +6249,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.multi_chip = true,
 		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
 		.ptp_support = true,
+		.qav = &mv88e6352_qav_info,
 		.ops = &mv88e6240_ops,
 	},
 
@@ -6440,6 +6463,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.multi_chip = true,
 		.edsa_support = MV88E6XXX_EDSA_SUPPORTED,
 		.ptp_support = true,
+		.qav = &mv88e6352_qav_info,
 		.ops = &mv88e6352_ops,
 	},
 	[MV88E6361] = {
@@ -6496,6 +6520,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.multi_chip = true,
 		.edsa_support = MV88E6XXX_EDSA_UNDOCUMENTED,
 		.ptp_support = true,
+		.qav = &mv88e6390_qav_info,
 		.ops = &mv88e6390_ops,
 	},
 	[MV88E6390X] = {
@@ -6523,6 +6548,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.multi_chip = true,
 		.edsa_support = MV88E6XXX_EDSA_UNDOCUMENTED,
 		.ptp_support = true,
+		.qav = &mv88e6390_qav_info,
 		.ops = &mv88e6390x_ops,
 	},
 
@@ -6551,6 +6577,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.pvt = true,
 		.multi_chip = true,
 		.ptp_support = true,
+		.qav = &mv88e6390_qav_info,
 		.ops = &mv88e6393x_ops,
 	},
 };
@@ -7172,6 +7199,101 @@ static int mv88e6xxx_crosschip_lag_leave(struct dsa_switch *ds, int sw_index,
 	return err_sync ? : err_pvt;
 }
 
+static int mv88e6xxx_setup_tc_cbs(struct dsa_switch *ds, int port,
+				  struct tc_cbs_qopt_offload *cbs)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	const struct mv88e6xxx_ops *ops = chip->info->ops;
+	const struct mv88e6xxx_avb_ops *avb_ops;
+	const struct mv88e6xxx_qav_info *qav;
+	int rate_reg;
+	int hilimit_reg;
+	u8 queue_bit;
+	u16 rate = 0;
+	u16 hi_limit;
+	int err;
+
+	avb_ops = ops->avb_ops;
+	qav = chip->info->qav;
+	if (!qav || !avb_ops || !avb_ops->port_qav_write ||
+	    !ops->port_set_scheduling_mode)
+		return -EOPNOTSUPP;
+
+	if (!dsa_is_user_port(ds, port))
+		return -EOPNOTSUPP;
+
+	if (cbs->queue < 0 || cbs->queue >= chip->info->num_tx_queues)
+		return -EINVAL;
+
+	if (!(qav->queue_mask & BIT(cbs->queue)))
+		return -EOPNOTSUPP;
+
+	queue_bit = BIT(cbs->queue);
+	rate_reg = cbs->queue * 2;
+	hilimit_reg = rate_reg + 1;
+
+	if (cbs->enable) {
+		if (cbs->idleslope <= 0 ||
+		    cbs->idleslope > qav->max_rate ||
+		    cbs->sendslope >= 0 || cbs->hicredit <= 0 ||
+		    cbs->hicredit > qav->hi_limit_mask)
+			return -ERANGE;
+
+		rate = DIV_ROUND_UP(cbs->idleslope, qav->rate_unit);
+		if (rate > qav->rate_mask)
+			return -ERANGE;
+	}
+
+	mv88e6xxx_reg_lock(chip);
+
+	if (!cbs->enable) {
+		err = avb_ops->port_qav_write(chip, port, rate_reg, 0);
+		if (err)
+			goto unlock;
+
+		if (!(chip->ports[port].cbs_active_queues & ~queue_bit)) {
+			err = ops->port_set_scheduling_mode(chip, port, 0);
+			if (err)
+				goto unlock;
+		}
+		chip->ports[port].cbs_active_queues &= ~queue_bit;
+		goto unlock;
+	}
+
+	hi_limit = cbs->hicredit & qav->hi_limit_mask;
+	err = avb_ops->port_qav_write(chip, port, hilimit_reg, hi_limit);
+	if (err)
+		goto unlock;
+
+	err = avb_ops->port_qav_write(chip, port, rate_reg, rate);
+	if (err)
+		goto unlock;
+
+	err = ops->port_set_scheduling_mode(chip, port,
+					    chip->info->num_tx_queues - 1);
+	if (err) {
+		avb_ops->port_qav_write(chip, port, rate_reg, 0);
+		goto unlock;
+	}
+	chip->ports[port].cbs_active_queues |= queue_bit;
+
+unlock:
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
+static int mv88e6xxx_port_setup_tc(struct dsa_switch *ds, int port,
+				   enum tc_setup_type type, void *type_data)
+{
+	switch (type) {
+	case TC_SETUP_QDISC_CBS:
+		return mv88e6xxx_setup_tc_cbs(ds, port, type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct phylink_mac_ops mv88e6xxx_phylink_mac_ops = {
 	.mac_select_pcs		= mv88e6xxx_mac_select_pcs,
 	.mac_prepare		= mv88e6xxx_mac_prepare,
@@ -7231,6 +7353,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_hwtstamp_get	= mv88e6xxx_port_hwtstamp_get,
 	.port_txtstamp		= mv88e6xxx_port_txtstamp,
 	.port_rxtstamp		= mv88e6xxx_port_rxtstamp,
+	.port_setup_tc		= mv88e6xxx_port_setup_tc,
 	.cls_flower_add		= mv88e6xxx_cls_flower_add,
 	.cls_flower_del         = mv88e6xxx_cls_flower_del,
 	.get_ts_info		= mv88e6xxx_get_ts_info,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 19d8eda19b78..81c9fb2f0e92 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -125,6 +125,7 @@ enum mv88e6xxx_edsa_support {
 };
 
 struct mv88e6xxx_ops;
+struct mv88e6xxx_qav_info;
 
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
@@ -177,6 +178,9 @@ struct mv88e6xxx_info {
 	/* Supports PTP */
 	bool ptp_support;
 
+	/* 802.1Qav credit based shaping */
+	const struct mv88e6xxx_qav_info *qav;
+
 	/* Internal PHY start index. 0 means that internal PHYs range starts at
 	 * port 0, 1 means internal PHYs range starts at port 1, etc
 	 */
@@ -304,6 +308,9 @@ struct mv88e6xxx_port {
 
 	/* MacAuth Bypass control flag */
 	bool mab;
+
+	/* Queues with CBS currently enabled. */
+	u8 cbs_active_queues;
 };
 
 enum mv88e6xxx_region_id {
@@ -607,6 +614,8 @@ struct mv88e6xxx_ops {
 				   size_t size);
 
 	int (*port_egress_rate_limiting)(struct mv88e6xxx_chip *chip, int port);
+	int (*port_set_scheduling_mode)(struct mv88e6xxx_chip *chip, int port,
+					u8 mode);
 	int (*port_pause_limit)(struct mv88e6xxx_chip *chip, int port, u8 in,
 				u8 out);
 	int (*port_disable_learn_limit)(struct mv88e6xxx_chip *chip, int port);
@@ -764,6 +773,10 @@ struct mv88e6xxx_avb_ops {
 	int (*tai_read)(struct mv88e6xxx_chip *chip, int addr, u16 *data,
 			int len);
 	int (*tai_write)(struct mv88e6xxx_chip *chip, int addr, u16 data);
+
+	/* Access port-scoped 802.1Qav registers */
+	int (*port_qav_write)(struct mv88e6xxx_chip *chip, int port, int addr,
+			      u16 data);
 };
 
 struct mv88e6xxx_ptp_ops {
@@ -799,6 +812,14 @@ struct mv88e6xxx_tcam_ops {
 	int (*flush_tcam)(struct mv88e6xxx_chip *chip);
 };
 
+struct mv88e6xxx_qav_info {
+	u32 max_rate; /* in kbps */
+	u16 rate_unit; /* in kbps */
+	u16 rate_mask; /* QPri Rate valid bits mask */
+	u16 hi_limit_mask; /* Qpri Hi Limit bits mask*/
+	u8 queue_mask; /* supported queues bitmask */
+};
+
 static inline bool mv88e6xxx_has_stu(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->max_sid > 0 &&
diff --git a/drivers/net/dsa/mv88e6xxx/global2_avb.c b/drivers/net/dsa/mv88e6xxx/global2_avb.c
index 657783e043ff..6b54e275d21a 100644
--- a/drivers/net/dsa/mv88e6xxx/global2_avb.c
+++ b/drivers/net/dsa/mv88e6xxx/global2_avb.c
@@ -110,6 +110,15 @@ static int mv88e6352_g2_avb_port_ptp_write(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g2_avb_write(chip, writeop, data);
 }
 
+static int mv88e6352_g2_avb_port_qav_write(struct mv88e6xxx_chip *chip,
+					   int port, int addr, u16 data)
+{
+	u16 writeop = MV88E6352_G2_AVB_CMD_OP_WRITE | (port << 8) |
+		      (MV88E6352_G2_AVB_CMD_BLOCK_QAV << 5) | addr;
+
+	return mv88e6xxx_g2_avb_write(chip, writeop, data);
+}
+
 static int mv88e6352_g2_avb_ptp_read(struct mv88e6xxx_chip *chip, int addr,
 				     u16 *data, int len)
 {
@@ -149,6 +158,7 @@ const struct mv88e6xxx_avb_ops mv88e6352_avb_ops = {
 	.ptp_write		= mv88e6352_g2_avb_ptp_write,
 	.tai_read		= mv88e6352_g2_avb_tai_read,
 	.tai_write		= mv88e6352_g2_avb_tai_write,
+	.port_qav_write		= mv88e6352_g2_avb_port_qav_write,
 };
 
 static int mv88e6165_g2_avb_tai_read(struct mv88e6xxx_chip *chip, int addr,
@@ -174,6 +184,7 @@ const struct mv88e6xxx_avb_ops mv88e6165_avb_ops = {
 	.ptp_write		= mv88e6352_g2_avb_ptp_write,
 	.tai_read		= mv88e6165_g2_avb_tai_read,
 	.tai_write		= mv88e6165_g2_avb_tai_write,
+	.port_qav_write		= mv88e6352_g2_avb_port_qav_write,
 };
 
 static int mv88e6390_g2_avb_port_ptp_read(struct mv88e6xxx_chip *chip,
@@ -197,6 +208,15 @@ static int mv88e6390_g2_avb_port_ptp_write(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g2_avb_write(chip, writeop, data);
 }
 
+static int mv88e6390_g2_avb_port_qav_write(struct mv88e6xxx_chip *chip,
+					   int port, int addr, u16 data)
+{
+	u16 writeop = MV88E6390_G2_AVB_CMD_OP_WRITE | (port << 8) |
+		      (MV88E6352_G2_AVB_CMD_BLOCK_QAV << 5) | addr;
+
+	return mv88e6xxx_g2_avb_write(chip, writeop, data);
+}
+
 static int mv88e6390_g2_avb_ptp_read(struct mv88e6xxx_chip *chip, int addr,
 				     u16 *data, int len)
 {
@@ -236,4 +256,5 @@ const struct mv88e6xxx_avb_ops mv88e6390_avb_ops = {
 	.ptp_write		= mv88e6390_g2_avb_ptp_write,
 	.tai_read		= mv88e6390_g2_avb_tai_read,
 	.tai_write		= mv88e6390_g2_avb_tai_write,
+	.port_qav_write		= mv88e6390_g2_avb_port_qav_write,
 };
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index c90117d2dd83..79422b8b01c8 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1348,6 +1348,44 @@ int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
 				    0x0001);
 }
 
+int mv88e6352_port_set_scheduling_mode(struct mv88e6xxx_chip *chip, int port,
+				       u8 mode)
+{
+	u16 reg;
+	int err;
+
+	if (mode > 3)
+		return -EINVAL;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_EGRESS_RATE_CTL2,
+				  &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK;
+	reg |= mode << MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_SHIFT;
+
+	return mv88e6xxx_port_write(chip, port,
+				    MV88E6XXX_PORT_EGRESS_RATE_CTL2, reg);
+}
+
+int mv88e6390_port_set_scheduling_mode(struct mv88e6xxx_chip *chip, int port,
+				       u8 mode)
+{
+	u16 reg;
+
+	if (mode > MV88E6390_PORT_QUEUE_CTL_SCHEDULE_MASK)
+		return -EINVAL;
+
+	reg = MV88E6390_PORT_QUEUE_CTL_UPDATE |
+	      (MV88E6390_PORT_QUEUE_CTL_SCHEDULE <<
+	       MV88E6390_PORT_QUEUE_CTL_PTR_SHIFT) |
+	      (mode & MV88E6390_PORT_QUEUE_CTL_SCHEDULE_MASK);
+
+	return mv88e6xxx_port_write(chip, port, MV88E6390_PORT_QUEUE_CTL,
+				    reg);
+}
+
 /* Offset 0x0B: Port Association Vector */
 
 int mv88e6xxx_port_set_assoc_vector(struct mv88e6xxx_chip *chip, int port,
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index f6041f91215e..c4b0ec1990b3 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -241,6 +241,18 @@
 
 /* Offset 0x0A: Egress Rate Control 2 */
 #define MV88E6XXX_PORT_EGRESS_RATE_CTL2		0x0a
+#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_MASK	0x3000
+#define MV88E6XXX_PORT_EGRESS_RATE_CTL2_SCHEDULE_SHIFT	12
+
+/* Offset 0x1C: Port Queue Control */
+#define MV88E6390_PORT_QUEUE_CTL			0x1c
+#define MV88E6390_PORT_QUEUE_CTL_UPDATE			0x8000
+#define MV88E6390_PORT_QUEUE_CTL_PTR_MASK		0x7f00
+#define MV88E6390_PORT_QUEUE_CTL_PTR_SHIFT		8
+#define MV88E6390_PORT_QUEUE_CTL_DATA_MASK		0x00ff
+#define MV88E6390_PORT_QUEUE_CTL_SCHEDULE		0x00
+#define MV88E6390_PORT_QUEUE_CTL_SCHEDULE_MASK		0x07
+
 
 /* Offset 0x0B: Port Association Vector */
 #define MV88E6XXX_PORT_ASSOC_VECTOR			0x0b
@@ -569,6 +581,10 @@ int mv88e6165_port_set_jumbo_size(struct mv88e6xxx_chip *chip, int port,
 				  size_t size);
 int mv88e6095_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
 int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port);
+int mv88e6352_port_set_scheduling_mode(struct mv88e6xxx_chip *chip, int port,
+				       u8 mode);
+int mv88e6390_port_set_scheduling_mode(struct mv88e6xxx_chip *chip, int port,
+				       u8 mode);
 int mv88e6xxx_port_set_assoc_vector(struct mv88e6xxx_chip *chip, int port,
 				    u16 pav);
 int mv88e6097_port_pause_limit(struct mv88e6xxx_chip *chip, int port, u8 in,

-- 
2.43.0

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

Hi Cedric,

We sent RFC patches for mv88e6xxx support for both MQPRIO [1] and CBS [2] last year. (We have since made a few small changes which can be found in our working tree at [3].)

We have been using these patches successfully for the past year with our SRP stack [4] on a custom board with a 88E6352. The CBS and MQPRIO patches remain unsubmitted because we also needed extend the Netlink API and software bridge with a flag indicating a FDB/MDB entry is managed by SRP.

I would be happy to work together to harmonise our patches to avoid duplicated effort.

Cheers,
Luke

[1] https://lists.openwall.net/netdev/2025/09/27/34
[2] https://lists.openwall.net/netdev/2025/09/27/26
[3] https://github.com/PADL/linux/tree/rpi-6.18.y-xebros-rmu
[4] https://github.com/PADL/OpenSRP

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

Hi Cedric,

One followup:

I could be missing some patches from you due to netdev volume but, do you have a separate patch set for hardware offloaded MQPRIO?

Your example uses hw 0 (software MQPRIO) which doesn’t program the switch’s FPri/QPri mapping. For switches excepting the 6390 family, this mapping is per-chip, which means one needs to validate it is identical for every port on which CBS can be enabled.

Cheers,
Luke

> On 26 May 2026, at 10:32, Luke Howard <lukeh@padl.com> wrote:
> 
> Hi Cedric,
> 
> We sent RFC patches for mv88e6xxx support for both MQPRIO [1] and CBS [2] last year. (We have since made a few small changes which can be found in our working tree at [3].)
> 
> We have been using these patches successfully for the past year with our SRP stack [4] on a custom board with a 88E6352. The CBS and MQPRIO patches remain unsubmitted because we also needed extend the Netlink API and software bridge with a flag indicating a FDB/MDB entry is managed by SRP.
> 
> I would be happy to work together to harmonise our patches to avoid duplicated effort.
> 
> Cheers,
> Luke
> 
> [1] https://lists.openwall.net/netdev/2025/09/27/34
> [2] https://lists.openwall.net/netdev/2025/09/27/26
> [3] https://github.com/PADL/linux/tree/rpi-6.18.y-xebros-rmu
> [4] https://github.com/PADL/OpenSRP

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Cedric Jehasse]

> From: Luke Howard <lukeh@padl.com>
> Sent: Tuesday, May 26, 2026 2:32 AM
> To: Cedric Jehasse
> Cc: Andrew Lunn; Vladimir Oltean; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Simon Horman; Russell King; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Kieran Tyrrell; Max Hunter
> Subject: Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper
> 
> Hi Cedric,
> 
> We sent RFC patches for mv88e6xxx support for both MQPRIO [1] and CBS [2] last year. (We have since made a few small changes which can be found in our working tree at [3].)
> 
> We have been using these patches successfully for the past year with our SRP stack [4] on a custom board with a 88E6352. The CBS and MQPRIO patches remain unsubmitted because we also needed extend the Netlink API and software bridge with a flag indicating a FDB/MDB entry is managed by SRP.
> 
> I would be happy to work together to harmonise our patches to avoid duplicated effort.
> 
> Cheers,
> Luke
> 
> [1] https://lists.openwall.net/netdev/2025/09/27/34
> [2] https://lists.openwall.net/netdev/2025/09/27/26
> [3] https://github.com/PADL/linux/tree/rpi-6.18.y-xebros-rmu
> [4] https://github.com/PADL/OpenSRP

I've looked at these patches before starting. I quickly ran into the limitation the driver only supports 3 traffic classes. I want to be able to send traffic from the cpu to other queues too.
I don't use MQPRIO hw offload, which i think is a big part of your changes. So it didn't make sense to continue on those patches. This also answers your other question: i haven't submitted another patch set for hardware offloaded MQPRIO. 
I'm trying to upstream changes one topic at a time. Eg. this CBS patch is only for configuring the shaper. How traffic ends up in a certain queue is not relevant for this patch.

Cedric

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

> I've looked at these patches before starting. I quickly ran into the limitation the driver only supports 3 traffic classes. I want to be able to send traffic from the cpu to other queues too.

Yes, fair point, a compromise because our goal was to support offloaded switching of SRP-managed AVB streams. Only the 6390 family of Marvell switch chips support CBS with more than two queues (the third being legacy).

> I'm trying to upstream changes one topic at a time. Eg. this CBS patch is only for configuring the shaper. How traffic ends up in a certain queue is not relevant for this patch.

Right, we did it in the reverse order (MQPRIO then CBS, as separate commits but in the same patch series).

Now I have a solution for flagging SRP-managed MDB entries, I should be in a position to resubmit the patch set after regression testing. One thing have incorporated from your patch is dynamically enabling strict priority scheduling (thank you).

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

Hi Cedric,

> On 26 May 2026, at 9:28 pm, Cedric Jehasse <cedric.jehasse@luminex.be> wrote:
> 
> I don't use MQPRIO hw offload, which i think is a big part of your changes. So it didn't make sense to continue on those patches. This also answers your other question: i haven't submitted another patch set for hardware offloaded MQPRIO. 
> I'm trying to upstream changes one topic at a time. Eg. this CBS patch is only for configuring the shaper. How traffic ends up in a certain queue is not relevant for this patch.

This actually makes more sense also now that you pointed out that AVB mode is set on the ingress, not egress, port: configuring CBS should stay out of setting any of the AVB mode bits. Are you planning to submit a MQPRIO patch? If not, shall I just wait for your CBS patch to land, and then rebase my AVB patch series on top of net-next?

We have different use cases: yours (as I understand it) is host-initiated traffic with as many queues as the hardware supports, and mine is hardware offloaded AVB switching with (essentially) three TCs (one queue per AVB TC). It would be great to find a solution that can support both.

Cheers,
Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Cedric Jehasse]

On Thu, May 28, 2026 at 2:17 AM Luke Howard <lukeh@padl.com> wrote:

> This actually makes more sense also now that you pointed out that AVB mode is set on the ingress, not egress, port: configuring CBS should stay out of setting any of the AVB mode bits. Are you planning to submit a MQPRIO patch? If not, shall I just wait for your CBS patch to land, and then rebase my AVB patch series on top of net-next?

No, I don't have changes to support MQPRIO in the driver.  I use the
default FPri/Qpri mappings at the moment.

>
> We have different use cases: yours (as I understand it) is host-initiated traffic with as many queues as the hardware supports, and mine is hardware offloaded AVB switching with (essentially) three TCs (one queue per AVB TC). It would be great to find a solution that can support both.

My use case is both: forwarding AVB traffic by the switch, but also
sending traffic from the host at a certain priority.

Cedric

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

> On 28 May 2026, at 6:11 pm, Cedric Jehasse <cedric.jehasse@gmail.com> wrote:
> 
> On Thu, May 28, 2026 at 2:17 AM Luke Howard <lukeh@padl.com> wrote:
> 
>> This actually makes more sense also now that you pointed out that AVB mode is set on the ingress, not egress, port: configuring CBS should stay out of setting any of the AVB mode bits. Are you planning to submit a MQPRIO patch? If not, shall I just wait for your CBS patch to land, and then rebase my AVB patch series on top of net-next?
> 
> No, I don't have changes to support MQPRIO in the driver.  I use the
> default FPri/Qpri mappings at the moment.

This is a salient point: in my MQPRIO patch series, offloaded TCs 2 and 1 represent AVB classes (with 0 representing non-AVB traffic). This is the most natural mapping to take advantage of the switches’ native AVB support.

But there’s an alternate mapping where TCs represent switch queues and can be used to configure arbitrary FPri/QPri mappings.

It would be good to support both, particularly for switches with >4 queues. Maybe this can be keyed the presence of the DT property (now renamed, marvell,avb-mode).

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Andrew Lunn]

> It would be good to support both, particularly for switches with >4
> queues. Maybe this can be keyed the presence of the DT property (now
> renamed, marvell,avb-mode).

You need to be careful with DT. It represents hardware. Configuration
of that hardware, or policy, should not be in DT.

	Andrew

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

> On 28 May 2026, at 10:46 pm, Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> It would be good to support both, particularly for switches with >4
>> queues. Maybe this can be keyed the presence of the DT property (now
>> renamed, marvell,avb-mode).
> 
> You need to be careful with DT. It represents hardware. Configuration
> of that hardware, or policy, should not be in DT.

Noted. The tradeoff is I don’t user space to be aware of specific hardware, i.e. a SRP daemon should not care whether it is using HW offload or a software bridge. But it would also be ideal to allow a 1:1 mapping of TCs to queues non-AVB uses such as Cedric’s.

A similarly sized hammer to DT would be a command-line option. Another option that requires a small user space change would be to add (say) TC_MQPRIO_MODE_CHANNEL_SRP, or to overload TC_MQPRIO_MODE_DCB for AVB.

Thoughts?

Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Paolo Abeni]

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net: dsa: mv88e6xxx: add support for credit based shaper

This commit adds support for credit-based shaping (802.1Qav) to several 
MV88E6xxx chip families (6352, 6390, 6393). It implements the UAPI 
structures allowing configuration of the idleslope and hicredit parameters 
per queue.

> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 1bd0615ea95b1..7ab0733f49154 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -7170,6 +7197,101 @@ static int mv88e6xxx_crosschip_lag_leave(struct dsa_switch *ds, int sw_index,
[ ... ]
> +	if (!cbs->enable) {
[ ... ]
> +		goto unlock;
> +	}
> +
> +	hi_limit = cbs->hicredit & qav->hi_limit_mask;

[Severity: High]
The tc_cbs_qopt_offload UAPI structure specifies hicredit in bytes, but this
appears to pass the value directly to the hardware register without scaling.
If standard tooling passes a correct byte value, will this result in the
hardware allocating significantly more credits than intended due to hardware
block size multipliers?

> +	err = avb_ops->port_qav_write(chip, port, hilimit_reg, hi_limit);
> +	if (err)
> +		goto unlock;
> +
> +	err = avb_ops->port_qav_write(chip, port, rate_reg, rate);
> +	if (err)
> +		goto unlock;
> +
> +	err = ops->port_set_scheduling_mode(chip, port,
> +					    chip->info->num_tx_queues - 1);
> +	if (err) {
> +		avb_ops->port_qav_write(chip, port, rate_reg, 0);
> +		goto unlock;

[Severity: Medium]
If ops->port_set_scheduling_mode() fails, the rate register is cleared, but
the queue_bit is not removed from chip->ports[port].cbs_active_queues.
Could this cause the software state to incorrectly believe the queue is
still active, preventing the port from returning to standard scheduling
mode when other queues are later disabled?

> +	}
> +	chip->ports[port].cbs_active_queues |= queue_bit;
> +
> +unlock:
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
[ ... ]
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index ea1fab71968a0..cadbc664608ff 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -1323,6 +1323,44 @@ int mv88e6097_port_egress_rate_limiting(struct mv88e6xxx_chip *chip, int port)
[ ... ]
> +int mv88e6390_port_set_scheduling_mode(struct mv88e6xxx_chip *chip, int port,
> +				       u8 mode)
> +{
> +	u16 reg;
> +
> +	if (mode > MV88E6390_PORT_QUEUE_CTL_SCHEDULE_MASK)
> +		return -EINVAL;
> +
> +	reg = MV88E6390_PORT_QUEUE_CTL_UPDATE |
> +	      (MV88E6390_PORT_QUEUE_CTL_SCHEDULE <<
> +	       MV88E6390_PORT_QUEUE_CTL_PTR_SHIFT) |
> +	      (mode & MV88E6390_PORT_QUEUE_CTL_SCHEDULE_MASK);
> +
> +	return mv88e6xxx_port_write(chip, port, MV88E6390_PORT_QUEUE_CTL,
> +				    reg);

[Severity: Medium]
This function sets the MV88E6390_PORT_QUEUE_CTL_UPDATE bit but returns
immediately. Should there be a wait or poll for the hardware to clear
this bit before returning, to prevent sequential configuration changes
from silently dropping updates while the hardware is still busy?

> +}
-- 
This is an AI-generated review.

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

Hi Cedric,

In anticipation of potentially rebasing my MQPRIO patch series upon this patch, I have some comments. (My MUA appears to have ruined the indentation, apologies.)

> Some of the chips supported by this driver have credit based shaper
> support. Support is added for the 6352, 6390 and 6393 families.

Would also be good to add support for the 6341, which as 4 queues but uses the same Qav multipliers as the 6390.

> + if (cbs->enable) {
> + if (cbs->idleslope <= 0 ||
> +    cbs->idleslope > qav->max_rate ||
> +    cbs->sendslope >= 0 || cbs->hicredit <= 0 ||
> +    cbs->hicredit > qav->hi_limit_mask)
> + return -ERANGE;

Other drivers (e.g. felix_vsc9959) round up and clamp, rather than validating. I would consider whether this validation is necessary at all, and/or whether it would suffice to omit the idleslope check and check only rate_mask. The idleslope and sendslope zero validation might be better placed in sch_cbs.c.

> + if (!cbs->enable) {
> + err = avb_ops->port_qav_write(chip, port, rate_reg, 0);

The convention is to provide a wrapper function, e.g. mv88e6xxx_port_qav_write(), returning -EOPNOTSUPP if the function pointer is NULL.

> + if (!(chip->ports[port].cbs_active_queues & ~queue_bit)) {
> + err = ops->port_set_scheduling_mode(chip, port, 0);
> + if (err)
> + goto unlock;
> + }
> + chip->ports[port].cbs_active_queues &= ~queue_bit;
> + goto unlock;
> + }
> +
> + hi_limit = cbs->hicredit & qav->hi_limit_mask;
> + err = avb_ops->port_qav_write(chip, port, hilimit_reg, hi_limit);
> + if (err)
> + goto unlock;
> +
> + err = avb_ops->port_qav_write(chip, port, rate_reg, rate);
> + if (err)
> + goto unlock;
> +
> + err = ops->port_set_scheduling_mode(chip, port,
> +    chip->info->num_tx_queues - 1);
> + if (err) {
> + avb_ops->port_qav_write(chip, port, rate_reg, 0);
> + goto unlock;
> + }
> + chip->ports[port].cbs_active_queues |= queue_bit;
> +
> +unlock:
> + mv88e6xxx_reg_unlock(chip);
> +
> + return err;

This may not matter in practice, but SMI errors will leave the register state in an inconsistent configuration. I would consider having multiple cleanup handlers that revert the state to that on function entry.

With respect to cbs_active_queues, my personal opinion is to not cache state both in the driver and registers where possible (to avoid the possibility they may get out of sync), but it is true that on a device with many queues, determining the CBS state dynamically would be expensive. I would be interested in others’ opinion here.

> +struct mv88e6xxx_qav_info {
> + u32 max_rate; /* in kbps */

Per above comment, perhaps consider whether max_rate is required. Also, queue_mask can be determined dynamically from chip->info->num_tx_queues, so can be elided.

Cheers,
Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Cedric Jehasse]

On Thu, May 28, 2026 at 1:52 AM Luke Howard <lukeh@padl.com> wrote:
> Would also be good to add support for the 6341, which as 4 queues but uses the same Qav multipliers as the 6390.
I'll have a look at the 6341

>
> > + if (cbs->enable) {
> > + if (cbs->idleslope <= 0 ||
> > +    cbs->idleslope > qav->max_rate ||
> > +    cbs->sendslope >= 0 || cbs->hicredit <= 0 ||
> > +    cbs->hicredit > qav->hi_limit_mask)
> > + return -ERANGE;
>
> Other drivers (e.g. felix_vsc9959) round up and clamp, rather than validating. I would consider whether this validation is necessary at all, and/or whether it would suffice to omit the idleslope check and check only rate_mask. The idleslope and sendslope zero validation might be better placed in sch_cbs.c.

I think some of these were the result of running the kernel review
prompts through Claude. Other drivers are probably from before LLM
reviews were introduced.

>
> > + if (!(chip->ports[port].cbs_active_queues & ~queue_bit)) {
> > + err = ops->port_set_scheduling_mode(chip, port, 0);
> > + if (err)
> > + goto unlock;
> > + }
> > + chip->ports[port].cbs_active_queues &= ~queue_bit;
> > + goto unlock;
> > + }
> > +
> > + hi_limit = cbs->hicredit & qav->hi_limit_mask;
> > + err = avb_ops->port_qav_write(chip, port, hilimit_reg, hi_limit);
> > + if (err)
> > + goto unlock;
> > +
> > + err = avb_ops->port_qav_write(chip, port, rate_reg, rate);
> > + if (err)
> > + goto unlock;
> > +
> > + err = ops->port_set_scheduling_mode(chip, port,
> > +    chip->info->num_tx_queues - 1);
> > + if (err) {
> > + avb_ops->port_qav_write(chip, port, rate_reg, 0);
> > + goto unlock;
> > + }
> > + chip->ports[port].cbs_active_queues |= queue_bit;
> > +
> > +unlock:
> > + mv88e6xxx_reg_unlock(chip);
> > +
> > + return err;
>
> This may not matter in practice, but SMI errors will leave the register state in an inconsistent configuration. I would consider having multiple cleanup handlers that revert the state to that on function entry.
>
> With respect to cbs_active_queues, my personal opinion is to not cache state both in the driver and registers where possible (to avoid the possibility they may get out of sync), but it is true that on a device with many queues, determining the CBS state dynamically would be expensive. I would be interested in others’ opinion here.

I have a different opinion. The driver is in control, if these get out of sync
you're having bigger problems. Either something else is writing to the device,
or the hardware is changing state by itself which means it's unstable.

>
> > +struct mv88e6xxx_qav_info {
> > + u32 max_rate; /* in kbps */
>
> Per above comment, perhaps consider whether max_rate is required. Also, queue_mask can be determined dynamically from chip->info->num_tx_queues, so can be elided.

Eg. for the 6393 the documentation says the maximum supported rate is 4000Mbps.
It could be this comes from the rate register set to 0xffff,
which is 4193240Kbps, and then rounded to 4000Mbps in the docs.

The 6020 family has 4 queues, but only 2 queues support CBS. That's why there's
a seperate queue mask, even if CBS support wasn't added for 6020.

Cedric

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

> I think some of these were the result of running the kernel review
> prompts through Claude. Other drivers are probably from before LLM
> reviews were introduced.

It seems to me it would be better to validate idleslope and sendslope in cbs_change() or cbs_enable_offload() so that all drivers can benefit from this? Also stylistically perhaps the comparison could be broken into >1 statements.

> I have a different opinion. The driver is in control, if these get out of sync
> you're having bigger problems. Either something else is writing to the device,
> or the hardware is changing state by itself which means it's unstable.

Valid. :)

> The 6020 family has 4 queues, but only 2 queues support CBS. That's why there's
> a seperate queue mask, even if CBS support wasn't added for 6020.

Ah, I wasn’t aware of this. I suppose this is the best solution then even though for the majority case it could be inferred.

Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Luke Howard]

> On 28 May 2026, at 7:19 pm, Cedric Jehasse <cedric.jehasse@gmail.com> wrote:
> 
> On Thu, May 28, 2026 at 1:52 AM Luke Howard <lukeh@padl.com> wrote:
>> Would also be good to add support for the 6341, which as 4 queues but uses the same Qav multipliers as the 6390.
> I'll have a look at the 6341

As an aside, the 6341 driver is buggy. It assumes the chip does not have a dedicated ATU FID register because it has 256 databases. This is incorrect and breaks VLAN-aware bridging. I will post a patch when the merge window reopens.

On the hardware side, the ATU hashing algorithm appears to easily collide. This causes problems with things such as installing the broadcast address into multiple FIDs, not to mention 802.1Q Dynamic Reservation Entries. The only workaround is to use the ‘direct’ algorithm which may have a performance impact (I am presuming it is a linear search; the data sheet warns against using it).

Luke

Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Andrew Lunn]

> As an aside, the 6341 driver is buggy. It assumes the chip does not
> have a dedicated ATU FID register because it has 256 databases. This
> is incorrect and breaks VLAN-aware bridging. I will post a patch
> when the merge window reopens.

You can post fixes for net any time. No need to wait for the merge
window to close.

	Andrew