> > Support basic stats for PRIO qdisc, which includes tx packets and bytes
> > count, drops count and backlog size. The rest of the stats are irrelevant
> > for this qdisc offload.
> > Since backlog is not only incremental but reflecting momentary value, in
> > case of a qdisc that stops being offloaded but is not destroyed, backlog
> > value needs to be updated about the un-offloading.
> > For that reason an unoffload function is being added to the ops struct.
> >
> > Signed-off-by: Nogah Frankel <nog...@mellanox.com>
> > Reviewed-by: Yuval Mintz <yuv...@mellanox.com>
> > Signed-off-by: Jiri Pirko <j...@mellanox.com>
> > ---
> > .../net/ethernet/mellanox/mlxsw/spectrum_qdisc.c | 92
> ++++++++++++++++++++++
> > 1 file changed, 92 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > index 9e83edde7b35..272c04951e5d 100644
> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
> > @@ -66,6 +66,11 @@ struct mlxsw_sp_qdisc_ops {
> > void *xstats_ptr);
> > void (*clean_stats)(struct mlxsw_sp_port *mlxsw_sp_port,
> > struct mlxsw_sp_qdisc *mlxsw_sp_qdisc);
> > + /* unoffload - to be used for a qdisc that stops being offloaded
> without
> > + * being destroyed.
> > + */
> > + void (*unoffload)(struct mlxsw_sp_port *mlxsw_sp_port,
> > + struct mlxsw_sp_qdisc *mlxsw_sp_qdisc, void
> *params);
>
> Hm. You you need this just because you didn't add the backlog pointer
> to destroy? AFAIK on destroy we are free to reset stats as well, thus
> simplifying your driver... Let me know if I misunderstand.
This is meant exactly for the scenario where qdisc didn't get destroyed
yet is no longer offloaded; E.g., if number of bands increased beyond
What we can offload. So we can't reset the statistics in this case.
[Although I might be the one to misunderstand you,
as the 'not destroyed' was explicitly mentioned twice above]
>
> > };
> >
> > struct mlxsw_sp_qdisc {
> > @@ -73,6 +78,9 @@ struct mlxsw_sp_qdisc {
> > u8 tclass_num;
> > union {
> > struct red_stats red;
> > + struct mlxsw_sp_qdisc_prio_stats {
> > + u64 backlog;
>
> This is not a prio stat, it's a standard qstat. I've added it to
> struct mlxsw_sp_qdisc_stats. The reason you need to treat it
> separately is that RED has non-standard backlog handling which I'm
> trying to fix...
>
> > + } prio;
> > } xstats_base;
> > struct mlxsw_sp_qdisc_stats {
> > u64 tx_bytes;
> > @@ -144,6 +152,9 @@ mlxsw_sp_qdisc_replace(struct mlxsw_sp_port
> *mlxsw_sp_port, u32 handle,
> >
> > err_bad_param:
> > err_config:
> > + if (mlxsw_sp_qdisc->handle == handle && ops->unoffload)
> > + ops->unoffload(mlxsw_sp_port, mlxsw_sp_qdisc, params);
> > +
> > mlxsw_sp_qdisc_destroy(mlxsw_sp_port, mlxsw_sp_qdisc);
> > return err;
> > }
>
> > @@ -479,6 +567,10 @@ int mlxsw_sp_setup_tc_prio(struct mlxsw_sp_port
> *mlxsw_sp_port,
> > switch (p->command) {
> > case TC_PRIO_DESTROY:
> > return mlxsw_sp_qdisc_destroy(mlxsw_sp_port,
> mlxsw_sp_qdisc);
> > + case TC_PRIO_STATS:
> > + return mlxsw_sp_qdisc_get_stats(mlxsw_sp_port,
> mlxsw_sp_qdisc,
> > + &p->stats);
> > +
>
> nit: extra new line intentional? :)
>
> > default:
> > return -EOPNOTSUPP;
> > }
