[PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[Oleksij Rempel]

changes v3:
- add tc_ets_supported to match supported devices
- dynamically regenerated default TC to queue map.
- add Acked-by to the first patch

changes v2:
- run egress limit configuration on all queue separately. Otherwise
  configuration may not apply correctly.

Oleksij Rempel (2):
  net: dsa: microchip: add ksz_setup_tc_mode() function
  net: dsa: microchip: add ETS Qdisc support for KSZ9477 series

 drivers/net/dsa/microchip/ksz_common.c | 238 ++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h |  18 +-
 2 files changed, 244 insertions(+), 12 deletions(-)

-- 
2.30.2

[PATCH net-next v3 1/2] net: dsa: microchip: add ksz_setup_tc_mode() function

[Oleksij Rempel]

Add ksz_setup_tc_mode() to make queue scheduling and shaping
configuration more visible.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 20 ++++++++++++--------
 drivers/net/dsa/microchip/ksz_common.h |  6 ++----
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 48e35a1d110e..ae05fe0b0a81 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -32,10 +32,6 @@
 #include "ksz9477.h"
 #include "lan937x.h"
 
-#define KSZ_CBS_ENABLE ((MTI_SCHEDULE_STRICT_PRIO << MTI_SCHEDULE_MODE_S) | \
-			(MTI_SHAPING_SRP << MTI_SHAPING_S))
-#define KSZ_CBS_DISABLE ((MTI_SCHEDULE_WRR << MTI_SCHEDULE_MODE_S) |\
-			 (MTI_SHAPING_OFF << MTI_SHAPING_S))
 #define MIB_COUNTER_NUM 0x20
 
 struct ksz_stats_raw {
@@ -3119,6 +3115,14 @@ static int cinc_cal(s32 idle_slope, s32 send_slope, u32 *bw)
 	return 0;
 }
 
