From aaca9408078914380fbfd8aef3c38a34b515a654 Mon Sep 17 00:00:00 2001 From: Petr Machata Date: Thu, 5 Mar 2020 09:16:40 +0200 Subject: [PATCH 1/5] net: sched: Make FIFO Qdisc offloadable Invoke ndo_setup_tc() as appropriate to signal init / replacement, destroying and dumping of pFIFO / bFIFO Qdisc. A lot of the FIFO logic is used for pFIFO_head_drop as well, but that's a semantically very different Qdisc that isn't really in the same boat as pFIFO / bFIFO. Split some of the functions to keep the Qdisc intact. Signed-off-by: Petr Machata Signed-off-by: Ido Schimmel Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- include/linux/netdevice.h | 1 + include/net/pkt_cls.h | 15 ++++++ net/sched/sch_fifo.c | 97 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index b6fedd54cd8ea..654808bfad83d 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -853,6 +853,7 @@ enum tc_setup_type { TC_SETUP_FT, TC_SETUP_QDISC_ETS, TC_SETUP_QDISC_TBF, + TC_SETUP_QDISC_FIFO, }; /* These structures hold the attributes of bpf state that are being passed diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 53946b509b519..341a66af8d59f 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -881,4 +881,19 @@ struct tc_tbf_qopt_offload { }; }; +enum tc_fifo_command { + TC_FIFO_REPLACE, + TC_FIFO_DESTROY, + TC_FIFO_STATS, +}; + +struct tc_fifo_qopt_offload { + enum tc_fifo_command command; + u32 handle; + u32 parent; + union { + struct tc_qopt_offload_stats stats; + }; +}; + #endif diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c index 37c8aa75d70c5..a579a4131d22d 100644 --- a/net/sched/sch_fifo.c +++ b/net/sched/sch_fifo.c @@ -12,6 +12,7 @@ #include #include #include +#include /* 1 band FIFO pseudo-"scheduler" */ @@ -51,8 +52,49 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch, return NET_XMIT_CN; } -static int fifo_init(struct Qdisc *sch, struct nlattr *opt, - struct netlink_ext_ack *extack) +static void fifo_offload_init(struct Qdisc *sch) +{ + struct net_device *dev = qdisc_dev(sch); + struct tc_fifo_qopt_offload qopt; + + if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc) + return; + + qopt.command = TC_FIFO_REPLACE; + qopt.handle = sch->handle; + qopt.parent = sch->parent; + dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_FIFO, &qopt); +} + +static void fifo_offload_destroy(struct Qdisc *sch) +{ + struct net_device *dev = qdisc_dev(sch); + struct tc_fifo_qopt_offload qopt; + + if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc) + return; + + qopt.command = TC_FIFO_DESTROY; + qopt.handle = sch->handle; + qopt.parent = sch->parent; + dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_FIFO, &qopt); +} + +static int fifo_offload_dump(struct Qdisc *sch) +{ + struct tc_fifo_qopt_offload qopt; + + qopt.command = TC_FIFO_STATS; + qopt.handle = sch->handle; + qopt.parent = sch->parent; + qopt.stats.bstats = &sch->bstats; + qopt.stats.qstats = &sch->qstats; + + return qdisc_offload_dump_helper(sch, TC_SETUP_QDISC_FIFO, &qopt); +} + +static int __fifo_init(struct Qdisc *sch, struct nlattr *opt, + struct netlink_ext_ack *extack) { bool bypass; bool is_bfifo = sch->ops == &bfifo_qdisc_ops; @@ -82,10 +124,35 @@ static int fifo_init(struct Qdisc *sch, struct nlattr *opt, sch->flags |= TCQ_F_CAN_BYPASS; else sch->flags &= ~TCQ_F_CAN_BYPASS; + return 0; } -static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb) +static int fifo_init(struct Qdisc *sch, struct nlattr *opt, + struct netlink_ext_ack *extack) +{ + int err; + + err = __fifo_init(sch, opt, extack); + if (err) + return err; + + fifo_offload_init(sch); + return 0; +} + +static int fifo_hd_init(struct Qdisc *sch, struct nlattr *opt, + struct netlink_ext_ack *extack) +{ + return __fifo_init(sch, opt, extack); +} + +static void fifo_destroy(struct Qdisc *sch) +{ + fifo_offload_destroy(sch); +} + +static int __fifo_dump(struct Qdisc *sch, struct sk_buff *skb) { struct tc_fifo_qopt opt = { .limit = sch->limit }; @@ -97,6 +164,22 @@ static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb) return -1; } +static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb) +{ + int err; + + err = fifo_offload_dump(sch); + if (err) + return err; + + return __fifo_dump(sch, skb); +} + +static int fifo_hd_dump(struct Qdisc *sch, struct sk_buff *skb) +{ + return __fifo_dump(sch, skb); +} + struct Qdisc_ops pfifo_qdisc_ops __read_mostly = { .id = "pfifo", .priv_size = 0, @@ -104,6 +187,7 @@ struct Qdisc_ops pfifo_qdisc_ops __read_mostly = { .dequeue = qdisc_dequeue_head, .peek = qdisc_peek_head, .init = fifo_init, + .destroy = fifo_destroy, .reset = qdisc_reset_queue, .change = fifo_init, .dump = fifo_dump, @@ -118,6 +202,7 @@ struct Qdisc_ops bfifo_qdisc_ops __read_mostly = { .dequeue = qdisc_dequeue_head, .peek = qdisc_peek_head, .init = fifo_init, + .destroy = fifo_destroy, .reset = qdisc_reset_queue, .change = fifo_init, .dump = fifo_dump, @@ -131,10 +216,10 @@ struct Qdisc_ops pfifo_head_drop_qdisc_ops __read_mostly = { .enqueue = pfifo_tail_enqueue, .dequeue = qdisc_dequeue_head, .peek = qdisc_peek_head, - .init = fifo_init, + .init = fifo_hd_init, .reset = qdisc_reset_queue, - .change = fifo_init, - .dump = fifo_dump, + .change = fifo_hd_init, + .dump = fifo_hd_dump, .owner = THIS_MODULE, }; From ee88450d2509f11fc5cb3fc8b4430b4186981886 Mon Sep 17 00:00:00 2001 From: Petr Machata Date: Thu, 5 Mar 2020 09:16:41 +0200 Subject: [PATCH 2/5] mlxsw: spectrum_qdisc: Introduce struct mlxsw_sp_qdisc_state In order to have a tidy structure where to put information related to Qdisc offloads, introduce a new structure. Move there the two existing pieces of data: root_qdisc and tclass_qdiscs. Embed them directly, because there's no reason to go through pointer anymore. Convert users, update init/fini functions. Signed-off-by: Petr Machata Signed-off-by: Ido Schimmel Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- .../net/ethernet/mellanox/mlxsw/spectrum.h | 4 +- .../ethernet/mellanox/mlxsw/spectrum_qdisc.c | 85 ++++++++++--------- 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h index 9708156e78719..f952fbf96b410 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h @@ -139,6 +139,7 @@ struct mlxsw_sp_port_type_speed_ops; struct mlxsw_sp_ptp_state; struct mlxsw_sp_ptp_ops; struct mlxsw_sp_span_ops; +struct mlxsw_sp_qdisc_state; struct mlxsw_sp_port_mapping { u8 module; @@ -276,8 +277,7 @@ struct mlxsw_sp_port { struct mlxsw_sp_port_sample *sample; struct list_head vlans_list; struct mlxsw_sp_port_vlan *default_vlan; - struct mlxsw_sp_qdisc *root_qdisc; - struct mlxsw_sp_qdisc *tclass_qdiscs; + struct mlxsw_sp_qdisc_state *qdisc; unsigned acl_rule_count; struct mlxsw_sp_acl_block *ing_acl_block; struct mlxsw_sp_acl_block *eg_acl_block; diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c index 13a054c0ce0f7..250b0069d1c10 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c @@ -22,6 +22,8 @@ enum mlxsw_sp_qdisc_type { MLXSW_SP_QDISC_TBF, }; +struct mlxsw_sp_qdisc; + struct mlxsw_sp_qdisc_ops { enum mlxsw_sp_qdisc_type type; int (*check_params)(struct mlxsw_sp_port *mlxsw_sp_port, @@ -64,6 +66,11 @@ struct mlxsw_sp_qdisc { struct mlxsw_sp_qdisc_ops *ops; }; +struct mlxsw_sp_qdisc_state { + struct mlxsw_sp_qdisc root_qdisc; + struct mlxsw_sp_qdisc tclass_qdiscs[IEEE_8021QAZ_MAX_TCS]; +}; + static bool mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle, enum mlxsw_sp_qdisc_type type) @@ -77,36 +84,38 @@ static struct mlxsw_sp_qdisc * mlxsw_sp_qdisc_find(struct mlxsw_sp_port *mlxsw_sp_port, u32 parent, bool root_only) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; int tclass, child_index; if (parent == TC_H_ROOT) - return mlxsw_sp_port->root_qdisc; + return &qdisc_state->root_qdisc; - if (root_only || !mlxsw_sp_port->root_qdisc || - !mlxsw_sp_port->root_qdisc->ops || - TC_H_MAJ(parent) != mlxsw_sp_port->root_qdisc->handle || + if (root_only || !qdisc_state || + !qdisc_state->root_qdisc.ops || + TC_H_MAJ(parent) != qdisc_state->root_qdisc.handle || TC_H_MIN(parent) > IEEE_8021QAZ_MAX_TCS) return NULL; child_index = TC_H_MIN(parent); tclass = MLXSW_SP_PRIO_CHILD_TO_TCLASS(child_index); - return &mlxsw_sp_port->tclass_qdiscs[tclass]; + return &qdisc_state->tclass_qdiscs[tclass]; } static struct mlxsw_sp_qdisc * mlxsw_sp_qdisc_find_by_handle(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; int i; - if (mlxsw_sp_port->root_qdisc->handle == handle) - return mlxsw_sp_port->root_qdisc; + if (qdisc_state->root_qdisc.handle == handle) + return &qdisc_state->root_qdisc; - if (mlxsw_sp_port->root_qdisc->handle == TC_H_UNSPEC) + if (qdisc_state->root_qdisc.handle == TC_H_UNSPEC) return NULL; for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) - if (mlxsw_sp_port->tclass_qdiscs[i].handle == handle) - return &mlxsw_sp_port->tclass_qdiscs[i]; + if (qdisc_state->tclass_qdiscs[i].handle == handle) + return &qdisc_state->tclass_qdiscs[i]; return NULL; } @@ -360,7 +369,8 @@ static int mlxsw_sp_qdisc_red_destroy(struct mlxsw_sp_port *mlxsw_sp_port, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc) { - struct mlxsw_sp_qdisc *root_qdisc = mlxsw_sp_port->root_qdisc; + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; + struct mlxsw_sp_qdisc *root_qdisc = &qdisc_state->root_qdisc; if (root_qdisc != mlxsw_sp_qdisc) root_qdisc->stats_base.backlog -= @@ -559,7 +569,8 @@ static int mlxsw_sp_qdisc_tbf_destroy(struct mlxsw_sp_port *mlxsw_sp_port, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc) { - struct mlxsw_sp_qdisc *root_qdisc = mlxsw_sp_port->root_qdisc; + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; + struct mlxsw_sp_qdisc *root_qdisc = &qdisc_state->root_qdisc; if (root_qdisc != mlxsw_sp_qdisc) root_qdisc->stats_base.backlog -= @@ -737,6 +748,7 @@ int mlxsw_sp_setup_tc_tbf(struct mlxsw_sp_port *mlxsw_sp_port, static int __mlxsw_sp_qdisc_ets_destroy(struct mlxsw_sp_port *mlxsw_sp_port) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; int i; for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) { @@ -746,8 +758,8 @@ __mlxsw_sp_qdisc_ets_destroy(struct mlxsw_sp_port *mlxsw_sp_port) MLXSW_REG_QEEC_HR_SUBGROUP, i, 0, false, 0); mlxsw_sp_qdisc_destroy(mlxsw_sp_port, - &mlxsw_sp_port->tclass_qdiscs[i]); - mlxsw_sp_port->tclass_qdiscs[i].prio_bitmap = 0; + &qdisc_state->tclass_qdiscs[i]); + qdisc_state->tclass_qdiscs[i].prio_bitmap = 0; } return 0; @@ -786,6 +798,7 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, const unsigned int *weights, const u8 *priomap) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; struct mlxsw_sp_qdisc *child_qdisc; int tclass, i, band, backlog; u8 old_priomap; @@ -793,7 +806,7 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, for (band = 0; band < nbands; band++) { tclass = MLXSW_SP_PRIO_BAND_TO_TCLASS(band); - child_qdisc = &mlxsw_sp_port->tclass_qdiscs[tclass]; + child_qdisc = &qdisc_state->tclass_qdiscs[tclass]; old_priomap = child_qdisc->prio_bitmap; child_qdisc->prio_bitmap = 0; @@ -825,7 +838,7 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, } for (; band < IEEE_8021QAZ_MAX_TCS; band++) { tclass = MLXSW_SP_PRIO_BAND_TO_TCLASS(band); - child_qdisc = &mlxsw_sp_port->tclass_qdiscs[tclass]; + child_qdisc = &qdisc_state->tclass_qdiscs[tclass]; child_qdisc->prio_bitmap = 0; mlxsw_sp_qdisc_destroy(mlxsw_sp_port, child_qdisc); mlxsw_sp_port_ets_set(mlxsw_sp_port, @@ -875,6 +888,7 @@ mlxsw_sp_qdisc_get_prio_stats(struct mlxsw_sp_port *mlxsw_sp_port, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, struct tc_qopt_offload_stats *stats_ptr) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; struct mlxsw_sp_qdisc *tc_qdisc; u64 tx_packets = 0; u64 tx_bytes = 0; @@ -883,7 +897,7 @@ mlxsw_sp_qdisc_get_prio_stats(struct mlxsw_sp_port *mlxsw_sp_port, int i; for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) { - tc_qdisc = &mlxsw_sp_port->tclass_qdiscs[i]; + tc_qdisc = &qdisc_state->tclass_qdiscs[i]; mlxsw_sp_qdisc_collect_tc_stats(mlxsw_sp_port, tc_qdisc, &tx_bytes, &tx_packets, &drops, &backlog); @@ -1009,11 +1023,12 @@ __mlxsw_sp_qdisc_ets_graft(struct mlxsw_sp_port *mlxsw_sp_port, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u8 band, u32 child_handle) { + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; int tclass_num = MLXSW_SP_PRIO_BAND_TO_TCLASS(band); struct mlxsw_sp_qdisc *old_qdisc; if (band < IEEE_8021QAZ_MAX_TCS && - mlxsw_sp_port->tclass_qdiscs[tclass_num].handle == child_handle) + qdisc_state->tclass_qdiscs[tclass_num].handle == child_handle) return 0; if (!child_handle) { @@ -1032,7 +1047,7 @@ __mlxsw_sp_qdisc_ets_graft(struct mlxsw_sp_port *mlxsw_sp_port, mlxsw_sp_qdisc_destroy(mlxsw_sp_port, old_qdisc); mlxsw_sp_qdisc_destroy(mlxsw_sp_port, - &mlxsw_sp_port->tclass_qdiscs[tclass_num]); + &qdisc_state->tclass_qdiscs[tclass_num]); return -EOPNOTSUPP; } @@ -1114,37 +1129,23 @@ int mlxsw_sp_setup_tc_ets(struct mlxsw_sp_port *mlxsw_sp_port, int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port) { - struct mlxsw_sp_qdisc *mlxsw_sp_qdisc; + struct mlxsw_sp_qdisc_state *qdisc_state; int i; - mlxsw_sp_qdisc = kzalloc(sizeof(*mlxsw_sp_qdisc), GFP_KERNEL); - if (!mlxsw_sp_qdisc) - goto err_root_qdisc_init; + qdisc_state = kzalloc(sizeof(*qdisc_state), GFP_KERNEL); + if (!qdisc_state) + return -ENOMEM; - mlxsw_sp_port->root_qdisc = mlxsw_sp_qdisc; - mlxsw_sp_port->root_qdisc->prio_bitmap = 0xff; - mlxsw_sp_port->root_qdisc->tclass_num = MLXSW_SP_PORT_DEFAULT_TCLASS; - - mlxsw_sp_qdisc = kcalloc(IEEE_8021QAZ_MAX_TCS, - sizeof(*mlxsw_sp_qdisc), - GFP_KERNEL); - if (!mlxsw_sp_qdisc) - goto err_tclass_qdiscs_init; - - mlxsw_sp_port->tclass_qdiscs = mlxsw_sp_qdisc; + qdisc_state->root_qdisc.prio_bitmap = 0xff; + qdisc_state->root_qdisc.tclass_num = MLXSW_SP_PORT_DEFAULT_TCLASS; for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) - mlxsw_sp_port->tclass_qdiscs[i].tclass_num = i; + qdisc_state->tclass_qdiscs[i].tclass_num = i; + mlxsw_sp_port->qdisc = qdisc_state; return 0; - -err_tclass_qdiscs_init: - kfree(mlxsw_sp_port->root_qdisc); -err_root_qdisc_init: - return -ENOMEM; } void mlxsw_sp_tc_qdisc_fini(struct mlxsw_sp_port *mlxsw_sp_port) { - kfree(mlxsw_sp_port->tclass_qdiscs); - kfree(mlxsw_sp_port->root_qdisc); + kfree(mlxsw_sp_port->qdisc); } From c4e372e2ac7c9752ded23f24d718cad4a8234077 Mon Sep 17 00:00:00 2001 From: Petr Machata Date: Thu, 5 Mar 2020 09:16:42 +0200 Subject: [PATCH 3/5] mlxsw: spectrum_qdisc: Add handle parameter to ..._ops.replace PRIO and ETS will need to check the value of qdisc handle in their handlers. Add it to the callback and propagate through. Signed-off-by: Petr Machata Signed-off-by: Ido Schimmel Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- .../ethernet/mellanox/mlxsw/spectrum_qdisc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c index 250b0069d1c10..55751faa9fa4f 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c @@ -29,7 +29,7 @@ struct mlxsw_sp_qdisc_ops { 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, + int (*replace)(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, 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); @@ -156,7 +156,7 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, if (err) goto err_bad_param; - err = ops->replace(mlxsw_sp_port, mlxsw_sp_qdisc, params); + err = ops->replace(mlxsw_sp_port, handle, mlxsw_sp_qdisc, params); if (err) goto err_config; @@ -409,7 +409,7 @@ mlxsw_sp_qdisc_red_check_params(struct mlxsw_sp_port *mlxsw_sp_port, } static int -mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, +mlxsw_sp_qdisc_red_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void *params) { @@ -657,7 +657,7 @@ mlxsw_sp_qdisc_tbf_check_params(struct mlxsw_sp_port *mlxsw_sp_port, } static int -mlxsw_sp_qdisc_tbf_replace(struct mlxsw_sp_port *mlxsw_sp_port, +mlxsw_sp_qdisc_tbf_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void *params) { @@ -792,7 +792,7 @@ mlxsw_sp_qdisc_prio_check_params(struct mlxsw_sp_port *mlxsw_sp_port, } static int -__mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, +__mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, unsigned int nbands, const unsigned int *quanta, const unsigned int *weights, @@ -849,14 +849,14 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, } static int -mlxsw_sp_qdisc_prio_replace(struct mlxsw_sp_port *mlxsw_sp_port, +mlxsw_sp_qdisc_prio_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void *params) { struct tc_prio_qopt_offload_params *p = params; unsigned int zeroes[TCQ_ETS_MAX_BANDS] = {0}; - return __mlxsw_sp_qdisc_ets_replace(mlxsw_sp_port, p->bands, + return __mlxsw_sp_qdisc_ets_replace(mlxsw_sp_port, handle, p->bands, zeroes, zeroes, p->priomap); } @@ -955,13 +955,13 @@ mlxsw_sp_qdisc_ets_check_params(struct mlxsw_sp_port *mlxsw_sp_port, } static int -mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, +mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void *params) { struct tc_ets_qopt_offload_replace_params *p = params; - return __mlxsw_sp_qdisc_ets_replace(mlxsw_sp_port, p->bands, + return __mlxsw_sp_qdisc_ets_replace(mlxsw_sp_port, handle, p->bands, p->quanta, p->weights, p->priomap); } From 7bec1a45d5708a19154b94758802d7c599b99119 Mon Sep 17 00:00:00 2001 From: Petr Machata Date: Thu, 5 Mar 2020 09:16:43 +0200 Subject: [PATCH 4/5] mlxsw: spectrum_qdisc: Support offloading of FIFO Qdisc There are two peculiarities about offloading FIFO: - sometimes the qdisc has an unspecified handle (it is "invisible") - it may be created before the qdisc that it will be a child of These features make the offload a bit more tricky. The approach chosen in this patch is to make note of all the FIFOs that needed to be rejected because their parents were not known. Later when the parent is created, they are offloaded FIFO is only offloaded for its counters, queue length is ignored. Signed-off-by: Petr Machata Signed-off-by: Ido Schimmel Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- .../net/ethernet/mellanox/mlxsw/spectrum.c | 2 + .../net/ethernet/mellanox/mlxsw/spectrum.h | 2 + .../ethernet/mellanox/mlxsw/spectrum_qdisc.c | 146 +++++++++++++++++- 3 files changed, 149 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c index 673fa2fd995c8..51709012593e3 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c @@ -1783,6 +1783,8 @@ static int mlxsw_sp_setup_tc(struct net_device *dev, enum tc_setup_type type, return mlxsw_sp_setup_tc_ets(mlxsw_sp_port, type_data); case TC_SETUP_QDISC_TBF: return mlxsw_sp_setup_tc_tbf(mlxsw_sp_port, type_data); + case TC_SETUP_QDISC_FIFO: + return mlxsw_sp_setup_tc_fifo(mlxsw_sp_port, type_data); default: return -EOPNOTSUPP; } diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h index f952fbf96b410..ff61cad74bb0e 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h @@ -867,6 +867,8 @@ int mlxsw_sp_setup_tc_ets(struct mlxsw_sp_port *mlxsw_sp_port, struct tc_ets_qopt_offload *p); int mlxsw_sp_setup_tc_tbf(struct mlxsw_sp_port *mlxsw_sp_port, struct tc_tbf_qopt_offload *p); +int mlxsw_sp_setup_tc_fifo(struct mlxsw_sp_port *mlxsw_sp_port, + struct tc_fifo_qopt_offload *p); /* spectrum_fid.c */ bool mlxsw_sp_fid_is_dummy(struct mlxsw_sp *mlxsw_sp, u16 fid_index); diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c index 55751faa9fa4f..b9f429ec0db4d 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c @@ -20,6 +20,7 @@ enum mlxsw_sp_qdisc_type { MLXSW_SP_QDISC_PRIO, MLXSW_SP_QDISC_ETS, MLXSW_SP_QDISC_TBF, + MLXSW_SP_QDISC_FIFO, }; struct mlxsw_sp_qdisc; @@ -69,6 +70,20 @@ struct mlxsw_sp_qdisc { struct mlxsw_sp_qdisc_state { struct mlxsw_sp_qdisc root_qdisc; struct mlxsw_sp_qdisc tclass_qdiscs[IEEE_8021QAZ_MAX_TCS]; + + /* When a PRIO or ETS are added, the invisible FIFOs in their bands are + * created first. When notifications for these FIFOs arrive, it is not + * known what qdisc their parent handle refers to. It could be a + * newly-created PRIO that will replace the currently-offloaded one, or + * it could be e.g. a RED that will be attached below it. + * + * As the notifications start to arrive, use them to note what the + * future parent handle is, and keep track of which child FIFOs were + * seen. Then when the parent is known, retroactively offload those + * FIFOs. + */ + u32 future_handle; + bool future_fifos[IEEE_8021QAZ_MAX_TCS]; }; static bool @@ -160,7 +175,11 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, if (err) goto err_config; - if (mlxsw_sp_qdisc->handle != handle) { + /* Check if the Qdisc changed. That includes a situation where an + * invisible Qdisc replaces another one, or is being added for the + * first time. + */ + if (mlxsw_sp_qdisc->handle != handle || handle == TC_H_UNSPEC) { mlxsw_sp_qdisc->ops = ops; if (ops->clean_stats) ops->clean_stats(mlxsw_sp_port, mlxsw_sp_qdisc); @@ -745,6 +764,118 @@ int mlxsw_sp_setup_tc_tbf(struct mlxsw_sp_port *mlxsw_sp_port, } } +static int +mlxsw_sp_qdisc_fifo_destroy(struct mlxsw_sp_port *mlxsw_sp_port, + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc) +{ + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; + struct mlxsw_sp_qdisc *root_qdisc = &qdisc_state->root_qdisc; + + if (root_qdisc != mlxsw_sp_qdisc) + root_qdisc->stats_base.backlog -= + mlxsw_sp_qdisc->stats_base.backlog; + return 0; +} + +static int +mlxsw_sp_qdisc_fifo_check_params(struct mlxsw_sp_port *mlxsw_sp_port, + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, + void *params) +{ + return 0; +} + +static int +mlxsw_sp_qdisc_fifo_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, + void *params) +{ + return 0; +} + +static int +mlxsw_sp_qdisc_get_fifo_stats(struct mlxsw_sp_port *mlxsw_sp_port, + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, + struct tc_qopt_offload_stats *stats_ptr) +{ + mlxsw_sp_qdisc_get_tc_stats(mlxsw_sp_port, mlxsw_sp_qdisc, + stats_ptr); + return 0; +} + +static struct mlxsw_sp_qdisc_ops mlxsw_sp_qdisc_ops_fifo = { + .type = MLXSW_SP_QDISC_FIFO, + .check_params = mlxsw_sp_qdisc_fifo_check_params, + .replace = mlxsw_sp_qdisc_fifo_replace, + .destroy = mlxsw_sp_qdisc_fifo_destroy, + .get_stats = mlxsw_sp_qdisc_get_fifo_stats, + .clean_stats = mlxsw_sp_setup_tc_qdisc_leaf_clean_stats, +}; + +int mlxsw_sp_setup_tc_fifo(struct mlxsw_sp_port *mlxsw_sp_port, + struct tc_fifo_qopt_offload *p) +{ + struct mlxsw_sp_qdisc_state *qdisc_state = mlxsw_sp_port->qdisc; + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc; + int tclass, child_index; + u32 parent_handle; + + /* Invisible FIFOs are tracked in future_handle and future_fifos. Make + * sure that not more than one qdisc is created for a port at a time. + * RTNL is a simple proxy for that. + */ + ASSERT_RTNL(); + + mlxsw_sp_qdisc = mlxsw_sp_qdisc_find(mlxsw_sp_port, p->parent, false); + if (!mlxsw_sp_qdisc && p->handle == TC_H_UNSPEC) { + parent_handle = TC_H_MAJ(p->parent); + if (parent_handle != qdisc_state->future_handle) { + /* This notifications is for a different Qdisc than + * previously. Wipe the future cache. + */ + memset(qdisc_state->future_fifos, 0, + sizeof(qdisc_state->future_fifos)); + qdisc_state->future_handle = parent_handle; + } + + child_index = TC_H_MIN(p->parent); + tclass = MLXSW_SP_PRIO_CHILD_TO_TCLASS(child_index); + if (tclass < IEEE_8021QAZ_MAX_TCS) { + if (p->command == TC_FIFO_REPLACE) + qdisc_state->future_fifos[tclass] = true; + else if (p->command == TC_FIFO_DESTROY) + qdisc_state->future_fifos[tclass] = false; + } + } + if (!mlxsw_sp_qdisc) + return -EOPNOTSUPP; + + if (p->command == TC_FIFO_REPLACE) { + return mlxsw_sp_qdisc_replace(mlxsw_sp_port, p->handle, + mlxsw_sp_qdisc, + &mlxsw_sp_qdisc_ops_fifo, NULL); + } + + if (!mlxsw_sp_qdisc_compare(mlxsw_sp_qdisc, p->handle, + MLXSW_SP_QDISC_FIFO)) + return -EOPNOTSUPP; + + switch (p->command) { + case TC_FIFO_DESTROY: + if (p->handle == mlxsw_sp_qdisc->handle) + return mlxsw_sp_qdisc_destroy(mlxsw_sp_port, + mlxsw_sp_qdisc); + return 0; + case TC_FIFO_STATS: + return mlxsw_sp_qdisc_get_stats(mlxsw_sp_port, mlxsw_sp_qdisc, + &p->stats); + case TC_FIFO_REPLACE: /* Handled above. */ + break; + } + + return -EOPNOTSUPP; +} + static int __mlxsw_sp_qdisc_ets_destroy(struct mlxsw_sp_port *mlxsw_sp_port) { @@ -835,6 +966,16 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, child_qdisc); child_qdisc->stats_base.backlog = backlog; } + + if (handle == qdisc_state->future_handle && + qdisc_state->future_fifos[tclass]) { + err = mlxsw_sp_qdisc_replace(mlxsw_sp_port, TC_H_UNSPEC, + child_qdisc, + &mlxsw_sp_qdisc_ops_fifo, + NULL); + if (err) + return err; + } } for (; band < IEEE_8021QAZ_MAX_TCS; band++) { tclass = MLXSW_SP_PRIO_BAND_TO_TCLASS(band); @@ -845,6 +986,9 @@ __mlxsw_sp_qdisc_ets_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle, MLXSW_REG_QEEC_HR_SUBGROUP, tclass, 0, false, 0); } + + qdisc_state->future_handle = TC_H_UNSPEC; + memset(qdisc_state->future_fifos, 0, sizeof(qdisc_state->future_fifos)); return 0; } From b9b72999eb86e650489a4321c8e95a4e2e3df20c Mon Sep 17 00:00:00 2001 From: Petr Machata Date: Thu, 5 Mar 2020 09:16:44 +0200 Subject: [PATCH 5/5] selftests: forwarding: ETS: Use Qdisc counters Currently the SW-datapath ETS selftests use "ip link" stats to obtain the number of packets that went through a given band. mlxsw then uses ethtool per-priority counters. Instead, change both to use qdiscs. In SW datapath this is the obvious choice, and now that mlxsw offloads FIFO, this should work on the offloaded datapath as well. This has the effect of verifying that the FIFO offload works. Signed-off-by: Petr Machata Signed-off-by: Ido Schimmel Signed-off-by: David S. Miller --- .../testing/selftests/drivers/net/mlxsw/sch_ets.sh | 14 +++++++++++--- tools/testing/selftests/net/forwarding/lib.sh | 10 ++++++++++ tools/testing/selftests/net/forwarding/sch_ets.sh | 9 ++++++--- .../selftests/net/forwarding/sch_ets_tests.sh | 10 +++------- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh index c9fc4d4885c17..94c37124a840f 100755 --- a/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh +++ b/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh @@ -56,11 +56,19 @@ switch_destroy() } # Callback from sch_ets_tests.sh -get_stats() +collect_stats() { - local band=$1; shift + local -a streams=("$@") + local stream - ethtool_stats_get "$h2" rx_octets_prio_$band + # Wait for qdisc counter update so that we don't get it mid-way through. + busywait_for_counter 1000 +1 \ + qdisc_parent_stats_get $swp2 10:$((${streams[0]} + 1)) .bytes \ + > /dev/null + + for stream in ${streams[@]}; do + qdisc_parent_stats_get $swp2 10:$((stream + 1)) .bytes + done } bail_on_lldpad diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 7ecce65d08f96..a4a7879b3bb95 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -655,6 +655,16 @@ qdisc_stats_get() | jq '.[] | select(.handle == "'"$handle"'") | '"$selector" } +qdisc_parent_stats_get() +{ + local dev=$1; shift + local parent=$1; shift + local selector=$1; shift + + tc -j -s qdisc show dev "$dev" invisible \ + | jq '.[] | select(.parent == "'"$parent"'") | '"$selector" +} + humanize() { local speed=$1; shift diff --git a/tools/testing/selftests/net/forwarding/sch_ets.sh b/tools/testing/selftests/net/forwarding/sch_ets.sh index 40e0ad1bc4f25..e60c8b4818cca 100755 --- a/tools/testing/selftests/net/forwarding/sch_ets.sh +++ b/tools/testing/selftests/net/forwarding/sch_ets.sh @@ -34,11 +34,14 @@ switch_destroy() } # Callback from sch_ets_tests.sh -get_stats() +collect_stats() { - local stream=$1; shift + local -a streams=("$@") + local stream - link_stats_get $h2.1$stream rx bytes + for stream in ${streams[@]}; do + qdisc_parent_stats_get $swp2 10:$((stream + 1)) .bytes + done } ets_run diff --git a/tools/testing/selftests/net/forwarding/sch_ets_tests.sh b/tools/testing/selftests/net/forwarding/sch_ets_tests.sh index 3c3b204d47e8e..cdf689e99458e 100644 --- a/tools/testing/selftests/net/forwarding/sch_ets_tests.sh +++ b/tools/testing/selftests/net/forwarding/sch_ets_tests.sh @@ -2,7 +2,7 @@ # Global interface: # $put -- port under test (e.g. $swp2) -# get_stats($band) -- A function to collect stats for band +# collect_stats($streams...) -- A function to get stats for individual streams # ets_start_traffic($band) -- Start traffic for this band # ets_change_qdisc($op, $dev, $nstrict, $quanta...) -- Add or change qdisc @@ -94,15 +94,11 @@ __ets_dwrr_test() sleep 10 - t0=($(for stream in ${streams[@]}; do - get_stats $stream - done)) + t0=($(collect_stats "${streams[@]}")) sleep 10 - t1=($(for stream in ${streams[@]}; do - get_stats $stream - done)) + t1=($(collect_stats "${streams[@]}")) d=($(for ((i = 0; i < ${#streams[@]}; i++)); do echo $((${t1[$i]} - ${t0[$i]})) done))