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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 29 May 2019 09:47:54 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        sthemmin@...rosoft.com, dsahern@...il.com, saeedm@...lanox.com,
        leon@...nel.org, f.fainelli@...il.com
Subject: Re: [patch net-next v2 7/7] netdevsim: implement fake flash
 updating with notifications

On Wed, 29 May 2019 10:00:16 +0200, Jiri Pirko wrote:
> Tue, May 28, 2019 at 10:01:15PM CEST, jakub.kicinski@...ronome.com wrote:
> >On Tue, 28 May 2019 13:48:46 +0200, Jiri Pirko wrote:  
> >> From: Jiri Pirko <jiri@...lanox.com>
> >> 
> >> Signed-off-by: Jiri Pirko <jiri@...lanox.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?

Well you're adding notifications, probably that the notifications
arrive correctly?  Plus that devlink doesn't hung when there are no
notifications.  It doesn't have to be a super advanced test, just
exercising the code paths in the kernel is fine.

In principle netdevsim is for testing and you add no tests, its not
the first time you're doing this.

> >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
> >> @@ -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.

Is my maths off?  The loop is 5 sec now:

 500000 / 1000 * 10 ms = 5000 ms = 5 sec?