From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6B8ED440F2; Tue, 28 May 2024 10:11:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1DC7040A7A; Tue, 28 May 2024 10:10:44 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by mails.dpdk.org (Postfix) with ESMTP id 8E09E406A2 for ; Tue, 28 May 2024 10:10:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1716883841; x=1748419841; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=WN8ejD3AJkKlE7kFAHKqkn8wVhh7lNpZAjhrliLICKg=; b=JpJzCExNBweDM5/vFHC4u39IuqJHj/k+LOpMnx11/JPNkMs3btFu2wCY raWWsyQloaJ77+ipOh08rTwQjG8KHffZBCdsD/J+Veas2ZfquIBv0ItV+ zmCBOirgSp9C+H/zkMj3xBnKfb2RCk1sqiSDCt6F0EqKFwk9D23hoA9+1 MaDnrk3nodLLzVXi0A6Di5kCt4pDoC0sWVNMEwMj1bGPQcMz05a3CJVrs PsVY+jdUuAprmr+nvApkDZEC0vFlzccsHPb8l+L9B5qpOWigATXjlzq1Z zLYwxxigIFNXTh3RxjMHod+54mhhUjLdABjXYgLLNOUFxgfD/jnRFG5cM A==; X-CSE-ConnectionGUID: +Lmr/JSVQ8a31tkBcDecUg== X-CSE-MsgGUID: 1ZPwrUH3RRa6FQq69SkhpA== X-IronPort-AV: E=McAfee;i="6600,9927,11085"; a="16161931" X-IronPort-AV: E=Sophos;i="6.08,194,1712646000"; d="scan'208";a="16161931" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 May 2024 01:10:40 -0700 X-CSE-ConnectionGUID: NulMc0LmSPmvRjw23WhKpw== X-CSE-MsgGUID: c6OjhfSYTv+Cg37w4Woadw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,194,1712646000"; d="scan'208";a="66197696" Received: from unknown (HELO npf-hyd-clx-03..) ([10.145.170.182]) by fmviesa001.fm.intel.com with ESMTP; 28 May 2024 01:10:39 -0700 From: Soumyadeep Hore To: yuying.zhang@intel.com, jingjing.wu@intel.com Cc: dev@dpdk.org Subject: [PATCH 07/25] common/idpf: avoid defensive programming Date: Tue, 28 May 2024 07:28:37 +0000 Message-ID: <20240528072839.867100-8-soumyadeep.hore@intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240528072839.867100-1-soumyadeep.hore@intel.com> References: <20240528072839.867100-1-soumyadeep.hore@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Based on the upstream feedback, driver should not use any defensive programming strategy by checking for NULL pointers and other conditional checks unnecessarily in the code flow to fall back, instead fail and fix the bug in a proper way. Some of the checks checks are identified and removed/wrapped in this patch: - As the control queue is freed and deleted from the list after the idpf_ctlq_shutdown call, there is no need to have the ring_size check in idpf_ctlq_shutdown. - From the upstream perspective shared code is part of the Linux driver and it doesn't make sense to add zero 'len' and 'buf_size' check in idpf_ctlq_add as to start with, driver provides valid sizes, if not it is a bug. - Remove cq NULL and zero ring_size check wherever possible as the IDPF driver code flow does not pass any NULL cq pointer to the control queue callbacks. If it passes then it is a bug and should be fixed rather than checking for NULL pointer and falling back which is not the right way. Note: Most of the checks are wrapped with __KERNEL__ flag and will not have any impact on other shared code consumers other than the IDPF Linux driver as I am not confident if the same reasoning works for other components as well. Signed-off-by: Soumyadeep Hore --- drivers/common/idpf/base/idpf_controlq.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/common/idpf/base/idpf_controlq.c b/drivers/common/idpf/base/idpf_controlq.c index 0ba7281a45..4d31c6e6d8 100644 --- a/drivers/common/idpf/base/idpf_controlq.c +++ b/drivers/common/idpf/base/idpf_controlq.c @@ -98,9 +98,6 @@ static void idpf_ctlq_shutdown(struct idpf_hw *hw, struct idpf_ctlq_info *cq) { idpf_acquire_lock(&cq->cq_lock); - if (!cq->ring_size) - goto shutdown_sq_out; - #ifdef SIMICS_BUILD wr32(hw, cq->reg.head, 0); wr32(hw, cq->reg.tail, 0); @@ -115,7 +112,6 @@ static void idpf_ctlq_shutdown(struct idpf_hw *hw, struct idpf_ctlq_info *cq) /* Set ring_size to 0 to indicate uninitialized queue */ cq->ring_size = 0; -shutdown_sq_out: idpf_release_lock(&cq->cq_lock); idpf_destroy_lock(&cq->cq_lock); } @@ -661,9 +657,6 @@ int idpf_ctlq_recv(struct idpf_ctlq_info *cq, u16 *num_q_msg, int ret_code = 0; u16 i = 0; - if (!cq || !cq->ring_size) - return -ENOBUFS; - if (*num_q_msg == 0) return 0; else if (*num_q_msg > cq->ring_size) -- 2.43.0