[patch net-next] netdevsim: implement support for devlink region and snapshots

[patch net-next] netdevsim: implement support for devlink region and snapshots

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Implement dummy region of size 32K and allow user to create snapshots
or random data using debugfs file trigger.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/netdevsim/dev.c       | 66 ++++++++++++++++++++++++++++++-
 drivers/net/netdevsim/netdevsim.h |  1 +
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 08ca59fc189b..e76ea6a3cb60 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -27,6 +27,41 @@
 
 static struct dentry *nsim_dev_ddir;
 
+#define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
+
+static ssize_t nsim_dev_take_snapshot_write(struct file *file,
+					    const char __user *data,
+					    size_t count, loff_t *ppos)
+{
+	struct nsim_dev *nsim_dev = file->private_data;
+	void *dummy_data;
+	u32 id;
+	int err;
+
+	dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL);
+	if (!dummy_data) {
+		pr_err("Failed to allocate memory for region snapshot\n");
+		goto out;
+	}
+
+	get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);
+
+	id = devlink_region_shapshot_id_get(priv_to_devlink(nsim_dev));
+	err = devlink_region_snapshot_create(nsim_dev->dummy_region,
+					     dummy_data, id, kfree);
+	if (err)
+		pr_err("Failed to create region snapshot\n");
+
+out:
+	return count;
+}
+
+static const struct file_operations nsim_dev_take_snapshot_fops = {
+	.open = simple_open,
+	.write = nsim_dev_take_snapshot_write,
+	.llseek = generic_file_llseek,
+};
+
 static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 {
 	char dev_ddir_name[16];
@@ -44,6 +79,8 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 			   &nsim_dev->max_macs);
 	debugfs_create_bool("test1", 0600, nsim_dev->ddir,
 			    &nsim_dev->test1);
+	debugfs_create_file("take_snapshot", 0200, nsim_dev->ddir, nsim_dev,
+			    &nsim_dev_take_snapshot_fops);
 	return 0;
 }
 
@@ -248,6 +285,26 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink)
 		nsim_dev->test1 = saved_value.vbool;
 }
 
+#define NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX 16
+
+static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev,
+				      struct devlink *devlink)
+{
+	nsim_dev->dummy_region =
+		devlink_region_create(devlink, "dummy",
+				      NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX,
+				      NSIM_DEV_DUMMY_REGION_SIZE);
+	if (IS_ERR(nsim_dev->dummy_region))
+		return PTR_ERR(nsim_dev->dummy_region);
+
+	return 0;
+}
+
+static void nsim_dev_dummy_region_exit(struct nsim_dev *nsim_dev)
+{
+	devlink_region_destroy(nsim_dev->dummy_region);
+}
+
 static int nsim_dev_reload(struct devlink *devlink,
 			   struct netlink_ext_ack *extack)
 {
@@ -363,10 +420,14 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
 		goto err_dl_unregister;
 	nsim_devlink_set_params_init_values(nsim_dev, devlink);
 
-	err = nsim_dev_debugfs_init(nsim_dev);
+	err = nsim_dev_dummy_region_init(nsim_dev, devlink);
 	if (err)
 		goto err_params_unregister;
 
+	err = nsim_dev_debugfs_init(nsim_dev);
+	if (err)
+		goto err_dummy_region_exit;
+
 	err = nsim_bpf_dev_init(nsim_dev);
 	if (err)
 		goto err_debugfs_exit;
@@ -376,6 +437,8 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
 
 err_debugfs_exit:
 	nsim_dev_debugfs_exit(nsim_dev);
+err_dummy_region_exit:
+	nsim_dev_dummy_region_exit(nsim_dev);
 err_params_unregister:
 	devlink_params_unregister(devlink, nsim_devlink_params,
 				  ARRAY_SIZE(nsim_devlink_params));
@@ -396,6 +459,7 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
 
 	nsim_bpf_dev_exit(nsim_dev);
 	nsim_dev_debugfs_exit(nsim_dev);
+	nsim_dev_dummy_region_exit(nsim_dev);
 	devlink_params_unregister(devlink, nsim_devlink_params,
 				  ARRAY_SIZE(nsim_devlink_params));
 	devlink_unregister(devlink);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 95751a817508..4c758c6919f5 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -160,6 +160,7 @@ struct nsim_dev {
 	bool fw_update_status;
 	u32 max_macs;
 	bool test1;
+	struct devlink_region *dummy_region;
 };
 
 int nsim_dev_init(void);
-- 
2.21.0

Re: [patch net-next] netdevsim: implement support for devlink region and snapshots

[Jakub Kicinski]

On Mon, 12 Aug 2019 12:16:20 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Implement dummy region of size 32K and allow user to create snapshots
> or random data using debugfs file trigger.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

I'm nacking all the netdevsim patches unless the selftest 
is posted at the same time :/

You're leaking those features one by one what if you get distracted 
and the tests never materialize :/

This is all dead code.

> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index 08ca59fc189b..e76ea6a3cb60 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -27,6 +27,41 @@
>  
>  static struct dentry *nsim_dev_ddir;
>  
> +#define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
> +
> +static ssize_t nsim_dev_take_snapshot_write(struct file *file,
> +					    const char __user *data,
> +					    size_t count, loff_t *ppos)
> +{
> +	struct nsim_dev *nsim_dev = file->private_data;
> +	void *dummy_data;
> +	u32 id;
> +	int err;
> +
> +	dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL);
> +	if (!dummy_data) {
> +		pr_err("Failed to allocate memory for region snapshot\n");
> +		goto out;
> +	}
> +
> +	get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);
> +
> +	id = devlink_region_shapshot_id_get(priv_to_devlink(nsim_dev));
> +	err = devlink_region_snapshot_create(nsim_dev->dummy_region,
> +					     dummy_data, id, kfree);
> +	if (err)
> +		pr_err("Failed to create region snapshot\n");
> +
> +out:
> +	return count;

