> +static int mlxsw_sp1_nve_vxlan_metrics_init(struct mlxsw_sp *mlxsw_sp)
> +{
> +     struct mlxsw_sp_nve_metrics *metrics = &mlxsw_sp->nve->metrics;
> +     struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
> +     int err;
> +
> +     err = mlxsw_sp1_nve_vxlan_counters_clear(mlxsw_sp);
> +     if (err)
> +             return err;
> +
> +     metrics->counter_encap =
> +             devlink_metric_counter_create(devlink, "nve_vxlan_encap",
> +                                           &mlxsw_sp1_nve_vxlan_encap_ops,
> +                                           mlxsw_sp);
> +     if (IS_ERR(metrics->counter_encap))
> +             return PTR_ERR(metrics->counter_encap);
> +
> +     metrics->counter_decap =
> +             devlink_metric_counter_create(devlink, "nve_vxlan_decap",
> +                                           &mlxsw_sp1_nve_vxlan_decap_ops,
> +                                           mlxsw_sp);
> +     if (IS_ERR(metrics->counter_decap)) {
> +             err = PTR_ERR(metrics->counter_decap);
> +             goto err_counter_decap;
> +     }
> +
> +     metrics->counter_decap_errors =
> +             devlink_metric_counter_create(devlink, "nve_vxlan_decap_errors",
> +                                           
> &mlxsw_sp1_nve_vxlan_decap_errors_ops,
> +                                           mlxsw_sp);
> +     if (IS_ERR(metrics->counter_decap_errors)) {
> +             err = PTR_ERR(metrics->counter_decap_errors);
> +             goto err_counter_decap_errors;
> +     }
> +
> +     metrics->counter_decap_discards =
> +             devlink_metric_counter_create(devlink, 
> "nve_vxlan_decap_discards",
> +                                           
> &mlxsw_sp1_nve_vxlan_decap_discards_ops,
> +                                           mlxsw_sp);
> +     if (IS_ERR(metrics->counter_decap_discards)) {
> +             err = PTR_ERR(metrics->counter_decap_discards);
> +             goto err_counter_decap_discards;
> +     }
> +
> +     return 0;

Looking at this, i wonder about the scalability of this API. With just
4 counters it looks pretty ugly. What about 50 counters?

Maybe move the name into the ops structure. Then add a call
devlink_metric_counters_create() where you can pass an array and array
size of op structures? There are plenty of other examples in the
kernel, e.g. sysfs groups, hwmon, etc. where you register a large
bunch of things with the core with a single call.

> +static void mlxsw_sp1_nve_vxlan_metrics_fini(struct mlxsw_sp *mlxsw_sp)
> +{
> +     struct mlxsw_sp_nve_metrics *metrics = &mlxsw_sp->nve->metrics;
> +     struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
> +
> +     devlink_metric_destroy(devlink, metrics->counter_decap_discards);
> +     devlink_metric_destroy(devlink, metrics->counter_decap_errors);
> +     devlink_metric_destroy(devlink, metrics->counter_decap);
> +     devlink_metric_destroy(devlink, metrics->counter_encap);
> +}

I guess the most frequent use case is to remove all counters,
e.g. driver unload, or when probe fails. So maybe provide a
devlink_metric_destroy_all(devlink) ?

    Andrew

Reply via email to