All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next 00/11] mlxsw qdisc refactoring
@ 2018-01-10 13:59 Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 01/11] mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated file Jiri Pirko
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 13:59 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Jiri Pirko <jiri@mellanox.com>

This patchset refactors the qdisc handling in mlxsw driver in order to make
it more object oriented like.
It helps readability, laying the groundwork for the offloading of
additional qdiscs by the driver
This patchset also makes the qdiscs statistics more generic.

Patch 1 moves the qdiscs declaration to the spectrum_qdisc.c
Patches 2-3 clean the offloaded stats requests. Patch 2 changes the RED
generic stats struct to be sharable by other offloaded qdiscs. Patch 3
changes the xstats request to be like the stats. Note that these patches
are outside the driver scope.
Patches 4-5 clean the statistics related functions and structs within the
driver.
Patches 6-7 decrease the need for the same parameters to be sent to many
functions.
Patches 8-11 create a functions pointers struct, to make the qdiscs
handling more object oriented like.

Nogah Frankel (11):
  mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated
    file
  net: sch: red: Change the name of the stats struct to be generic
  net: sch: red: Change offloaded xstats to be incremental
  mlxsw: spectrum: qdiscs: Clean qdisc statistics structs
  mlxsw: spectrum: qdiscs: Make the clean stats function to be for RED
    only
  mlxsw: spectrum: qdiscs: Add tclass number to the mlxsw_sp_qdisc
  mlxsw: spectrum: qdiscs: Unite all handle checks
  mlxsw: spectrum: qdiscs: Add an ops struct
  mlxsw: spectrum: qdiscs: Create a generic destroy function
  mlxsw: spectrum: qdiscs: Create a generic replace function
  mlxsw: spectrum: qdiscs: Remove qdisc before setting a new one

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  10 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  27 +-
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 362 ++++++++++++++-------
 include/net/pkt_cls.h                              |  11 +-
 net/sched/sch_red.c                                |  24 +-
 5 files changed, 280 insertions(+), 154 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [patch net-next 01/11] mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated file
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
@ 2018-01-10 13:59 ` Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 02/11] net: sch: red: Change the name of the stats struct to be generic Jiri Pirko
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 13:59 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Move all the qdisc related data from the spectrum.h to spectrum_qdisc.c.
Create an init and fini functions for the qdiscs.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 10 ++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     | 27 ++-------------
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 39 +++++++++++++++++++++-
 3 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index fedd231affd6..54c7d9202e81 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3085,6 +3085,13 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 		goto err_port_fids_init;
 	}
 
+	err = mlxsw_sp_tc_qdisc_init(mlxsw_sp_port);
+	if (err) {
+		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to initialize TC qdiscs\n",
+			mlxsw_sp_port->local_port);
+		goto err_port_qdiscs_init;
+	}
+
 	mlxsw_sp_port_vlan = mlxsw_sp_port_vlan_get(mlxsw_sp_port, 1);
 	if (IS_ERR(mlxsw_sp_port_vlan)) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to create VID 1\n",
@@ -3113,6 +3120,8 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_put(mlxsw_sp_port_vlan);
 err_port_vlan_get:
+	mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
+err_port_qdiscs_init:
 	mlxsw_sp_port_fids_fini(mlxsw_sp_port);
 err_port_fids_init:
 	mlxsw_sp_port_dcb_fini(mlxsw_sp_port);
@@ -3148,6 +3157,7 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	mlxsw_sp->ports[local_port] = NULL;
 	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_flush(mlxsw_sp_port);
+	mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
 	mlxsw_sp_port_fids_fini(mlxsw_sp_port);
 	mlxsw_sp_port_dcb_fini(mlxsw_sp_port);
 	mlxsw_sp_port_swid_set(mlxsw_sp_port, MLXSW_PORT_SWID_DISABLED_PORT);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 346b8b688b6f..b6f475e83474 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -204,29 +204,6 @@ struct mlxsw_sp_port_vlan {
 	struct list_head bridge_vlan_node;
 };
 
-enum mlxsw_sp_qdisc_type {
-	MLXSW_SP_QDISC_NO_QDISC,
-	MLXSW_SP_QDISC_RED,
-};
-
-struct mlxsw_sp_qdisc {
-	u32 handle;
-	enum mlxsw_sp_qdisc_type type;
-	struct red_stats xstats_base;
-	union {
-		struct {
-			u64 tail_drop_base;
-			u64 ecn_base;
-			u64 wred_drop_base;
-		} red;
-	} xstats;
-
-	u64 tx_bytes;
-	u64 tx_packets;
-	u64 drops;
-	u64 overlimits;
-};
-
 /* No need an internal lock; At worse - miss a single periodic iteration */
 struct mlxsw_sp_port_xstats {
 	u64 ecn;
@@ -269,7 +246,7 @@ struct mlxsw_sp_port {
 	} periodic_hw_stats;
 	struct mlxsw_sp_port_sample *sample;
 	struct list_head vlans_list;
-	struct mlxsw_sp_qdisc root_qdisc;
+	struct mlxsw_sp_qdisc *root_qdisc;
 	unsigned acl_rule_count;
 };
 
@@ -584,6 +561,8 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp_port *mlxsw_sp_port, bool ingress,
 			  struct tc_cls_flower_offload *f);
 
 /* spectrum_qdisc.c */
+int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port);
+void mlxsw_sp_tc_qdisc_fini(struct mlxsw_sp_port *mlxsw_sp_port);
 int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 			  struct tc_red_qopt_offload *p);
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index c33beac5def0..8172cc902f12 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -41,6 +41,28 @@
 #include "spectrum.h"
 #include "reg.h"
 
+enum mlxsw_sp_qdisc_type {
+	MLXSW_SP_QDISC_NO_QDISC,
+	MLXSW_SP_QDISC_RED,
+};
+
+struct mlxsw_sp_qdisc {
+	u32 handle;
+	enum mlxsw_sp_qdisc_type type;
+	struct red_stats xstats_base;
+	union {
+		struct {
+			u64 tail_drop_base;
+			u64 ecn_base;
+			u64 wred_drop_base;
+		} red;
+	} xstats;
+	u64 tx_bytes;
+	u64 tx_packets;
+	u64 drops;
+	u64 overlimits;
+};
+
 static int
 mlxsw_sp_tclass_congestion_enable(struct mlxsw_sp_port *mlxsw_sp_port,
 				  int tclass_num, u32 min, u32 max,
@@ -251,7 +273,7 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 	if (p->parent != TC_H_ROOT)
 		return -EOPNOTSUPP;
 
-	mlxsw_sp_qdisc = &mlxsw_sp_port->root_qdisc;
+	mlxsw_sp_qdisc = mlxsw_sp_port->root_qdisc;
 	tclass_num = MLXSW_SP_PORT_DEFAULT_TCLASS;
 
 	switch (p->command) {
@@ -274,3 +296,18 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 		return -EOPNOTSUPP;
 	}
 }
+
+int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port)
+{
+	mlxsw_sp_port->root_qdisc = kzalloc(sizeof(*mlxsw_sp_port->root_qdisc),
+					    GFP_KERNEL);
+	if (!mlxsw_sp_port->root_qdisc)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void mlxsw_sp_tc_qdisc_fini(struct mlxsw_sp_port *mlxsw_sp_port)
+{
+	kfree(mlxsw_sp_port->root_qdisc);
+}
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 02/11] net: sch: red: Change the name of the stats struct to be generic
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 01/11] mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated file Jiri Pirko
@ 2018-01-10 13:59 ` Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 03/11] net: sch: red: Change offloaded xstats to be incremental Jiri Pirko
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 13:59 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Change the name of the stats struct to be generic, so it could be used for
other qdisc offload, that will be added in the next patches.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c |  2 +-
 include/net/pkt_cls.h                                | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 8172cc902f12..3e2841872f64 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -229,7 +229,7 @@ static int
 mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			     struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			     int tclass_num,