why not return an error?

> +}
> +
> +static const struct file_operations nsim_dev_take_snapshot_fops = {
> +	.open = simple_open,
> +	.write = nsim_dev_take_snapshot_write,
> +	.llseek = generic_file_llseek,
> +};
> +
>  static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>  {
>  	char dev_ddir_name[16];
> @@ -44,6 +79,8 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>  			   &nsim_dev->max_macs);
>  	debugfs_create_bool("test1", 0600, nsim_dev->ddir,
>  			    &nsim_dev->test1);
> +	debugfs_create_file("take_snapshot", 0200, nsim_dev->ddir, nsim_dev,
> +			    &nsim_dev_take_snapshot_fops);
>  	return 0;
>  }
>  
> @@ -248,6 +285,26 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink)
>  		nsim_dev->test1 = saved_value.vbool;
>  }
>  
> +#define NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX 16
> +
> +static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev,
> +				      struct devlink *devlink)
> +{
> +	nsim_dev->dummy_region =
> +		devlink_region_create(devlink, "dummy",
> +				      NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX,
> +				      NSIM_DEV_DUMMY_REGION_SIZE);
> +	if (IS_ERR(nsim_dev->dummy_region))
> +		return PTR_ERR(nsim_dev->dummy_region);
> +
> +	return 0;

PTR_ERR_OR_ZERO()

> +}
> +
> +static void nsim_dev_dummy_region_exit(struct nsim_dev *nsim_dev)
> +{
> +	devlink_region_destroy(nsim_dev->dummy_region);
> +}
> +
>  static int nsim_dev_reload(struct devlink *devlink,
>  			   struct netlink_ext_ack *extack)
>  {

Re: [patch net-next] netdevsim: implement support for devlink region and snapshots

[kbuild test robot]

Hi Jiri,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jiri-Pirko/netdevsim-implement-support-for-devlink-region-and-snapshots/20190813-002135

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


coccinelle warnings: (new ones prefixed by >>)

>> drivers/net/netdevsim/dev.c:297:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] netdevsim: fix ptr_ret.cocci warnings

[kbuild test robot]

From: kbuild test robot <lkp@intel.com>

drivers/net/netdevsim/dev.c:297:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

Fixes: e9cf98183f96 ("netdevsim: implement support for devlink region and snapshots")
CC: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: kbuild test robot <lkp@intel.com>
---

url:    https://github.com/0day-ci/linux/commits/Jiri-Pirko/netdevsim-implement-support-for-devlink-region-and-snapshots/20190813-002135

 dev.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -294,10 +294,7 @@ static int nsim_dev_dummy_region_init(st
 		devlink_region_create(devlink, "dummy",
 				      NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX,
 				      NSIM_DEV_DUMMY_REGION_SIZE);
-	if (IS_ERR(nsim_dev->dummy_region))
-		return PTR_ERR(nsim_dev->dummy_region);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(nsim_dev->dummy_region);
 }
 
 static void nsim_dev_dummy_region_exit(struct nsim_dev *nsim_dev)

