Thread (43 messages) 43 messages, 6 authors, 2016-03-22

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.h
b/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 --git
a/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 */ -static
uint8_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, enum
i40e_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);
 }

 /*
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help