-			     struct tc_red_qopt_offload_stats *res)
+			     struct tc_qopt_offload_stats *res)
 {
 	u64 tx_bytes, tx_packets, overlimits, drops;
 	struct mlxsw_sp_port_xstats *xstats;
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index c4f4e46ea8d6..0d1343cba84c 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -731,6 +731,11 @@ struct tc_cookie {
 	u32 len;
 };
 
+struct tc_qopt_offload_stats {
+	struct gnet_stats_basic_packed *bstats;
+	struct gnet_stats_queue *qstats;
+};
+
 enum tc_red_command {
 	TC_RED_REPLACE,
 	TC_RED_DESTROY,
@@ -744,10 +749,6 @@ struct tc_red_qopt_offload_params {
 	u32 probability;
 	bool is_ecn;
 };
-struct tc_red_qopt_offload_stats {
-	struct gnet_stats_basic_packed *bstats;
-	struct gnet_stats_queue *qstats;
-};
 
 struct tc_red_qopt_offload {
 	enum tc_red_command command;
@@ -755,7 +756,7 @@ struct tc_red_qopt_offload {
 	u32 parent;
 	union {
 		struct tc_red_qopt_offload_params set;
-		struct tc_red_qopt_offload_stats stats;
+		struct tc_qopt_offload_stats stats;
 		struct red_stats *xstats;
 	};
 };
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 03/11] net: sch: red: Change offloaded xstats to be incremental
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 01/11] mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated file Jiri Pirko
  2018-01-10 13:59 ` [patch net-next 02/11] net: sch: red: Change the name of the stats struct to be generic Jiri Pirko
@ 2018-01-10 13:59 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 04/11] mlxsw: spectrum: qdiscs: Clean qdisc statistics structs Jiri Pirko
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 13:59 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Change the value of the xstats requested from the driver for offloaded RED
to be incremental, like the normal stats.
It increases consistency - if a qdisc stops being offloaded its xstats
don't change.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 15 +++++++++++---
 net/sched/sch_red.c                                | 24 ++++++++--------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 3e2841872f64..55e4e4d0dad3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -212,6 +212,7 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 {
 	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base;
 	struct mlxsw_sp_port_xstats *xstats;
+	int early_drops, marks, pdrops;
 
 	if (mlxsw_sp_qdisc->handle != handle ||
 	    mlxsw_sp_qdisc->type != MLXSW_SP_QDISC_RED)
@@ -219,9 +220,17 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 
-	res->prob_drop = xstats->wred_drop[tclass_num] - xstats_base->prob_drop;
-	res->prob_mark = xstats->ecn - xstats_base->prob_mark;
-	res->pdrop = xstats->tail_drop[tclass_num] - xstats_base->pdrop;
+	early_drops = xstats->wred_drop[tclass_num] - xstats_base->prob_drop;
+	marks = xstats->ecn - xstats_base->prob_mark;
+	pdrops = xstats->tail_drop[tclass_num] - xstats_base->pdrop;
+
+	res->pdrop += pdrops;
+	res->prob_drop += early_drops;
+	res->prob_mark += marks;
+
+	xstats_base->pdrop += pdrops;
+	xstats_base->prob_drop += early_drops;
+	xstats_base->prob_mark += marks;
 	return 0;
 }
 
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index a392eaa4a0b4..0af1c1254e0b 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -344,32 +344,24 @@ static int red_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 {
 	struct red_sched_data *q = qdisc_priv(sch);
 	struct net_device *dev = qdisc_dev(sch);
-	struct tc_red_xstats st = {
-		.early	= q->stats.prob_drop + q->stats.forced_drop,
-		.pdrop	= q->stats.pdrop,
-		.other	= q->stats.other,
-		.marked	= q->stats.prob_mark + q->stats.forced_mark,
-	};
+	struct tc_red_xstats st = {0};
 
 	if (sch->flags & TCQ_F_OFFLOADED) {
-		struct red_stats hw_stats = {0};
 		struct tc_red_qopt_offload hw_stats_request = {
 			.command = TC_RED_XSTATS,
 			.handle = sch->handle,
 			.parent = sch->parent,
 			{
-				.xstats = &hw_stats,
+				.xstats = &q->stats,
 			},
 		};
-		if (!dev->netdev_ops->ndo_setup_tc(dev,
-						   TC_SETUP_QDISC_RED,
-						   &hw_stats_request)) {
-			st.early += hw_stats.prob_drop + hw_stats.forced_drop;
-			st.pdrop += hw_stats.pdrop;
-			st.other += hw_stats.other;
-			st.marked += hw_stats.prob_mark + hw_stats.forced_mark;
-		}
+		dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_RED,
+					      &hw_stats_request);
 	}
+	st.early = q->stats.prob_drop + q->stats.forced_drop;
+	st.pdrop = q->stats.pdrop;
+	st.other = q->stats.other;
+	st.marked = q->stats.prob_mark + q->stats.forced_mark;
 
 	return gnet_stats_copy_app(d, &st, sizeof(st));
 }
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 04/11] mlxsw: spectrum: qdiscs: Clean qdisc statistics structs
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (2 preceding siblings ...)
  2018-01-10 13:59 ` [patch net-next 03/11] net: sch: red: Change offloaded xstats to be incremental Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 05/11] mlxsw: spectrum: qdiscs: Make the clean stats function to be for RED only Jiri Pirko
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Clean RED offloaded stats and make them more generic by breaking the
generic qdisc stats to a struct of their own.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 63 +++++++++++-----------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 55e4e4d0dad3..0b6e553978b3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -49,18 +49,15 @@ enum mlxsw_sp_qdisc_type {
 struct mlxsw_sp_qdisc {
 	u32 handle;
 	enum mlxsw_sp_qdisc_type type;
-	struct red_stats xstats_base;
 	union {
-		struct {
-			u64 tail_drop_base;
-			u64 ecn_base;
-			u64 wred_drop_base;
-		} red;
-	} xstats;
-	u64 tx_bytes;
-	u64 tx_packets;
-	u64 drops;
-	u64 overlimits;
+		struct red_stats red;
+	} xstats_base;
+	struct mlxsw_sp_qdisc_stats {
+		u64 tx_bytes;
+		u64 tx_packets;
+		u64 drops;
+		u64 overlimits;
+	} stats_base;
 };
 
 static int