+static int ksz_setup_tc_mode(struct ksz_device *dev, int port, u8 scheduler,
+			     u8 shaper)
+{
+	return ksz_pwrite8(dev, port, REG_PORT_MTI_QUEUE_CTRL_0,
+			   FIELD_PREP(MTI_SCHEDULE_MODE_M, scheduler) |
+			   FIELD_PREP(MTI_SHAPING_M, shaper));
+}
+
 static int ksz_setup_tc_cbs(struct dsa_switch *ds, int port,
 			    struct tc_cbs_qopt_offload *qopt)
 {
@@ -3138,8 +3142,8 @@ static int ksz_setup_tc_cbs(struct dsa_switch *ds, int port,
 		return ret;
 
 	if (!qopt->enable)
-		return ksz_pwrite8(dev, port, REG_PORT_MTI_QUEUE_CTRL_0,
-				   KSZ_CBS_DISABLE);
+		return ksz_setup_tc_mode(dev, port, MTI_SCHEDULE_WRR,
+					 MTI_SHAPING_OFF);
 
 	/* High Credit */
 	ret = ksz_pwrite16(dev, port, REG_PORT_MTI_HI_WATER_MARK,
@@ -3164,8 +3168,8 @@ static int ksz_setup_tc_cbs(struct dsa_switch *ds, int port,
 			return ret;
 	}
 
-	return ksz_pwrite8(dev, port, REG_PORT_MTI_QUEUE_CTRL_0,
-			   KSZ_CBS_ENABLE);
+	return ksz_setup_tc_mode(dev, port, MTI_SCHEDULE_STRICT_PRIO,
+				 MTI_SHAPING_SRP);
 }
 
 static int ksz_setup_tc(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 10c732b1cea8..f53834bbe896 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -662,12 +662,10 @@ static inline int is_lan937x(struct ksz_device *dev)
 
 #define REG_PORT_MTI_QUEUE_CTRL_0	0x0914
 
-#define MTI_SCHEDULE_MODE_M		0x3
-#define MTI_SCHEDULE_MODE_S		6
+#define MTI_SCHEDULE_MODE_M		GENMASK(7, 6)
 #define MTI_SCHEDULE_STRICT_PRIO	0
 #define MTI_SCHEDULE_WRR		2
-#define MTI_SHAPING_M			0x3
-#define MTI_SHAPING_S			4
+#define MTI_SHAPING_M			GENMASK(5, 4)
 #define MTI_SHAPING_OFF			0
 #define MTI_SHAPING_SRP			1
 #define MTI_SHAPING_TIME_AWARE		2
-- 
2.30.2

[PATCH net-next v3 2/2] net: dsa: microchip: add ETS Qdisc support for KSZ9477 series

[Oleksij Rempel]

Add ETS Qdisc support for KSZ9477 of switches. Current implementation is
limited to strict priority mode.

Tested on KSZ8563R with following configuration:
tc qdisc replace dev lan2 root handle 1: ets strict 4 \
  priomap 3 3 2 2 1 1 0 0
ip link add link lan2 name v1 type vlan id 1 \
  egress-qos-map 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7

and patched iperf3 version:
https://github.com/esnet/iperf/pull/1476
iperf3 -c 172.17.0.1 -b100M  -l1472 -t100 -u -R --sock-prio 2

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz_common.c | 218 +++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  12 ++
 2 files changed, 230 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index ae05fe0b0a81..54d75ec22ef0 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1087,6 +1087,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 3,
 		.num_tx_queues = 4,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &ksz9477_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1227,6 +1228,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 4,
 		.num_tx_queues = 4,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &ksz9477_dev_ops,
 		.phy_errata_9477 = true,
 		.mib_names = ksz9477_mib_names,
@@ -1352,6 +1354,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 3,
 		.num_tx_queues = 4,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &ksz9477_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1379,6 +1382,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 3,
 		.num_tx_queues = 4,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &ksz9477_dev_ops,
 		.phy_errata_9477 = true,
 		.mib_names = ksz9477_mib_names,
@@ -1411,6 +1415,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &lan937x_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1437,6 +1442,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &lan937x_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1463,6 +1469,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &lan937x_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1493,6 +1500,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &lan937x_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -1523,6 +1531,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
+		.tc_ets_supported = true,
 		.ops = &lan937x_dev_ops,
 		.mib_names = ksz9477_mib_names,
 		.mib_cnt = ARRAY_SIZE(ksz9477_mib_names),
@@ -3172,12 +3181,221 @@ static int ksz_setup_tc_cbs(struct dsa_switch *ds, int port,
 				 MTI_SHAPING_SRP);
 }
 
+static int ksz_disable_egress_rate_limit(struct ksz_device *dev, int port)
+{
+	int queue, ret;
+
+	/* Configuration will not take effect until the last Port Queue X
+	 * Egress Limit Control Register is written.
+	 */
+	for (queue = 0; queue < dev->info->num_tx_queues; queue++) {
+		ret = ksz_pwrite8(dev, port, KSZ9477_REG_PORT_OUT_RATE_0 + queue,
+				  KSZ9477_OUT_RATE_NO_LIMIT);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ksz_ets_band_to_queue(struct tc_ets_qopt_offload_replace_params *p,
+				 int band)
+{
+	/* Compared to queues, bands prioritize packets differently. In strict
+	 * priority mode, the lowest priority is assigned to Queue 0 while the
+	 * highest priority is given to Band 0.
+	 */
+	return p->bands - 1 - band;
+}
+
+static int ksz_queue_set_strict(struct ksz_device *dev, int port, int queue)
+{
+	int ret;
+
+	ret = ksz_pwrite32(dev, port, REG_PORT_MTI_QUEUE_INDEX__4, queue);
+	if (ret)
+		return ret;
+
+	return ksz_setup_tc_mode(dev, port, MTI_SCHEDULE_STRICT_PRIO,
+				 MTI_SHAPING_OFF);
+}
+
+static int ksz_queue_set_wrr(struct ksz_device *dev, int port, int queue,
+			     int weight)
+{
+	int ret;
+
+	ret = ksz_pwrite32(dev, port, REG_PORT_MTI_QUEUE_INDEX__4, queue);
+	if (ret)
+		return ret;
+
+	ret = ksz_setup_tc_mode(dev, port, MTI_SCHEDULE_WRR,
+				MTI_SHAPING_OFF);
+	if (ret)
+		return ret;
+
+	return ksz_pwrite8(dev, port, KSZ9477_PORT_MTI_QUEUE_CTRL_1, weight);
+}
+
+static int ksz_tc_ets_add(struct ksz_device *dev, int port,
+			  struct tc_ets_qopt_offload_replace_params *p)
+{
+	int ret, band, tc_prio;
+	u32 queue_map = 0;
+
+	/* In order to ensure proper prioritization, it is necessary to set the
+	 * rate limit for the related queue to zero. Otherwise strict priority
+	 * or WRR mode will not work. This is a hardware limitation.
+	 */
+	ret = ksz_disable_egress_rate_limit(dev, port);
+	if (ret)
+		return ret;
+
+	/* Configure queue scheduling mode for all bands. Currently only strict
+	 * prio mode is supported.
+	 */
+	for (band = 0; band < p->bands; band++) {
+		int queue = ksz_ets_band_to_queue(p, band);
+
+		ret = ksz_queue_set_strict(dev, port, queue);
+		if (ret)
+			return ret;
+	}
+
+	/* Configure the mapping between traffic classes and queues. Note:
+	 * priomap variable support 16 traffic classes, but the chip can handle
+	 * only 8 classes.
+	 */
+	for (tc_prio = 0; tc_prio < ARRAY_SIZE(p->priomap); tc_prio++) {
+		int queue;
+
+		if (tc_prio > KSZ9477_MAX_TC_PRIO)
+			break;
+
+		queue = ksz_ets_band_to_queue(p, p->priomap[tc_prio]);
+		queue_map |= queue << (tc_prio * KSZ9477_PORT_TC_MAP_S);
+	}
+
+	return ksz_pwrite32(dev, port, KSZ9477_PORT_MRI_TC_MAP__4, queue_map);
+}
+
+static int ksz_tc_ets_del(struct ksz_device *dev, int port)
+{
+	int ret, queue, tc_prio, s;
+	u32 queue_map = 0;
+
+	/* To restore the default chip configuration, set all queues to use the
+	 * WRR scheduler with a weight of 1.
+	 */
+	for (queue = 0; queue < dev->info->num_tx_queues; queue++) {
+		ret = ksz_queue_set_wrr(dev, port, queue,
+					KSZ9477_DEFAULT_WRR_WEIGHT);
+		if (ret)
+			return ret;
+	}
+
+	switch (dev->info->num_tx_queues) {
+	case 2:
+		s = 2;
+		break;
+	case 4:
+		s = 1;
+		break;
+	case 8:
+		s = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Revert the queue mapping for TC-priority to its default setting on
+	 * the chip.
+	 */
+	for (tc_prio = 0; tc_prio <= KSZ9477_MAX_TC_PRIO; tc_prio++) {
+		int queue;
+
+		queue = tc_prio >> s;
+		queue_map |= queue << (tc_prio * KSZ9477_PORT_TC_MAP_S);
+	}
+
+	return ksz_pwrite32(dev, port, KSZ9477_PORT_MRI_TC_MAP__4, queue_map);
+}
+
+static int ksz_tc_ets_validate(struct ksz_device *dev, int port,
+			       struct tc_ets_qopt_offload_replace_params *p)
+{
+	int band;
+
+	/* Since it is not feasible to share one port among multiple qdisc,
+	 * the user must configure all available queues appropriately.
+	 */
+	if (p->bands != dev->info->num_tx_queues) {
+		dev_err(dev->dev, "Not supported amount of bands. It should be %d\n",
+			dev->info->num_tx_queues);
+		return -EOPNOTSUPP;
+	}
+
+	for (band = 0; band < p->bands; ++band) {
+		/* The KSZ switches utilize a weighted round robin configuration
+		 * where a certain number of packets can be transmitted from a
+		 * queue before the next queue is serviced. For more information
+		 * on this, refer to section 5.2.8.4 of the KSZ8565R
+		 * documentation on the Port Transmit Queue Control 1 Register.
+		 * However, the current ETS Qdisc implementation (as of February
+		 * 2023) assigns a weight to each queue based on the number of
+		 * bytes or extrapolated bandwidth in percentages. Since this
+		 * differs from the KSZ switches' method and we don't want to
+		 * fake support by converting bytes to packets, it is better to
+		 * return an error instead.
+		 */
+		if (p->quanta[band]) {
+			dev_err(dev->dev, "Quanta/weights configuration is not supported.\n");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
+static int ksz_tc_setup_qdisc_ets(struct dsa_switch *ds, int port,
+				  struct tc_ets_qopt_offload *qopt)
+{
+	struct ksz_device *dev = ds->priv;
+	int ret;
+
+	if (!dev->info->tc_ets_supported)
+		return -EOPNOTSUPP;
+
+	if (qopt->parent != TC_H_ROOT) {
+		dev_err(dev->dev, "Parent should be \"root\"\n");
+		return -EOPNOTSUPP;
+	}
+
+	switch (qopt->command) {
+	case TC_ETS_REPLACE:
+		ret = ksz_tc_ets_validate(dev, port, &qopt->replace_params);
+		if (ret)
+			return ret;
+
+		return ksz_tc_ets_add(dev, port, &qopt->replace_params);
+	case TC_ETS_DESTROY:
+		return ksz_tc_ets_del(dev, port);
+	case TC_ETS_STATS:
+	case TC_ETS_GRAFT:
+		return -EOPNOTSUPP;
+	}
+
+	return -EOPNOTSUPP;
+}
+
 static int ksz_setup_tc(struct dsa_switch *ds, int port,
 			enum tc_setup_type type, void *type_data)
 {
 	switch (type) {
 	case TC_SETUP_QDISC_CBS:
 		return ksz_setup_tc_cbs(ds, port, type_data);
+	case TC_SETUP_QDISC_ETS:
+		return ksz_tc_setup_qdisc_ets(ds, port, type_data);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index f53834bbe896..20e5da47ac7b 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -51,6 +51,7 @@ struct ksz_chip_data {
 	u8 port_nirqs;
 	u8 num_tx_queues;
 	bool tc_cbs_supported;
+	bool tc_ets_supported;
 	const struct ksz_dev_ops *ops;
 	bool phy_errata_9477;
 	bool ksz87xx_eee_link_erratum;
@@ -657,6 +658,14 @@ static inline int is_lan937x(struct ksz_device *dev)
 #define KSZ8_LEGAL_PACKET_SIZE		1518
 #define KSZ9477_MAX_FRAME_SIZE		9000
 
+#define KSZ9477_REG_PORT_OUT_RATE_0	0x0420
+#define KSZ9477_OUT_RATE_NO_LIMIT	0
+
+#define KSZ9477_PORT_MRI_TC_MAP__4	0x0808
+
+#define KSZ9477_PORT_TC_MAP_S		4
+#define KSZ9477_MAX_TC_PRIO		7
+
 /* CBS related registers */
 #define REG_PORT_MTI_QUEUE_INDEX__4	0x0900
 
@@ -670,6 +679,9 @@ static inline int is_lan937x(struct ksz_device *dev)
 #define MTI_SHAPING_SRP			1
 #define MTI_SHAPING_TIME_AWARE		2
 
+#define KSZ9477_PORT_MTI_QUEUE_CTRL_1	0x0915
+#define KSZ9477_DEFAULT_WRR_WEIGHT	1
+
 #define REG_PORT_MTI_HI_WATER_MARK	0x0916
 #define REG_PORT_MTI_LO_WATER_MARK	0x0918
 
-- 
2.30.2

Re: [PATCH net-next v3 2/2] net: dsa: microchip: add ETS Qdisc support for KSZ9477 series

[Arun.Ramadoss]

Hi Oleksij,
On Fri, 2023-03-10 at 10:08 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Add ETS Qdisc support for KSZ9477 of switches. Current implementation
> is
> limited to strict priority mode.
> 
> Tested on KSZ8563R with following configuration:
> tc qdisc replace dev lan2 root handle 1: ets strict 4 \
>   priomap 3 3 2 2 1 1 0 0
> ip link add link lan2 name v1 type vlan id 1 \
>   egress-qos-map 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
> 
> and patched iperf3 version:
> https://github.com/esnet/iperf/pull/1476
> iperf3 -c 172.17.0.1 -b100M  -l1472 -t100 -u -R --sock-prio 2
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 218
> +++++++++++++++++++++++++
>  drivers/net/dsa/microchip/ksz_common.h |  12 ++
>  2 files changed, 230 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c
> b/drivers/net/dsa/microchip/ksz_common.c
> index ae05fe0b0a81..54d75ec22ef0 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1087,6 +1087,7 @@ const struct ksz_chip_data ksz_switch_chips[] =
> {
>                 .port_nirqs = 3,
>                 .num_tx_queues = 4,
>                 .tc_cbs_supported = true,
> +               .tc_ets_supported = true,

Whether the switch which are supporting cbs will also support ets or
not. If CBS and ETS are related, then is it possible to use single flag
controlling both the feature. I could infer that switch which has
tc_cbs_supported  true, also has tc_ets_supported also true.

If both are different, patch looks good to me.

Re: [PATCH net-next v3 2/2] net: dsa: microchip: add ETS Qdisc support for KSZ9477 series

[Oleksij Rempel]

On Mon, Mar 13, 2023 at 03:34:16AM +0000, Arun.Ramadoss@microchip.com wrote:
> Hi Oleksij,
> On Fri, 2023-03-10 at 10:08 +0100, Oleksij Rempel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > Add ETS Qdisc support for KSZ9477 of switches. Current implementation
> > is
> > limited to strict priority mode.
> > 
> > Tested on KSZ8563R with following configuration:
> > tc qdisc replace dev lan2 root handle 1: ets strict 4 \
> >   priomap 3 3 2 2 1 1 0 0
> > ip link add link lan2 name v1 type vlan id 1 \
> >   egress-qos-map 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
> > 
> > and patched iperf3 version:
> > https://github.com/esnet/iperf/pull/1476
> > iperf3 -c 172.17.0.1 -b100M  -l1472 -t100 -u -R --sock-prio 2
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/dsa/microchip/ksz_common.c | 218
> > +++++++++++++++++++++++++
> >  drivers/net/dsa/microchip/ksz_common.h |  12 ++
> >  2 files changed, 230 insertions(+)
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index ae05fe0b0a81..54d75ec22ef0 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -1087,6 +1087,7 @@ const struct ksz_chip_data ksz_switch_chips[] =
> > {
> >                 .port_nirqs = 3,
> >                 .num_tx_queues = 4,
> >                 .tc_cbs_supported = true,
> > +               .tc_ets_supported = true,
> 
> Whether the switch which are supporting cbs will also support ets or
> not. If CBS and ETS are related, then is it possible to use single flag
> controlling both the feature. I could infer that switch which has
> tc_cbs_supported  true, also has tc_ets_supported also true.
> 
> If both are different, patch looks good to me.

Both are different. For example on ksz8 switches it is possible to
implement tc-etc but not tc-cbs.

Regatds,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v3 2/2] net: dsa: microchip: add ETS Qdisc support for KSZ9477 series

[Arun.Ramadoss]

On Mon, 2023-03-13 at 06:36 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Mon, Mar 13, 2023 at 03:34:16AM +0000, Arun.Ramadoss@microchip.com
>  wrote:
> > Hi Oleksij,
> > On Fri, 2023-03-10 at 10:08 +0100, Oleksij Rempel wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > > 
> > > Add ETS Qdisc support for KSZ9477 of switches. Current
> > > implementation
> > > is
> > > limited to strict priority mode.
> > > 
> > > Tested on KSZ8563R with following configuration:
> > > tc qdisc replace dev lan2 root handle 1: ets strict 4 \
> > >   priomap 3 3 2 2 1 1 0 0
> > > ip link add link lan2 name v1 type vlan id 1 \
> > >   egress-qos-map 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
> > > 
> > > and patched iperf3 version:
> > > https://github.com/esnet/iperf/pull/1476
> > > iperf3 -c 172.17.0.1 -b100M  -l1472 -t100 -u -R --sock-prio 2
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  drivers/net/dsa/microchip/ksz_common.c | 218
> > > +++++++++++++++++++++++++
> > >  drivers/net/dsa/microchip/ksz_common.h |  12 ++
> > >  2 files changed, 230 insertions(+)
> > > 
> > > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > > b/drivers/net/dsa/microchip/ksz_common.c
> > > index ae05fe0b0a81..54d75ec22ef0 100644
> > > --- a/drivers/net/dsa/microchip/ksz_common.c
> > > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > > @@ -1087,6 +1087,7 @@ const struct ksz_chip_data
> > > ksz_switch_chips[] =
> > > {
> > >                 .port_nirqs = 3,
> > >                 .num_tx_queues = 4,
> > >                 .tc_cbs_supported = true,
> > > +               .tc_ets_supported = true,
> > 
> > Whether the switch which are supporting cbs will also support ets
> > or
> > not. If CBS and ETS are related, then is it possible to use single
> > flag
> > controlling both the feature. I could infer that switch which has
> > tc_cbs_supported  true, also has tc_ets_supported also true.
> > 
> > If both are different, patch looks good to me.
> 
> Both are different. For example on ksz8 switches it is possible to
> implement tc-etc but not tc-cbs.

Ok. 

Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>

> 
> Regatds,
> Oleksij
> --
> Pengutronix
> e.K.                           |                             |
> Steuerwalder Str. 21                       | 
> http://www.pengutronix.de/e/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-
> 0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-
> 5555 |

Re: [PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[Arun.Ramadoss]

Hi Oleksij,
On Fri, 2023-03-10 at 10:08 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe

Cover letter patch version is still v2.
[PATCH net-next v2]

> 
> changes v3:
> - add tc_ets_supported to match supported devices
> - dynamically regenerated default TC to queue map.
> - add Acked-by to the first patch
> 
> changes v2:
> - run egress limit configuration on all queue separately. Otherwise
>   configuration may not apply correctly.
> 
> Oleksij Rempel (2):
>   net: dsa: microchip: add ksz_setup_tc_mode() function
>   net: dsa: microchip: add ETS Qdisc support for KSZ9477 series
> 
>  drivers/net/dsa/microchip/ksz_common.c | 238
> ++++++++++++++++++++++++-
>  drivers/net/dsa/microchip/ksz_common.h |  18 +-
>  2 files changed, 244 insertions(+), 12 deletions(-)
> 
> --
> 2.30.2
> 

Re: [PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[Jakub Kicinski]

On Fri, 10 Mar 2023 10:08:07 +0100 Oleksij Rempel wrote:
> changes v3:
> - add tc_ets_supported to match supported devices
> - dynamically regenerated default TC to queue map.
> - add Acked-by to the first patch
> 
> changes v2:
> - run egress limit configuration on all queue separately. Otherwise
>   configuration may not apply correctly.

I thought Vladimir was suggesting mqprio, could you summarize the take
aways from that discussion?

Re: [PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[Oleksij Rempel]

On Mon, Mar 13, 2023 at 12:18:33PM -0700, Jakub Kicinski wrote:
> On Fri, 10 Mar 2023 10:08:07 +0100 Oleksij Rempel wrote:
> > changes v3:
> > - add tc_ets_supported to match supported devices
> > - dynamically regenerated default TC to queue map.
> > - add Acked-by to the first patch
> > 
> > changes v2:
> > - run egress limit configuration on all queue separately. Otherwise
> >   configuration may not apply correctly.
> 
> I thought Vladimir was suggesting mqprio, could you summarize the take
> aways from that discussion?

Both Qdiscs are suitable for my use case, but I prefer tc-ets because it
aligns better with the abilities of the KSZ9477 family of switches.
However, I won't be able to support only the deficit round-robin
functionality of tc-ets with this hardware. On the other hand, tc-mqprio
has more features that are not supported by this switch, such as TXQ
grouping, bandwidth limit, and DCB support. The advanced mapping
functionality of tc-mqprio, which involves mapping SO_PRIORITY to TCs
and then to TXQ groups, can also be confusing. For my use case, only TC
to TXQ mapping is needed.

Futures of tc-mqprio:
- all TXQ groups use a strict priority transmission selection algorithm
  (TSA). Within one TXQ group probably round robin robin TSA is used.
- Number of supported traffic classes (TCs) is equal to the number of
  TXQ groups.
- TCs have predefined priorities. TC0 == minimal prio and transmitted
  last if TCn> TC0 exists.
- flexible mapping of all SO_PRIORITYs to TCs
- flexible mapping of TCs to TXQ groups.
- configurable bandwidth limit per TXQ group.

Futures of tc-ets:
- supports strict priority and deficit round robin (DRR) TSA. Both
  variants can be combined.
- quantum of each DRR band can be flexibly configured.
- each band has predefined priority and mapped directly to a TXQ.
- Supports max 16 TCs. Each TC can be flexibly mapped to a band==TXQ.

Futures of KSZ9477 family:
- Supports up to 4 TXQs. LANxxxx variants support up to 8 TXQs
- Each TXQ can be configured to strict priority or to weighted round
  robin (WRR) TSA.
- If bandwidth configuration per TXQ is used TSA falls back to simple
  round robin.
- TXQs can’t be grouped without breaking strict priority TSA
- Max 8 TCs are supported and can be flexibly mapped to TXQs

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 0/2] net: dsa: microchip: tc-ets support

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 10 Mar 2023 10:08:07 +0100 you wrote:
> changes v3:
> - add tc_ets_supported to match supported devices
> - dynamically regenerated default TC to queue map.
> - add Acked-by to the first patch
> 
> changes v2:
> - run egress limit configuration on all queue separately. Otherwise
>   configuration may not apply correctly.
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/2] net: dsa: microchip: add ksz_setup_tc_mode() function
    https://git.kernel.org/netdev/net-next/c/69444581d002
  - [net-next,v3,2/2] net: dsa: microchip: add ETS Qdisc support for KSZ9477 series
    https://git.kernel.org/netdev/net-next/c/c570f861fa05

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html