* [dpdk-stable] [PATCH v4] net/i40e: fix modify the number of qps in VF [not found] <20200715072811.12592-1-alvinx.zhang@intel.com> @ 2020-07-16 1:52 ` alvinx.zhang 2020-07-16 4:24 ` [dpdk-stable] [PATCH v5] " alvinx.zhang 0 siblings, 1 reply; 8+ messages in thread From: alvinx.zhang @ 2020-07-16 1:52 UTC (permalink / raw) To: jia.guo; +Cc: beilei.xing, dev, Alvin Zhang, stable From: Alvin Zhang <alvinx.zhang@intel.com> If a VF request PF to allocate more number of queue pairs, the PF will free the queue pairs which have been allocated and reset the VF. So, VF should stop to work until all the process is done. This patch modify the process of the request queue pairs. To improve efficiency and eliminate code redundancy, the promiscuous ops were also updated. Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") Cc: stable@dpdk.org Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> --- V2: Update git log and modify codes according to comments. V3: All the code was refactored. --- drivers/net/i40e/i40e_ethdev_vf.c | 106 +++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 48 deletions(-) diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index eca716a..faff77e 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40evf_dev_close(struct rte_eth_dev *dev); -static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_check_vf_reset_done(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ -519,10 +520,19 @@ struct rte_i40evf_xstats_name_off { err = i40evf_execute_vf_cmd(dev, &args); - if (err) + if (err) { PMD_DRV_LOG(ERR, "fail to execute command " "CONFIG_PROMISCUOUS_MODE"); - return err; + + if (err == I40E_NOT_SUPPORTED) + return -ENOTSUP; + + return -EAGAIN; + } + + vf->promisc_unicast_enabled = enable_unicast; + vf->promisc_multicast_enabled = enable_multicast; + return 0; } static int @@ -1081,12 +1091,31 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, args.out_size = I40E_AQ_BUF_SZ; rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); + err = i40evf_execute_vf_cmd(dev, &args); - if (err) + if (err != I40E_SUCCESS) { + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, + i40evf_dev_alarm_handler, dev); PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES"); + return err; + } rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, i40evf_dev_alarm_handler, dev); + + /* + * The PF will issue a reset to the VF when change the number of + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, then + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch + * the moment that COMPLETE is set. So, for vf, we'll try to wait a + * long time. + */ + rte_delay_ms(100); + + err = i40evf_check_vf_reset_done(dev); + if (err) + PMD_DRV_LOG(ERR, "VF is still resetting"); + return err; } @@ -1514,7 +1543,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, hw->bus.device = pci_dev->addr.devid; hw->bus.func = pci_dev->addr.function; hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; - hw->adapter_stopped = 0; + hw->adapter_stopped = 1; hw->adapter_closed = 0; /* Pass the information to the rte_eth_dev_close() that it should also @@ -1610,10 +1639,26 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) ad->tx_vec_allowed = true; if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { - int ret = 0; + struct i40e_hw *hw; + int ret; + + /* For secondary processes, we don't configure queue pairs + * any further as primary has already done this work. + */ + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + PMD_DRV_LOG(ERR, + "For secondary processes, change queue pairs is not supported!"); + return -ENOTSUP; + } + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", vf->vsi_res->num_queue_pairs, num_queue_pairs); + if (hw->adapter_stopped == 0) { + PMD_DRV_LOG(ERR, "Device must be stopped first!"); + return -EBUSY; + } + ret = i40evf_request_queues(dev, num_queue_pairs); if (ret != 0) return ret; @@ -2182,68 +2227,32 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, true, vf->promisc_multicast_enabled); } static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, false, vf->promisc_multicast_enabled); } static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); - if (ret == 0) - vf->promisc_multicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, true); } static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); - if (ret == 0) - vf->promisc_multicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, false); } static int @@ -2365,8 +2374,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) * it is a workaround solution when work with kernel driver * and it is not the normal way */ - i40evf_dev_promiscuous_disable(dev); - i40evf_dev_allmulticast_disable(dev); + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) + i40evf_config_promisc(dev, false, false); + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); i40evf_reset_vf(dev); -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5] net/i40e: fix modify the number of qps in VF 2020-07-16 1:52 ` [dpdk-stable] [PATCH v4] net/i40e: fix modify the number of qps in VF alvinx.zhang @ 2020-07-16 4:24 ` alvinx.zhang 2020-07-16 4:57 ` [dpdk-stable] [dpdk-dev] " Yang, Qiming 2020-07-16 5:12 ` [dpdk-stable] " alvinx.zhang 0 siblings, 2 replies; 8+ messages in thread From: alvinx.zhang @ 2020-07-16 4:24 UTC (permalink / raw) To: jia.guo; +Cc: beilei.xing, dev, Alvin Zhang, stable From: Alvin Zhang <alvinx.zhang@intel.com> If a VF request PF to allocate more number of queue pairs, the PF will free the queue pairs which have been allocated and reset the VF. So, VF should stop to work until all the process is done. This patch modify the process of the request queue pairs. To improve efficiency and eliminate code redundancy, the promiscuous ops were also updated. Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") Cc: stable@dpdk.org Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> --- V2: Update git log and modify codes according to comments. V3: All the code was refactored. V4, V5: Modify codes according to comments. --- drivers/net/i40e/i40e_ethdev_vf.c | 104 ++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index eca716a..69cab8e 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40evf_dev_close(struct rte_eth_dev *dev); -static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_check_vf_reset_done(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ -519,10 +520,19 @@ struct rte_i40evf_xstats_name_off { err = i40evf_execute_vf_cmd(dev, &args); - if (err) + if (err) { PMD_DRV_LOG(ERR, "fail to execute command " "CONFIG_PROMISCUOUS_MODE"); - return err; + + if (err == I40E_NOT_SUPPORTED) + return -ENOTSUP; + + return -EAGAIN; + } + + vf->promisc_unicast_enabled = enable_unicast; + vf->promisc_multicast_enabled = enable_multicast; + return 0; } static int @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, args.out_size = I40E_AQ_BUF_SZ; rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); + err = i40evf_execute_vf_cmd(dev, &args); - if (err) + + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, i40evf_dev_alarm_handler, dev); + + if (err != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES"); + return err; + } + + /* The PF will issue a reset to the VF when change the number of + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, then + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch + * the moment that COMPLETE is set. So, for vf, we'll try to wait a + * long time. + */ + rte_delay_ms(100); + + err = i40evf_check_vf_reset_done(dev); + if (err) + PMD_DRV_LOG(ERR, "VF is still resetting"); - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, - i40evf_dev_alarm_handler, dev); return err; } @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, hw->bus.device = pci_dev->addr.devid; hw->bus.func = pci_dev->addr.function; hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; - hw->adapter_stopped = 0; + hw->adapter_stopped = 1; hw->adapter_closed = 0; /* Pass the information to the rte_eth_dev_close() that it should also @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) ad->tx_vec_allowed = true; if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { - int ret = 0; + struct i40e_hw *hw; + int ret; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + PMD_DRV_LOG(ERR, + "For secondary processes, change queue pairs is not supported!"); + return -ENOTSUP; + } + + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); + if (!hw->adapter_stopped) { + PMD_DRV_LOG(ERR, "Device must be stopped first!"); + return -EBUSY; + } PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", vf->vsi_res->num_queue_pairs, num_queue_pairs); @@ -2182,68 +2221,32 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, true, vf->promisc_multicast_enabled); } static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, false, vf->promisc_multicast_enabled); } static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); - if (ret == 0) - vf->promisc_multicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, true); } static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); - if (ret == 0) - vf->promisc_multicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, false); } static int @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) * it is a workaround solution when work with kernel driver * and it is not the normal way */ - i40evf_dev_promiscuous_disable(dev); - i40evf_dev_allmulticast_disable(dev); + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) + i40evf_config_promisc(dev, false, false); + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); i40evf_reset_vf(dev); -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5] net/i40e: fix modify the number of qps in VF 2020-07-16 4:24 ` [dpdk-stable] [PATCH v5] " alvinx.zhang @ 2020-07-16 4:57 ` Yang, Qiming 2020-07-16 5:13 ` Zhang, AlvinX 2020-07-16 5:12 ` [dpdk-stable] " alvinx.zhang 1 sibling, 1 reply; 8+ messages in thread From: Yang, Qiming @ 2020-07-16 4:57 UTC (permalink / raw) To: Zhang, AlvinX, Guo, Jia; +Cc: Xing, Beilei, dev, Zhang, AlvinX, stable > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of alvinx.zhang@intel.com > Sent: Thursday, July 16, 2020 12:24 > To: Guo, Jia <jia.guo@intel.com> > Cc: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org; Zhang, AlvinX > <alvinx.zhang@intel.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v5] net/i40e: fix modify the number of qps in VF Fix and modify are all verbs, I think you want to fix the queue pair modify process. > From: Alvin Zhang <alvinx.zhang@intel.com> > > If a VF request PF to allocate more number of queue pairs, the PF will free > the queue pairs which have been allocated and reset the VF. So, VF should > stop to work until all the process is done. This patch modify the process of > the request queue pairs. To improve efficiency and eliminate code > redundancy, the promiscuous ops were also updated. > > Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") > Cc: stable@dpdk.org > > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> > --- > > V2: Update git log and modify codes according to comments. > V3: All the code was refactored. > V4, V5: Modify codes according to comments. > --- > drivers/net/i40e/i40e_ethdev_vf.c | 104 ++++++++++++++++++++----------- > ------- > 1 file changed, 54 insertions(+), 50 deletions(-) > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c > b/drivers/net/i40e/i40e_ethdev_vf.c > index eca716a..69cab8e 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev > *dev, > uint16_t vlan_id, int on); > static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static > void i40evf_dev_close(struct rte_eth_dev *dev); -static int > i40evf_dev_reset(struct rte_eth_dev *dev); > +static int i40evf_dev_reset(struct rte_eth_dev *dev); static int > +i40evf_check_vf_reset_done(struct rte_eth_dev *dev); > static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static > int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); static int > i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ -519,10 > +520,19 @@ struct rte_i40evf_xstats_name_off { > > err = i40evf_execute_vf_cmd(dev, &args); > > - if (err) > + if (err) { > PMD_DRV_LOG(ERR, "fail to execute command " > "CONFIG_PROMISCUOUS_MODE"); > - return err; > + > + if (err == I40E_NOT_SUPPORTED) > + return -ENOTSUP; > + > + return -EAGAIN; > + } > + > + vf->promisc_unicast_enabled = enable_unicast; > + vf->promisc_multicast_enabled = enable_multicast; > + return 0; > } > > static int > @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct > rte_eth_dev *dev, > args.out_size = I40E_AQ_BUF_SZ; > > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > + > err = i40evf_execute_vf_cmd(dev, &args); > - if (err) > + > + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > i40evf_dev_alarm_handler, > +dev); > + > + if (err != I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "fail to execute command > OP_REQUEST_QUEUES"); > + return err; > + } > + > + /* The PF will issue a reset to the VF when change the number of > + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, > then > + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch > + * the moment that COMPLETE is set. So, for vf, we'll try to wait a > + * long time. > + */ > + rte_delay_ms(100); > + > + err = i40evf_check_vf_reset_done(dev); > + if (err) > + PMD_DRV_LOG(ERR, "VF is still resetting"); > > - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > - i40evf_dev_alarm_handler, dev); > return err; > } > > @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev > *dev, > hw->bus.device = pci_dev->addr.devid; > hw->bus.func = pci_dev->addr.function; > hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; > - hw->adapter_stopped = 0; > + hw->adapter_stopped = 1; > hw->adapter_closed = 0; > > /* Pass the information to the rte_eth_dev_close() that it should > also @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct > rte_pci_device *pci_dev) > ad->tx_vec_allowed = true; > > if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { > - int ret = 0; > + struct i40e_hw *hw; > + int ret; > + > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > + PMD_DRV_LOG(ERR, > + "For secondary processes, change queue > pairs is not supported!"); > + return -ENOTSUP; > + } > + > + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + if (!hw->adapter_stopped) { > + PMD_DRV_LOG(ERR, "Device must be stopped > first!"); > + return -EBUSY; > + } > > PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", > vf->vsi_res->num_queue_pairs, > num_queue_pairs); @@ -2182,68 +2221,32 @@ static int > eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); > - if (ret == 0) > - vf->promisc_unicast_enabled = TRUE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, true, > +vf->promisc_multicast_enabled); > } > > static int > i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); > - if (ret == 0) > - vf->promisc_unicast_enabled = FALSE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, false, > +vf->promisc_multicast_enabled); > } > > static int > i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); > - if (ret == 0) > - vf->promisc_multicast_enabled = TRUE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, > true); > } > > static int > i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); > - if (ret == 0) > - vf->promisc_multicast_enabled = FALSE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, > false); > } > > static int > @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct > rte_pci_device *pci_dev) > * it is a workaround solution when work with kernel driver > * and it is not the normal way > */ > - i40evf_dev_promiscuous_disable(dev); > - i40evf_dev_allmulticast_disable(dev); > + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) > + i40evf_config_promisc(dev, false, false); > + > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > > i40evf_reset_vf(dev); > -- > 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5] net/i40e: fix modify the number of qps in VF 2020-07-16 4:57 ` [dpdk-stable] [dpdk-dev] " Yang, Qiming @ 2020-07-16 5:13 ` Zhang, AlvinX 0 siblings, 0 replies; 8+ messages in thread From: Zhang, AlvinX @ 2020-07-16 5:13 UTC (permalink / raw) To: Yang, Qiming, Guo, Jia; +Cc: Xing, Beilei, dev, stable Thanks Qiming, I have updated it and resubmitted the patch. > -----Original Message----- > From: Yang, Qiming <qiming.yang@intel.com> > Sent: Thursday, July 16, 2020 12:58 PM > To: Zhang, AlvinX <alvinx.zhang@intel.com>; Guo, Jia <jia.guo@intel.com> > Cc: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org; Zhang, AlvinX > <alvinx.zhang@intel.com>; stable@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix modify the number of qps in VF > > > > > -----Original Message----- > > From: dev <dev-bounces@dpdk.org> On Behalf Of alvinx.zhang@intel.com > > Sent: Thursday, July 16, 2020 12:24 > > To: Guo, Jia <jia.guo@intel.com> > > Cc: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org; Zhang, AlvinX > > <alvinx.zhang@intel.com>; stable@dpdk.org > > Subject: [dpdk-dev] [PATCH v5] net/i40e: fix modify the number of qps > > in VF > > Fix and modify are all verbs, I think you want to fix the queue pair modify > process. > > > From: Alvin Zhang <alvinx.zhang@intel.com> > > > > If a VF request PF to allocate more number of queue pairs, the PF will > > free the queue pairs which have been allocated and reset the VF. So, > > VF should stop to work until all the process is done. This patch > > modify the process of the request queue pairs. To improve efficiency > > and eliminate code redundancy, the promiscuous ops were also updated. > > > > Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") > > Cc: stable@dpdk.org > > > > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> > > --- > > > > V2: Update git log and modify codes according to comments. > > V3: All the code was refactored. > > V4, V5: Modify codes according to comments. > > --- > > drivers/net/i40e/i40e_ethdev_vf.c | 104 > > ++++++++++++++++++++----------- > > ------- > > 1 file changed, 54 insertions(+), 50 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c > > b/drivers/net/i40e/i40e_ethdev_vf.c > > index eca716a..69cab8e 100644 > > --- a/drivers/net/i40e/i40e_ethdev_vf.c > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > > @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev > > *dev, > > uint16_t vlan_id, int on); > > static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); static void i40evf_dev_close(struct rte_eth_dev *dev); -static > > int i40evf_dev_reset(struct rte_eth_dev *dev); > > +static int i40evf_dev_reset(struct rte_eth_dev *dev); static int > > +i40evf_check_vf_reset_done(struct rte_eth_dev *dev); > > static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); > > static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); > > static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ > > -519,10 > > +520,19 @@ struct rte_i40evf_xstats_name_off { > > > > err = i40evf_execute_vf_cmd(dev, &args); > > > > - if (err) > > + if (err) { > > PMD_DRV_LOG(ERR, "fail to execute command " > > "CONFIG_PROMISCUOUS_MODE"); > > - return err; > > + > > + if (err == I40E_NOT_SUPPORTED) > > + return -ENOTSUP; > > + > > + return -EAGAIN; > > + } > > + > > + vf->promisc_unicast_enabled = enable_unicast; > > + vf->promisc_multicast_enabled = enable_multicast; > > + return 0; > > } > > > > static int > > @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct > > rte_eth_dev *dev, > > args.out_size = I40E_AQ_BUF_SZ; > > > > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > > + > > err = i40evf_execute_vf_cmd(dev, &args); > > - if (err) > > + > > + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > > i40evf_dev_alarm_handler, > > +dev); > > + > > + if (err != I40E_SUCCESS) { > > PMD_DRV_LOG(ERR, "fail to execute command > OP_REQUEST_QUEUES"); > > + return err; > > + } > > + > > + /* The PF will issue a reset to the VF when change the number of > > + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, > > then > > + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch > > + * the moment that COMPLETE is set. So, for vf, we'll try to wait a > > + * long time. > > + */ > > + rte_delay_ms(100); > > + > > + err = i40evf_check_vf_reset_done(dev); > > + if (err) > > + PMD_DRV_LOG(ERR, "VF is still resetting"); > > > > - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > > - i40evf_dev_alarm_handler, dev); > > return err; > > } > > > > @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct > > rte_eth_dev *dev, > > hw->bus.device = pci_dev->addr.devid; > > hw->bus.func = pci_dev->addr.function; > > hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; > > - hw->adapter_stopped = 0; > > + hw->adapter_stopped = 1; > > hw->adapter_closed = 0; > > > > /* Pass the information to the rte_eth_dev_close() that it should > > also @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct > > rte_pci_device *pci_dev) > > ad->tx_vec_allowed = true; > > > > if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { > > - int ret = 0; > > + struct i40e_hw *hw; > > + int ret; > > + > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > > + PMD_DRV_LOG(ERR, > > + "For secondary processes, change queue > > pairs is not supported!"); > > + return -ENOTSUP; > > + } > > + > > + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > + if (!hw->adapter_stopped) { > > + PMD_DRV_LOG(ERR, "Device must be stopped > > first!"); > > + return -EBUSY; > > + } > > > > PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", > > vf->vsi_res->num_queue_pairs, > > num_queue_pairs); @@ -2182,68 +2221,32 @@ static int > > eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > > i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { > > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > > >dev_private); > > - int ret; > > - > > - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); > > - if (ret == 0) > > - vf->promisc_unicast_enabled = TRUE; > > - else if (ret == I40E_NOT_SUPPORTED) > > - ret = -ENOTSUP; > > - else > > - ret = -EAGAIN; > > > > - return ret; > > + return i40evf_config_promisc(dev, true, > > +vf->promisc_multicast_enabled); > > } > > > > static int > > i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { > > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > > >dev_private); > > - int ret; > > - > > - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); > > - if (ret == 0) > > - vf->promisc_unicast_enabled = FALSE; > > - else if (ret == I40E_NOT_SUPPORTED) > > - ret = -ENOTSUP; > > - else > > - ret = -EAGAIN; > > > > - return ret; > > + return i40evf_config_promisc(dev, false, > > +vf->promisc_multicast_enabled); > > } > > > > static int > > i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { > > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > > >dev_private); > > - int ret; > > - > > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); > > - if (ret == 0) > > - vf->promisc_multicast_enabled = TRUE; > > - else if (ret == I40E_NOT_SUPPORTED) > > - ret = -ENOTSUP; > > - else > > - ret = -EAGAIN; > > > > - return ret; > > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, > > true); > > } > > > > static int > > i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { > > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data- > > >dev_private); > > - int ret; > > - > > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); > > - if (ret == 0) > > - vf->promisc_multicast_enabled = FALSE; > > - else if (ret == I40E_NOT_SUPPORTED) > > - ret = -ENOTSUP; > > - else > > - ret = -EAGAIN; > > > > - return ret; > > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, > > false); > > } > > > > static int > > @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct > > rte_pci_device *pci_dev) > > * it is a workaround solution when work with kernel driver > > * and it is not the normal way > > */ > > - i40evf_dev_promiscuous_disable(dev); > > - i40evf_dev_allmulticast_disable(dev); > > + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) > > + i40evf_config_promisc(dev, false, false); > > + > > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > > > > i40evf_reset_vf(dev); > > -- > > 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5] net/i40e: fix modify the number of qps in VF 2020-07-16 4:24 ` [dpdk-stable] [PATCH v5] " alvinx.zhang 2020-07-16 4:57 ` [dpdk-stable] [dpdk-dev] " Yang, Qiming @ 2020-07-16 5:12 ` alvinx.zhang 2020-07-16 6:33 ` [dpdk-stable] [PATCH v5] net/i40e: fix qps configuration " alvinx.zhang 1 sibling, 1 reply; 8+ messages in thread From: alvinx.zhang @ 2020-07-16 5:12 UTC (permalink / raw) To: jia.guo; +Cc: beilei.xing, dev, Alvin Zhang, stable From: Alvin Zhang <alvinx.zhang@intel.com> If a VF request PF to allocate more number of queue pairs, the PF will free the queue pairs which have been allocated and reset the VF. So, VF should stop to work until all the process is done. This patch modify the process of the request queue pairs. To improve efficiency and eliminate code redundancy, the promiscuous ops were also updated. Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") Cc: stable@dpdk.org Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> --- V2: Update git log and modify codes according to comments. V3: All the code was refactored. V4, V5: Modify codes according to comments. --- drivers/net/i40e/i40e_ethdev_vf.c | 104 ++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index eca716a..69cab8e 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40evf_dev_close(struct rte_eth_dev *dev); -static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_check_vf_reset_done(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ -519,10 +520,19 @@ struct rte_i40evf_xstats_name_off { err = i40evf_execute_vf_cmd(dev, &args); - if (err) + if (err) { PMD_DRV_LOG(ERR, "fail to execute command " "CONFIG_PROMISCUOUS_MODE"); - return err; + + if (err == I40E_NOT_SUPPORTED) + return -ENOTSUP; + + return -EAGAIN; + } + + vf->promisc_unicast_enabled = enable_unicast; + vf->promisc_multicast_enabled = enable_multicast; + return 0; } static int @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, args.out_size = I40E_AQ_BUF_SZ; rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); + err = i40evf_execute_vf_cmd(dev, &args); - if (err) + + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, i40evf_dev_alarm_handler, dev); + + if (err != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES"); + return err; + } + + /* The PF will issue a reset to the VF when change the number of + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, then + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch + * the moment that COMPLETE is set. So, for vf, we'll try to wait a + * long time. + */ + rte_delay_ms(100); + + err = i40evf_check_vf_reset_done(dev); + if (err) + PMD_DRV_LOG(ERR, "VF is still resetting"); - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, - i40evf_dev_alarm_handler, dev); return err; } @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, hw->bus.device = pci_dev->addr.devid; hw->bus.func = pci_dev->addr.function; hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; - hw->adapter_stopped = 0; + hw->adapter_stopped = 1; hw->adapter_closed = 0; /* Pass the information to the rte_eth_dev_close() that it should also @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) ad->tx_vec_allowed = true; if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { - int ret = 0; + struct i40e_hw *hw; + int ret; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + PMD_DRV_LOG(ERR, + "For secondary processes, change queue pairs is not supported!"); + return -ENOTSUP; + } + + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); + if (!hw->adapter_stopped) { + PMD_DRV_LOG(ERR, "Device must be stopped first!"); + return -EBUSY; + } PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", vf->vsi_res->num_queue_pairs, num_queue_pairs); @@ -2182,68 +2221,32 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, true, vf->promisc_multicast_enabled); } static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, false, vf->promisc_multicast_enabled); } static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); - if (ret == 0) - vf->promisc_multicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, true); } static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); - if (ret == 0) - vf->promisc_multicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, false); } static int @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) * it is a workaround solution when work with kernel driver * and it is not the normal way */ - i40evf_dev_promiscuous_disable(dev); - i40evf_dev_allmulticast_disable(dev); + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) + i40evf_config_promisc(dev, false, false); + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); i40evf_reset_vf(dev); -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5] net/i40e: fix qps configuration in VF 2020-07-16 5:12 ` [dpdk-stable] " alvinx.zhang @ 2020-07-16 6:33 ` alvinx.zhang 2020-07-16 7:30 ` Jeff Guo 0 siblings, 1 reply; 8+ messages in thread From: alvinx.zhang @ 2020-07-16 6:33 UTC (permalink / raw) To: jia.guo, qiming.yang; +Cc: dev, Alvin Zhang, stable From: Alvin Zhang <alvinx.zhang@intel.com> If a VF request PF to allocate more number of queue pairs, the PF will free the queue pairs which have been allocated and reset the VF. So, VF should stop to work until all the process is done. This patch modify the process of the request queue pairs. To improve efficiency and eliminate code redundancy, the promiscuous ops were also updated. Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") Cc: stable@dpdk.org Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> --- V2: Update git log and modify codes according to comments. V3: All the code was refactored. V4, V5: Modify codes according to comments. --- drivers/net/i40e/i40e_ethdev_vf.c | 104 ++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index eca716a..69cab8e 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40evf_dev_close(struct rte_eth_dev *dev); -static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_dev_reset(struct rte_eth_dev *dev); +static int i40evf_check_vf_reset_done(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); @@ -519,10 +520,19 @@ struct rte_i40evf_xstats_name_off { err = i40evf_execute_vf_cmd(dev, &args); - if (err) + if (err) { PMD_DRV_LOG(ERR, "fail to execute command " "CONFIG_PROMISCUOUS_MODE"); - return err; + + if (err == I40E_NOT_SUPPORTED) + return -ENOTSUP; + + return -EAGAIN; + } + + vf->promisc_unicast_enabled = enable_unicast; + vf->promisc_multicast_enabled = enable_multicast; + return 0; } static int @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, args.out_size = I40E_AQ_BUF_SZ; rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); + err = i40evf_execute_vf_cmd(dev, &args); - if (err) + + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, i40evf_dev_alarm_handler, dev); + + if (err != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES"); + return err; + } + + /* The PF will issue a reset to the VF when change the number of + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, then + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch + * the moment that COMPLETE is set. So, for vf, we'll try to wait a + * long time. + */ + rte_delay_ms(100); + + err = i40evf_check_vf_reset_done(dev); + if (err) + PMD_DRV_LOG(ERR, "VF is still resetting"); - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, - i40evf_dev_alarm_handler, dev); return err; } @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, hw->bus.device = pci_dev->addr.devid; hw->bus.func = pci_dev->addr.function; hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; - hw->adapter_stopped = 0; + hw->adapter_stopped = 1; hw->adapter_closed = 0; /* Pass the information to the rte_eth_dev_close() that it should also @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) ad->tx_vec_allowed = true; if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { - int ret = 0; + struct i40e_hw *hw; + int ret; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { + PMD_DRV_LOG(ERR, + "For secondary processes, change queue pairs is not supported!"); + return -ENOTSUP; + } + + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); + if (!hw->adapter_stopped) { + PMD_DRV_LOG(ERR, "Device must be stopped first!"); + return -EBUSY; + } PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", vf->vsi_res->num_queue_pairs, num_queue_pairs); @@ -2182,68 +2221,32 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, true, vf->promisc_multicast_enabled); } static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); - if (ret == 0) - vf->promisc_unicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, false, vf->promisc_multicast_enabled); } static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); - if (ret == 0) - vf->promisc_multicast_enabled = TRUE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, true); } static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); - int ret; - - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); - if (ret == 0) - vf->promisc_multicast_enabled = FALSE; - else if (ret == I40E_NOT_SUPPORTED) - ret = -ENOTSUP; - else - ret = -EAGAIN; - return ret; + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, false); } static int @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) * it is a workaround solution when work with kernel driver * and it is not the normal way */ - i40evf_dev_promiscuous_disable(dev); - i40evf_dev_allmulticast_disable(dev); + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) + i40evf_config_promisc(dev, false, false); + rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); i40evf_reset_vf(dev); -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [PATCH v5] net/i40e: fix qps configuration in VF 2020-07-16 6:33 ` [dpdk-stable] [PATCH v5] net/i40e: fix qps configuration " alvinx.zhang @ 2020-07-16 7:30 ` Jeff Guo 2020-07-16 9:14 ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z 0 siblings, 1 reply; 8+ messages in thread From: Jeff Guo @ 2020-07-16 7:30 UTC (permalink / raw) To: alvinx.zhang, qiming.yang; +Cc: dev, stable Acked-by: Jeff Guo <jia.guo@intel.com> On 7/16/2020 2:33 PM, alvinx.zhang@intel.com wrote: > From: Alvin Zhang <alvinx.zhang@intel.com> > > If a VF request PF to allocate more number of queue pairs, the PF will > free the queue pairs which have been allocated and reset the VF. So, > VF should stop to work until all the process is done. This patch modify > the process of the request queue pairs. To improve efficiency and > eliminate code redundancy, the promiscuous ops were also updated. > > Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") > Cc: stable@dpdk.org > > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> > --- > > V2: Update git log and modify codes according to comments. > V3: All the code was refactored. > V4, V5: Modify codes according to comments. > --- > drivers/net/i40e/i40e_ethdev_vf.c | 104 ++++++++++++++++++++------------------ > 1 file changed, 54 insertions(+), 50 deletions(-) > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c > index eca716a..69cab8e 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -91,7 +91,8 @@ static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, > uint16_t vlan_id, int on); > static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static void i40evf_dev_close(struct rte_eth_dev *dev); > -static int i40evf_dev_reset(struct rte_eth_dev *dev); > +static int i40evf_dev_reset(struct rte_eth_dev *dev); > +static int i40evf_check_vf_reset_done(struct rte_eth_dev *dev); > static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); > static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); > static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); > @@ -519,10 +520,19 @@ struct rte_i40evf_xstats_name_off { > > err = i40evf_execute_vf_cmd(dev, &args); > > - if (err) > + if (err) { > PMD_DRV_LOG(ERR, "fail to execute command " > "CONFIG_PROMISCUOUS_MODE"); > - return err; > + > + if (err == I40E_NOT_SUPPORTED) > + return -ENOTSUP; > + > + return -EAGAIN; > + } > + > + vf->promisc_unicast_enabled = enable_unicast; > + vf->promisc_multicast_enabled = enable_multicast; > + return 0; > } > > static int > @@ -1081,12 +1091,28 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, > args.out_size = I40E_AQ_BUF_SZ; > > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > + > err = i40evf_execute_vf_cmd(dev, &args); > - if (err) > + > + rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, i40evf_dev_alarm_handler, dev); > + > + if (err != I40E_SUCCESS) { > PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES"); > + return err; > + } > + > + /* The PF will issue a reset to the VF when change the number of > + * queues. The PF will set I40E_VFGEN_RSTAT to COMPLETE first, then > + * wait 10ms and set it to ACTIVE. In this duration, vf may not catch > + * the moment that COMPLETE is set. So, for vf, we'll try to wait a > + * long time. > + */ > + rte_delay_ms(100); > + > + err = i40evf_check_vf_reset_done(dev); > + if (err) > + PMD_DRV_LOG(ERR, "VF is still resetting"); > > - rte_eal_alarm_set(I40EVF_ALARM_INTERVAL, > - i40evf_dev_alarm_handler, dev); > return err; > } > > @@ -1514,7 +1540,7 @@ static int i40evf_dev_xstats_get(struct rte_eth_dev *dev, > hw->bus.device = pci_dev->addr.devid; > hw->bus.func = pci_dev->addr.function; > hw->hw_addr = (void *)pci_dev->mem_resource[0].addr; > - hw->adapter_stopped = 0; > + hw->adapter_stopped = 1; > hw->adapter_closed = 0; > > /* Pass the information to the rte_eth_dev_close() that it should also > @@ -1610,7 +1636,20 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > ad->tx_vec_allowed = true; > > if (num_queue_pairs > vf->vsi_res->num_queue_pairs) { > - int ret = 0; > + struct i40e_hw *hw; > + int ret; > + > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > + PMD_DRV_LOG(ERR, > + "For secondary processes, change queue pairs is not supported!"); > + return -ENOTSUP; > + } > + > + hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + if (!hw->adapter_stopped) { > + PMD_DRV_LOG(ERR, "Device must be stopped first!"); > + return -EBUSY; > + } > > PMD_DRV_LOG(INFO, "change queue pairs from %u to %u", > vf->vsi_res->num_queue_pairs, num_queue_pairs); > @@ -2182,68 +2221,32 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev) > { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, 1, vf->promisc_multicast_enabled); > - if (ret == 0) > - vf->promisc_unicast_enabled = TRUE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, true, vf->promisc_multicast_enabled); > } > > static int > i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) > { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, 0, vf->promisc_multicast_enabled); > - if (ret == 0) > - vf->promisc_unicast_enabled = FALSE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, false, vf->promisc_multicast_enabled); > } > > static int > i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) > { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); > - if (ret == 0) > - vf->promisc_multicast_enabled = TRUE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, true); > } > > static int > i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) > { > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); > - int ret; > - > - ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); > - if (ret == 0) > - vf->promisc_multicast_enabled = FALSE; > - else if (ret == I40E_NOT_SUPPORTED) > - ret = -ENOTSUP; > - else > - ret = -EAGAIN; > > - return ret; > + return i40evf_config_promisc(dev, vf->promisc_unicast_enabled, false); > } > > static int > @@ -2365,8 +2368,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > * it is a workaround solution when work with kernel driver > * and it is not the normal way > */ > - i40evf_dev_promiscuous_disable(dev); > - i40evf_dev_allmulticast_disable(dev); > + if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled) > + i40evf_config_promisc(dev, false, false); > + > rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev); > > i40evf_reset_vf(dev); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5] net/i40e: fix qps configuration in VF 2020-07-16 7:30 ` Jeff Guo @ 2020-07-16 9:14 ` Zhang, Qi Z 0 siblings, 0 replies; 8+ messages in thread From: Zhang, Qi Z @ 2020-07-16 9:14 UTC (permalink / raw) To: Guo, Jia, Zhang, AlvinX, Yang, Qiming; +Cc: dev, stable > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Jeff Guo > Sent: Thursday, July 16, 2020 3:31 PM > To: Zhang, AlvinX <alvinx.zhang@intel.com>; Yang, Qiming > <qiming.yang@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5] net/i40e: fix qps configuration in VF > > Acked-by: Jeff Guo <jia.guo@intel.com> > > On 7/16/2020 2:33 PM, alvinx.zhang@intel.com wrote: > > From: Alvin Zhang <alvinx.zhang@intel.com> > > > > If a VF request PF to allocate more number of queue pairs, the PF will > > free the queue pairs which have been allocated and reset the VF. So, > > VF should stop to work until all the process is done. This patch > > modify the process of the request queue pairs. To improve efficiency > > and eliminate code redundancy, the promiscuous ops were also updated. > > > > Fixes: c48eb308ed13 ("net/i40e: support VF request more queues") > > Cc: stable@dpdk.org > > > > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com> Applied to dpdk-next-net-intel. Thanks Qi ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-16 9:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200715072811.12592-1-alvinx.zhang@intel.com>
2020-07-16 1:52 ` [dpdk-stable] [PATCH v4] net/i40e: fix modify the number of qps in VF alvinx.zhang
2020-07-16 4:24 ` [dpdk-stable] [PATCH v5] " alvinx.zhang
2020-07-16 4:57 ` [dpdk-stable] [dpdk-dev] " Yang, Qiming
2020-07-16 5:13 ` Zhang, AlvinX
2020-07-16 5:12 ` [dpdk-stable] " alvinx.zhang
2020-07-16 6:33 ` [dpdk-stable] [PATCH v5] net/i40e: fix qps configuration " alvinx.zhang
2020-07-16 7:30 ` Jeff Guo
2020-07-16 9:14 ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z
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).