@@ -105,26 +102,28 @@ mlxsw_sp_setup_tc_qdisc_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 				    struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 				    int tclass_num)
 {
-	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base;
+	struct mlxsw_sp_qdisc_stats *stats_base;
 	struct mlxsw_sp_port_xstats *xstats;
 	struct rtnl_link_stats64 *stats;
+	struct red_stats *red_base;
 
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 	stats = &mlxsw_sp_port->periodic_hw_stats.stats;
+	stats_base = &mlxsw_sp_qdisc->stats_base;
 
-	mlxsw_sp_qdisc->tx_packets = stats->tx_packets;
-	mlxsw_sp_qdisc->tx_bytes = stats->tx_bytes;
+	stats_base->tx_packets = stats->tx_packets;
+	stats_base->tx_bytes = stats->tx_bytes;
 
 	switch (mlxsw_sp_qdisc->type) {
 	case MLXSW_SP_QDISC_RED:
-		xstats_base->prob_mark = xstats->ecn;
-		xstats_base->prob_drop = xstats->wred_drop[tclass_num];
-		xstats_base->pdrop = xstats->tail_drop[tclass_num];
-
-		mlxsw_sp_qdisc->overlimits = xstats_base->prob_drop +
-					     xstats_base->prob_mark;
-		mlxsw_sp_qdisc->drops = xstats_base->prob_drop +
-					xstats_base->pdrop;
+		red_base = &mlxsw_sp_qdisc->xstats_base.red;
+		red_base->prob_mark = xstats->ecn;
+		red_base->prob_drop = xstats->wred_drop[tclass_num];
+		red_base->pdrop = xstats->tail_drop[tclass_num];
+
+		stats_base->overlimits = red_base->prob_drop +
+					 red_base->prob_mark;
+		stats_base->drops = red_base->prob_drop + red_base->pdrop;
 		break;
 	default:
 		break;
@@ -210,7 +209,7 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			      struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			      int tclass_num, struct red_stats *res)
 {
-	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base;
+	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base.red;
 	struct mlxsw_sp_port_xstats *xstats;
 	int early_drops, marks, pdrops;
 
@@ -241,6 +240,7 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			     struct tc_qopt_offload_stats *res)
 {
 	u64 tx_bytes, tx_packets, overlimits, drops;
+	struct mlxsw_sp_qdisc_stats *stats_base;
 	struct mlxsw_sp_port_xstats *xstats;
 	struct rtnl_link_stats64 *stats;
 
@@ -250,13 +250,14 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 	stats = &mlxsw_sp_port->periodic_hw_stats.stats;
+	stats_base = &mlxsw_sp_qdisc->stats_base;
 
-	tx_bytes = stats->tx_bytes - mlxsw_sp_qdisc->tx_bytes;
-	tx_packets = stats->tx_packets - mlxsw_sp_qdisc->tx_packets;
+	tx_bytes = stats->tx_bytes - stats_base->tx_bytes;
+	tx_packets = stats->tx_packets - stats_base->tx_packets;
 	overlimits = xstats->wred_drop[tclass_num] + xstats->ecn -
-		     mlxsw_sp_qdisc->overlimits;
+		     stats_base->overlimits;
 	drops = xstats->wred_drop[tclass_num] + xstats->tail_drop[tclass_num] -
-		mlxsw_sp_qdisc->drops;
+		stats_base->drops;
 
 	_bstats_update(res->bstats, tx_bytes, tx_packets);
 	res->qstats->overlimits += overlimits;
@@ -264,10 +265,10 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	res->qstats->backlog += mlxsw_sp_cells_bytes(mlxsw_sp_port->mlxsw_sp,
 						xstats->backlog[tclass_num]);
 
-	mlxsw_sp_qdisc->drops +=  drops;
-	mlxsw_sp_qdisc->overlimits += overlimits;
-	mlxsw_sp_qdisc->tx_bytes += tx_bytes;
-	mlxsw_sp_qdisc->tx_packets += tx_packets;
+	stats_base->drops +=  drops;
+	stats_base->overlimits += overlimits;
+	stats_base->tx_bytes += tx_bytes;
+	stats_base->tx_packets += tx_packets;
 	return 0;
 }
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 05/11] mlxsw: spectrum: qdiscs: Make the clean stats function to be for RED only
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (3 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 04/11] mlxsw: spectrum: qdiscs: Clean qdisc statistics structs Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 06/11] mlxsw: spectrum: qdiscs: Add tclass number to the mlxsw_sp_qdisc Jiri Pirko
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Improve readability by changing the clean stats function to handle only
RED. Qdiscs that will be offloaded in the future will have a clean stats
function of their own.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 33 +++++++++-------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 0b6e553978b3..6f7687bfa1e2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -98,9 +98,9 @@ mlxsw_sp_tclass_congestion_disable(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static void
-mlxsw_sp_setup_tc_qdisc_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
-				    struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-				    int tclass_num)
+mlxsw_sp_setup_tc_qdisc_red_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
+					struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+					int tclass_num)
 {
 	struct mlxsw_sp_qdisc_stats *stats_base;
 	struct mlxsw_sp_port_xstats *xstats;
@@ -110,24 +110,17 @@ mlxsw_sp_setup_tc_qdisc_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 	stats = &mlxsw_sp_port->periodic_hw_stats.stats;
 	stats_base = &mlxsw_sp_qdisc->stats_base;
+	red_base = &mlxsw_sp_qdisc->xstats_base.red;
 
 	stats_base->tx_packets = stats->tx_packets;
 	stats_base->tx_bytes = stats->tx_bytes;
 
-	switch (mlxsw_sp_qdisc->type) {
-	case MLXSW_SP_QDISC_RED:
-		red_base = &mlxsw_sp_qdisc->xstats_base.red;
-		red_base->prob_mark = xstats->ecn;
-		red_base->prob_drop = xstats->wred_drop[tclass_num];
-		red_base->pdrop = xstats->tail_drop[tclass_num];
-
-		stats_base->overlimits = red_base->prob_drop +
-					 red_base->prob_mark;
-		stats_base->drops = red_base->prob_drop + red_base->pdrop;
-		break;
-	default:
-		break;
-	}
+	red_base->prob_mark = xstats->ecn;
+	red_base->prob_drop = xstats->wred_drop[tclass_num];
+	red_base->pdrop = xstats->tail_drop[tclass_num];
+
+	stats_base->overlimits = red_base->prob_drop + red_base->prob_mark;
+	stats_base->drops = red_base->prob_drop + red_base->pdrop;
 }
 
 static int
