On Wed, 9 Oct 2019 13:04:44 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <j...@mellanox.com>
>
> Implement "empty" and "dummy" reporters. The first one is really simple
> and does nothing. The other one has debugfs files to trigger breakage
> and it is able to do recovery. The ops also implement dummy fmsg
> content.
>
> Signed-off-by: Jiri Pirko <j...@mellanox.com>
> diff --git a/drivers/net/netdevsim/health.c b/drivers/net/netdevsim/health.c
> new file mode 100644
> index 000000000000..088ae8fd89fc
> --- /dev/null
> +++ b/drivers/net/netdevsim/health.c
> +static int
> +nsim_dev_dummy_reporter_recover(struct devlink_health_reporter *reporter,
> + void *priv_ctx,
> + struct netlink_ext_ack *extack)
> +{
> + struct nsim_dev_health *health = devlink_health_reporter_priv(reporter);
> + struct nsim_dev_dummy_reporter_ctx *ctx = priv_ctx;
> +
> + if (health->fail_recover) {
> + /* For testing purposes, user set debugfs fail_recover
> + * value to true. Fail right away.
> + */
> + NL_SET_ERR_MSG_MOD(extack, "User setup the recover to fail for
> testing purposes");
> + return -EINVAL;
> + }
> + if (ctx) {
> + health->recovered_break_msg = kstrdup(ctx->break_msg,
Can't there already be a message there? wouldn't this leak it?
> + GFP_KERNEL);
> + if (!health->recovered_break_msg)
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +static ssize_t nsim_dev_health_break_write(struct file *file,
> + const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + struct nsim_dev_health *health = file->private_data;
> + struct nsim_dev_dummy_reporter_ctx ctx;
> + char *break_msg;
> + int err;
> +
> + break_msg = kmalloc(count + 1, GFP_KERNEL);
> + if (!break_msg)
> + return -ENOMEM;
> +
> + if (copy_from_user(break_msg, data, count)) {
> + err = -EFAULT;
> + goto out;
> + }
> + break_msg[count] = '\0';
> + if (break_msg[count - 1] == '\n')
> + break_msg[count - 1] = '\0';
> +
> + ctx.break_msg = break_msg;
> + err = devlink_health_report(health->dummy_reporter, break_msg, &ctx);
> + if (err)
> + goto out;
> +
> +out:
> + kfree(break_msg);
> + return err ? err : count;
nit: ?: works here, like you used below
> +}
> +
> +static const struct file_operations nsim_dev_health_break_fops = {
> + .open = simple_open,
> + .write = nsim_dev_health_break_write,
> + .llseek = generic_file_llseek,
> +};
> +
> +int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink)
> +{
> + struct nsim_dev_health *health = &nsim_dev->health;
> + int err;
> +
> + health->empty_reporter =
> + devlink_health_reporter_create(devlink,
> + &nsim_dev_empty_reporter_ops,
> + 0, false, health);
> + if (IS_ERR(health->empty_reporter))
> + return PTR_ERR(health->empty_reporter);
> +
> + health->dummy_reporter =
> + devlink_health_reporter_create(devlink,
> + &nsim_dev_dummy_reporter_ops,
> + 0, false, health);
> + if (IS_ERR(health->dummy_reporter)) {
> + err = PTR_ERR(health->dummy_reporter);
> + goto err_empty_reporter_destroy;
> + }
> +
> + health->ddir = debugfs_create_dir("health", nsim_dev->ddir);
> + if (IS_ERR_OR_NULL(health->ddir))
> + return PTR_ERR_OR_ZERO(health->ddir) ?: -EINVAL;
goto err_dummy_reporter_destroy?
> + health->recovered_break_msg = NULL;
> + debugfs_create_file("break_health", 0200, health->ddir, health,
> + &nsim_dev_health_break_fops);
> + health->binary_len = 16;
> + debugfs_create_u32("binary_len", 0600, health->ddir,
> + &health->binary_len);
> + health->fail_recover = false;
> + debugfs_create_bool("fail_recover", 0600, health->ddir,
> + &health->fail_recover);
> + return 0;
> +
> +err_empty_reporter_destroy:
> + devlink_health_reporter_destroy(health->empty_reporter);
> + return err;
> +}
> +
> +void nsim_dev_health_exit(struct nsim_dev *nsim_dev)
> +{
> + struct nsim_dev_health *health = &nsim_dev->health;
> +
> + debugfs_remove_recursive(health->ddir);
> + kfree(health->recovered_break_msg);
> + devlink_health_reporter_destroy(health->dummy_reporter);
> + devlink_health_reporter_destroy(health->empty_reporter);
> +}
> diff --git a/drivers/net/netdevsim/netdevsim.h
> b/drivers/net/netdevsim/netdevsim.h
> index 24358385d869..657cbae50293 100644
> --- a/drivers/net/netdevsim/netdevsim.h
> +++ b/drivers/net/netdevsim/netdevsim.h
> @@ -18,6 +18,7 @@
> #include <linux/list.h>
> #include <linux/netdevice.h>
> #include <linux/u64_stats_sync.h>
> +#include <linux/debugfs.h>
I don't think this include is needed? Forward declaration of
struct dentry, should be sufficient, if needed, no?
> #include <net/devlink.h>
> #include <net/xdp.h>
>
> @@ -134,6 +135,18 @@ enum nsim_resource_id {
> NSIM_RESOURCE_IPV6_FIB_RULES,
> };
>
> +struct nsim_dev_health {
> + struct devlink_health_reporter *empty_reporter;
> + struct devlink_health_reporter *dummy_reporter;
> + struct dentry *ddir;
> + char *recovered_break_msg;
> + u32 binary_len;
> + bool fail_recover;
> +};
> +
> +int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink);
> +void nsim_dev_health_exit(struct nsim_dev *nsim_dev);
> +
> struct nsim_dev_port {
> struct list_head list;
> struct devlink_port devlink_port;
> @@ -164,6 +177,7 @@ struct nsim_dev {
> bool dont_allow_reload;
> bool fail_reload;
> struct devlink_region *dummy_region;
> + struct nsim_dev_health health;
> };
>
> static inline struct net *nsim_dev_net(struct nsim_dev *nsim_dev)
