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: <4ff0b209-2770-3790-ae93-3ea81c15a03e@intel.com>
Date:   Mon, 12 Sep 2022 15:08:27 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Subject: Re: [PATCH net-next v1] drivers/net/ethernet/intel/e100: check the
 return value of e100_exec_cmd()
Content-Language: en-US
To:     Li Zhong <floridsleeves@gmail.com>, <linux-kernel@vger.kernel.org>,
        <netdev@vger.kernel.org>, <intel-wired-lan@lists.osuosl.org>
CC:     <pabeni@redhat.com>, <kuba@kernel.org>, <edumazet@google.com>,
        <davem@davemloft.net>, <jesse.brandeburg@intel.com>
References: <20220909041645.2612842-1-floridsleeves@gmail.com>
From:   Tony Nguyen <anthony.l.nguyen@intel.com>
In-Reply-To: <20220909041645.2612842-1-floridsleeves@gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a1BxTktYUjJsWDZHcFUxS2hJeDRrSitjR1NLcGV2L2RaS25lc1RDTjlMTmpQ?=
 =?utf-8?B?eUoyWTFZVGlpRWx5L3kzd1V5ZFJnZ2RocEcraVR0aWptOHYvN2RhZE15dmNr?=
 =?utf-8?B?SGlKMUxxcmt5STY0S2JFdFcwVkJnRGxURmtBMUVibnhibTVDSHhYaVNNaDI3?=
 =?utf-8?B?bzFQRVI5M01aT3F1UXdFTkVFb3ZjeHBrMW91bEhQVDRqcjgwQ3l5eWxoc3k1?=
 =?utf-8?B?dU1WWmw4cDlKZXJ0RXExNkRqSkhCdXlGcnpLQmYxNkRaditzbzhmMTJXRGw3?=
 =?utf-8?B?SU5WeXFEUG9qbFMraG1TVkNsdGpmVEovdXkzUU1aTWF6YWIvanFSd2JyTWti?=
 =?utf-8?B?bUJWMmFZVTVHbVlBS0FSZGNReEI2NmhEY2o2Ym00KysrbUc4VGdDTVd3TThF?=
 =?utf-8?B?cXp5OW0rQk85eHdRZmFlcDZkMG43NGNKeXhLaWloY2RabEVRSm9oQkdqZFA5?=
 =?utf-8?B?UHdXbG9OWnFYUVMzL3Q1Yjk3aVA2R2dzdllQZGdYYnl5U3ZLVWdIRG9lbzhX?=
 =?utf-8?B?eTJsSzVUdTVMNmxYNk80ampoVHZUNUZlTmNsazNWSVRRNVJZQTZjb3p3eGhl?=
 =?utf-8?B?d0FHN3Y0U1dmY0tCelFWSHZwdnA0cnM1YmdnSDVWUjJnODgwaGFoaGtkYlVK?=
 =?utf-8?B?V2F3aXVxR1VvaUVSa3VBWEpCTXpJcTc1eXN6ZjhzSElkMEd4b3VIZzkyNmtL?=
 =?utf-8?B?SVVFeWJocTJQS3psOW9RQ1NQZVhCQXdOeTI3UW54ZGx0dU9zQmV3bklsKzhl?=
 =?utf-8?B?Lzh1SFRQSjRhVCtDSHhseFo1c3ljVER2bzExSXdtcmRUQ2lucXQzN0p4Mnox?=
 =?utf-8?B?YXkxcktGbm10anRTckZhNnJ0YWRGUm9QMzh5VWRNSFdmWlIvUmFic2FZL0Ni?=
 =?utf-8?B?U3VsKzVBeTdYUzE5SWZ4cHBMYzZIT1p1UzZ1azBvcGY5WWpsTEViaGE5L1Zj?=
 =?utf-8?B?YUxDckdYbjRacWR3amVVUnRHZUlNYWtDUTJWdGVyWTh5VGtkdEhUbnVuOFdj?=
 =?utf-8?B?T3FJTGVmK2ZCRGQ5M0RjZEpMVHg5VGJtV2JPSlFmVVJuU211RW9iMDNrUzJm?=
 =?utf-8?B?S1kvNFpOdTBpOW5idHMyNElnL0tYWFVGUFlNdGJCR1FFYUhjWmNtUDlkTUVu?=
 =?utf-8?B?RTVqNitrUEJFUjNYSXhpenZmeks5QkJWQjRaZ0RadFlLQmV0VVlFTjkvZ2RC?=
 =?utf-8?B?TDA0bFlwUTJkbERoUHJTSFpMeUJGT0NYcUFWck9iaWVaOCtYV3ZOQzZEcDR0?=
 =?utf-8?B?MEJjMFVFVEo3b0UzSmgzL2lVOGxUZ3c5L2xjTk5wYURXSmI5aDhVMEkzNllR?=
 =?utf-8?B?bTgzNDRLREFrN21lNWhpL0JsZ013NUp6S0lRSG9kWXVNejhEcGR5OUdzNElK?=
 =?utf-8?B?UEdSeGRndzV6c01nNCtCRXBwOGtHc05HeUpvcUpzUElwVU9aSG5lVk5GQ1B5?=
 =?utf-8?B?YWh1ckZvVUo3djk3VXp2NHYyUDVaTmJaY1dWa3lTSStuY05EeWtKZnIrQWZy?=
 =?utf-8?B?MkQzK0JIRDh2bjIxRC9sMlJxeTlWczZYNWpEMkx6dzhvNjJCZ0ZnWnJXbmtW?=
 =?utf-8?B?RHRodXZJZkRhME4vemswOEk0Z05wZ0tTODQ0NVh0UUNSR2dLWWQ2Q2xVdUVX?=
 =?utf-8?B?WnQyY2d0WE5WcDIwS1JCbmhlazBPY21WRmhsZ3NVTmV5eWlqeDRSNWtqVEt3?=
 =?utf-8?B?ckQ3WEpTbitCRUNIZldYUTJURUk5dHUyTXNRejhYeDBHM2pzZ0w3RUtWWWlh?=
 =?utf-8?B?NzVUNEZwK0tLMHFWZ3cyZ0VSN29uREJaV0xhMkdkNVlYUjdQbC9TUWY1NHlj?=
 =?utf-8?B?Qk12WGFzOVlJblNRcDJ0UER6VXRlamo5MzYrb3FjRmRxVmdodWM4Q0JTL2lC?=
 =?utf-8?B?a0hZQThIWkprY0k1eFRDcU92UkRpYUI5TVJDQzVQRXVPZkE1aDR0V1NWTk5E?=
 =?utf-8?B?YnhHcWdsWVBnVWNKdmY1WTNETUw2VDlXUDFxUzk4MjZ0bXR4NjB0VFc5NVVz?=
 =?utf-8?B?bklybWpHSEhkaTNpUFFrcXF5WjZxazFHMHFTR00wUG9PN28vR2k3K0xRRXNY?=
 =?utf-8?B?VDFROEo3bnNoL2dZSjdUeTI2SkYrWlowN3RBQmVBaDZ6eHpHNTVjZ2tKQXRW?=
 =?utf-8?B?cEwwbzhTV3YzK3o1VXQzMDgrakRBc1gyL1ZUVjRxdWhFclF1R2NUeElhdmE3?=
 =?utf-8?B?WFE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6c49a238-ebcf-435d-2747-08da950b53ce
