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]

Message-ID: <20191003161730.6c61b48c@cakuba.hsd1.ca.comcast.net>
Date:   Thu, 3 Oct 2019 16:17:30 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
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

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. 

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,