netdev - Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix fwlog after driver reinit

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: <4583cf1b-46b0-4664-a0b3-f02331685955@linux.intel.com>
Date: Tue, 25 Feb 2025 14:15:53 +0100
From: "Szapar-Mudlaw, Martyna" <martyna.szapar-mudlaw@...ux.intel.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix fwlog after driver
 reinit



On 2/20/2025 9:11 PM, Paul Menzel wrote:
> Dear Martyna,
> 
> 
> Thank you for your patch.
> 
> Am 20.02.25 um 16:04 schrieb Martyna Szapar-Mudlaw:
>> Fix an issue when firmware logging stops after devlink reload action
>> driver_reinit or driver reset. Fix it by restoring fw logging when
> 
> Maybe elaborate, why/how driver reinit or reset disables fwlog.
>

ok, I can expand commit message

>> it was previously registered before these events.
> 
> I’d add a blank line between paragraphs.>

ok

>> Restoring fw logging in these cases was faultily removed with new
>> debugfs fw logging implementation.
>> Failure to init fw logging is not a critical error so it is safely
>> ignored.
> 
> How can this be tested?

By examining debugfs fwlog data file, information from fw is no longer 
logged to this file after driver reinit (devlink dev reload 
pci/{PCI_Bus} action driver_reinit). In patches 73671c3162c8 ("ice: 
enable FW logging") and 9d3535e71985 ("ice: add ability to read and 
configure FW log data") it is described how to enable and collect ice fw 
logs.
In case of failure during fwlog registration, debug prints indicating 
this are logged.

> 
>> Fixes: 73671c3162c8 ("ice: enable FW logging")
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>> Signed-off-by: Martyna Szapar-Mudlaw <martyna.szapar- 
>> mudlaw@...ux.intel.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ 
>> ethernet/intel/ice/ice_main.c
>> index a03e1819e6d5..6d6873003bcb 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -5151,6 +5151,13 @@ int ice_load(struct ice_pf *pf)
>>       devl_assert_locked(priv_to_devlink(pf));
>> +    if (pf->hw.fwlog_cfg.options & ICE_FWLOG_OPTION_IS_REGISTERED) {
>> +        err = ice_fwlog_register(&pf->hw);
>> +        if (err)
>> +            pf->hw.fwlog_cfg.options &=
>> +                ~ICE_FWLOG_OPTION_IS_REGISTERED;
> 
> Should an error be logged in the failure case?

ice_fwlog_register function already provides logs in case of failure

Thank you for the review,
Martyna

> 
>> +    }
>> +
>>       vsi = ice_get_main_vsi(pf);
>>       /* init channel list */
>> @@ -7701,6 +7708,13 @@ static void ice_rebuild(struct ice_pf *pf, enum 
>> ice_reset_req reset_type)
>>           goto err_init_ctrlq;
>>       }
>> +    if (hw->fwlog_cfg.options & ICE_FWLOG_OPTION_IS_REGISTERED) {
>> +        err = ice_fwlog_register(hw);
>> +        if (err)
>> +            hw->fwlog_cfg.options &=
>> +                ~ICE_FWLOG_OPTION_IS_REGISTERED;
>> +    }
> 
> Ditto.
> 
>> +
>>       /* if DDP was previously loaded successfully */
>>       if (!ice_is_safe_mode(pf)) {
>>           /* reload the SW DB of filter tables */
> 
> 
> Kind regards,
> 
> Paul
>