netdev - Re: [PATCH net-next 01/10] netdevsim: fib: Convert the current occupancy to an atomic variable

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: <b307a304-09ef-d8e8-7296-92ddddfc348c@gmail.com>
Date:   Tue, 26 Jan 2021 21:33:27 -0700
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...sch.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, amcohen@...dia.com,
        roopa@...dia.com, sharpd@...dia.com, bpoirier@...dia.com,
        mlxsw@...dia.com, Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH net-next 01/10] netdevsim: fib: Convert the current
 occupancy to an atomic variable

On 1/26/21 6:23 AM, Ido Schimmel wrote:
> @@ -889,22 +882,29 @@ static void nsim_nexthop_destroy(struct nsim_nexthop *nexthop)
>  static int nsim_nexthop_account(struct nsim_fib_data *data, u64 occ,
>  				bool add, struct netlink_ext_ack *extack)
>  {
> -	int err = 0;
> +	int i, err = 0;
>  
>  	if (add) {
> -		if (data->nexthops.num + occ <= data->nexthops.max) {
> -			data->nexthops.num += occ;
> -		} else {
> -			err = -ENOSPC;
> -			NL_SET_ERR_MSG_MOD(extack, "Exceeded number of supported nexthops");
> -		}
> +		for (i = 0; i < occ; i++)
> +			if (!atomic64_add_unless(&data->nexthops.num, 1,
> +						 data->nexthops.max)) {

seems like this can be
		if (!atomic64_add_unless(&data->nexthops.num, occ,
					 data->nexthops.max)) {

and then the err_num_decrease is not needed

> +				err = -ENOSPC;
> +				NL_SET_ERR_MSG_MOD(extack, "Exceeded number of supported nexthops");
> +				goto err_num_decrease;
> +			}
>  	} else {
> -		if (WARN_ON(occ > data->nexthops.num))
> +		if (WARN_ON(occ > atomic64_read(&data->nexthops.num)))
>  			return -EINVAL;
> -		data->nexthops.num -= occ;
> +		atomic64_sub(occ, &data->nexthops.num);
>  	}
>  
>  	return err;
> +
> +err_num_decrease:
> +	for (i--; i >= 0; i--)
> +		atomic64_dec(&data->nexthops.num);

and if this path is really needed, why not atomic64_sub here?

> +	return err;
> +
>  }
>  
>  static int nsim_nexthop_add(struct nsim_fib_data *data,
>