netdev - Re: [PATCH net 2/7] Revert "ibmvnic: remove duplicate napi

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:   Thu, 24 Jun 2021 00:07:06 -0700
From:   Rick Lindsley <ricklind@...ux.vnet.ibm.com>
To:     Lijun Pan <lijunp213@...il.com>,
        Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
Cc:     Networking <netdev@...r.kernel.org>,
        Dany Madden <drt@...ux.ibm.com>,
        Rick Lindsley <ricklind@...ux.ibm.com>,
        Brian King <brking@...ux.ibm.com>,
        Cristobal Forno <cforno12@...ux.ibm.com>,
        Abdul Haleem <abdhalee@...ibm.com>
Subject: Re: [PATCH net 2/7] Revert "ibmvnic: remove duplicate napi_schedule
 call in open function"

On 6/23/21 11:20 PM, Lijun Pan wrote:

>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
>> index f13ad6bc67cd..fe1627ea9762 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>> @@ -1234,6 +1234,11 @@ static int __ibmvnic_open(struct net_device *netdev)
>>
>>          netif_tx_start_all_queues(netdev);
>>
>> +       if (prev_state == VNIC_CLOSED) {
>> +               for (i = 0; i < adapter->req_rx_queues; i++)
>> +                       napi_schedule(&adapter->napi[i]);
>> +       }
>> +
> 
> interrupt_rx will schedule the napi, so not necessary here.

You keep saying this, but yet there is some case with the original patch that leaves napi unscheduled and the device unresponsive.  Until that is better understood, this patch should be reverted - especially when you consider that calling napi_schedule() when the rx_queue is already scheduled is harmless.  The original patch did not address any specific problem, but did introduce one.

Rick