@@ -189,9 +182,9 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 
 	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_RED;
 	if (mlxsw_sp_qdisc->handle != handle)
-		mlxsw_sp_setup_tc_qdisc_clean_stats(mlxsw_sp_port,
-						    mlxsw_sp_qdisc,
-						    tclass_num);
+		mlxsw_sp_setup_tc_qdisc_red_clean_stats(mlxsw_sp_port,
+							mlxsw_sp_qdisc,
+							tclass_num);
 
 	mlxsw_sp_qdisc->handle = handle;
 	return 0;
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 06/11] mlxsw: spectrum: qdiscs: Add tclass number to the mlxsw_sp_qdisc
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (4 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 05/11] mlxsw: spectrum: qdiscs: Make the clean stats function to be for RED only Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 07/11] mlxsw: spectrum: qdiscs: Unite all handle checks Jiri Pirko
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Tclass number is needed for most of the operations related to the qdisc in
the driver. Create a field for it in the mlxsw_sp_qdisc instead of passing
it to every function as parameter.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 36 +++++++++++-----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 6f7687bfa1e2..dcf6d7c932f9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -49,6 +49,7 @@ enum mlxsw_sp_qdisc_type {
 struct mlxsw_sp_qdisc {
 	u32 handle;
 	enum mlxsw_sp_qdisc_type type;
+	u8 tclass_num;
 	union {
 		struct red_stats red;
 	} xstats_base;
@@ -99,9 +100,9 @@ mlxsw_sp_tclass_congestion_disable(struct mlxsw_sp_port *mlxsw_sp_port,
 
 static void
 mlxsw_sp_setup_tc_qdisc_red_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
-					struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-					int tclass_num)
+					struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
 {
+	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
 	struct mlxsw_sp_qdisc_stats *stats_base;
 	struct mlxsw_sp_port_xstats *xstats;
 	struct rtnl_link_stats64 *stats;
@@ -125,15 +126,15 @@ mlxsw_sp_setup_tc_qdisc_red_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 
 static int
 mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
-			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			   int tclass_num)
+			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
 {
 	int err;
 
 	if (mlxsw_sp_qdisc->handle != handle)
 		return 0;
 
-	err = mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port, tclass_num);
+	err = mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port,
+						 mlxsw_sp_qdisc->tclass_num);
 	mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
 	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_NO_QDISC;
 
@@ -143,10 +144,10 @@ mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 static int
 mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			   int tclass_num,
 			   struct tc_red_qopt_offload_params *p)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
 	u32 min, max;
 	u64 prob;
 	int err = 0;
@@ -183,8 +184,7 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_RED;
 	if (mlxsw_sp_qdisc->handle != handle)
 		mlxsw_sp_setup_tc_qdisc_red_clean_stats(mlxsw_sp_port,
-							mlxsw_sp_qdisc,
-							tclass_num);
+							mlxsw_sp_qdisc);
 
 	mlxsw_sp_qdisc->handle = handle;
 	return 0;
@@ -193,16 +193,17 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	err = -EINVAL;
 err_config:
 	mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, mlxsw_sp_qdisc->handle,
-				   mlxsw_sp_qdisc, tclass_num);
+				   mlxsw_sp_qdisc);
 	return err;
 }
 
 static int
 mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			      struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			      int tclass_num, struct red_stats *res)
+			      struct red_stats *res)
 {
 	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base.red;
+	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
 	struct mlxsw_sp_port_xstats *xstats;
 	int early_drops, marks, pdrops;
 
@@ -229,10 +230,10 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 static int
 mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			     struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			     int tclass_num,
 			     struct tc_qopt_offload_stats *res)
 {
 	u64 tx_bytes, tx_packets, overlimits, drops;
+	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
 	struct mlxsw_sp_qdisc_stats *stats_base;
 	struct mlxsw_sp_port_xstats *xstats;
 	struct rtnl_link_stats64 *stats;
@@ -271,29 +272,26 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 			  struct tc_red_qopt_offload *p)
 {
 	struct mlxsw_sp_qdisc *mlxsw_sp_qdisc;
-	int tclass_num;
 
 	if (p->parent != TC_H_ROOT)
 		return -EOPNOTSUPP;
 
 	mlxsw_sp_qdisc = mlxsw_sp_port->root_qdisc;
-	tclass_num = MLXSW_SP_PORT_DEFAULT_TCLASS;
 
 	switch (p->command) {
 	case TC_RED_REPLACE:
 		return mlxsw_sp_qdisc_red_replace(mlxsw_sp_port, p->handle,
-						  mlxsw_sp_qdisc, tclass_num,
-						  &p->set);
+						  mlxsw_sp_qdisc, &p->set);
 	case TC_RED_DESTROY:
 		return mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, p->handle,
-						  mlxsw_sp_qdisc, tclass_num);
+						  mlxsw_sp_qdisc);
 	case TC_RED_XSTATS:
 		return mlxsw_sp_qdisc_get_red_xstats(mlxsw_sp_port, p->handle,
-						     mlxsw_sp_qdisc, tclass_num,
+						     mlxsw_sp_qdisc,
 						     p->xstats);
 	case TC_RED_STATS:
 		return mlxsw_sp_qdisc_get_red_stats(mlxsw_sp_port, p->handle,
-						    mlxsw_sp_qdisc, tclass_num,
+						    mlxsw_sp_qdisc,
 						    &p->stats);
 	default:
 		return -EOPNOTSUPP;
@@ -307,6 +305,8 @@ int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port)
 	if (!mlxsw_sp_port->root_qdisc)
 		return -ENOMEM;
 
+	mlxsw_sp_port->root_qdisc->tclass_num = MLXSW_SP_PORT_DEFAULT_TCLASS;
+
 	return 0;
 }
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 07/11] mlxsw: spectrum: qdiscs: Unite all handle checks
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (5 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 06/11] mlxsw: spectrum: qdiscs: Add tclass number to the mlxsw_sp_qdisc Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 08/11] mlxsw: spectrum: qdiscs: Add an ops struct Jiri Pirko
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Every qdisc op gets the qdisc handle ID as well as its location.  Each one
of them, beside replace, checks if the handle doesn't match the qdisc in
the given location, and if so, it returns without running the actual op.
Unite these checks to one comparison function and avoid sending the handle
id to these ops.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 43 +++++++++++-----------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index dcf6d7c932f9..51ac8090b735 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -61,6 +61,14 @@ struct mlxsw_sp_qdisc {
 	} stats_base;
 };
 
