On Mon, Jan 29, 2018 at 11:25 AM, Ananyev, Konstantin < konstantin.anan...@intel.com> wrote:
> > > -----Original Message----- > > From: Chas Williams [mailto:3ch...@gmail.com] > > Sent: Monday, January 29, 2018 3:21 PM > > To: dev@dpdk.org > > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Ananyev, Konstantin < > konstantin.anan...@intel.com>; Charles (Chas) Williams > > <ch...@att.com> > > Subject: [PATCH] net/ixgbe: fix reconfiguration of rx queues > > > > From: "Charles (Chas) Williams" <ch...@att.com> > > > > .dev_configure() may be called again after RX queues have been setup. > > This has the effect of clearing whatever setting the RX queues made for > > rx_bulk_alloc_allowed or rx_vec_allowed. Only reset this configuration > > is there aren't any currently allocated queues. > > > > Fixes: 01fa1d6215fa ("ixgbe: unify Rx setup") > > > > Signed-off-by: Chas Williams <ch...@att.com> > > --- > > drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ > ethdev.c > > index 37eb668..b39249a 100644 > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > @@ -2348,6 +2348,7 @@ ixgbe_dev_configure(struct rte_eth_dev *dev) > > struct ixgbe_adapter *adapter = > > (struct ixgbe_adapter *)dev->data->dev_private; > > int ret; > > + uint16_t i; > > > > PMD_INIT_FUNC_TRACE(); > > /* multipe queue mode checking */ > > @@ -2363,11 +2364,17 @@ ixgbe_dev_configure(struct rte_eth_dev *dev) > > > > /* > > * Initialize to TRUE. If any of Rx queues doesn't meet the bulk > > - * allocation or vector Rx preconditions we will reset it. > > + * allocation or vector Rx preconditions we will reset it. We > > + * can only do this is there aren't any existing RX queues. > > */ > > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > > + if (dev->data->rx_queues[i]) > > + goto out; > > + } > > I don't see why this is needed. > It surely should be possible to reconfigure device with different > number of queues. > Konstantin > Yes, you can add new queues but you shouldn't reset the bulk and vec settings that have already been chosen by the previously allocated queues. If those queues set rx_bulk_alloc_allowed to be false, then this is going to cause an issue with queue release later on. This breaks: rte_eth_dev_configure(..., 1, 1, ...); rx_queue_setup(1) [rx_queue_setup decides that it can't support rx_bulk_alloc_allowed] .. Later, you want to add some more queues, you call eth_eth_dev_configure(..., 2, 2, ...); rx_queue_setup(2) [rx_queue_setup hopefully makes the same choice as rxqid = 1?] ... Is one supposed to release all queues before calling rte_eth_dev_configure()? If that is true, it seems like the change_mtu examples I see are possibly wrong. As suggested in kenrel_nic_interface.rst: ret = rte_eth_dev_configure(port_id, 1, 1, &conf); if (ret < 0) { RTE_LOG(ERR, APP, "Fail to reconfigure port %d\n", port_id); return ret; } /* Restart specific port */ ret = rte_eth_dev_start(port_id); if (ret < 0) { RTE_LOG(ERR, APP, "Fail to restart port %d\n", port_id); return ret; } This is will obviously reset the rx_bulk_alloc_allowed and not reallocated the RX queues. > > > adapter->rx_bulk_alloc_allowed = true; > > adapter->rx_vec_allowed = true; > > > > +out: > > return 0; > > } > > > > @@ -4959,6 +4966,7 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev) > > struct rte_eth_conf *conf = &dev->data->dev_conf; > > struct ixgbe_adapter *adapter = > > (struct ixgbe_adapter *)dev->data->dev_private; > > + uint16_t i; > > > > PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d", > > dev->data->port_id); > > @@ -4981,11 +4989,17 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev) > > > > /* > > * Initialize to TRUE. If any of Rx queues doesn't meet the bulk > > - * allocation or vector Rx preconditions we will reset it. > > + * allocation or vector Rx preconditions we will reset it. We > > + * can only do this is there aren't any existing RX queues. > > */ > > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > > + if (dev->data->rx_queues[i]) > > + goto out; > > + } > > adapter->rx_bulk_alloc_allowed = true; > > adapter->rx_vec_allowed = true; > > > > +out: > > return 0; > > } > > > > -- > > 2.9.5 > >
