From 3043ce5011aa7075b32c80c79b9db96199938602 Mon Sep 17 00:00:00 2001 From: Helin Zhang Date: Tue, 9 Sep 2014 15:21:38 +0800 Subject: [PATCH] i40e/base: fix arq_event_info struct 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 Tested-by: HuilongX Xu --- 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 ++--- lib/librte_pmd_i40e/i40e_ethdev.c | 8 +++--- lib/librte_pmd_i40e/i40e_ethdev_vf.c | 10 +++---- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lib/librte_pmd_i40e/i40e/i40e_adminq.c b/lib/librte_pmd_i40e/i40e/i40e_adminq.c index 80da710775..e098ed694b 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 27f284391a..ea611bd9d8 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 ffd68a5ea6..ffaa777ead 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 f819f9aef0..f3215cf8b4 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); diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c index 26f179909e..52f01a529e 100644 --- a/lib/librte_pmd_i40e/i40e_ethdev.c +++ b/lib/librte_pmd_i40e/i40e_ethdev.c @@ -3337,8 +3337,8 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev) uint16_t pending, opcode; int ret; - info.msg_size = I40E_AQ_BUF_SZ; - info.msg_buf = rte_zmalloc("msg_buffer", I40E_AQ_BUF_SZ, 0); + info.buf_len = I40E_AQ_BUF_SZ; + info.msg_buf = rte_zmalloc("msg_buffer", info.buf_len, 0); if (!info.msg_buf) { PMD_DRV_LOG(ERR, "Failed to allocate mem"); return; @@ -3363,15 +3363,13 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev) rte_le_to_cpu_32(info.desc.cookie_high), rte_le_to_cpu_32(info.desc.cookie_low), info.msg_buf, - info.msg_size); + info.msg_len); break; default: PMD_DRV_LOG(ERR, "Request %u is not supported yet", opcode); break; } - /* Reset the buffer after processing one */ - info.msg_size = I40E_AQ_BUF_SZ; } rte_free(info.msg_buf); } diff --git a/lib/librte_pmd_i40e/i40e_ethdev_vf.c b/lib/librte_pmd_i40e/i40e_ethdev_vf.c index 809c1f0024..f7a932de4c 100644 --- a/lib/librte_pmd_i40e/i40e_ethdev_vf.c +++ b/lib/librte_pmd_i40e/i40e_ethdev_vf.c @@ -78,6 +78,7 @@ struct i40evf_arq_msg_info { enum i40e_virtchnl_ops ops; enum i40e_status_code result; + uint16_t buf_len; uint16_t msg_len; uint8_t *msg; }; @@ -225,8 +226,8 @@ i40evf_parse_pfmsg(struct i40e_vf *vf, } else { /* async reply msg on command issued by vf previously */ ret = I40EVF_MSG_CMD; - /* Actual buffer length read from PF */ - data->msg_len = event->msg_size; + /* Actual data length read from PF */ + data->msg_len = event->msg_len; } /* fill the ops and result to notify VF */ data->result = retval; @@ -247,7 +248,7 @@ i40evf_read_pfmsg(struct rte_eth_dev *dev, struct i40evf_arq_msg_info *data) int ret; enum i40evf_aq_result result = I40EVF_MSG_NON; - event.msg_size = data->msg_len; + event.buf_len = data->buf_len; event.msg_buf = data->msg; ret = i40e_clean_arq_element(hw, &event, NULL); /* Can't read any msg from adminQ */ @@ -281,7 +282,6 @@ i40evf_wait_cmd_done(struct rte_eth_dev *dev, /* Delay some time first */ rte_delay_ms(ASQ_DELAY_MS); ret = i40evf_read_pfmsg(dev, data); - if (ret == I40EVF_MSG_CMD) return 0; else if (ret == I40EVF_MSG_ERR) @@ -331,7 +331,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args) return -1; info.msg = args->out_buffer; - info.msg_len = args->out_size; + info.buf_len = args->out_size; info.ops = I40E_VIRTCHNL_OP_UNKNOWN; info.result = I40E_SUCCESS; -- 2.20.1