+static bool
+mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle,
+		       enum mlxsw_sp_qdisc_type type)
+{
+	return mlxsw_sp_qdisc && mlxsw_sp_qdisc->handle == handle &&
+	       mlxsw_sp_qdisc->type == type;
+}
+
 static int
 mlxsw_sp_tclass_congestion_enable(struct mlxsw_sp_port *mlxsw_sp_port,
 				  int tclass_num, u32 min, u32 max,
@@ -125,14 +133,11 @@ mlxsw_sp_setup_tc_qdisc_red_clean_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int
-mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
+mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
 			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
 {
 	int err;
 
-	if (mlxsw_sp_qdisc->handle != handle)
-		return 0;
-
 	err = mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port,
 						 mlxsw_sp_qdisc->tclass_num);
 	mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
@@ -192,13 +197,12 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 err_bad_param:
 	err = -EINVAL;
 err_config:
-	mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, mlxsw_sp_qdisc->handle,
-				   mlxsw_sp_qdisc);
+	mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
 	return err;
 }
 
 static int
-mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
+mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port,
 			      struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			      struct red_stats *res)
 {
@@ -207,10 +211,6 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	struct mlxsw_sp_port_xstats *xstats;
 	int early_drops, marks, pdrops;
 
-	if (mlxsw_sp_qdisc->handle != handle ||
-	    mlxsw_sp_qdisc->type != MLXSW_SP_QDISC_RED)
-		return -EOPNOTSUPP;
-
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 
 	early_drops = xstats->wred_drop[tclass_num] - xstats_base->prob_drop;
@@ -228,7 +228,7 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 }
 
 static int
-mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
+mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 			     struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			     struct tc_qopt_offload_stats *res)
 {
@@ -238,10 +238,6 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	struct mlxsw_sp_port_xstats *xstats;
 	struct rtnl_link_stats64 *stats;
 
-	if (mlxsw_sp_qdisc->handle != handle ||
-	    mlxsw_sp_qdisc->type != MLXSW_SP_QDISC_RED)
-		return -EOPNOTSUPP;
-
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
 	stats = &mlxsw_sp_port->periodic_hw_stats.stats;
 	stats_base = &mlxsw_sp_qdisc->stats_base;
@@ -278,19 +274,24 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	mlxsw_sp_qdisc = mlxsw_sp_port->root_qdisc;
 
-	switch (p->command) {
-	case TC_RED_REPLACE:
+	if (p->command == TC_RED_REPLACE)
 		return mlxsw_sp_qdisc_red_replace(mlxsw_sp_port, p->handle,
 						  mlxsw_sp_qdisc, &p->set);
+
+	if (!mlxsw_sp_qdisc_compare(mlxsw_sp_qdisc, p->handle,
+				    MLXSW_SP_QDISC_RED))
+		return -EOPNOTSUPP;
+
+	switch (p->command) {
 	case TC_RED_DESTROY:
-		return mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, p->handle,
+		return mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port,
 						  mlxsw_sp_qdisc);
 	case TC_RED_XSTATS:
-		return mlxsw_sp_qdisc_get_red_xstats(mlxsw_sp_port, p->handle,
+		return mlxsw_sp_qdisc_get_red_xstats(mlxsw_sp_port,
 						     mlxsw_sp_qdisc,
 						     p->xstats);
 	case TC_RED_STATS:
-		return mlxsw_sp_qdisc_get_red_stats(mlxsw_sp_port, p->handle,
+		return mlxsw_sp_qdisc_get_red_stats(mlxsw_sp_port,
 						    mlxsw_sp_qdisc,
 						    &p->stats);
 	default:
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 08/11] mlxsw: spectrum: qdiscs: Add an ops struct
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (6 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 07/11] mlxsw: spectrum: qdiscs: Unite all handle checks Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 09/11] mlxsw: spectrum: qdiscs: Create a generic destroy function Jiri Pirko
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Qdisc struct have the Qdisc_class_ops struct.
This patch introduces the similar ops struct for the mlxsw_sp_qdisc_ops
struct. It allows better readability as well as code reusability for the
common parts of some functions like destroy.
The first operations to be added are the statistics getters.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 76 ++++++++++++++++++----
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 51ac8090b735..417350ceda7f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -46,6 +46,15 @@ enum mlxsw_sp_qdisc_type {
 	MLXSW_SP_QDISC_RED,
 };
 
+struct mlxsw_sp_qdisc_ops {
+	int (*get_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
+			 struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			 struct tc_qopt_offload_stats *stats_ptr);
+	int (*get_xstats)(struct mlxsw_sp_port *mlxsw_sp_port,
+			  struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			  void *xstats_ptr);
+};
+
 struct mlxsw_sp_qdisc {
 	u32 handle;
 	enum mlxsw_sp_qdisc_type type;
@@ -59,6 +68,8 @@ struct mlxsw_sp_qdisc {
 		u64 drops;
 		u64 overlimits;
 	} stats_base;
+
+	struct mlxsw_sp_qdisc_ops *ops;
 };
 
 static bool
@@ -69,6 +80,34 @@ mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle,
 	       mlxsw_sp_qdisc->type == type;
 }
 
+static int
+mlxsw_sp_qdisc_get_stats(struct mlxsw_sp_port *mlxsw_sp_port,
+			 struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			 struct tc_qopt_offload_stats *stats_ptr)
+{
+	if (mlxsw_sp_qdisc && mlxsw_sp_qdisc->ops &&
+	    mlxsw_sp_qdisc->ops->get_stats)
+		return mlxsw_sp_qdisc->ops->get_stats(mlxsw_sp_port,
+						      mlxsw_sp_qdisc,
+						      stats_ptr);
+
+	return -EOPNOTSUPP;
+}
+
+static int
+mlxsw_sp_qdisc_get_xstats(struct mlxsw_sp_port *mlxsw_sp_port,
+			  struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			  void *xstats_ptr)
+{
+	if (mlxsw_sp_qdisc && mlxsw_sp_qdisc->ops &&
+	    mlxsw_sp_qdisc->ops->get_xstats)
+		return mlxsw_sp_qdisc->ops->get_xstats(mlxsw_sp_port,
+						      mlxsw_sp_qdisc,
+						      xstats_ptr);
+
+	return -EOPNOTSUPP;
+}
+
 static int
 mlxsw_sp_tclass_congestion_enable(struct mlxsw_sp_port *mlxsw_sp_port,
 				  int tclass_num, u32 min, u32 max,
@@ -149,6 +188,7 @@ mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
 static int
 mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			   struct mlxsw_sp_qdisc_ops *ops,
 			   struct tc_red_qopt_offload_params *p)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
@@ -187,6 +227,7 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 		goto err_config;
 
 	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_RED;
+	mlxsw_sp_qdisc->ops = ops;
 	if (mlxsw_sp_qdisc->handle != handle)
 		mlxsw_sp_setup_tc_qdisc_red_clean_stats(mlxsw_sp_port,
 							mlxsw_sp_qdisc);
@@ -204,11 +245,12 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 static int
 mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port,
 			      struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			      struct red_stats *res)
