From be5466e4e9cc547f7792f04bee1f1d593d8072b8 Mon Sep 17 00:00:00 2001 From: Yahui Cao Date: Tue, 24 Dec 2019 12:13:13 +0800 Subject: [PATCH] net/iavf: fix virtual channel return In iavf_handle_virtchnl_msg(), it is not appropriate for _clear_cmd() to be used as a notification to foreground thread. So introduce _notify_cmd() to fix this error. In addition, since _notify_cmd() contains rte_wmb(), rte_compiler_barrier() is not necessary. Sending msg from VF to PF is mainly by calling iavf_execute_vf_cmd(), the whole virtchnl msg process is like, iavf_execute_vf_cmd() will call iavf_aq_send_msg_to_pf() to send msg and then polling the cmd done flag as "if (vf->pend_cmd == VIRTCHNL_OP_UNKNOWN)" When reply msg is returned by pf, iavf_handle_virtchnl_msg() in isr will read msg return value by "vf->cmd_retval = msg_ret" and immediately set the cmd done flag by calling _clear_cmd() to notify the iavf_execute_vf_cmd(). iavf_execute_vf_cmd() find the cmd done flag is set and then check whether command return value vf->cmd_retval is success or not. However _clear_cmd() also resets the vf->cmd_retval to success, overwriting the actual return value which is used for diagnosis. So iavf_execute_vf_cmd() will always find vf->cmd_retval is success and then return success. Fixes: 22b123a36d07 ("net/avf: initialize PMD") Cc: stable@dpdk.org Signed-off-by: Yahui Cao Acked-by: Qi Zhang Acked-by: Xiaolong Ye --- drivers/net/iavf/iavf.h | 11 +++++++++++ drivers/net/iavf/iavf_vchnl.c | 9 +++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index 77b7068939..9466808917 100644 --- a/drivers/net/iavf/iavf.h +++ b/drivers/net/iavf/iavf.h @@ -174,6 +174,17 @@ struct iavf_cmd_info { uint32_t out_size; /* buffer size for response */ }; +/* notify current command done. Only call in case execute + * _atomic_set_cmd successfully. + */ +static inline void +_notify_cmd(struct iavf_info *vf, uint32_t msg_ret) +{ + vf->cmd_retval = msg_ret; + rte_wmb(); + vf->pend_cmd = VIRTCHNL_OP_UNKNOWN; +} + /* clear current command. Only call in case execute * _atomic_set_cmd successfully. */ diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index bf87ab6115..303b515386 100644 --- a/drivers/net/iavf/iavf_vchnl.c +++ b/drivers/net/iavf/iavf_vchnl.c @@ -210,12 +210,9 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev) info.msg_len); } else { /* read message and it's expected one */ - if (msg_opc == vf->pend_cmd) { - vf->cmd_retval = msg_ret; - /* prevent compiler reordering */ - rte_compiler_barrier(); - _clear_cmd(vf); - } else + if (msg_opc == vf->pend_cmd) + _notify_cmd(vf, msg_ret); + else PMD_DRV_LOG(ERR, "command mismatch," "expect %u, get %u", vf->pend_cmd, msg_opc); -- 2.20.1