Re: [patch net-next] netdevsim: implement support for devlink region and snapshots

[Jiri Pirko]

Tue, Aug 13, 2019 at 02:58:59AM CEST, jakub.kicinski@netronome.com wrote:
>On Mon, 12 Aug 2019 12:16:20 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Implement dummy region of size 32K and allow user to create snapshots
>> or random data using debugfs file trigger.
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>I'm nacking all the netdevsim patches unless the selftest 
>is posted at the same time :/
>
>You're leaking those features one by one what if you get distracted 
>and the tests never materialize :/
>
>This is all dead code.

Okay, fair enough. Will add it now.


>
>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>> index 08ca59fc189b..e76ea6a3cb60 100644
>> --- a/drivers/net/netdevsim/dev.c
>> +++ b/drivers/net/netdevsim/dev.c
>> @@ -27,6 +27,41 @@
>>  
>>  static struct dentry *nsim_dev_ddir;
>>  
>> +#define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
>> +
>> +static ssize_t nsim_dev_take_snapshot_write(struct file *file,
>> +					    const char __user *data,
>> +					    size_t count, loff_t *ppos)
>> +{
>> +	struct nsim_dev *nsim_dev = file->private_data;
>> +	void *dummy_data;
>> +	u32 id;
>> +	int err;
>> +
>> +	dummy_data = kmalloc(NSIM_DEV_DUMMY_REGION_SIZE, GFP_KERNEL);
>> +	if (!dummy_data) {
>> +		pr_err("Failed to allocate memory for region snapshot\n");
>> +		goto out;
>> +	}
>> +
>> +	get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);
>> +
>> +	id = devlink_region_shapshot_id_get(priv_to_devlink(nsim_dev));
>> +	err = devlink_region_snapshot_create(nsim_dev->dummy_region,
>> +					     dummy_data, id, kfree);
>> +	if (err)
>> +		pr_err("Failed to create region snapshot\n");
>> +
>> +out:
>> +	return count;
>
>why not return an error?

Okay.


>
>> +}
>> +
>> +static const struct file_operations nsim_dev_take_snapshot_fops = {
>> +	.open = simple_open,
>> +	.write = nsim_dev_take_snapshot_write,
>> +	.llseek = generic_file_llseek,
>> +};
>> +
>>  static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>>  {
>>  	char dev_ddir_name[16];
>> @@ -44,6 +79,8 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>>  			   &nsim_dev->max_macs);
>>  	debugfs_create_bool("test1", 0600, nsim_dev->ddir,
>>  			    &nsim_dev->test1);
>> +	debugfs_create_file("take_snapshot", 0200, nsim_dev->ddir, nsim_dev,
>> +			    &nsim_dev_take_snapshot_fops);
>>  	return 0;
>>  }
>>  
>> @@ -248,6 +285,26 @@ static void nsim_devlink_param_load_driverinit_values(struct devlink *devlink)
>>  		nsim_dev->test1 = saved_value.vbool;
>>  }
>>  
>> +#define NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX 16
>> +
>> +static int nsim_dev_dummy_region_init(struct nsim_dev *nsim_dev,
>> +				      struct devlink *devlink)
>> +{
>> +	nsim_dev->dummy_region =
>> +		devlink_region_create(devlink, "dummy",
>> +				      NSIM_DEV_DUMMY_REGION_SNAPSHOT_MAX,
>> +				      NSIM_DEV_DUMMY_REGION_SIZE);
>> +	if (IS_ERR(nsim_dev->dummy_region))
>> +		return PTR_ERR(nsim_dev->dummy_region);
>> +
>> +	return 0;
>
>PTR_ERR_OR_ZERO()

Okay.


>
>> +}
>> +
>> +static void nsim_dev_dummy_region_exit(struct nsim_dev *nsim_dev)
>> +{
>> +	devlink_region_destroy(nsim_dev->dummy_region);
>> +}
>> +
>>  static int nsim_dev_reload(struct devlink *devlink,
>>  			   struct netlink_ext_ack *extack)
>>  {
>