+			      void *xstats_ptr)
 {
 	struct red_stats *xstats_base = &mlxsw_sp_qdisc->xstats_base.red;
 	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
 	struct mlxsw_sp_port_xstats *xstats;
+	struct red_stats *res = xstats_ptr;
 	int early_drops, marks, pdrops;
 
 	xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
@@ -230,7 +272,7 @@ mlxsw_sp_qdisc_get_red_xstats(struct mlxsw_sp_port *mlxsw_sp_port,
 static int
 mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 			     struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			     struct tc_qopt_offload_stats *res)
+			     struct tc_qopt_offload_stats *stats_ptr)
 {
 	u64 tx_bytes, tx_packets, overlimits, drops;
 	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
@@ -249,11 +291,12 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 	drops = xstats->wred_drop[tclass_num] + xstats->tail_drop[tclass_num] -
 		stats_base->drops;
 
-	_bstats_update(res->bstats, tx_bytes, tx_packets);
-	res->qstats->overlimits += overlimits;
-	res->qstats->drops += drops;
-	res->qstats->backlog += mlxsw_sp_cells_bytes(mlxsw_sp_port->mlxsw_sp,
-						xstats->backlog[tclass_num]);
+	_bstats_update(stats_ptr->bstats, tx_bytes, tx_packets);
+	stats_ptr->qstats->overlimits += overlimits;
+	stats_ptr->qstats->drops += drops;
+	stats_ptr->qstats->backlog +=
+			mlxsw_sp_cells_bytes(mlxsw_sp_port->mlxsw_sp,
+					     xstats->backlog[tclass_num]);
 
 	stats_base->drops +=  drops;
 	stats_base->overlimits += overlimits;
@@ -264,6 +307,11 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 
 #define MLXSW_SP_PORT_DEFAULT_TCLASS 0
 
+static struct mlxsw_sp_qdisc_ops mlxsw_sp_qdisc_ops_red = {
+	.get_stats = mlxsw_sp_qdisc_get_red_stats,
+	.get_xstats = mlxsw_sp_qdisc_get_red_xstats,
+};
+
 int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 			  struct tc_red_qopt_offload *p)
 {
@@ -276,7 +324,9 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	if (p->command == TC_RED_REPLACE)
 		return mlxsw_sp_qdisc_red_replace(mlxsw_sp_port, p->handle,
-						  mlxsw_sp_qdisc, &p->set);
+						  mlxsw_sp_qdisc,
+						  &mlxsw_sp_qdisc_ops_red,
+						  &p->set);
 
 	if (!mlxsw_sp_qdisc_compare(mlxsw_sp_qdisc, p->handle,
 				    MLXSW_SP_QDISC_RED))
@@ -287,13 +337,11 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 		return mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port,
 						  mlxsw_sp_qdisc);
 	case TC_RED_XSTATS:
-		return mlxsw_sp_qdisc_get_red_xstats(mlxsw_sp_port,
-						     mlxsw_sp_qdisc,
-						     p->xstats);
+		return mlxsw_sp_qdisc_get_xstats(mlxsw_sp_port, mlxsw_sp_qdisc,
+						 p->xstats);
 	case TC_RED_STATS:
-		return mlxsw_sp_qdisc_get_red_stats(mlxsw_sp_port,
-						    mlxsw_sp_qdisc,
-						    &p->stats);
+		return mlxsw_sp_qdisc_get_stats(mlxsw_sp_port, mlxsw_sp_qdisc,
+						&p->stats);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 09/11] mlxsw: spectrum: qdiscs: Create a generic destroy function
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (7 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 08/11] mlxsw: spectrum: qdiscs: Add an ops struct Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 10/11] mlxsw: spectrum: qdiscs: Create a generic replace function Jiri Pirko
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Add a destroy function to the qdiscs ops struct.
Create a generic qdisc destroy function, that clears the qdisc metadata as
well as calling the specific qdisc destroy function.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 37 +++++++++++++++-------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 417350ceda7f..9b23dfc95ad9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -47,6 +47,8 @@ enum mlxsw_sp_qdisc_type {
 };
 
 struct mlxsw_sp_qdisc_ops {
+	int (*destroy)(struct mlxsw_sp_port *mlxsw_sp_port,
+		       struct mlxsw_sp_qdisc *mlxsw_sp_qdisc);
 	int (*get_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
 			 struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			 struct tc_qopt_offload_stats *stats_ptr);
@@ -80,6 +82,25 @@ mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle,
 	       mlxsw_sp_qdisc->type == type;
 }
 
+static int
+mlxsw_sp_qdisc_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
+		       struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
+{
+	int err = 0;
+
+	if (!mlxsw_sp_qdisc)
+		return 0;
+
+	if (mlxsw_sp_qdisc->ops && mlxsw_sp_qdisc->ops->destroy)
+		err = mlxsw_sp_qdisc->ops->destroy(mlxsw_sp_port,
+						   mlxsw_sp_qdisc);
+
+	mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
+	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_NO_QDISC;
+	mlxsw_sp_qdisc->ops = NULL;
+	return err;
+}
+
 static int
 mlxsw_sp_qdisc_get_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 			 struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
@@ -175,14 +196,8 @@ static int
 mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
 			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
 {
-	int err;
-
-	err = mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port,
-						 mlxsw_sp_qdisc->tclass_num);
-	mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
-	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_NO_QDISC;
-
-	return err;
+	return mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port,
+						  mlxsw_sp_qdisc->tclass_num);
 }
 
 static int
@@ -238,7 +253,7 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 err_bad_param:
 	err = -EINVAL;
 err_config:
-	mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
+	mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
 	return err;
 }
 
@@ -308,6 +323,7 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 #define MLXSW_SP_PORT_DEFAULT_TCLASS 0
 
 static struct mlxsw_sp_qdisc_ops mlxsw_sp_qdisc_ops_red = {
+	.destroy = mlxsw_sp_qdisc_red_destroy,
 	.get_stats = mlxsw_sp_qdisc_get_red_stats,
 	.get_xstats = mlxsw_sp_qdisc_get_red_xstats,
 };
