netdev - Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting sriov

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: <3176d23e-cc51-2855-7880-ca41779746e4@redhat.com>
Date:   Tue, 17 Aug 2021 05:23:51 -0500
From:   Ken Cox <jkc@...hat.com>
To:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "kuba@...nel.org" <kuba@...nel.org>
Cc:     "Szlosek, Marek" <marek.szlosek@...el.com>,
        "Yang, Lihong" <lihong.yang@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Venkataramanan, Anirudh" <anirudh.venkataramanan@...el.com>
Subject: Re: [PATCH net 1/1] ixgbe: Add locking to prevent panic when setting
 sriov_numvfs to zero



On 8/16/21 12:52 PM, Nguyen, Anthony L wrote:
> On Fri, 2021-08-13 at 17:20 -0700, Jakub Kicinski wrote:
>> On Thu, 12 Aug 2021 10:18:56 -0700 Tony Nguyen wrote:
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> index 214a38de3f41..0a1a8756f1fd 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>> @@ -206,8 +206,12 @@ int ixgbe_disable_sriov(struct ixgbe_adapter
>>> *adapter)
>>>   	unsigned int num_vfs = adapter->num_vfs, vf;
>>>   	int rss;
>>>   
>>> +	while (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter-
>>>> state))
>>> +		usleep_range(1000, 2000);
>>> +
>>>   	/* set num VFs to 0 to prevent access to vfinfo */
>>>   	adapter->num_vfs = 0;
>>> +	clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>>>   
>>>   	/* put the reference to all of the vf devices */
>>>   	for (vf = 0; vf < num_vfs; ++vf) {
>>> @@ -1307,6 +1311,9 @@ void ixgbe_msg_task(struct ixgbe_adapter
>>> *adapter)
>>>   	struct ixgbe_hw *hw = &adapter->hw;
>>>   	u32 vf;
>>>   
>>> +	if (test_and_set_bit(__IXGBE_DISABLING_VFS, &adapter->state))
>>> +		return;
>>> +
>>>   	for (vf = 0; vf < adapter->num_vfs; vf++) {
>>>   		/* process any reset requests */
>>>   		if (!ixgbe_check_for_rst(hw, vf))
>>> @@ -1320,6 +1327,7 @@ void ixgbe_msg_task(struct ixgbe_adapter
>>> *adapter)
>>>   		if (!ixgbe_check_for_ack(hw, vf))
>>>   			ixgbe_rcv_ack_from_vf(adapter, vf);
>>>   	}
>>> +	clear_bit(__IXGBE_DISABLING_VFS, &adapter->state);
>>
>> Like I've already said two or three times. No flag based locking.
> 
> Ken,
> 
> Did you want to make this change or did you want Intel to do it?

Hi Tony,

It would be great if Intel could make the change for the locking.

Thanks,
Ken