Re: [patch net-next v2 7/7] netdevsim: implement fake flash updating with notifications

[Jiri Pirko <jiri@resnulli.us>]

Tue, May 28, 2019 at 10:01:15PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 28 May 2019 13:48:46 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> v1->v2:
>> - added debugfs toggle to enable/disable flash status notifications
>
>Could you please add a selftest making use of netdevsim code?

How do you imagine the selftest should look like. What should it test
exactly?


>
>Sorry, I must have liked the feature so much first time I missed this :)
>
>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>> index b509b941d5ca..c5c417a3c0ce 100644
>> --- a/drivers/net/netdevsim/dev.c
>> +++ b/drivers/net/netdevsim/dev.c
>> @@ -38,6 +38,8 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>>  	nsim_dev->ports_ddir = debugfs_create_dir("ports", nsim_dev->ddir);
>>  	if (IS_ERR_OR_NULL(nsim_dev->ports_ddir))
>>  		return PTR_ERR_OR_ZERO(nsim_dev->ports_ddir) ?: -EINVAL;
>> +	debugfs_create_bool("fw_update_status", 0600, nsim_dev->ddir,
>> +			    &nsim_dev->fw_update_status);
>>  	return 0;
>>  }
>>  
>> @@ -220,8 +222,49 @@ static int nsim_dev_reload(struct devlink *devlink,
>>  	return 0;
>>  }
>>  
>> +#define NSIM_DEV_FLASH_SIZE 500000
>> +#define NSIM_DEV_FLASH_CHUNK_SIZE 1000
>> +#define NSIM_DEV_FLASH_CHUNK_TIME_MS 10
>> +
>> +static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
>> +				 const char *component,
>> +				 struct netlink_ext_ack *extack)
>> +{
>> +	struct nsim_dev *nsim_dev = devlink_priv(devlink);
>> +	int i;
>> +
>> +	if (nsim_dev->fw_update_status) {
>> +		devlink_flash_update_begin_notify(devlink);
>> +		devlink_flash_update_status_notify(devlink,
>> +						   "Preparing to flash",
>> +						   component, 0, 0);
>> +	}
>> +
>> +	for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
>> +		if (nsim_dev->fw_update_status)
>> +			devlink_flash_update_status_notify(devlink, "Flashing",
>> +							   component,
>> +							   i * NSIM_DEV_FLASH_CHUNK_SIZE,
>> +							   NSIM_DEV_FLASH_SIZE);
>> +		msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
>
>In automated testing it may be a little annoying if this takes > 5sec

I wanted to emulate real device. I can make this 5 sec if you want, no
problem.


>
>> +	}
>> +
>> +	if (nsim_dev->fw_update_status) {
>> +		devlink_flash_update_status_notify(devlink, "Flashing",
>> +						   component,
>> +						   NSIM_DEV_FLASH_SIZE,
>> +						   NSIM_DEV_FLASH_SIZE);
>> +		devlink_flash_update_status_notify(devlink, "Flashing done",
>> +						   component, 0, 0);
>> +		devlink_flash_update_end_notify(devlink);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static const struct devlink_ops nsim_dev_devlink_ops = {
>>  	.reload = nsim_dev_reload,
>> +	.flash_update = nsim_dev_flash_update,
>>  };
>>  
>>  static struct nsim_dev *