| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20180111053849.14250-3-jakub.kicinski@netronome.com>
Date: Wed, 10 Jan 2018 21:38:49 -0800
From: Jakub Kicinski <jakub.kicinski@...ronome.com>
To: davem@...emloft.net, jiri@...nulli.us,
Nogah Frankel <nogahf@...lanox.com>
Cc: netdev@...r.kernel.org, oss-drivers@...ronome.com,
john.fastabend@...il.com, xiyou.wangcong@...il.com,
edumazet@...gle.com, Yuval Mintz <yuvalm@...lanox.com>,
Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: [PATCH net-next 2/2] net: sched: red: don't reset the backlog on every stat dump
Commit 0dfb33a0d7e2 ("sch_red: report backlog information") copied
child's backlog into RED's backlog. Back then RED did not maintain
its own backlog counts. This has changed after commit 2ccccf5fb43f
("net_sched: update hierarchical backlog too") and commit d7f4f332f082
("sch_red: update backlog as well"). Copying is no longer necessary.
As we add support for offload of other qdiscs this dependency on a
second copy of backlog length for resetting to pre-offload values
will become cumbersome.
Tested:
$ tc -s qdisc show dev veth0
qdisc red 1: root refcnt 2 limit 400000b min 30000b max 30000b ecn
Sent 20942 bytes 221 pkt (dropped 0, overlimits 0 requeues 0)
backlog 1260b 14p requeues 14
marked 0 early 0 pdrop 0 other 0
qdisc tbf 2: parent 1: rate 1Kbit burst 15000b lat 3585.0s
Sent 20942 bytes 221 pkt (dropped 0, overlimits 138 requeues 0)
backlog 1260b 14p requeues 14
Recently RED offload was added. We need to make sure drivers don't
depend on resetting the stats. This means backlog should be treated
like any other statistic:
total_stat = new_hw_stat - prev_hw_stat;
Unlike other stats drivers also need to remove their adjustment
on destroy (or replace with unsupported parameters). Note that
new_hw_stat < prev_hw_stat can be true for momentary stats.
Adjust mlxsw.
Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 35 ++++++++++++++--------
include/net/pkt_cls.h | 5 +++-
net/sched/sch_red.c | 3 +-
3 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index 273300b75a68..50031ab484af 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -54,7 +54,8 @@ struct mlxsw_sp_qdisc_ops {
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);
+ struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+ struct gnet_stats_queue *qstats);
int (*get_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
struct tc_qopt_offload_stats *stats_ptr);
@@ -76,6 +77,7 @@ struct mlxsw_sp_qdisc {
u64 tx_packets;
u64 drops;
u64 overlimits;
+ u64 backlog;
} stats_base;
struct mlxsw_sp_qdisc_ops *ops;
@@ -92,7 +94,8 @@ mlxsw_sp_qdisc_compare(struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, u32 handle,
static int
mlxsw_sp_qdisc_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
- struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
+ struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+ struct gnet_stats_queue *qstats)
{
int err = 0;
@@ -101,7 +104,8 @@ mlxsw_sp_qdisc_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
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,
+ qstats);
mlxsw_sp_qdisc->handle = TC_H_UNSPEC;
mlxsw_sp_qdisc->ops = NULL;
@@ -111,7 +115,8 @@ mlxsw_sp_qdisc_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
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)
+ struct mlxsw_sp_qdisc_ops *ops, void *params,
+ struct gnet_stats_queue *qstats)
{
int err;
@@ -121,7 +126,7 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
* Otherwise, we need to remove the old qdisc before setting the
* new one.
*/
- mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
+ mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc, qstats);
err = ops->check_params(mlxsw_sp_port, mlxsw_sp_qdisc, params);
if (err)
goto err_bad_param;
@@ -141,7 +146,7 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port *mlxsw_sp_port, u32 handle,
err_bad_param:
err_config:
- mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
+ mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc, qstats);
return err;
}
@@ -238,8 +243,10 @@ 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,
- struct mlxsw_sp_qdisc *mlxsw_sp_qdisc)
+ struct mlxsw_sp_qdisc *mlxsw_sp_qdisc,
+ struct gnet_stats_queue *qstats)
{
+ qstats->backlog -= mlxsw_sp_qdisc->stats_base.backlog;
return mlxsw_sp_tclass_congestion_disable(mlxsw_sp_port,
mlxsw_sp_qdisc->tclass_num);
}
@@ -330,6 +337,7 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp_qdisc_stats *stats_base;
struct mlxsw_sp_port_xstats *xstats;
struct rtnl_link_stats64 *stats;
+ s64 backlog;
xstats = &mlxsw_sp_port->periodic_hw_stats.xstats;
stats = &mlxsw_sp_port->periodic_hw_stats.stats;
@@ -341,18 +349,20 @@ mlxsw_sp_qdisc_get_red_stats(struct mlxsw_sp_port *mlxsw_sp_port,
stats_base->overlimits;
drops = xstats->wred_drop[tclass_num] + xstats->tail_drop[tclass_num] -
stats_base->drops;
+ backlog = mlxsw_sp_cells_bytes(mlxsw_sp_port->mlxsw_sp,
+ xstats->backlog[tclass_num]) -
+ stats_base->backlog;
_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_ptr->qstats->backlog += backlog;
stats_base->drops += drops;
stats_base->overlimits += overlimits;
stats_base->tx_bytes += tx_bytes;
stats_base->tx_packets += tx_packets;
+ stats_base->backlog += backlog;
return 0;
}
@@ -382,7 +392,7 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
return mlxsw_sp_qdisc_replace(mlxsw_sp_port, p->handle,
mlxsw_sp_qdisc,
&mlxsw_sp_qdisc_ops_red,
- &p->set);
+ &p->set, p->qstats);
if (!mlxsw_sp_qdisc_compare(mlxsw_sp_qdisc, p->handle,
MLXSW_SP_QDISC_RED))
@@ -390,7 +400,8 @@ int mlxsw_sp_setup_tc_red(struct mlxsw_sp_port *mlxsw_sp_port,
switch (p->command) {
case TC_RED_DESTROY:
- return mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
+ return mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc,
+ p->qstats);
case TC_RED_XSTATS:
return mlxsw_sp_qdisc_get_xstats(mlxsw_sp_port, mlxsw_sp_qdisc,
p->xstats);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 0d1343cba84c..d64db7777d69 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -755,7 +755,10 @@ struct tc_red_qopt_offload {
u32 handle;
u32 parent;
union {
- struct tc_red_qopt_offload_params set;
+ struct {
+ struct tc_red_qopt_offload_params set;
+ struct gnet_stats_queue *qstats;
+ };
struct tc_qopt_offload_stats stats;
struct red_stats *xstats;
};
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 0af1c1254e0b..8845654ef84a 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -167,8 +167,10 @@ static int red_offload(struct Qdisc *sch, bool enable)
opt.set.max = q->parms.qth_max >> q->parms.Wlog;
opt.set.probability = q->parms.max_P;
opt.set.is_ecn = red_use_ecn(q);
+ opt.qstats = &sch->qstats;
} else {
opt.command = TC_RED_DESTROY;
+ opt.qstats = &sch->qstats;
}
return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_RED, &opt);
@@ -322,7 +324,6 @@ static int red_dump(struct Qdisc *sch, struct sk_buff *skb)
};
int err;
- sch->qstats.backlog = q->qdisc->qstats.backlog;
err = red_dump_offload_stats(sch, &opt);
if (err)
goto nla_put_failure;
--
2.15.1
Powered by blists - more mailing lists