Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: support VF request more queues

["Yan, Zhirun" <zhirun.yan@intel.com>]

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