netdev - Re: [PATCH net-next v2 2/2] net: openvswitch: make masks cache size configurable

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] [day] [month] [year] [list]

Message-ID: <5A55F49F-24E5-4149-A212-8F00589D6777@redhat.com>
Date:   Fri, 24 Jul 2020 10:00:16 +0200
From:   "Eelco Chaudron" <echaudro@...hat.com>
To:     "Jakub Kicinski" <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, dev@...nvswitch.org,
        pabeni@...hat.com, pshelar@....org, fw@...len.de
Subject: Re: [PATCH net-next v2 2/2] net: openvswitch: make masks cache size
 configurable



On 23 Jul 2020, at 18:42, Jakub Kicinski wrote:

> On Thu, 23 Jul 2020 14:58:31 +0200 Eelco Chaudron wrote:
>> +	if ((size & (size - 1)) != 0 ||
>
> is_power_of_2() ?

Was not aware of this macro, will replace…
>
>> +	    (size * sizeof(struct mask_cache_entry)) > PCPU_MIN_UNIT_SIZE)
>> +		return NULL;
>
>> +	new->cache_size = size;
>> +	if (new->cache_size > 0) {
>> +		cache = __alloc_percpu(sizeof(struct mask_cache_entry) *
>> +				       new->cache_size,
>
> array_size() ?

Will change to:

		cache = __alloc_percpu(array_size(
					       sizeof(struct mask_cache_entry),
					       new->cache_size),
				       __alignof__(struct mask_cache_entry));
		if (!cache) {
>
>> +				       __alignof__(struct mask_cache_entry));
>> +
>
> No need for the new line here

Will remove

>> +		if (!cache) {
>> +			kfree(new);
>> +			return NULL;
>> +		}
>> +	}
>
>> +	if (size == mc->cache_size || (size & (size - 1)) != 0)
>
> why check "is power of 2" twice?

Will remove as it will always call tbl_mask_cache_alloc() which has the 
check above.

>
>> @@ -454,7 +516,7 @@ void ovs_flow_tbl_destroy(struct flow_table 
>> *table)
>>  	struct table_instance *ti = rcu_dereference_raw(table->ti);
>>  	struct table_instance *ufid_ti = 
>> rcu_dereference_raw(table->ufid_ti);
>>
>> -	free_percpu(table->mask_cache);
>> +	call_rcu(&table->mask_cache->rcu, mask_cache_rcu_cb);
>>  	call_rcu(&table->mask_array->rcu, mask_array_rcu_cb);
>>  	table_instance_destroy(table, ti, ufid_ti, false);
>>  }
>
> This adds a new warning :(
>
> net/openvswitch/flow_table.c:519:24: warning: incorrect type in 
> argument 1 (different address spaces)
> net/openvswitch/flow_table.c:519:24:    expected struct callback_head 
> *head
> net/openvswitch/flow_table.c:519:24:    got struct callback_head 
> [noderef] __rcu *

:( Yes my diff with older did not work… Sure I’ve fixed them all now 
:)


Will sent out a V3 after some testing…