@@ -334,8 +350,7 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 
 	switch (p->command) {
 	case TC_RED_DESTROY:
-		return mlxsw_sp_qdisc_red_destroy(mlxsw_sp_port,
-						  mlxsw_sp_qdisc);
+		return mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
 	case TC_RED_XSTATS:
 		return mlxsw_sp_qdisc_get_xstats(mlxsw_sp_port, mlxsw_sp_qdisc,
 						 p->xstats);
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 10/11] mlxsw: spectrum: qdiscs: Create a generic replace function
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (8 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 09/11] mlxsw: spectrum: qdiscs: Create a generic destroy function Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 14:00 ` [patch net-next 11/11] mlxsw: spectrum: qdiscs: Remove qdisc before setting a new one Jiri Pirko
  2018-01-10 21:08 ` [patch net-next 00/11] mlxsw qdisc refactoring David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

Create a generic qdisc replace function.
For that goal, add three functions to the qdisc ops struct:
* check_params: Checks if the given parameters are offloadable.
* replace: Offload the given parameters.
* clean_stats: clean the qdisc stats for the offloaded qdisc.
integrate RED offloading into using the new internal replace API.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c   | 109 ++++++++++++++-------
 1 file changed, 71 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 9b23dfc95ad9..c896ee721523 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -47,6 +47,12 @@ enum mlxsw_sp_qdisc_type {
 };
 
 struct mlxsw_sp_qdisc_ops {
+	enum mlxsw_sp_qdisc_type type;
+	int (*check_params)(struct mlxsw_sp_port *mlxsw_sp_port,
+			    struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			    void *params);
+	int (*replace)(struct mlxsw_sp_port *mlxsw_sp_port,
+		       struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void *params);
 	int (*destroy)(struct mlxsw_sp_port *mlxsw_sp_port,
 		       struct mlxsw_sp_qdisc *mlxsw_sp_qdisc);
 	int (*get_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
@@ -55,11 +61,12 @@ struct mlxsw_sp_qdisc_ops {
 	int (*get_xstats)(struct mlxsw_sp_port *mlxsw_sp_port,
 			  struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
 			  void *xstats_ptr);
+	void (*clean_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
+			    struct mlxsw_sp_qdisc *mlxsw_sp_qdisc);
 };
 
 struct mlxsw_sp_qdisc {
 	u32 handle;
-	enum mlxsw_sp_qdisc_type type;
 	u8 tclass_num;
 	union {
 		struct red_stats red;
@@ -78,8 +85,9 @@ static bool
 mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle,
 		       enum mlxsw_sp_qdisc_type type)
 {
-	return mlxsw_sp_qdisc && mlxsw_sp_qdisc->handle == handle &&
-	       mlxsw_sp_qdisc->type == type;
+	return mlxsw_sp_qdisc && mlxsw_sp_qdisc->ops &&
+	       mlxsw_sp_qdisc->ops->type == type &&
+	       mlxsw_sp_qdisc->handle == handle;
 }
 
 static int
@@ -96,11 +104,40 @@ mlxsw_sp_qdisc_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
 						   mlxsw_sp_qdisc);
 
 	mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
-	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_NO_QDISC;
 	mlxsw_sp_qdisc->ops = NULL;
 	return err;
 }
 
+static int
+mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
+		       struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+		       struct mlxsw_sp_qdisc_ops *ops, void *params)
+{
+	int err;
+
+	err = ops->check_params(mlxsw_sp_port, mlxsw_sp_qdisc, params);
+	if (err)
+		goto err_bad_param;
+
+	err = ops->replace(mlxsw_sp_port, mlxsw_sp_qdisc, params);
+	if (err)
+		goto err_config;
+
+	if (mlxsw_sp_qdisc->handle != handle) {
+		mlxsw_sp_qdisc->ops = ops;
+		if (ops->clean_stats)
+			ops->clean_stats(mlxsw_sp_port, mlxsw_sp_qdisc);
+	}
+
+	mlxsw_sp_qdisc->handle = handle;
+	return 0;
+
+err_bad_param:
+err_config:
+	mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
+	return err;
+}
+
 static int
 mlxsw_sp_qdisc_get_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 			 struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
@@ -201,33 +238,42 @@ mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
 }
 
 static int
-mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
-			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
-			   struct mlxsw_sp_qdisc_ops *ops,
-			   struct tc_red_qopt_offload_params *p)
+mlxsw_sp_qdisc_red_check_params(struct mlxsw_sp_port *mlxsw_sp_port,
+				struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+				void *params)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
-	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
-	u32 min, max;
-	u64 prob;
-	int err = 0;
+	struct tc_red_qopt_offload_params *p = params;
 
 	if (p->min > p->max) {
 		dev_err(mlxsw_sp->bus_info->dev,
 			"spectrum: RED: min %u is bigger then max %u\n", p->min,
 			p->max);
-		goto err_bad_param;
+		return -EINVAL;
 	}
 	if (p->max > MLXSW_CORE_RES_GET(mlxsw_sp->core, MAX_BUFFER_SIZE)) {
 		dev_err(mlxsw_sp->bus_info->dev,
 			"spectrum: RED: max value %u is too big\n", p->max);
-		goto err_bad_param;
+		return -EINVAL;
 	}
 	if (p->min == 0 || p->max == 0) {
 		dev_err(mlxsw_sp->bus_info->dev,
 			"spectrum: RED: 0 value is illegal for min and max\n");
-		goto err_bad_param;
+		return -EINVAL;
 	}
+	return 0;
+}
+
+static int
+mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port,
+			   struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+			   void *params)
+{
+	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+	struct tc_red_qopt_offload_params *p = params;
+	u8 tclass_num = mlxsw_sp_qdisc->tclass_num;
+	u32 min, max;
+	u64 prob;
 
 	/* calculate probability in percentage */
 	prob = p->probability;
@@ -236,25 +282,8 @@ mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 	prob = DIV_ROUND_UP(prob, 1 << 16);
 	min = mlxsw_sp_bytes_cells(mlxsw_sp, p->min);
 	max = mlxsw_sp_bytes_cells(mlxsw_sp, p->max);
-	err = mlxsw_sp_tclass_congestion_enable(mlxsw_sp_port, tclass_num, min,
-						max, prob, p->is_ecn);
-	if (err)
-		goto err_config;
-
-	mlxsw_sp_qdisc->type = MLXSW_SP_QDISC_RED;
-	mlxsw_sp_qdisc->ops = ops;
-	if (mlxsw_sp_qdisc->handle != handle)
-		mlxsw_sp_setup_tc_qdisc_red_clean_stats(mlxsw_sp_port,
-							mlxsw_sp_qdisc);
-
-	mlxsw_sp_qdisc->handle = handle;
-	return 0;
-
-err_bad_param:
-	err = -EINVAL;
-err_config:
-	mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
-	return err;
+	return mlxsw_sp_tclass_congestion_enable(mlxsw_sp_port, tclass_num, min,
+						 max, prob, p->is_ecn);
 }
 
 static int
