From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 85615222580 for ; Thu, 19 Dec 2024 10:43:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734604995; cv=none; b=suFwuBxdmLabkReKUg2YTjmoorhxDofL+afbWwQuMyzXRG770iYdLvXoLIWzp1To1wZDAJ+pNuSQYNuPCTqqqFgKTEcs8Zb6Ptpg9azmmTJ3mEf8yu3uNpOlnnX5HI2xyNGqi39r39AlaroMloNByF1KbvETMtPPi9MEXBc97hM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734604995; c=relaxed/simple; bh=xctGrKNjyEqrQNBes8QDh00bSqkeoLPqCWNniZ+3mDw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=j4LPhyjrAfP/tMg8PXpUEsGXR2qlz6fKlFPLDXvGKKGMB9tXLgYigtbAsRqTEW6WeuoDsG87LZD36dERj+cTEPz2KBQ9AGKanwxHAWU+zUJfY4KIT0DrdXIqfkqlUgFnHHfmTBHYn5vMNoorp5X7ZJ7aGmm0rOwxKOH6ry9KjJE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=WqRel8W3; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="WqRel8W3" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1734604992; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dpPwc0bxzpk/ODMpU/DpXxSrREQ7QEq1IBIxImcqo4g=; b=WqRel8W3eD3hHwIai1bjsBU9HBCGbTZgmGV2e86gfgEyl1j3tIkWf/oRnbzhAuukG2TlLq 636wX5FVbm45tKSysL40D1SEEjZgSL0giBKuD7JU6MIkMUMCrDxP9pMtJn1D/YuuE9I4ag 9EcVesQMSJPokBz/kzqaSA3sxDuVF0Y= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-14-c9ArHwbBO0yD2Z29lieDgQ-1; Thu, 19 Dec 2024 05:43:10 -0500 X-MC-Unique: c9ArHwbBO0yD2Z29lieDgQ-1 X-Mimecast-MFC-AGG-ID: c9ArHwbBO0yD2Z29lieDgQ Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-385df115288so320334f8f.2 for ; Thu, 19 Dec 2024 02:43:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734604989; x=1735209789; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=dpPwc0bxzpk/ODMpU/DpXxSrREQ7QEq1IBIxImcqo4g=; b=Y6/WnPhfh4/ZM4ohckukJll1v9DlZaxBK8TOc0rVh7QFGRk3TlDsto67w5ymEfiesQ wHXdpFuKfmfU9+OCF8QyHpBSDBh3jIGs8j3S6bJyQML41wz+O6ZaxXxMvuY4OGNdXBxl dAwgMAWuzT2xMjST+RWJWubTzoAXtkhOlU9d7AC5qwQcuN8CL6iLw9FD31LV3oPRiYuJ +xYsFYoiFcFMsnNdLD0z59LaAjYSSCQFuEr5uHF4n24fDfjWIEORwJkBJIyoR9YrOv0K yx8xBdQUQto5L5hUEocyQM4yR3tcbZERq1rq6WgqPyv2oKQK/GbTVxTFZCMfeZ7OdZlb N2Ng== X-Forwarded-Encrypted: i=1; AJvYcCWY2uSakfcETtvbawPJdyBkIggrLbh/1mo5HY9tZjhzx5FmbkeqYGAeo1/qO3YD9OT+uW5AOShg1bJCKp0=@vger.kernel.org X-Gm-Message-State: AOJu0YwgiJOK8IWkcEM787utUqG3yaPR1q9J4HVVIixj721HIT1KKvgS i3l5TfBtO4I20AR7KS4rSNVodxD8rOFAkGp8yr3NoFtiGErsmNOrOTsYMCxbkKBn+PLWJgWk0is tjC/AbJ4iYTbD5vg+jAaZti33H+KSGoMwRGK7bzbH9eDTgxXZVQSW0AyAywr5VQ== X-Gm-Gg: ASbGncvjGv+IR6DqbuJuzw2/mnhe2eZwk/fbcRQ4Acwag77YkAL0d9pcJuCaodN7r9n mq4sAnA71QH4kOgXY+QpArFI/iMeWIUqravanUUgJkcjbR5J6B/kop4PNgsnO+6MhP0oJu/yoq8 bVpuV9J+h9DWcZa8gWD5zAfnDmbbWz2KNsRQfPrGfeZIG+kUamIZMncx1bjujg3widGVKvl8GT5 DieZRVA85UyHuOhDAxZ9pK4X3dB13r1A41iUjfzteYdaxU3dvlrxG4Bmg6rpirzqqN4xBrZk8JG ljr0F9Vrvw== X-Received: by 2002:a05:6000:1fa2:b0:386:3357:b4ac with SMTP id ffacd0b85a97d-38a19b1e727mr2518819f8f.42.1734604989569; Thu, 19 Dec 2024 02:43:09 -0800 (PST) X-Google-Smtp-Source: AGHT+IGJSKr2/YYlBAUa3pwKhmIgjUJgPNJ1Yka1xNUjygqzb6ORUFcMicfDNAljlC4zVw6nGPGEeQ== X-Received: by 2002:a05:6000:1fa2:b0:386:3357:b4ac with SMTP id ffacd0b85a97d-38a19b1e727mr2518793f8f.42.1734604989184; Thu, 19 Dec 2024 02:43:09 -0800 (PST) Received: from [192.168.88.24] (146-241-54-197.dyn.eolo.it. [146.241.54.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c89e528sm1261604f8f.83.2024.12.19.02.43.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Dec 2024 02:43:08 -0800 (PST) Message-ID: <85e10807-c2ea-41c8-a5b1-64105f7f30ce@redhat.com> Date: Thu, 19 Dec 2024 11:43:07 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RESEND V2 net 1/7] net: hns3: fixed reset failure issues caused by the incorrect reset type To: Michal Swiatkowski , Jijie Shao Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org, andrew+netdev@lunn.ch, horms@kernel.org, shenjian15@huawei.com, wangpeiyang1@huawei.com, liuyonglong@huawei.com, chenhao418@huawei.com, jonathan.cameron@huawei.com, shameerali.kolothum.thodi@huawei.com, salil.mehta@huawei.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20241217010839.1742227-1-shaojijie@huawei.com> <20241217010839.1742227-2-shaojijie@huawei.com> <8a789f23-a17a-456d-ba2a-de8207d65503@redhat.com> Content-Language: en-US From: Paolo Abeni In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 12/19/24 11:11, Michal Swiatkowski wrote: > On Thu, Dec 19, 2024 at 10:41:53AM +0100, Paolo Abeni wrote: >> On 12/18/24 10:02, Michal Swiatkowski wrote: >>> On Tue, Dec 17, 2024 at 09:08:33AM +0800, Jijie Shao wrote: >>>> From: Hao Lan >>>> >>>> When a reset type that is not supported by the driver is input, a reset >>>> pending flag bit of the HNAE3_NONE_RESET type is generated in >>>> reset_pending. The driver does not have a mechanism to clear this type >>>> of error. As a result, the driver considers that the reset is not >>>> complete. This patch provides a mechanism to clear the >>>> HNAE3_NONE_RESET flag and the parameter of >>>> hnae3_ae_ops.set_default_reset_request is verified. >>>> >>>> The error message: >>>> hns3 0000:39:01.0: cmd failed -16 >>>> hns3 0000:39:01.0: hclge device re-init failed, VF is disabled! >>>> hns3 0000:39:01.0: failed to reset VF stack >>>> hns3 0000:39:01.0: failed to reset VF(4) >>>> hns3 0000:39:01.0: prepare reset(2) wait done >>>> hns3 0000:39:01.0 eth4: already uninitialized >>>> >>>> Use the crash tool to view struct hclgevf_dev: >>>> struct hclgevf_dev { >>>> ... >>>> default_reset_request = 0x20, >>>> reset_level = HNAE3_NONE_RESET, >>>> reset_pending = 0x100, >>>> reset_type = HNAE3_NONE_RESET, >>>> ... >>>> }; >>>> >>>> Fixes: 720bd5837e37 ("net: hns3: add set_default_reset_request in the hnae3_ae_ops") >>>> Signed-off-by: Hao Lan >>>> Signed-off-by: Jijie Shao >>>> Signed-off-by: Paolo Abeni >> >> I haven't signed-off this patch. >> >> Still no need to repost (yet) for this if the following points are >> solved rapidly (as I may end-up merging the series and really adding my >> SoB), but please avoid this kind of issue in the future. >> >>>> @@ -4227,7 +4240,7 @@ static bool hclge_reset_err_handle(struct hclge_dev *hdev) >>>> return false; >>>> } else if (hdev->rst_stats.reset_fail_cnt < MAX_RESET_FAIL_CNT) { >>>> hdev->rst_stats.reset_fail_cnt++; >>>> - set_bit(hdev->reset_type, &hdev->reset_pending); >>>> + hclge_set_reset_pending(hdev, hdev->reset_type); >>> Sth is unclear for me here. Doesn't HNAE3_NONE_RESET mean that there is >>> no reset? If yes, why in this case reset_fail_cnt++ is increasing? >>> >>> Maybe the check for NONE_RESET should be done in this else if check to >>> prevent reset_fail_cnt from increasing (and also solve the problem with >>> pending bit set) >> >> @Michal: I don't understand your comment above. hclge_reset_err_handle() >> handles attempted reset failures. I don't see it triggered when >> reset_type == HNAE3_NONE_RESET. >> > > Maybe I missed sth. The hclge_set_reset_pending() is added to check if > reset type isn't HNAE3_NONE_RESET. If it is the set_bit isn't called. It > is the only place where hclge_set_reset_pending() is called with a > variable, so I assumed the fix is for this place. > > This means that code can be reach here with HNAE3_NONE_RESET which is > unclear for me why to increment resets if rest_type in NONE. If it is > true that hclge_reset_err_handle() is never called with reset_type > HNAE3_NONE_RESET it shouldn't be needed to have the > hclge_set_reset_pending() function. You are right, I felt off-track. @Jijie: how can 'reset_type' be set to an unsupported value?!? I don't see that in the code, short of a memory corruption on uninit problem. Are you sure you are not papering over a different issue here? At least some more info (either in the commit description or in a code comment) is IMHO needed. Otherwise you should probably catch that before hclge_reset_err_handle() time. Thanks! Paolo