Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9ec85f45-4724-4695-a853-16c5efcf8151@intel.com>
Date:   Tue, 8 Aug 2023 15:10:59 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH net 0/2] Fix VF to VM attach detach
Content-Language: en-US
To:     Petr Oros <poros@redhat.com>, <netdev@vger.kernel.org>
CC:     <jesse.brandeburg@intel.com>, <anthony.l.nguyen@intel.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <przemyslawx.patynowski@intel.com>,
        <kamil.maziarz@intel.com>, <dawidx.wesierski@intel.com>,
        <mateusz.palczewski@intel.com>, <slawomirx.laba@intel.com>,
        <norbertx.zulinski@intel.com>, <intel-wired-lan@lists.osuosl.org>,
        <linux-kernel@vger.kernel.org>
References: <20230807094831.696626-1-poros@redhat.com>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20230807094831.696626-1-poros@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UFBnRXdRbUdEYVBYWE9UWE5EbW4vc3p1QnJIZ0UxSTNjdi9hZm45bld6MnNs?=
 =?utf-8?B?ekhSckpiZytScEV3dUhPMG4vWWpmOHlNTWt4SnhINE9jNkRjZG4yWkFFTWlG?=
 =?utf-8?B?d3RMcnFGcFQ3WEU5TjdNQlVRRDRwZzlIZWxWK0xZOEp0L1hGd3UxcFFweUx3?=
 =?utf-8?B?WWxOR3FHMlAxNU45Z2lZQXZLSHRnYzRvUkhBN2dwU2E2ZE1zUjd1SkYrM3RZ?=
 =?utf-8?B?aGVuaEwvbllxZ2laTFQxc2RYZFFub01zM2E1UnBjTUw4UDhlSGdDQi9WWitq?=
 =?utf-8?B?Y01JeDRTWitmbmNDRGFsVnEwZ29DVW1vTWpvb2hNRHVZWWRXcTNJbTdxMUhR?=
 =?utf-8?B?TFdCdEJiWVpCeHpwSW9aRU5NUkVnUXJYMUdvUnBORmRwREI3dEJBSmFPOHl5?=
 =?utf-8?B?MkJaK0oxek82eEw0cnhaYjFJdTFvM0RjNFp1OW9iU1Y3OEZKV0liclEwcHd2?=
 =?utf-8?B?ZnlIT0s5V0FuUzNtamFZMzZoRTNQbUZQdUhhaVhNd1d0NzVabUZGT0ZqSVlJ?=
 =?utf-8?B?cDNlbUdTSlZia3ZFeTZYZ1ROUGUxN0xIajRWL0pQZmQ0U0NDa0t0UWxWMFlW?=
 =?utf-8?B?NmFBNkdCdHdnQ0ZCYjI3SUhmNXk4aGFYNThtYTMvWjNnVHBFTE8zc2dzb2ww?=
 =?utf-8?B?K3h5UTM5bXFodDJxcnM5LzRFZ3lIb3RUZlpjN2FRUWdxaUZHWlNZN25RQkxZ?=
 =?utf-8?B?N0cvSHFKZGJGOHBWb3lmZ2dpOTZzNGd0TGVudElrZE5jbjJyYkFMd2x1cXlp?=
 =?utf-8?B?ZndIRS9sU3RPVUJ1d3ExVmFtYkc3b250MWF0dW14ZTc4V3BOak5Za3J4Vzd0?=
 =?utf-8?B?TWtsZlplVVBRRFBXOTE5WmxrS3lJWkQyQWlKZkpwWjdNM1ZMNTBOMzR6U3lH?=
 =?utf-8?B?dTlnTzljaHVndElQeVZIbjJ1YVFFUjJrSmJjMlJtYjAwTXFpSGNwdDBxNU5D?=
 =?utf-8?B?dW9VYlR1S3gzNEhFWVVGVVRrS3J5NnlEUzh6WGNiN1dLYnhVWmwwNm9Kaklk?=
 =?utf-8?B?OWZIZGU1L3g2WnlOVEtjYkR0MWt0VE9hYlJ1Z0hRa3V4cFNBVTY0ZE5rOG1v?=
 =?utf-8?B?TFljOXhGUzhRdHFHL0oydE1ZSHdMM1V6anVJenBOM1dTaXFvYTlqWU9pbEt5?=
 =?utf-8?B?dFBaLzUwYTR4bVMzWVZCVlRsNXJteGJTUEZJUHQvU2E1Sk8zSWp2NVMxVnJ1?=
 =?utf-8?B?MkUrT09PNUhjcDdBQVNjVjlKbTh2UlY0R21VKzZOS2g1TENkN3RVZGRGdWVm?=
 =?utf-8?B?WkQ3eXN5aU1mM0lKa3BidUhmRDRIdk9jM3lSc0NNa1hUbHRyZGt6Nk9DdzFt?=
 =?utf-8?B?MXhKN1RibkhCSi9xdkdkRzZqMHlEbFFmSm13ZWJKeld6QVN5c1RjRlJadnd1?=
 =?utf-8?B?eUkzeUlqamdLc0F1K0ZEWjk0OWFVVnY3cHVVUXcrbGFRQW9vbzcvUThzYUxw?=
 =?utf-8?B?TFU1enA2bzgyRDZMNEdPRzJqUzMyY2JpMVdhZ1ZSeC9ac2VEVWtnWFQydlMw?=
 =?utf-8?B?T3NRMXozcjB5VmdVVE5UQy90bkxOdlJ5ZjEyRnppWHFvdE02aGE1Wk1GUXJC?=
 =?utf-8?B?ZFpzYVZ6MTJPM2VpOTNQR1hxZElZT05iVmh1c1V1M1BJYStlNklUR1h2NExS?=
 =?utf-8?B?WFNOaXo0UzJwMW42ajg4bmRMQmw4QzA0Q1d0YlhDdWk3alFURTRYbFFEbEV0?=
 =?utf-8?B?WE15cUV4LzltTUM1RldXQVBuOU5iVzd3M1AzQkJUdndhSHJxbFRYVU1LRnoy?=
 =?utf-8?B?aExzSzhBTm1oSFpxSTBDRWVvSHdRQytjTXRBZDdoVDV0LzRISEE2NnlZd2s3?=
 =?utf-8?B?b1N0emluazh6SFJZa29jU3Q5ejlYem1OWHBzOXFFVUxIWlN0Tzk1cWV5cjZr?=
 =?utf-8?B?eWFLbGNFTXJNeWNsWkpDZ2oyZ0U3OWI0aTdvWkRmaFNtbXN5MnhPVEJxRnRY?=
 =?utf-8?B?YWZudVlNQm41Wkp0WXM3V3piMlozYWdrRFZKRmxKRWdNL2krK3NmaG00L1lD?=
 =?utf-8?B?MDZhSEVOYVJ1eHN2a3AvOHQ5VjVmNXQzV2xoT3J4M2o5ZHBza1NaUUFBSkp1?=
 =?utf-8?B?ZEF0eXFnTHZZOTgrd1lrNHBTNTZraXFkYjcwTUxUTTNyem5XamswSjlDQXBL?=
 =?utf-8?B?bHIvR2ZiYVhadi8zODE2OTFaUVZVdks5Wnd3R3BwaDdXNkpuOXpPdUpMS3k5?=
 =?utf-8?B?eWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 28e82a61-2c28-4be1-7f8f-08db985c5a0e
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2023 22:11:01.9963
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: p+6i9mkt/WN6NmXG6FRIuzS/vyAWy3MXn9hwHSZA1ulDFjTKobEZmoPEezbIL2zgNgklEOw0EWZxnIMGsrCh66Tu+RYNIVKRe+uC3WOnAbg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW6PR11MB8340
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/7/2023 2:48 AM, Petr Oros wrote:
> Petr Oros (2):
>   Revert "ice: Fix ice VF reset during iavf initialization"
>   ice: Fix NULL pointer deref during VF reset
> 
>  drivers/net/ethernet/intel/ice/ice_sriov.c    |  8 ++---
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 34 +++++--------------
>  drivers/net/ethernet/intel/ice/ice_vf_lib.h   |  1 -
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |  1 -
>  4 files changed, 12 insertions(+), 32 deletions(-)
> 


I reviewed the commit message for the reverted commit and I concur that
any sort of issue that it fixed with respect to concurrent resets is
better fixed by the 2nd commit of this series.

The motivation for the original commit appears to be to prevent a reset
from happening while the VF is still resetting. I think this motivation
is incorrect for a few reasons:

1) as described by the revert above, if the VF has not had iAVF load on
it yet (such as when its been assigned to the pass through PCI driver),
it will never initialize, and thus we can't ever reset the device.

2) It does not make sense for the PF to rely on or assume behavior of
the VF (i.e. that it will properly initialize) and I believe it should
be allowed to reset the device without regard to the state of the VF
driver. It is a bug in the VF driver if this causes problems for it. I
think we recently fixed several issues here in iAVF.

With that in mind, I think this series of fixes is good.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>