netdev - Re: [PATCH iwl-net] ice: Add netif_device

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <40210c88-1e3a-44d2-8907-1530500eab91@linux.intel.com>
Date: Wed, 14 Aug 2024 12:44:56 +0200
From: Dawid Osuchowski <dawid.osuchowski@...ux.intel.com>
To: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 Jakub Kicinski <kuba@...nel.org>, Igor Bagnucki <igor.bagnucki@...el.com>
Subject: Re: [PATCH iwl-net] ice: Add netif_device_attach/detach into PF reset
 flow

On 14.08.2024 05:19, Kalesh Anakkur Purayil wrote:
> On Mon, Aug 12, 2024 at 3:52 PM Dawid Osuchowski
> <dawid.osuchowski@...ux.intel.com> wrote:
>> @@ -7568,11 +7570,13 @@ static void ice_update_pf_netdev_link(struct ice_pf *pf)
>>
>>                  ice_get_link_status(pf->vsi[i]->port_info, &link_up);
>>                  if (link_up) {
>> +                       netif_device_attach(pf->vsi[i]->netdev);
>>                          netif_carrier_on(pf->vsi[i]->netdev);
>>                          netif_tx_wake_all_queues(pf->vsi[i]->netdev);
>>                  } else {
>>                          netif_carrier_off(pf->vsi[i]->netdev);
>>                          netif_tx_stop_all_queues(pf->vsi[i]->netdev);
>> +                       netif_device_detach(pf->vsi[i]->netdev);
> [Kalesh] Is there any reason to attach back the netdev only if link is
> up? IMO, you should attach the device back irrespective of physical
> link status. In ice_prepare_for_reset(), you are detaching the device
> unconditionally.
> 
> I may be missing something here.

Hey Kalesh,

I think you are right, it is a mistake on my end. I have already sent a 
v2 but without this change. I just tested if this works with the attach 
irrespective of link status and it also resolves the reported issue that 
the patch is supposed to fix and doesn't introduce any regression that I 
am aware of. I will forward your concern to the v2 thread and will post 
a v3 with the change.

--Dawid

>>                  }
>>          }
>>   }
> 
>> --
>> 2.44.0
>>
>>
> 
> 
> --
> Regards,
> Kalesh A P