Re: [dpdk-dev] [PATCH 3/3] net/i40e: fix inadvertent override of vector RX allowance

["Ergin, Mesut A" <mesut.a.ergin@intel.com>]

Hi,

> > > > i40e_rx_vec_dev_conf_condition_check_default(struct rte_eth_dev *dev)
> > > >  	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
> > > >  		return -1;
> > > >
> > > > +	/* Should not override if vector was already disallowed */
> > >
> > > It is possible a device be reconfigured between dev_stop/dev_start,
> > > vector mode may fit for the new configure, so the old rx_vec_allowd
> > > should be ignored,
> > >
> >
> > i40e_dev_configure would reset rx_vec_allowed already. Am I missing another
> > reconfiguration path?
> 
> Look at below scenario,
> 
> 1. dev_configure (rx_vec_allowed is reset to true)
> 2. queue_setup (the ring size is not power of 2)
> 3. dev_start (vector will not be selected due to ring size, rx_vec_allowed set to
> false)
> 4. dev_stop
> 5. queue_setup  (this time, with power of 2 ring size)
> 6. dev_start (assume vector path should be selected, and rx_vec_allowed should
> be overwrite to true, but your patch will prevent it)
> 
> Also, I may not get the point of the gap you observed, would you share more
> detail scenario?
> 
> Regards
> Qi

It seems like queue setup should reset vector_allowed flag, too. Because 
i40e_dev_rx_queue_setup_runtime is already doing it, though covering
only half of the state space (i.e. device already started).

Do you think this patch plus that would be a preferred approach?

> 
> >
> > Mesut
> >
> > > Regards
> > > Qi
> > >
> > > > +	if (!ad->rx_vec_allowed)
> > > > +	return -1;
> > > > +
> > > >  	/**
> > > >  	 * Vector mode is allowed only when number of Rx queue
> > > >  	 * descriptor is power of 2.
> > > > --
> > > > 2.7.4