netdev - Re: [patch net-next v3 11/15] netdevsim: implement proper devlink reload

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:   Fri, 4 Oct 2019 08:19:14 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, idosch@...lanox.com,
        dsahern@...il.com, tariqt@...lanox.com, saeedm@...lanox.com,
        kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org, shuah@...nel.org,
        mlxsw@...lanox.com
Subject: Re: [patch net-next v3 11/15] netdevsim: implement proper devlink
 reload

Fri, Oct 04, 2019 at 01:17:30AM CEST, jakub.kicinski@...ronome.com wrote:
>On Thu,  3 Oct 2019 11:49:36 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...lanox.com>
>> 
>> During devlink reload, all driver objects should be reinstantiated with
>> the exception of devlink instance and devlink resources and params.
>> Move existing devlink_resource_size_get() calls into fib_create() just
>> before fib notifier is registered. Also, make sure that extack is
>> propagated down to fib_notifier_register() call.
>> 
>> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
>
>Acked-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>
>> diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
>> index d2aeac0f4c2c..fdc682f3a09a 100644
>> --- a/drivers/net/netdevsim/fib.c
>> +++ b/drivers/net/netdevsim/fib.c
>> @@ -63,12 +63,10 @@ u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
>>  	return max ? entry->max : entry->num;
>>  }
>>  
>> -int nsim_fib_set_max(struct nsim_fib_data *fib_data,
>> -		     enum nsim_resource_id res_id, u64 val,
>> -		     struct netlink_ext_ack *extack)
>> +static void nsim_fib_set_max(struct nsim_fib_data *fib_data,
>> +			     enum nsim_resource_id res_id, u64 val)
>>  {
>>  	struct nsim_fib_entry *entry;
>> -	int err = 0;
>>  
>>  	switch (res_id) {
>>  	case NSIM_RESOURCE_IPV4_FIB:
>> @@ -84,20 +82,10 @@ int nsim_fib_set_max(struct nsim_fib_data *fib_data,
>>  		entry = &fib_data->ipv6.rules;
>>  		break;
>>  	default:
>> -		return 0;
>> -	}
>> -
>> -	/* not allowing a new max to be less than curren occupancy
>> -	 * --> no means of evicting entries
>> -	 */
>> -	if (val < entry->num) {
>> -		NL_SET_ERR_MSG_MOD(extack, "New size is less than current occupancy");
>> -		err = -EINVAL;
>
>This change in behaviour should perhaps be mentioned in the commit
>message. The reload will no longer fail if the resources are
>insufficient. 

Reload is going to fail if the resources are insufficient. I have a
selftest for that, please see the last patch.

>
>Since we want to test reload more widely than just for the FIB limits
>that does make sense to me. Is that the thinking?
>
>> -	} else {
>> -		entry->max = val;
>> +		WARN_ON(1);
>> +		return;
>>  	}
>> -
>> -	return err;
>> +	entry->max = val;
>>  }
>>  
>>  static int nsim_fib_rule_account(struct nsim_fib_entry *entry, bool add,