X-MS-Exchange-CrossTenant-AuthSource: SN6PR11MB3229.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Sep 2022 22:08:31.1107
 (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: voZQu5y6HZUAlrC8D3TScaTRRzVFCDbM5jqxfoUutmMdWUr6FnhrEKFr3Jb5gT51kMmi+qguYU6l8mQrMjTVtl29OiDnMCBnmY1kv1x2eFw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6851
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-6.6 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_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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 9/8/2022 9:16 PM, Li Zhong wrote:
> Check the return value of e100_exec_cmd() which could return error code
> when execution fails.

Are you coming across this as a real bug or as something reported by 
static analysis? If the latter, I suggest checking the return value and 
reporting it as debug, however, not changing existing behavior. We don't 
have validation on this driver so there is limited ability to check for 
regressions and the code has been like this for a long time without 
reported issues.

Thanks,
Tony

> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> ---
>   drivers/net/ethernet/intel/e100.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
> index 11a884aa5082..3b84745376fe 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -1911,7 +1911,8 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
>   
>   	/* (Re)start RU if suspended or idle and RFA is non-NULL */
>   	if (rx->skb) {
> -		e100_exec_cmd(nic, ruc_start, rx->dma_addr);
> +		if (!e100_exec_cmd(nic, ruc_start, rx->dma_addr))
> +			return;
>   		nic->ru_running = RU_RUNNING;
>   	}
>   }