@@ -323,9 +352,13 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
 #define MLXSW_SP_PORT_DEFAULT_TCLASS 0
 
 static struct mlxsw_sp_qdisc_ops mlxsw_sp_qdisc_ops_red = {
+	.type = MLXSW_SP_QDISC_RED,
+	.check_params = mlxsw_sp_qdisc_red_check_params,
+	.replace = mlxsw_sp_qdisc_red_replace,
 	.destroy = mlxsw_sp_qdisc_red_destroy,
 	.get_stats = mlxsw_sp_qdisc_get_red_stats,
 	.get_xstats = mlxsw_sp_qdisc_get_red_xstats,
+	.clean_stats = mlxsw_sp_setup_tc_qdisc_red_clean_stats,
 };
 
 int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
@@ -339,10 +372,10 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
 	mlxsw_sp_qdisc = mlxsw_sp_port->root_qdisc;
 
 	if (p->command == TC_RED_REPLACE)
-		return mlxsw_sp_qdisc_red_replace(mlxsw_sp_port, p->handle,
-						  mlxsw_sp_qdisc,
-						  &mlxsw_sp_qdisc_ops_red,
-						  &p->set);
+		return mlxsw_sp_qdisc_replace(mlxsw_sp_port, p->handle,
+					      mlxsw_sp_qdisc,
+					      &mlxsw_sp_qdisc_ops_red,
+					      &p->set);
 
 	if (!mlxsw_sp_qdisc_compare(mlxsw_sp_qdisc, p->handle,
 				    MLXSW_SP_QDISC_RED))
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [patch net-next 11/11] mlxsw: spectrum: qdiscs: Remove qdisc before setting a new one
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (9 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 10/11] mlxsw: spectrum: qdiscs: Create a generic replace function Jiri Pirko
@ 2018-01-10 14:00 ` Jiri Pirko
  2018-01-10 21:08 ` [patch net-next 00/11] mlxsw qdisc refactoring David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Jiri Pirko @ 2018-01-10 14:00 UTC (permalink / raw)
  To: netdev; +Cc: nogahf, davem, idosch, mlxsw, jhs, xiyou.wangcong

From: Nogah Frankel <nogahf@mellanox.com>

If a qdisc is being replaced by another qdisc of the same type, it can
simply override over its configuration.
However, if it replaces a qdisc of another type, it needs to be removed
before setting the new qdisc.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Yuval Mintz <yuvalm@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index c896ee721523..273300b75a68 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -115,6 +115,13 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
 {
 	int err;
 
+	if (mlxsw_sp_qdisc->ops && mlxsw_sp_qdisc->ops->type != ops->type)
+		/* In case this location contained a different qdisc of the
+		 * same type we can override the old qdisc configuration.
+		 * Otherwise, we need to remove the old qdisc before setting the
+		 * new one.
+		 */
+		mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
 	err = ops->check_params(mlxsw_sp_port, mlxsw_sp_qdisc, params);
 	if (err)
 		goto err_bad_param;
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [patch net-next 00/11] mlxsw qdisc refactoring
  2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
                   ` (10 preceding siblings ...)
  2018-01-10 14:00 ` [patch net-next 11/11] mlxsw: spectrum: qdiscs: Remove qdisc before setting a new one Jiri Pirko
@ 2018-01-10 21:08 ` David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2018-01-10 21:08 UTC (permalink / raw)
  To: jiri; +Cc: netdev, nogahf, idosch, mlxsw, jhs, xiyou.wangcong

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 10 Jan 2018 14:59:56 +0100

> From: Jiri Pirko <jiri@mellanox.com>
> 
> This patchset refactors the qdisc handling in mlxsw driver in order to make
> it more object oriented like.
> It helps readability, laying the groundwork for the offloading of
> additional qdiscs by the driver
> This patchset also makes the qdiscs statistics more generic.
> 
> Patch 1 moves the qdiscs declaration to the spectrum_qdisc.c
> Patches 2-3 clean the offloaded stats requests. Patch 2 changes the RED
> generic stats struct to be sharable by other offloaded qdiscs. Patch 3
> changes the xstats request to be like the stats. Note that these patches
> are outside the driver scope.
> Patches 4-5 clean the statistics related functions and structs within the
> driver.
> Patches 6-7 decrease the need for the same parameters to be sent to many
> functions.
> Patches 8-11 create a functions pointers struct, to make the qdiscs
> handling more object oriented like.

Series applied, thanks Jiri.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-01-10 21:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 13:59 [patch net-next 00/11] mlxsw qdisc refactoring Jiri Pirko
2018-01-10 13:59 ` [patch net-next 01/11] mlxsw: spectrum: qdiscs: Move qdisc's declarations to its designated file Jiri Pirko
2018-01-10 13:59 ` [patch net-next 02/11] net: sch: red: Change the name of the stats struct to be generic Jiri Pirko
2018-01-10 13:59 ` [patch net-next 03/11] net: sch: red: Change offloaded xstats to be incremental Jiri Pirko
2018-01-10 14:00 ` [patch net-next 04/11] mlxsw: spectrum: qdiscs: Clean qdisc statistics structs Jiri Pirko
2018-01-10 14:00 ` [patch net-next 05/11] mlxsw: spectrum: qdiscs: Make the clean stats function to be for RED only Jiri Pirko
2018-01-10 14:00 ` [patch net-next 06/11] mlxsw: spectrum: qdiscs: Add tclass number to the mlxsw_sp_qdisc Jiri Pirko
2018-01-10 14:00 ` [patch net-next 07/11] mlxsw: spectrum: qdiscs: Unite all handle checks Jiri Pirko
2018-01-10 14:00 ` [patch net-next 08/11] mlxsw: spectrum: qdiscs: Add an ops struct Jiri Pirko
2018-01-10 14:00 ` [patch net-next 09/11] mlxsw: spectrum: qdiscs: Create a generic destroy function Jiri Pirko
2018-01-10 14:00 ` [patch net-next 10/11] mlxsw: spectrum: qdiscs: Create a generic replace function Jiri Pirko
2018-01-10 14:00 ` [patch net-next 11/11] mlxsw: spectrum: qdiscs: Remove qdisc before setting a new one Jiri Pirko
2018-01-10 21:08 ` [patch net-next 00/11] mlxsw qdisc refactoring David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.