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

[Ferruh Yigit]

On 9/26/2017 9:54 AM, Zhao1, Wei wrote:
> Hi,  Ferruh

Hi Wei,

>> -----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.

RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);

Normally we don't need to worry about this in PMD because abstraction
layer APIs do this check already. But since this is public API, user can
give any value as port_id parameter, better to verify it.

> 
>>
>> > +      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;
>>