[v3,1/2] net/i40e: support VF request more queues
Checks
Commit Message
Before this patch, VF gets a default number of queues from the PF.
This patch enables VF to request a different number. When VF configures
more queues, it will send VIRTCHNL_OP_REQUEST_QUEUES to PF to request
more queues, if success, PF will reset the VF.
User can run "port stop all", "port config port_id rxq/txq queue_num"
and "port start all" to reconfigure queue number.
Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
drivers/net/i40e/i40e_ethdev_vf.c | 56 ++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 4 deletions(-)
Comments
Hello, Zhirun,
On Thu, Dec 13, 2018 at 7:28 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
> @@ -1515,8 +1545,12 @@ RTE_PMD_REGISTER_KMOD_DEP(net_i40e_vf, "* igb_uio |
> vfio-pci");
> static int
> i40evf_dev_configure(struct rte_eth_dev *dev)
> {
> + struct i40e_vf *vf =
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> struct i40e_adapter *ad =
> I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> + uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
> + dev->data->nb_tx_queues);
> + int ret = 0;
>
> /* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
> * allocation or vector Rx preconditions we will reset it.
> @@ -1526,7 +1560,21 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
> ad->tx_simple_allowed = true;
> ad->tx_vec_allowed = true;
>
> - return i40evf_init_vlan(dev);
> + if (num_queue_pairs != vf->vsi_res->num_queue_pairs) {
> + PMD_DRV_LOG(INFO, "change queue pairs from %u to %u",
> + vf->vsi_res->num_queue_pairs,
> num_queue_pairs);
> + ret = i40evf_request_queues(dev, num_queue_pairs);
> + if (ret != 0)
> + return ret;
> +
> + ret = i40evf_dev_reset(dev);
> + if (ret != 0)
> + return ret;
> + }
> +
> + i40evf_init_vlan(dev);
> +
> + return ret;
> }
>
> static int
>
Did not look too much into this code, but I noticed that with this change,
you always return 0, whatever happened with i40evf_init_vlan().
I would have put the "int ret" declaration in the new block and left the
return i40evf_init_vlan(dev); as is.
Do you have a rationale not to do so ?
> -----Original Message-----
> From: Yan, Zhirun
> Sent: Thursday, December 13, 2018 10:05 PM
> To: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Yan, Zhirun <zhirun.yan@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>
> Subject: [PATCH v3 1/2] net/i40e: support VF request more queues
>
> Before this patch, VF gets a default number of queues from the PF.
> This patch enables VF to request a different number. When VF configures more
> queues, it will send VIRTCHNL_OP_REQUEST_QUEUES to PF to request more
> queues, if success, PF will reset the VF.
>
> User can run "port stop all", "port config port_id rxq/txq queue_num"
> and "port start all" to reconfigure queue number.
>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev_vf.c | 56 ++++++++++++++++++++++++++++---
> 1 file changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index cecedc91f..50e3f1f4f 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -343,6 +343,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct
> vf_cmd_info *args)
> err = 0;
> break;
> case VIRTCHNL_OP_VERSION:
> + case VIRTCHNL_OP_REQUEST_QUEUES:
> case VIRTCHNL_OP_GET_VF_RESOURCES:
> /* for init adminq commands, need to poll the response */
> err = -1;
> @@ -350,8 +351,15 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct
> vf_cmd_info *args)
> ret = i40evf_read_pfmsg(dev, &info);
> vf->cmd_retval = info.result;
> if (ret == I40EVF_MSG_CMD) {
> - err = 0;
> + if (info.ops != VIRTCHNL_OP_REQUEST_QUEUES)
> + err = 0;
> break;
> + } else if (ret == I40EVF_MSG_SYS) {
> + if (args->ops == VIRTCHNL_OP_REQUEST_QUEUES &&
OK, for VIRTCHNL_OP_REQUEST_QUEUES, we will ignore async reply (I40EVF_MSG_CMD) but only wait for system message (I40EVF_MSG_SYS)
I think it's better to create new branch for case VIRTCHNL_OP_REQUEST_QUEUES to make code more readable.
> + vf->vf_reset) {
> + err = 0;
> + break;
> + }
> } else if (ret == I40EVF_MSG_ERR)
> break;
> rte_delay_ms(ASQ_DELAY_MS);
Hi David,
The i40evf_init_vlan() always returns 0. So the change doesn’t affect it.
I agree with you. So I will put the "int ret" declaration in the new block in next version.
Thanks.
From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Thursday, December 13, 2018 4:26 PM
To: Yan, Zhirun <zhirun.yan@intel.com>
Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: support VF request more queues
Hello, Zhirun,
On Thu, Dec 13, 2018 at 7:28 AM Zhirun Yan <zhirun.yan@intel.com<mailto:zhirun.yan@intel.com>> wrote:
@@ -1515,8 +1545,12 @@ RTE_PMD_REGISTER_KMOD_DEP(net_i40e_vf, "* igb_uio | vfio-pci");
static int
i40evf_dev_configure(struct rte_eth_dev *dev)
{
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
struct i40e_adapter *ad =
I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
+ dev->data->nb_tx_queues);
+ int ret = 0;
/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
* allocation or vector Rx preconditions we will reset it.
@@ -1526,7 +1560,21 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
ad->tx_simple_allowed = true;
ad->tx_vec_allowed = true;
- return i40evf_init_vlan(dev);
+ if (num_queue_pairs != vf->vsi_res->num_queue_pairs) {
+ PMD_DRV_LOG(INFO, "change queue pairs from %u to %u",
+ vf->vsi_res->num_queue_pairs, num_queue_pairs);
+ ret = i40evf_request_queues(dev, num_queue_pairs);
+ if (ret != 0)
+ return ret;
+
+ ret = i40evf_dev_reset(dev);
+ if (ret != 0)
+ return ret;
+ }
+
+ i40evf_init_vlan(dev);
+
+ return ret;
}
static int
Did not look too much into this code, but I noticed that with this change, you always return 0, whatever happened with i40evf_init_vlan().
I would have put the "int ret" declaration in the new block and left the return i40evf_init_vlan(dev); as is.
Do you have a rationale not to do so ?
--
David Marchand
Hi Qi,
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Thursday, December 13, 2018 6:49 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>; dev@dpdk.org
> Cc: Wang, Haiyue <haiyue.wang@intel.com>
> Subject: RE: [PATCH v3 1/2] net/i40e: support VF request more queues
>
>
>
> > -----Original Message-----
> > From: Yan, Zhirun
> > Sent: Thursday, December 13, 2018 10:05 PM
> > To: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: Yan, Zhirun <zhirun.yan@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>
> > Subject: [PATCH v3 1/2] net/i40e: support VF request more queues
> >
> > Before this patch, VF gets a default number of queues from the PF.
> > This patch enables VF to request a different number. When VF
> > configures more queues, it will send VIRTCHNL_OP_REQUEST_QUEUES to PF
> > to request more queues, if success, PF will reset the VF.
> >
> > User can run "port stop all", "port config port_id rxq/txq queue_num"
> > and "port start all" to reconfigure queue number.
> >
> > Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev_vf.c | 56
> > ++++++++++++++++++++++++++++---
> > 1 file changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index cecedc91f..50e3f1f4f 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -343,6 +343,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,
> > struct vf_cmd_info *args)
> > err = 0;
> > break;
> > case VIRTCHNL_OP_VERSION:
> > + case VIRTCHNL_OP_REQUEST_QUEUES:
> > case VIRTCHNL_OP_GET_VF_RESOURCES:
> > /* for init adminq commands, need to poll the response */
> > err = -1;
> > @@ -350,8 +351,15 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev,
> > struct vf_cmd_info *args)
> > ret = i40evf_read_pfmsg(dev, &info);
> > vf->cmd_retval = info.result;
> > if (ret == I40EVF_MSG_CMD) {
> > - err = 0;
> > + if (info.ops !=
> VIRTCHNL_OP_REQUEST_QUEUES)
> > + err = 0;
> > break;
> > + } else if (ret == I40EVF_MSG_SYS) {
> > + if (args->ops ==
> VIRTCHNL_OP_REQUEST_QUEUES &&
>
> OK, for VIRTCHNL_OP_REQUEST_QUEUES, we will ignore async reply
> (I40EVF_MSG_CMD) but only wait for system message (I40EVF_MSG_SYS) I
> think it's better to create new branch for case
> VIRTCHNL_OP_REQUEST_QUEUES to make code more readable.
>
Yes, I will send a new version without it and create new patchs for handling msg from pf.
> > + vf->vf_reset) {
> > + err = 0;
> > + break;
> > + }
> > } else if (ret == I40EVF_MSG_ERR)
> > break;
> > rte_delay_ms(ASQ_DELAY_MS);
@@ -343,6 +343,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
err = 0;
break;
case VIRTCHNL_OP_VERSION:
+ case VIRTCHNL_OP_REQUEST_QUEUES:
case VIRTCHNL_OP_GET_VF_RESOURCES:
/* for init adminq commands, need to poll the response */
err = -1;
@@ -350,8 +351,15 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct vf_cmd_info *args)
ret = i40evf_read_pfmsg(dev, &info);
vf->cmd_retval = info.result;
if (ret == I40EVF_MSG_CMD) {
- err = 0;
+ if (info.ops != VIRTCHNL_OP_REQUEST_QUEUES)
+ err = 0;
break;
+ } else if (ret == I40EVF_MSG_SYS) {
+ if (args->ops == VIRTCHNL_OP_REQUEST_QUEUES &&
+ vf->vf_reset) {
+ err = 0;
+ break;
+ }
} else if (ret == I40EVF_MSG_ERR)
break;
rte_delay_ms(ASQ_DELAY_MS);
@@ -1012,6 +1020,28 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
return err;
}
+static int
+i40evf_request_queues(struct rte_eth_dev *dev, uint16_t num)
+{
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+ struct virtchnl_vf_res_request vfres;
+ struct vf_cmd_info args;
+ int err;
+
+ vfres.num_queue_pairs = num;
+
+ args.ops = VIRTCHNL_OP_REQUEST_QUEUES;
+ args.in_args = (u8 *)&vfres;
+ args.in_args_size = sizeof(vfres);
+ args.out_buffer = vf->aq_resp;
+ args.out_size = I40E_AQ_BUF_SZ;
+ err = i40evf_execute_vf_cmd(dev, &args);
+ if (err)
+ PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES");
+
+ return err;
+}
+
static int
i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
{
@@ -1515,8 +1545,12 @@ RTE_PMD_REGISTER_KMOD_DEP(net_i40e_vf, "* igb_uio | vfio-pci");
static int
i40evf_dev_configure(struct rte_eth_dev *dev)
{
+ struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
struct i40e_adapter *ad =
I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ uint16_t num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
+ dev->data->nb_tx_queues);
+ int ret = 0;
/* Initialize to TRUE. If any of Rx queues doesn't meet the bulk
* allocation or vector Rx preconditions we will reset it.
@@ -1526,7 +1560,21 @@ i40evf_dev_configure(struct rte_eth_dev *dev)
ad->tx_simple_allowed = true;
ad->tx_vec_allowed = true;
- return i40evf_init_vlan(dev);
+ if (num_queue_pairs != vf->vsi_res->num_queue_pairs) {
+ PMD_DRV_LOG(INFO, "change queue pairs from %u to %u",
+ vf->vsi_res->num_queue_pairs, num_queue_pairs);
+ ret = i40evf_request_queues(dev, num_queue_pairs);
+ if (ret != 0)
+ return ret;
+
+ ret = i40evf_dev_reset(dev);
+ if (ret != 0)
+ return ret;
+ }
+
+ i40evf_init_vlan(dev);
+
+ return ret;
}
static int
@@ -2137,8 +2185,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
- dev_info->max_rx_queues = vf->vsi_res->num_queue_pairs;
- dev_info->max_tx_queues = vf->vsi_res->num_queue_pairs;
+ dev_info->max_rx_queues = I40E_MAX_QP_NUM_PER_VF;
+ dev_info->max_tx_queues = I40E_MAX_QP_NUM_PER_VF;
dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) * sizeof(uint32_t);