[dpdk-dev] [PATCH v3 1/2] net/i40e: queue region set and flush

[Zhao1, Wei]

Hi,  Ferruh



> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Wednesday, September 20, 2017 6:36 PM

> To: Zhao1, Wei <wei.zhao1 at intel.com>; dev at dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: queue region set and flush

>

> On 9/15/2017 4:13 AM, Wei Zhao wrote:

> > This feature enable queue regions configuration for RSS in PF/VF, so

> > that different traffic classes or different packet classification

> > types can be separated to different queues in different queue

> > regions.This patch can set queue region range, it include queue number

> > in a region and the index of first queue.

>

> Is following correct:

> So instead of distributing packets to the multiple queues, this will distribute

> packets into queue reqions which may consists of multiple queues.

>

> If so, is there a way to control how packets distributed within same queue

> region to multiple queues?

>

> And is this feature only supported with RSS? Can it be part of RSS

> configuration instead of PMD specific API?

>

> > This patch enable mapping between different priorities (UP) and

>

> User priorities (UP)

>

> > different traffic classes.It also enable mapping between a region

> > index and a sepcific flowtype(PCTYPE).It also provide the solution of

> > flush all configuration about queue region the above described.

> >

> > Signed-off-by: Wei Zhao <wei.zhao1 at intel.com<mailto:wei.zhao1 at intel.com>>

> > ---

> >  drivers/net/i40e/i40e_ethdev.c            |  19 +-

> >  drivers/net/i40e/i40e_ethdev.h            |  30 ++

> >  drivers/net/i40e/rte_pmd_i40e.c           | 482

> ++++++++++++++++++++++++++++++

> >  drivers/net/i40e/rte_pmd_i40e.h           |  38 +++

> >  drivers/net/i40e/rte_pmd_i40e_version.map |   1 +

> >  5 files changed, 566 insertions(+), 4 deletions(-)

> >

>

> <...>

>

> > +static int

> > +i40e_vsi_update_queue_region_mapping(struct i40e_hw *hw,

> > +                                            struct i40e_pf *pf)

> > +{

> > +      uint16_t i;

> > +      struct i40e_vsi *vsi = pf->main_vsi;

> > +      uint16_t queue_offset, bsf, tc_index;

> > +      struct i40e_vsi_context ctxt;

> > +      struct i40e_aqc_vsi_properties_data *vsi_info;

> > +      struct i40e_queue_region_info *region_info =

> > +                                                      &pf->queue_region;

> > +      uint32_t ret = -EINVAL;

> > +

> > +      if (!region_info->queue_region_number) {



....................



> > +int rte_pmd_i40e_queue_region_conf(uint8_t port,

> > +                                      struct rte_i40e_rss_region_conf *conf_ptr) {

> > +      struct rte_eth_dev *dev = &rte_eth_devices[port];

>

> you need to verify port_id, since this is public API now. Please check other

> APIs in this file.



I have already "if (!is_i40e_supported(dev))" code in v3 in function  rte_pmd_i40e_queue_region_conf.

So, I do not know what is your meaning.







>

> > +      struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-

> >dev_private);

> > +      struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

> >dev_private);

> > +      enum rte_pmd_i40e_queue_region_op op_type = conf_ptr->op;

> > +      uint32_t ret;

>

> This should be signed variable, since you are using it for return and assigning

> negative values.

>

> > +

> > +      if (!is_i40e_supported(dev))

> > +                      return -ENOTSUP;

> > +

> > +      switch (op_type) {

> > +      case RTE_PMD_I40E_QUEUE_REGION_SET:

> > +                      ret = i40e_set_queue_region(pf, conf_ptr);

> > +                      break;

>