From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id A6300B3B4 for ; Tue, 9 Sep 2014 09:18:05 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 09 Sep 2014 00:16:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,491,1406617200"; d="scan'208";a="600087352" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by orsmga002.jf.intel.com with ESMTP; 09 Sep 2014 00:22:22 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id s897MKcp008895; Tue, 9 Sep 2014 15:22:20 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id s897MIUQ004536; Tue, 9 Sep 2014 15:22:20 +0800 Received: (from hzhan75@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id s897MIBU004525; Tue, 9 Sep 2014 15:22:18 +0800 From: Helin Zhang To: dev@dpdk.org Date: Tue, 9 Sep 2014 15:21:38 +0800 Message-Id: <1410247299-4365-15-git-send-email-helin.zhang@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1410247299-4365-1-git-send-email-helin.zhang@intel.com> References: <1410247299-4365-1-git-send-email-helin.zhang@intel.com> Subject: [dpdk-dev] [PATCH 14/15] i40e: fix and enhancement in arq_event_info struct X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Sep 2014 07:18:06 -0000 Overloading the 'msg_size' field in the 'arq_event_info' struct is a bad idea. It leads to bugs when the structure is used in a loop, since the input value (buffer size) is overwritten by the output value (actual message length). The fix introduces one more field of 'buf_len' for the buffer size, and renames the field of 'msg_size' to 'msg_len' for the real message size. Signed-off-by: Helin Zhang Reviewed-by: Chen Jing --- lib/librte_pmd_i40e/i40e/i40e_adminq.c | 33 ++++++++++++++++--------------- lib/librte_pmd_i40e/i40e/i40e_adminq.h | 3 ++- lib/librte_pmd_i40e/i40e/i40e_common.c | 8 ++++++-- lib/librte_pmd_i40e/i40e/i40e_prototype.h | 6 ++---- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/lib/librte_pmd_i40e/i40e/i40e_adminq.c b/lib/librte_pmd_i40e/i40e/i40e_adminq.c index 80da710..e098ed6 100644 --- a/lib/librte_pmd_i40e/i40e/i40e_adminq.c +++ b/lib/librte_pmd_i40e/i40e/i40e_adminq.c @@ -867,7 +867,8 @@ enum i40e_status_code i40e_asq_send_command(struct i40e_hw *hw, /* bump the tail */ i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQTX: desc and buffer:\n"); - i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc_on_ring, buff); + i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc_on_ring, + buff, buff_size); (hw->aq.asq.next_to_use)++; if (hw->aq.asq.next_to_use == hw->aq.asq.count) hw->aq.asq.next_to_use = 0; @@ -917,11 +918,9 @@ enum i40e_status_code i40e_asq_send_command(struct i40e_hw *hw, hw->aq.asq_last_status = (enum i40e_admin_queue_err)retval; } - if (LE16_TO_CPU(desc->datalen) == buff_size) { - i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, - "AQTX: desc and buffer writeback:\n"); - i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, buff); - } + i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, + "AQTX: desc and buffer writeback:\n"); + i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, buff, buff_size); /* update the error if time out occurred */ if ((!cmd_completed) && @@ -1000,6 +999,7 @@ enum i40e_status_code i40e_clean_arq_element(struct i40e_hw *hw, /* now clean the next descriptor */ desc = I40E_ADMINQ_DESC(hw->aq.arq, ntc); desc_idx = ntc; + flags = LE16_TO_CPU(desc->flags); if (flags & I40E_AQ_FLAG_ERR) { ret_code = I40E_ERR_ADMIN_QUEUE_ERROR; @@ -1009,19 +1009,20 @@ enum i40e_status_code i40e_clean_arq_element(struct i40e_hw *hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: Event received with error 0x%X.\n", hw->aq.arq_last_status); - } else { - i40e_memcpy(&e->desc, desc, sizeof(struct i40e_aq_desc), - I40E_DMA_TO_NONDMA); - datalen = LE16_TO_CPU(desc->datalen); - e->msg_size = min(datalen, e->msg_size); - if (e->msg_buf != NULL && (e->msg_size != 0)) - i40e_memcpy(e->msg_buf, - hw->aq.arq.r.arq_bi[desc_idx].va, - e->msg_size, I40E_DMA_TO_NONDMA); } + i40e_memcpy(&e->desc, desc, sizeof(struct i40e_aq_desc), + I40E_DMA_TO_NONDMA); + datalen = LE16_TO_CPU(desc->datalen); + e->msg_len = min(datalen, e->buf_len); + if (e->msg_buf != NULL && (e->msg_len != 0)) + i40e_memcpy(e->msg_buf, + hw->aq.arq.r.arq_bi[desc_idx].va, + e->msg_len, I40E_DMA_TO_NONDMA); + i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: desc and buffer:\n"); - i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf); + i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf, + hw->aq.arq_buf_size); /* Restore the original datalen and buffer address in the desc, * FW updates datalen to indicate the event message diff --git a/lib/librte_pmd_i40e/i40e/i40e_adminq.h b/lib/librte_pmd_i40e/i40e/i40e_adminq.h index 27f2843..ea611bd 100644 --- a/lib/librte_pmd_i40e/i40e/i40e_adminq.h +++ b/lib/librte_pmd_i40e/i40e/i40e_adminq.h @@ -83,7 +83,8 @@ struct i40e_asq_cmd_details { /* ARQ event information */ struct i40e_arq_event_info { struct i40e_aq_desc desc; - u16 msg_size; + u16 msg_len; + u16 buf_len; u8 *msg_buf; }; diff --git a/lib/librte_pmd_i40e/i40e/i40e_common.c b/lib/librte_pmd_i40e/i40e/i40e_common.c index ffd68a5..ffaa777 100644 --- a/lib/librte_pmd_i40e/i40e/i40e_common.c +++ b/lib/librte_pmd_i40e/i40e/i40e_common.c @@ -89,13 +89,15 @@ STATIC enum i40e_status_code i40e_set_mac_type(struct i40e_hw *hw) * @mask: debug mask * @desc: pointer to admin queue descriptor * @buffer: pointer to command buffer + * @buf_len: max length of buffer * * Dumps debug log about adminq command with descriptor contents. **/ void i40e_debug_aq(struct i40e_hw *hw, enum i40e_debug_mask mask, void *desc, - void *buffer) + void *buffer, u16 buf_len) { struct i40e_aq_desc *aq_desc = (struct i40e_aq_desc *)desc; + u16 len = LE16_TO_CPU(aq_desc->datalen); u8 *aq_buffer = (u8 *)buffer; u32 data[4]; u32 i = 0; @@ -119,7 +121,9 @@ void i40e_debug_aq(struct i40e_hw *hw, enum i40e_debug_mask mask, void *desc, if ((buffer != NULL) && (aq_desc->datalen != 0)) { i40e_memset(data, 0, sizeof(data), I40E_NONDMA_MEM); i40e_debug(hw, mask, "AQ CMD Buffer:\n"); - for (i = 0; i < LE16_TO_CPU(aq_desc->datalen); i++) { + if (buf_len < len) + len = buf_len; + for (i = 0; i < len; i++) { data[((i % 16) / 4)] |= ((u32)aq_buffer[i]) << (8 * (i % 4)); if ((i % 16) == 15) { diff --git a/lib/librte_pmd_i40e/i40e/i40e_prototype.h b/lib/librte_pmd_i40e/i40e/i40e_prototype.h index f819f9a..f3215cf 100644 --- a/lib/librte_pmd_i40e/i40e/i40e_prototype.h +++ b/lib/librte_pmd_i40e/i40e/i40e_prototype.h @@ -69,10 +69,8 @@ enum i40e_status_code i40e_asq_send_command(struct i40e_hw *hw, bool i40e_asq_done(struct i40e_hw *hw); /* debug function for adminq */ -void i40e_debug_aq(struct i40e_hw *hw, - enum i40e_debug_mask mask, - void *desc, - void *buffer); +void i40e_debug_aq(struct i40e_hw *hw, enum i40e_debug_mask mask, + void *desc, void *buffer, u16 buf_len); void i40e_idle_aq(struct i40e_hw *hw); void i40e_resume_aq(struct i40e_hw *hw); -- 1.8.1.4