Wed, May 29, 2019 at 06:47:54PM CEST, jakub.kicin...@netronome.com wrote:
>On Wed, 29 May 2019 10:00:16 +0200, Jiri Pirko wrote:
>> Tue, May 28, 2019 at 10:01:15PM CEST, jakub.kicin...@netronome.com wrote:
>> >On Tue, 28 May 2019 13:48:46 +0200, Jiri Pirko wrote:
>> >> From: Jiri Pirko <j...@mellanox.com>
>> >>
>> >> Signed-off-by: Jiri Pirko <j...@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?
>
>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.
:/
Will add tests and send v3. Monday. Thanks!
>
>> >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?
Ah, yes. Originally I had this 20 sec. Pardon me.
