Re: [PATCH v2] net/i40e: fix PF notify issue when VF not up
From: Li, Xiaoyun <hidden>
Date: 2017-07-28 07:06:15
1. At first, I add vs state modification as follows, but it will show a warning when I check the patch.
The warning says that usually the else after return or break would not be executed.
if (i >= VFRESET_MAX_WAIT_CNT) {
PMD_DRV_LOG(ERR, "VF reset timeout");
return -ETIMEDOUT;
} else
vf->state = I40E_VF_ACTIVE;
2. Does it mean if vf state isn't inactive, the function will do "ret = i40e_aq_send_msg_to_vf();" and following statements ?
-----Original Message-----
From: Wu, Jingjing
Sent: Friday, July 28, 2017 14:53
To: Li, Xiaoyun <redacted>
Cc: dev@dpdk.org; stable@dpdk.org
Subject: RE: [PATCH v2] net/i40e: fix PF notify issue when VF not up
quoted hunk ↗ jump to hunk
-----Original Message----- From: Li, Xiaoyun Sent: Friday, July 28, 2017 7:40 PM To: Wu, Jingjing <redacted> Cc: dev@dpdk.org; Li, Xiaoyun <redacted>; stable@dpdk.org Subject: [PATCH v2] net/i40e: fix PF notify issue when VF not up This patch modifies PF notify error to warning when not starting up VF and modifies VF state to active when VF reset is completed. Fixes: 4861cde46116 ("i40e: new poll mode driver") Cc: stable@dpdk.org Signed-off-by: Xiaoyun Li <redacted> --- v2 changes: * add VF state modification when VF reset is done. --- drivers/net/i40e/i40e_pf.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c index b4cf57f..5d5d24a 100644 --- a/drivers/net/i40e/i40e_pf.c +++ b/drivers/net/i40e/i40e_pf.c@@ -152,22 +152,23 @@ i40e_pf_host_vf_reset(struct i40e_pf_vf *vf, bool do_hw_reset) val |= I40E_VPGEN_VFRTRIG_VFSWR_MASK; I40E_WRITE_REG(hw, I40E_VPGEN_VFRTRIG(vf_id), val); I40E_WRITE_FLUSH(hw); - } #define VFRESET_MAX_WAIT_CNT 100 - /* Wait until VF reset is done */ - for (i = 0; i < VFRESET_MAX_WAIT_CNT; i++) { - rte_delay_us(10); - val = I40E_READ_REG(hw, I40E_VPGEN_VFRSTAT(vf_id)); - if (val & I40E_VPGEN_VFRSTAT_VFRD_MASK) - break; - } + /* Wait until VF reset is done */ + for (i = 0; i < VFRESET_MAX_WAIT_CNT; i++) { + rte_delay_us(10); + val = I40E_READ_REG(hw, I40E_VPGEN_VFRSTAT(vf_id)); + if (val & I40E_VPGEN_VFRSTAT_VFRD_MASK) { + vf->state = I40E_VF_ACTIVE; + break; + } + } - if (i >= VFRESET_MAX_WAIT_CNT) { - PMD_DRV_LOG(ERR, "VF reset timeout"); - return -ETIMEDOUT; + if (i >= VFRESET_MAX_WAIT_CNT) { + PMD_DRV_LOG(ERR, "VF reset timeout"); + return -ETIMEDOUT; + }
It's much clearer to add "vf->state = I40E_VF_ACTIVE;" here but not above.
quoted hunk ↗ jump to hunk
} - /* This is not first time to do reset, do cleanup job first */ if (vf->vsi) { /* Disable queues */@@ -267,8 +268,12 @@ i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf, ret = i40e_aq_send_msg_to_vf(hw, abs_vf_id, opcode, retval, msg, msglen, NULL); if (ret) { - PMD_INIT_LOG(ERR, "Fail to send message to VF, err %u", - hw->aq.asq_last_status); + if (vf->state == I40E_VF_INACTIVE) + PMD_DRV_LOG(WARNING, "Warning! VF %u is inactive now!", + abs_vf_id);
How about just don't send msg to inactive VF but not print warning?
+ else + PMD_INIT_LOG(ERR, "Fail to send message to VF, err %u", + hw->aq.asq_last_status); } return ret; -- 2.7.4