netdev - Re: [PATCH net 4/4] ice: Add correct PHY lane assignment

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: <a7b0cf34-4445-40cf-9a8a-b3c24be08fc9@intel.com>
Date: Wed, 11 Dec 2024 11:36:03 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Paolo Abeni <pabeni@...hat.com>
CC: Karol Kolacinski <karol.kolacinski@...el.com>, <richardcochran@...il.com>,
	<horms@...nel.org>, Arkadiusz Kubalewski <Arkadiusz.kubalewski@...el.com>,
	Grzegorz Nitka <grzegorz.nitka@...el.com>, Pucha Himasekhar Reddy
	<himasekharx.reddy.pucha@...el.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <kuba@...nel.org>,
	<edumazet@...gle.com>, <andrew+netdev@...n.ch>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net 4/4] ice: Add correct PHY lane assignment

On 12/10/24 15:54, Paolo Abeni wrote:
> On 12/6/24 20:35, Tony Nguyen wrote:
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
>> index 496d86cbd13f..ab25ccd7e8ec 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>> @@ -4095,6 +4095,51 @@ ice_aq_set_port_option(struct ice_hw *hw, u8 lport, u8 lport_valid,
>>   	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
>>   }
>>   
>> +/**
>> + * ice_get_phy_lane_number - Get PHY lane number for current adapter
>> + * @hw: pointer to the hw struct
>> + *
>> + * Return: PHY lane number on success, negative error code otherwise.
>> + */
>> +int ice_get_phy_lane_number(struct ice_hw *hw)
>> +{
>> +	struct ice_aqc_get_port_options_elem *options __free(kfree);
> 
> Please avoid the __free() construct:
> 
> https://elixir.bootlin.com/linux/v6.13-rc2/source/Documentation/process/maintainer-netdev.rst#L393

My understanding was that conversions to __free() (w/o any other reason)
are bad. But for new code, it's fine. I get the "discourage" part from
the doc, as "don't use __free() by default, for all pointers, or all
allocations, but apply your judgement and sanity to tell if that makes
given function better".
I still believe that this function is better with __free(). We develop
(new code) with such assumptions for the better part of the year.

I think that static analysis tools/Reviewers already got used to that
(after the first false-positive memleak reported). Developers (and
Reviewers for sure) know that those pointers could not be left
uninitialized at function return. The only concern that is unresolved
for me yet, is: "there is a lot of characters to type", but that is also
good in some way, as one needs bigger function to justify the added
"complexity".

> 
> Thanks,
> 
> Paolo
>