> +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