* [dpdk-dev] [PATCH] net/i40evf: fix casting between structs
@ 2016-11-27 9:35 Jingjing Wu
2016-11-29 16:07 ` Ferruh Yigit
2016-11-30 2:02 ` [dpdk-dev] [PATCH v2] " Jingjing Wu
0 siblings, 2 replies; 5+ messages in thread
From: Jingjing Wu @ 2016-11-27 9:35 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
Casting from structs which lay out data in typed members
to structs which have flat memory buffers, will cause
problems if the alignment of the former isn't as expected.
This patch removes the casting between structs.
Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/i40e_ethdev_vf.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..53d7c87 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
struct i40e_arq_event_info info;
- struct i40e_virtchnl_msg *v_msg;
- uint16_t pending, opcode;
+ uint16_t pending, aq_opc;
+ enum i40e_virtchnl_ops msg_opc;
+ enum i40e_status_code msg_ret;
int ret;
info.buf_len = I40E_AQ_BUF_SZ;
@@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
return;
}
info.msg_buf = vf->aq_resp;
- v_msg = (struct i40e_virtchnl_msg *)&info.desc;
pending = 1;
while (pending) {
@@ -1357,32 +1357,35 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
"ret: %d", ret);
break;
}
- opcode = rte_le_to_cpu_16(info.desc.opcode);
-
- switch (opcode) {
+ aq_opc = rte_le_to_cpu_16(info.desc.opcode);
+ msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
+ info.desc.cookie_high);
+ msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
+ info.desc.cookie_low);
+ switch (aq_opc) {
case i40e_aqc_opc_send_msg_to_vf:
- if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
+ if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
/* process event*/
i40evf_handle_pf_event(dev, info.msg_buf,
info.msg_len);
else {
/* read message and it's expected one */
- if (v_msg->v_opcode == vf->pend_cmd) {
- vf->cmd_retval = v_msg->v_retval;
+ if (msg_opc == vf->pend_cmd) {
+ vf->cmd_retval = msg_ret;
/* prevent compiler reordering */
rte_compiler_barrier();
_clear_cmd(vf);
} else
PMD_DRV_LOG(ERR, "command mismatch,"
"expect %u, get %u",
- vf->pend_cmd, v_msg->v_opcode);
+ vf->pend_cmd, msg_ret);
PMD_DRV_LOG(DEBUG, "adminq response is received,"
- " opcode = %d\n", v_msg->v_opcode);
+ " opcode = %d\n", msg_ret);
}
break;
default:
PMD_DRV_LOG(ERR, "Request %u is not supported yet",
- opcode);
+ aq_opc);
break;
}
}
--
2.4.11
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40evf: fix casting between structs
2016-11-27 9:35 [dpdk-dev] [PATCH] net/i40evf: fix casting between structs Jingjing Wu
@ 2016-11-29 16:07 ` Ferruh Yigit
2016-11-30 0:35 ` Wu, Jingjing
2016-11-30 2:02 ` [dpdk-dev] [PATCH v2] " Jingjing Wu
1 sibling, 1 reply; 5+ messages in thread
From: Ferruh Yigit @ 2016-11-29 16:07 UTC (permalink / raw)
To: Jingjing Wu, dev; +Cc: helin.zhang
On 11/27/2016 9:35 AM, Jingjing Wu wrote:
> Casting from structs which lay out data in typed members
> to structs which have flat memory buffers, will cause
> problems if the alignment of the former isn't as expected.
> This patch removes the casting between structs.
>
> Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev_vf.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index aa306d6..53d7c87 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> struct i40e_arq_event_info info;
> - struct i40e_virtchnl_msg *v_msg;
> - uint16_t pending, opcode;
> + uint16_t pending, aq_opc;
> + enum i40e_virtchnl_ops msg_opc;
> + enum i40e_status_code msg_ret;
> int ret;
>
> info.buf_len = I40E_AQ_BUF_SZ;
> @@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> return;
> }
> info.msg_buf = vf->aq_resp;
> - v_msg = (struct i40e_virtchnl_msg *)&info.desc;
>
> pending = 1;
> while (pending) {
> @@ -1357,32 +1357,35 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> "ret: %d", ret);
> break;
> }
> - opcode = rte_le_to_cpu_16(info.desc.opcode);
> -
> - switch (opcode) {
> + aq_opc = rte_le_to_cpu_16(info.desc.opcode);
> + msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
> + info.desc.cookie_high);
> + msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
> + info.desc.cookie_low);
What do you think commenting cookie_high is opcode and cookie_low is
return_value?
> + switch (aq_opc) {
> case i40e_aqc_opc_send_msg_to_vf:
> - if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
> + if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
> /* process event*/
> i40evf_handle_pf_event(dev, info.msg_buf,
> info.msg_len);
> else {
> /* read message and it's expected one */
> - if (v_msg->v_opcode == vf->pend_cmd) {
> - vf->cmd_retval = v_msg->v_retval;
> + if (msg_opc == vf->pend_cmd) {
> + vf->cmd_retval = msg_ret;
> /* prevent compiler reordering */
> rte_compiler_barrier();
> _clear_cmd(vf);
> } else
> PMD_DRV_LOG(ERR, "command mismatch,"
> "expect %u, get %u",
> - vf->pend_cmd, v_msg->v_opcode);
> + vf->pend_cmd, msg_ret);
s/msg_ret/msg_opc/ ?
> PMD_DRV_LOG(DEBUG, "adminq response is received,"
> - " opcode = %d\n", v_msg->v_opcode);
> + " opcode = %d\n", msg_ret);
s/msg_ret/msg_opc/ ?
> }
> break;
> default:
> PMD_DRV_LOG(ERR, "Request %u is not supported yet",
> - opcode);
> + aq_opc);
> break;
> }
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40evf: fix casting between structs
2016-11-29 16:07 ` Ferruh Yigit
@ 2016-11-30 0:35 ` Wu, Jingjing
0 siblings, 0 replies; 5+ messages in thread
From: Wu, Jingjing @ 2016-11-30 0:35 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: Zhang, Helin
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, November 30, 2016 12:08 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: fix casting between structs
>
> On 11/27/2016 9:35 AM, Jingjing Wu wrote:
> > Casting from structs which lay out data in typed members to structs
> > which have flat memory buffers, will cause problems if the alignment
> > of the former isn't as expected.
> > This patch removes the casting between structs.
> >
> > Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev_vf.c | 27 +++++++++++++++------------
> > 1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index aa306d6..53d7c87 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
> > struct i40e_arq_event_info info;
> > - struct i40e_virtchnl_msg *v_msg;
> > - uint16_t pending, opcode;
> > + uint16_t pending, aq_opc;
> > + enum i40e_virtchnl_ops msg_opc;
> > + enum i40e_status_code msg_ret;
> > int ret;
> >
> > info.buf_len = I40E_AQ_BUF_SZ;
> > @@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > return;
> > }
> > info.msg_buf = vf->aq_resp;
> > - v_msg = (struct i40e_virtchnl_msg *)&info.desc;
> >
> > pending = 1;
> > while (pending) {
> > @@ -1357,32 +1357,35 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > "ret: %d", ret);
> > break;
> > }
> > - opcode = rte_le_to_cpu_16(info.desc.opcode);
> > -
> > - switch (opcode) {
> > + aq_opc = rte_le_to_cpu_16(info.desc.opcode);
> > + msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
> > + info.desc.cookie_high);
> > + msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
> > + info.desc.cookie_low);
>
> What do you think commenting cookie_high is opcode and cookie_low is
> return_value?
>
OK. Will add some comments.
> > + switch (aq_opc) {
> > case i40e_aqc_opc_send_msg_to_vf:
> > - if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
> > + if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
> > /* process event*/
> > i40evf_handle_pf_event(dev, info.msg_buf,
> > info.msg_len);
> > else {
> > /* read message and it's expected one */
> > - if (v_msg->v_opcode == vf->pend_cmd) {
> > - vf->cmd_retval = v_msg->v_retval;
> > + if (msg_opc == vf->pend_cmd) {
> > + vf->cmd_retval = msg_ret;
> > /* prevent compiler reordering */
> > rte_compiler_barrier();
> > _clear_cmd(vf);
> > } else
> > PMD_DRV_LOG(ERR, "command
> mismatch,"
> > "expect %u, get %u",
> > - vf->pend_cmd, v_msg-
> >v_opcode);
> > + vf->pend_cmd, msg_ret);
>
> s/msg_ret/msg_opc/ ?
Yes, should use msg_opc here. Thanks!
Will update!
Thanks
Jingjing
^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] [PATCH v2] net/i40evf: fix casting between structs
2016-11-27 9:35 [dpdk-dev] [PATCH] net/i40evf: fix casting between structs Jingjing Wu
2016-11-29 16:07 ` Ferruh Yigit
@ 2016-11-30 2:02 ` Jingjing Wu
2016-11-30 13:46 ` Ferruh Yigit
1 sibling, 1 reply; 5+ messages in thread
From: Jingjing Wu @ 2016-11-30 2:02 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
Casting from structs which lay out data in typed members
to structs which have flat memory buffers, will cause
problems if the alignment of the former isn't as expected.
This patch removes the casting between structs.
Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
v2 change:
correct the arguments in log.
add more comments.
drivers/net/i40e/i40e_ethdev_vf.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..5ec5264 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
struct i40e_arq_event_info info;
- struct i40e_virtchnl_msg *v_msg;
- uint16_t pending, opcode;
+ uint16_t pending, aq_opc;
+ enum i40e_virtchnl_ops msg_opc;
+ enum i40e_status_code msg_ret;
int ret;
info.buf_len = I40E_AQ_BUF_SZ;
@@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
return;
}
info.msg_buf = vf->aq_resp;
- v_msg = (struct i40e_virtchnl_msg *)&info.desc;
pending = 1;
while (pending) {
@@ -1357,32 +1357,39 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
"ret: %d", ret);
break;
}
- opcode = rte_le_to_cpu_16(info.desc.opcode);
-
- switch (opcode) {
+ aq_opc = rte_le_to_cpu_16(info.desc.opcode);
+ /* For the message sent from pf to vf, opcode is stored in
+ * cookie_high of struct i40e_aq_desc, while return error code
+ * are stored in cookie_low, Which is done by
+ * i40e_aq_send_msg_to_vf in PF driver.*/
+ msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
+ info.desc.cookie_high);
+ msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
+ info.desc.cookie_low);
+ switch (aq_opc) {
case i40e_aqc_opc_send_msg_to_vf:
- if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
+ if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
/* process event*/
i40evf_handle_pf_event(dev, info.msg_buf,
info.msg_len);
else {
/* read message and it's expected one */
- if (v_msg->v_opcode == vf->pend_cmd) {
- vf->cmd_retval = v_msg->v_retval;
+ if (msg_opc == vf->pend_cmd) {
+ vf->cmd_retval = msg_ret;
/* prevent compiler reordering */
rte_compiler_barrier();
_clear_cmd(vf);
} else
PMD_DRV_LOG(ERR, "command mismatch,"
"expect %u, get %u",
- vf->pend_cmd, v_msg->v_opcode);
+ vf->pend_cmd, msg_opc);
PMD_DRV_LOG(DEBUG, "adminq response is received,"
- " opcode = %d\n", v_msg->v_opcode);
+ " opcode = %d\n", msg_opc);
}
break;
default:
PMD_DRV_LOG(ERR, "Request %u is not supported yet",
- opcode);
+ aq_opc);
break;
}
}
--
2.4.11
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40evf: fix casting between structs
2016-11-30 2:02 ` [dpdk-dev] [PATCH v2] " Jingjing Wu
@ 2016-11-30 13:46 ` Ferruh Yigit
0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2016-11-30 13:46 UTC (permalink / raw)
To: Jingjing Wu, dev; +Cc: helin.zhang
On 11/30/2016 2:02 AM, Jingjing Wu wrote:
> Casting from structs which lay out data in typed members
> to structs which have flat memory buffers, will cause
> problems if the alignment of the former isn't as expected.
> This patch removes the casting between structs.
>
> Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-30 13:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-27 9:35 [dpdk-dev] [PATCH] net/i40evf: fix casting between structs Jingjing Wu
2016-11-29 16:07 ` Ferruh Yigit
2016-11-30 0:35 ` Wu, Jingjing
2016-11-30 2:02 ` [dpdk-dev] [PATCH v2] " Jingjing Wu
2016-11-30 13:46 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).