netdev - Re: [RFC net-next 2/2] ice: make use of DECLARE_FLEX() in ice

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: Tue, 1 Aug 2023 16:36:57 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
CC: Kees Cook <keescook@...omium.org>, Jacob Keller
	<jacob.e.keller@...el.com>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>
Subject: Re: [RFC net-next 2/2] ice: make use of DECLARE_FLEX() in
 ice_switch.c

On 8/1/23 15:48, Alexander Lobakin wrote:
> From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Date: Tue, 1 Aug 2023 13:19:23 +0200
> 
>> Use DECLARE_FLEX() macro for 1-elem flex array members of ice_switch.c
>>
>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice_switch.c | 53 +++++----------------
>>   1 file changed, 12 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
>> index a7afb612fe32..41679cb6b548 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
>> @@ -1812,15 +1812,11 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
>>   			   enum ice_sw_lkup_type lkup_type,
>>   			   enum ice_adminq_opc opc)
>>   {
>> -	struct ice_aqc_alloc_free_res_elem *sw_buf;
>> +	DECLARE_FLEX(struct ice_aqc_alloc_free_res_elem *, sw_buf, elem, 1);
>> +	u16 buf_len = struct_size(sw_buf, elem, 1);
>>   	struct ice_aqc_res_elem *vsi_ele;
>> -	u16 buf_len;
>>   	int status;
>>   
>> -	buf_len = struct_size(sw_buf, elem, 1);
>> -	sw_buf = devm_kzalloc(ice_hw_to_dev(hw), buf_len, GFP_KERNEL);
>> -	if (!sw_buf)
>> -		return -ENOMEM;
>>   	sw_buf->num_elems = cpu_to_le16(1);
>>   
>>   	if (lkup_type == ICE_SW_LKUP_MAC ||
>> @@ -1840,8 +1836,7 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,
>>   			sw_buf->res_type =
>>   				cpu_to_le16(ICE_AQC_RES_TYPE_VSI_LIST_PRUNE);
>>   	} else {
>> -		status = -EINVAL;
>> -		goto ice_aq_alloc_free_vsi_list_exit;
>> +		return -EINVAL;
>>   	}
>>   
>>   	if (opc == ice_aqc_opc_free_res)
> 
> bloat-o-meter results would be nice to have in the commitmsg.

I will do next time, perhaps you could tell me if I get the results 
right here:

./scripts/bloat-o-meter ice_switch.o{ld,}
add/remove: 2/2 grow/shrink: 3/5 up/down: 560/-483 (77)
Function                                     old     new   delta
ice_create_vsi_list_rule                       -     241    +241
ice_remove_vsi_list_rule                     139     270    +131
ice_add_adv_rule                            6047    6139     +92
ice_add_sw_recipe                           2892    2972     +80
__pfx_ice_create_vsi_list_rule                 -      16     +16
ice_alloc_recipe                             124     113     -11
__pfx_ice_aq_alloc_free_vsi_list              16       -     -16
ice_free_res_cntr                            185     155     -30
ice_alloc_res_cntr                           154     124     -30
ice_add_update_vsi_list                     1037     994     -43
ice_add_vlan_internal                       1027     953     -74
ice_aq_alloc_free_vsi_list                   279       -    -279
Total: Before=42183, After=42260, chg +0.18%

My guess here is that compiler did different decisions about what to 
inline where, what is biggest difference.
And biggest gain here is avoidance of heap allocs, perhaps that enables 
gcc to shuffle things a bit too.
Another guess is that b-o-m ignores heap bloat, so slight growth is 
expected.

Values reported for ice.ko are the same, with bigger base to compute the 
percent off.

 > [...]
 >
 > Thanks,
 > Olek

Thank you too, also for our initial talk about on the topic.