Re: [PATCH v2 1/2] i40evf: allocate virtchnl cmd buffer for each vf
From: Zhang, Helin <hidden>
Date: 2016-02-22 07:26:39
quoted hunk ↗ jump to hunk
-----Original Message----- From: Wu, Jingjing Sent: Wednesday, January 27, 2016 9:50 AM To: dev@dpdk.org Cc: Wu, Jingjing; Zhang, Helin; Lu, Wenzhuo; Pei, Yulong Subject: [PATCH v2 1/2] i40evf: allocate virtchnl cmd buffer for each vf Currently, i40evf PMD uses a global static buffer to send virtchnl command to host driver. It is shared by multi VFs. This patch changed to allocate virtchnl cmd buffer for each VF. Signed-off-by: Jingjing Wu <redacted> --- drivers/net/i40e/i40e_ethdev.h | 2 + drivers/net/i40e/i40e_ethdev_vf.c | 181 +++++++++++++++------------------- ---- 2 files changed, 74 insertions(+), 109 deletions(-)diff --git a/drivers/net/i40e/i40e_ethdev.hb/drivers/net/i40e/i40e_ethdev.h index 1f9792b..93122ad 100644--- a/drivers/net/i40e/i40e_ethdev.h +++ b/drivers/net/i40e/i40e_ethdev.h@@ -494,7 +494,9 @@ struct i40e_vf { bool link_up; bool vf_reset; volatile uint32_t pend_cmd; /* pending command not finished yet */ + uint32_t cmd_retval; /* return value of the cmd response from PF */ u16 pend_msg; /* flags indicates events from pf not handled yet */ + uint8_t *aq_resp; /* buffer to store the adminq response from PF */ /* VSI info */ struct i40e_virtchnl_vf_resource *vf_res; /* All VSIs */ diff --gita/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index 14d2a50..64e6957 100644--- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c@@ -103,9 +103,6 @@ enum i40evf_aq_result { I40EVF_MSG_CMD, /* Read async command result */ }; -/* A share buffer to store the command result from PF driver */ -staticuint8_t cmd_result_buffer[I40E_AQ_BUF_SZ]; - static int i40evf_dev_configure(struct rte_eth_dev *dev); static int i40evf_dev_start(struct rte_eth_dev *dev); static void i40evf_dev_stop(struct rte_eth_dev *dev); @@ -237,31 +234,39 @@ i40evf_set_mac_type(struct i40e_hw *hw) } /* - * Parse admin queue message. - * - * return value: - * < 0: meet error - * 0: read sys msg - * > 0: read cmd result + * Read data in admin queue to get msg from pf driver */ static enum i40evf_aq_result -i40evf_parse_pfmsg(struct i40e_vf *vf, - struct i40e_arq_event_info *event, - struct i40evf_arq_msg_info *data) +i40evf_read_pfmsg(struct rte_eth_dev *dev, struct i40evf_arq_msg_info +*data) { - enum i40e_virtchnl_ops opcode = (enum i40e_virtchnl_ops)\ - rte_le_to_cpu_32(event->desc.cookie_high); - enum i40e_status_code retval = (enum i40e_status_code)\ - rte_le_to_cpu_32(event->desc.cookie_low); - enum i40evf_aq_result ret = I40EVF_MSG_CMD; + struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-quoted
dev_private);+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-quoted
dev_private);+ struct i40e_arq_event_info event; + enum i40e_virtchnl_ops opcode; + enum i40e_status_code retval; + int ret; + enum i40evf_aq_result result = I40EVF_MSG_NON; + 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 */ + if (ret) { + if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK) + result = I40EVF_MSG_NON;
No need to assign I40EVF_MSG_NON, as it was initialized at first.
quoted hunk ↗ jump to hunk
+ else + result = I40EVF_MSG_ERR; + return result; + } + + opcode = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(event.desc.cookie_high); + retval = (enum +i40e_status_code)rte_le_to_cpu_32(event.desc.cookie_low); /* pf sys event */ if (opcode == I40E_VIRTCHNL_OP_EVENT) { struct i40e_virtchnl_pf_event *vpe = - (struct i40e_virtchnl_pf_event *)event->msg_buf; + (struct i40e_virtchnl_pf_event *)event.msg_buf; - /* Initialize ret to sys event */ - ret = I40EVF_MSG_SYS; + result = I40EVF_MSG_SYS; switch (vpe->event) { case I40E_VIRTCHNL_EVENT_LINK_CHANGE: vf->link_up =@@ -286,74 +291,17 @@ i40evf_parse_pfmsg(struct i40e_vf *vf, } } else { /* async reply msg on command issued by vf previously */ - ret = I40EVF_MSG_CMD; + result = I40EVF_MSG_CMD; /* Actual data length read from PF */ - data->msg_len = event->msg_len; + data->msg_len = event.msg_len; } - /* fill the ops and result to notify VF */ + data->result = retval; data->ops = opcode; - return ret; -} - -/* - * Read data in admin queue to get msg from pf driver - */ -static enum i40evf_aq_result -i40evf_read_pfmsg(struct rte_eth_dev *dev, struct i40evf_arq_msg_info*data) -{ - struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-quoted
dev_private);- struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-quoted
dev_private);- struct i40e_arq_event_info event; - int ret; - enum i40evf_aq_result result = I40EVF_MSG_NON; - - 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 */ - if (ret) { - if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK) - result = I40EVF_MSG_NON; - else - result = I40EVF_MSG_ERR; - return result; - } - - /* Parse the event */ - result = i40evf_parse_pfmsg(vf, &event, data); - return result; } -/* - * Polling read until command result return from pf driver or meet error. - */ -static int -i40evf_wait_cmd_done(struct rte_eth_dev *dev, - struct i40evf_arq_msg_info *data) -{ - int i = 0; - enum i40evf_aq_result ret; - -#define MAX_TRY_TIMES 20 -#define ASQ_DELAY_MS 100 - do { - /* 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) - return -1; - - /* If don't read msg or read sys event, continue */ - } while(i++ < MAX_TRY_TIMES); - - return -1; -} - /** * clear current command. Only call in case execute * _atomic_set_cmd successfully.@@ -380,13 +328,18 @@ _atomic_set_cmd(struct i40e_vf *vf, enumi40e_virtchnl_ops ops) return !ret; } +#define MAX_TRY_TIMES 20 +#define ASQ_DELAY_MS 100 + static int i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args) { struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-quoted
dev_private);struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-quoted
dev_private);- int err = -1; struct i40evf_arq_msg_info info; + enum i40evf_aq_result ret; + int err = -1; + int i = 0; if (_atomic_set_cmd(vf, args->ops)) return -1;@@ -404,19 +357,22 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,struct vf_cmd_info *args) return err; } - err = i40evf_wait_cmd_done(dev, &info); - /* read message and it's expected one */ - if (!err && args->ops == info.ops) - _clear_cmd(vf); - else if (err) { - PMD_DRV_LOG(ERR, "Failed to read message from AdminQ"); - _clear_cmd(vf); - } - else if (args->ops != info.ops) - PMD_DRV_LOG(ERR, "command mismatch, expect %u, get %u", - args->ops, info.ops); + do { + /* Delay some time first */ + rte_delay_ms(ASQ_DELAY_MS);
It would be better to read first, but not delay first. In addition, the value of ASG_DELAY_MS shouldn't be 100ms. It is a bit too long, try 5ms or 10ms, or even shorter, and see if it is good. I remember that there is a complaint here for the interval. Note that if ASG_DELAY_MS changed, MAX_TRY_TIMES should also be modified.
+ ret = i40evf_read_pfmsg(dev, &info);
+ if (ret == I40EVF_MSG_CMD) {
+ err = 0;
+ break;
+ } else if (ret == I40EVF_MSG_ERR) {
+ err = -1;
+ break;
+ }
+ /* If don't read msg or read sys event, continue */
+ } while (i++ < MAX_TRY_TIMES);
+ _clear_cmd(vf);
- return (err | info.result);
+ return (err | vf->cmd_retval);
}
/*