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

Message ID 20170824032602.107045-2-wei.zhao1@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Zhao1, Wei Aug. 24, 2017, 3:26 a.m. UTC
  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.
This patch enable mapping between different priorities (UP) and
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@intel.com>
---
 drivers/net/i40e/i40e_ethdev.h            |   6 +
 drivers/net/i40e/rte_pmd_i40e.c           | 287 ++++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           |  39 ++++
 drivers/net/i40e/rte_pmd_i40e_version.map |   7 +
 4 files changed, 339 insertions(+)
  

Comments

Ferruh Yigit Aug. 31, 2017, 4:18 p.m. UTC | #1
On 8/24/2017 4:26 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.
> This patch enable mapping between different priorities (UP) and
> 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@intel.com>

Hi Wei,

I wonder if this can be implemented using rte_flow, instead of PMD
specific API?

And if not, what is missing in rte_flow for this?

Thanks,
ferruh
  
Zhao1, Wei Sept. 1, 2017, 2:38 a.m. UTC | #2
HI,  Ferruh

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

> From: Yigit, Ferruh

> Sent: Friday, September 1, 2017 12:18 AM

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

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

> 

> On 8/24/2017 4:26 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.

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

> > 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@intel.com>

> 

> Hi Wei,

> 

> I wonder if this can be implemented using rte_flow, instead of PMD specific

> API?

> 

> And if not, what is missing in rte_flow for this?


Yes, at first I have plan to use rte_flow to implement this feature.
But there are many opposition for this, for example, this  feature is only support by i40e but not igb/ixgbe
From hardware capacity, not all NIC. So, private API is selected.

> 

> Thanks,

> ferruh
  
Chilikin, Andrey Sept. 5, 2017, 11:52 p.m. UTC | #3
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Wednesday, August 23, 2017 8:26 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
> 
> 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.
> This patch enable mapping between different priorities (UP) and
> 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@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.h            |   6 +
>  drivers/net/i40e/rte_pmd_i40e.c           | 287
> ++++++++++++++++++++++++++++++
>  drivers/net/i40e/rte_pmd_i40e.h           |  39 ++++
>  drivers/net/i40e/rte_pmd_i40e_version.map |   7 +
>  4 files changed, 339 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h
> b/drivers/net/i40e/i40e_ethdev.h
> index 48abc05..1d6e9b2 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -260,6 +260,12 @@ enum i40e_flxpld_layer_idx {
>  #define I40E_QOS_BW_WEIGHT_MIN 1
>  /* The max bandwidth weight is 127. */
>  #define I40E_QOS_BW_WEIGHT_MAX 127
> +/* The max queue region index is 7. */
> +#define I40E_TCREGION_MAX_INDEX 7
> +/* The max queue region userpriority is 7. */
> +#define I40E_REGION_USERPRIORITY_MAX_INDEX 7
> +/* The max pctype index is 63. */
> +#define I40E_REGION_PCTYPE_MAX_INDEX 63
> 
>  /**
>   * The overhead from MTU to max frame size.
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> b/drivers/net/i40e/rte_pmd_i40e.c
> index 950a0d6..5d1e1d4 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -2117,3 +2117,290 @@ int
> rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
> 
>  	return 0;
>  }
> +
> +static int
> +i40e_set_queue_region(struct i40e_hw *hw, struct i40e_pf *pf,
> +				struct rte_i40e_rss_region_conf *conf_ptr)
> +{
> +	uint32_t TCREGION, TC_OFFSET, TC_SIZE;
Please follow DPDK coding style guide and use only low case for variables names. 

> +	uint16_t TC_SIZE_TB[7] = {1, 2, 4, 8, 16, 32, 64};
> +	uint16_t i, index;
> +	uint16_t main_vsi_seid = pf->main_vsi_seid;

<snip>

> 
>  /**
> @@ -146,6 +159,19 @@ struct rte_pmd_i40e_ptype_mapping {
>  };
> 
>  /**
> + * Queue region information get from CLI.
> + */
> +struct rte_i40e_rss_region_conf {
> +	uint8_t region_id;
> +	uint8_t flowtype;
As this is internal i40e flow type (PCTYPE), it is better name it hw_flowtype. As an option - use RTE level sw_flowtype and map it to internal hw_flowtype using corresponding i40e API.

> +	uint8_t queue_startIndex;
> +	uint8_t queue_num;
> +	uint8_t UserPriority;
Low case for naming.

> +	uint8_t TrafficClasses;
> +	enum rte_pmd_i40e_queue_region_op  op;
> +};
> +
> +/**
>   * Notify VF when PF link status changes.
>   *
>   * @param port
> @@ -637,4 +663,17 @@ int rte_pmd_i40e_ptype_mapping_replace(uint8_t
> port,
>  				       uint8_t mask,
>  				       uint32_t pkt_type);
> 
> +/**
> + * Get RSS queue region info from CLI and do configuration for
> + * that port as the command otion type
> + *
> + * @param port
> + *    pointer to port identifier of the device
> + * @param conf_ptr
> + *    pointer to the struct that contain all the
> + *    region configuration parameters
> + */
> +int rte_pmd_i40e_queue_region_conf(uint8_t port,
> +		struct rte_i40e_rss_region_conf *conf_ptr);
> +
>  #endif /* _PMD_I40E_H_ */
> diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
> b/drivers/net/i40e/rte_pmd_i40e_version.map
> index 20cc980..77ac385 100644
> --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> @@ -45,3 +45,10 @@ DPDK_17.08 {
>  	rte_pmd_i40e_get_ddp_info;
> 
>  } DPDK_17.05;
> +
> +DPDK_17.11 {
> +	global:
> +
> +	rte_pmd_i40e_queue_region_conf;
> +
> +} DPDK_17.08;
> --
> 2.9.3
  
Zhao1, Wei Sept. 6, 2017, 7:21 a.m. UTC | #4
Hi,  Andrey

> -----Original Message-----
> From: Chilikin, Andrey
> Sent: Wednesday, September 6, 2017 7:52 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Wednesday, August 23, 2017 8:26 PM
> > To: dev@dpdk.org
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
> >
> > 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.
> > This patch enable mapping between different priorities (UP) and
> > 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@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.h            |   6 +
> >  drivers/net/i40e/rte_pmd_i40e.c           | 287
> > ++++++++++++++++++++++++++++++
> >  drivers/net/i40e/rte_pmd_i40e.h           |  39 ++++
> >  drivers/net/i40e/rte_pmd_i40e_version.map |   7 +
> >  4 files changed, 339 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.h
> > b/drivers/net/i40e/i40e_ethdev.h index 48abc05..1d6e9b2 100644
> > --- a/drivers/net/i40e/i40e_ethdev.h
> > +++ b/drivers/net/i40e/i40e_ethdev.h
> > @@ -260,6 +260,12 @@ enum i40e_flxpld_layer_idx {  #define
> > I40E_QOS_BW_WEIGHT_MIN 1
> >  /* The max bandwidth weight is 127. */  #define
> > I40E_QOS_BW_WEIGHT_MAX 127
> > +/* The max queue region index is 7. */ #define
> > +I40E_TCREGION_MAX_INDEX 7
> > +/* The max queue region userpriority is 7. */ #define
> > +I40E_REGION_USERPRIORITY_MAX_INDEX 7
> > +/* The max pctype index is 63. */
> > +#define I40E_REGION_PCTYPE_MAX_INDEX 63
> >
> >  /**
> >   * The overhead from MTU to max frame size.
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> > b/drivers/net/i40e/rte_pmd_i40e.c index 950a0d6..5d1e1d4 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.c
> > +++ b/drivers/net/i40e/rte_pmd_i40e.c
> > @@ -2117,3 +2117,290 @@ int
> > rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
> >
> >  	return 0;
> >  }
> > +
> > +static int
> > +i40e_set_queue_region(struct i40e_hw *hw, struct i40e_pf *pf,
> > +				struct rte_i40e_rss_region_conf *conf_ptr) {
> > +	uint32_t TCREGION, TC_OFFSET, TC_SIZE;
> Please follow DPDK coding style guide and use only low case for variables
> names.
> 
> > +	uint16_t TC_SIZE_TB[7] = {1, 2, 4, 8, 16, 32, 64};
> > +	uint16_t i, index;
> > +	uint16_t main_vsi_seid = pf->main_vsi_seid;
> 
> <snip>
> 
> >
> >  /**
> > @@ -146,6 +159,19 @@ struct rte_pmd_i40e_ptype_mapping {  };
> >
> >  /**
> > + * Queue region information get from CLI.
> > + */
> > +struct rte_i40e_rss_region_conf {
> > +	uint8_t region_id;
> > +	uint8_t flowtype;
> As this is internal i40e flow type (PCTYPE), it is better name it hw_flowtype.
> As an option - use RTE level sw_flowtype and map it to internal hw_flowtype
> using corresponding i40e API.

Ok, I will change that name to  " hw_flowtype " in v2 patch set.

> 
> > +	uint8_t queue_startIndex;
> > +	uint8_t queue_num;
> > +	uint8_t UserPriority;
> Low case for naming.

Ok, I will change code style in v2 patch set.

> 
> > +	uint8_t TrafficClasses;
> > +	enum rte_pmd_i40e_queue_region_op  op; };
> > +
> > +/**
> >   * Notify VF when PF link status changes.
> >   *
> >   * @param port
> > @@ -637,4 +663,17 @@ int
> rte_pmd_i40e_ptype_mapping_replace(uint8_t
> > port,
> >  				       uint8_t mask,
> >  				       uint32_t pkt_type);
> >
> > +/**
> > + * Get RSS queue region info from CLI and do configuration for
> > + * that port as the command otion type
> > + *
> > + * @param port
> > + *    pointer to port identifier of the device
> > + * @param conf_ptr
> > + *    pointer to the struct that contain all the
> > + *    region configuration parameters
> > + */
> > +int rte_pmd_i40e_queue_region_conf(uint8_t port,
> > +		struct rte_i40e_rss_region_conf *conf_ptr);
> > +
> >  #endif /* _PMD_I40E_H_ */
> > diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
> > b/drivers/net/i40e/rte_pmd_i40e_version.map
> > index 20cc980..77ac385 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> > +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> > @@ -45,3 +45,10 @@ DPDK_17.08 {
> >  	rte_pmd_i40e_get_ddp_info;
> >
> >  } DPDK_17.05;
> > +
> > +DPDK_17.11 {
> > +	global:
> > +
> > +	rte_pmd_i40e_queue_region_conf;
> > +
> > +} DPDK_17.08;
> > --
> > 2.9.3
  
Ferruh Yigit Sept. 6, 2017, 9:11 a.m. UTC | #5
On 9/1/2017 3:38 AM, Zhao1, Wei wrote:
> HI,  Ferruh
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, September 1, 2017 12:18 AM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
>>
>> On 8/24/2017 4:26 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.
>>> This patch enable mapping between different priorities (UP) and
>>> 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@intel.com>
>>
>> Hi Wei,
>>
>> I wonder if this can be implemented using rte_flow, instead of PMD specific
>> API?
>>
>> And if not, what is missing in rte_flow for this?
> 
> Yes, at first I have plan to use rte_flow to implement this feature.
> But there are many opposition for this, for example, this  feature is only support by i40e but not igb/ixgbe
> From hardware capacity, not all NIC. So, private API is selected.

I guess rte_flow headers needs to be updated for this support, how big
that update, what is missing in rte_flow for this?

Even this is only for i40e, rte_flow can be an option. I believe this
increases the usability of the feature for the driver.

> 
>>
>> Thanks,
>> ferruh
  
Ferruh Yigit Sept. 15, 2017, 11 a.m. UTC | #6
On 9/6/2017 10:11 AM, Ferruh Yigit wrote:
> On 9/1/2017 3:38 AM, Zhao1, Wei wrote:
>> HI,  Ferruh
>>
>>> -----Original Message-----
>>> From: Yigit, Ferruh
>>> Sent: Friday, September 1, 2017 12:18 AM
>>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
>>>
>>> On 8/24/2017 4:26 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.
>>>> This patch enable mapping between different priorities (UP) and
>>>> 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@intel.com>
>>>
>>> Hi Wei,
>>>
>>> I wonder if this can be implemented using rte_flow, instead of PMD specific
>>> API?
>>>
>>> And if not, what is missing in rte_flow for this?
>>
>> Yes, at first I have plan to use rte_flow to implement this feature.
>> But there are many opposition for this, for example, this  feature is only support by i40e but not igb/ixgbe
>> From hardware capacity, not all NIC. So, private API is selected.
> 
> I guess rte_flow headers needs to be updated for this support, how big
> that update, what is missing in rte_flow for this?

Hi Wei,

Would you mind answering this?

Thanks,
ferruh

> 
> Even this is only for i40e, rte_flow can be an option. I believe this
> increases the usability of the feature for the driver.
> 
>>
>>>
>>> Thanks,
>>> ferruh
>
  
Zhao1, Wei Sept. 18, 2017, 3:38 a.m. UTC | #7
Hi, Ferruh

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

> From: Yigit, Ferruh

> Sent: Wednesday, September 6, 2017 5:11 PM

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

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

> 

> On 9/1/2017 3:38 AM, Zhao1, Wei wrote:

> > HI,  Ferruh

> >

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

> >> From: Yigit, Ferruh

> >> Sent: Friday, September 1, 2017 12:18 AM

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

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

> >> flush

> >>

> >> On 8/24/2017 4:26 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.

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

> >>> 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@intel.com>

> >>

> >> Hi Wei,

> >>

> >> I wonder if this can be implemented using rte_flow, instead of PMD

> >> specific API?

> >>

> >> And if not, what is missing in rte_flow for this?

> >

> > Yes, at first I have plan to use rte_flow to implement this feature.

> > But there are many opposition for this, for example, this  feature is

> > only support by i40e but not igb/ixgbe From hardware capacity, not all NIC.

> So, private API is selected.

> 

> I guess rte_flow headers needs to be updated for this support, how big that

> update, what is missing in rte_flow for this?

> 


If we want rte_flow also to support this feature, although there will be many new CLI command need to be
Add to testpmd app, and some code change in ethdev, for example  add new member in enum rte_filter_type and so on.
But the biggest problem is to get recognize and support from expert and authority in DPDK community for this new change.
By now, rte_flow has no support for rss config application, but only filter configuration. But maybe this can be a work in next release.
  

> Even this is only for i40e, rte_flow can be an option. I believe this increases

> the usability of the feature for the driver.

> 

> >

> >>

> >> Thanks,

> >> ferruh
  
Zhao1, Wei Sept. 20, 2017, 3:20 a.m. UTC | #8
Hi, Ferruh

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

> From: Yigit, Ferruh

> Sent: Friday, September 15, 2017 7:01 PM

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

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

> 

> On 9/6/2017 10:11 AM, Ferruh Yigit wrote:

> > On 9/1/2017 3:38 AM, Zhao1, Wei wrote:

> >> HI,  Ferruh

> >>

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

> >>> From: Yigit, Ferruh

> >>> Sent: Friday, September 1, 2017 12:18 AM

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

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

> >>> flush

> >>>

> >>> On 8/24/2017 4:26 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.

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

> >>>> 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@intel.com>

> >>>

> >>> Hi Wei,

> >>>

> >>> I wonder if this can be implemented using rte_flow, instead of PMD

> >>> specific API?

> >>>

> >>> And if not, what is missing in rte_flow for this?

> >>

> >> Yes, at first I have plan to use rte_flow to implement this feature.

> >> But there are many opposition for this, for example, this  feature is

> >> only support by i40e but not igb/ixgbe From hardware capacity, not all NIC.

> So, private API is selected.

> >

> > I guess rte_flow headers needs to be updated for this support, how big

> > that update, what is missing in rte_flow for this?

> 

> Hi Wei,

> 

> Would you mind answering this?


If we want rte_flow also to support this feature,
although there will be many new CLI command need to be Add to testpmd app,
and some code change in ethdev, for example  add new member in enum rte_filter_type and so on.
But the biggest problem is to get recognize and support from expert and authority in DPDK community for this new change.
By now, rte_flow has no support for rss config application, but only filter configuration. But maybe this can be a work in next release.


> 

> Thanks,

> ferruh

> 

> >

> > Even this is only for i40e, rte_flow can be an option. I believe this

> > increases the usability of the feature for the driver.

> >

> >>

> >>>

> >>> Thanks,

> >>> ferruh

> >
  
Ferruh Yigit Sept. 20, 2017, 10:32 a.m. UTC | #9
On 9/20/2017 4:20 AM, Zhao1, Wei wrote:
> Hi, Ferruh
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, September 15, 2017 7:01 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and flush
>>
>> On 9/6/2017 10:11 AM, Ferruh Yigit wrote:
>>> On 9/1/2017 3:38 AM, Zhao1, Wei wrote:
>>>> HI,  Ferruh
>>>>
>>>>> -----Original Message-----
>>>>> From: Yigit, Ferruh
>>>>> Sent: Friday, September 1, 2017 12:18 AM
>>>>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>>>>> Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: queue region set and
>>>>> flush
>>>>>
>>>>> On 8/24/2017 4:26 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.
>>>>>> This patch enable mapping between different priorities (UP) and
>>>>>> 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@intel.com>
>>>>>
>>>>> Hi Wei,
>>>>>
>>>>> I wonder if this can be implemented using rte_flow, instead of PMD
>>>>> specific API?
>>>>>
>>>>> And if not, what is missing in rte_flow for this?
>>>>
>>>> Yes, at first I have plan to use rte_flow to implement this feature.
>>>> But there are many opposition for this, for example, this  feature is
>>>> only support by i40e but not igb/ixgbe From hardware capacity, not all NIC.
>> So, private API is selected.
>>>
>>> I guess rte_flow headers needs to be updated for this support, how big
>>> that update, what is missing in rte_flow for this?
>>
>> Hi Wei,
>>
>> Would you mind answering this?
> 
> If we want rte_flow also to support this feature,
> although there will be many new CLI command need to be Add to testpmd app,
> and some code change in ethdev, for example  add new member in enum rte_filter_type and so on.

new CLI part is side effect of the rte_flow, to demonstrate how to use
flow modes. I was more interested in how much change required in
rte_flow header.

> But the biggest problem is to get recognize and support from expert and authority in DPDK community for this new change.

Is this is referring previous push back for rte_filter_type update, I
think this can be evaluated case by case. As far as I remember
previously it is rejected in favor of improving rte_flow, this time
update is required to be able to use rte_flow, so I assume can be OK.
I believe this can be discussed out of this patch scope.

> By now, rte_flow has no support for rss config application, but only filter configuration. But maybe this can be a work in next release.

At least it is good that this effort can be re-used later for rte_flow,
I still believe this option worth investigating for further releases.

> 
> 
>>
>> Thanks,
>> ferruh
>>
>>>
>>> Even this is only for i40e, rte_flow can be an option. I believe this
>>> increases the usability of the feature for the driver.
>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> ferruh
>>>
>
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 48abc05..1d6e9b2 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -260,6 +260,12 @@  enum i40e_flxpld_layer_idx {
 #define I40E_QOS_BW_WEIGHT_MIN 1
 /* The max bandwidth weight is 127. */
 #define I40E_QOS_BW_WEIGHT_MAX 127
+/* The max queue region index is 7. */
+#define I40E_TCREGION_MAX_INDEX 7
+/* The max queue region userpriority is 7. */
+#define I40E_REGION_USERPRIORITY_MAX_INDEX 7
+/* The max pctype index is 63. */
+#define I40E_REGION_PCTYPE_MAX_INDEX 63
 
 /**
  * The overhead from MTU to max frame size.
diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index 950a0d6..5d1e1d4 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -2117,3 +2117,290 @@  int rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
 
 	return 0;
 }
+
+static int
+i40e_set_queue_region(struct i40e_hw *hw, struct i40e_pf *pf,
+				struct rte_i40e_rss_region_conf *conf_ptr)
+{
+	uint32_t TCREGION, TC_OFFSET, TC_SIZE;
+	uint16_t TC_SIZE_TB[7] = {1, 2, 4, 8, 16, 32, 64};
+	uint16_t i, index;
+	uint16_t main_vsi_seid = pf->main_vsi_seid;
+	struct i40e_vsi *main_vsi = pf->main_vsi;
+	uint32_t ret = -EINVAL;
+
+	index = conf_ptr->region_id >> 1;
+	for (i = 0; i < I40E_TCREGION_MAX_INDEX; i++)
+		if (conf_ptr->queue_num == TC_SIZE_TB[i])
+			break;
+
+	if (i == I40E_TCREGION_MAX_INDEX) {
+		printf("The region sizes should be any of the following "
+		"values: 1, 2, 4, 8, 16, 32, 64 as long as the "
+		"total number of queues do not exceed the VSI allocation");
+		return ret;
+	}
+
+	if (conf_ptr->region_id >= I40E_TCREGION_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the queue region max index is 7");
+		return ret;
+	}
+
+	if ((conf_ptr->queue_startIndex + conf_ptr->queue_num)
+					> main_vsi->nb_qps) {
+		PMD_INIT_LOG(ERR, "the queue index exceeds the VSI range");
+		return ret;
+	}
+
+	TCREGION = i40e_read_rx_ctl(hw,
+				I40E_VSIQF_TCREGION(index, main_vsi_seid));
+	if ((conf_ptr->region_id & 1) == 0) {
+		TC_OFFSET = (conf_ptr->queue_startIndex <<
+					I40E_VSIQF_TCREGION_TC_OFFSET_SHIFT) &
+					I40E_VSIQF_TCREGION_TC_OFFSET_MASK;
+		TC_SIZE = i << I40E_VSIQF_TCREGION_TC_SIZE_SHIFT;
+	} else {
+		TC_OFFSET = (conf_ptr->queue_startIndex <<
+					I40E_VSIQF_TCREGION_TC_OFFSET2_SHIFT) &
+					I40E_VSIQF_TCREGION_TC_OFFSET2_MASK;
+		TC_SIZE = i << I40E_VSIQF_TCREGION_TC_SIZE2_SHIFT;
+	}
+	TCREGION |= TC_OFFSET;
+	TCREGION |= TC_SIZE;
+	i40e_write_rx_ctl(hw, I40E_VSIQF_TCREGION(index, main_vsi_seid),
+						TCREGION);
+
+	return 0;
+}
+
+static int
+i40e_set_region_flowtype_pf(struct i40e_hw *hw,
+				struct rte_i40e_rss_region_conf *conf_ptr)
+{
+	uint32_t PFQF_HREGION;
+	uint32_t ret = -EINVAL;
+	uint16_t index;
+
+	if (conf_ptr->region_id > I40E_PFQF_HREGION_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the queue region max index is 7");
+		return ret;
+	}
+
+	if (conf_ptr->flowtype >= I40E_FILTER_PCTYPE_MAX) {
+		PMD_INIT_LOG(ERR, "the flowtype or PCTYPE max index is 63");
+		return ret;
+	}
+
+	index = conf_ptr->flowtype >> 3;
+	PFQF_HREGION = i40e_read_rx_ctl(hw, I40E_PFQF_HREGION(index));
+
+	if ((conf_ptr->flowtype & 0x7) == 0) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_0_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_0_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 1) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_1_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_1_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 2) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_2_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_2_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 3) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_3_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_3_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 4) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_4_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_4_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 5) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_5_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_5_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 6) {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_6_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_6_SHIFT;
+	} else {
+		PFQF_HREGION |= conf_ptr->region_id <<
+				I40E_PFQF_HREGION_REGION_7_SHIFT;
+		PFQF_HREGION |= 1 << I40E_PFQF_HREGION_OVERRIDE_ENA_7_SHIFT;
+	}
+
+	i40e_write_rx_ctl(hw, I40E_PFQF_HREGION(index), PFQF_HREGION);
+
+	return 0;
+}
+
+static int
+i40e_set_region_flowtype_vf(struct i40e_hw *hw,
+				struct rte_i40e_rss_region_conf *conf_ptr)
+{
+	uint32_t VFQF_HREGION;
+	uint32_t ret = -EINVAL;
+	uint16_t index;
+
+	if (conf_ptr->region_id > I40E_VFQF_HREGION_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the queue region max index is 7");
+		return ret;
+	}
+
+	if (conf_ptr->flowtype > I40E_REGION_PCTYPE_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the flowtype or PCTYPE max index is 63");
+		return ret;
+	}
+
+	index = conf_ptr->flowtype >> 3;
+	VFQF_HREGION = i40e_read_rx_ctl(hw, I40E_VFQF_HREGION(index));
+
+	if ((conf_ptr->flowtype & 0x7) == 0) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_0_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_0_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 1) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_1_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_1_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 2) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_2_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_2_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 3) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_3_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_3_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 4) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_4_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_4_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 5) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_5_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_5_SHIFT;
+	} else if ((conf_ptr->flowtype & 0x7) == 6) {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_6_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_6_SHIFT;
+	} else {
+		VFQF_HREGION |= conf_ptr->region_id <<
+				I40E_VFQF_HREGION_REGION_7_SHIFT;
+		VFQF_HREGION |= 1 << I40E_VFQF_HREGION_OVERRIDE_ENA_7_SHIFT;
+	}
+
+	i40e_write_rx_ctl(hw, I40E_VFQF_HREGION(index), VFQF_HREGION);
+
+	return 0;
+}
+
+static int
+i40e_set_up_tc(struct i40e_hw *hw,
+				struct rte_i40e_rss_region_conf *conf_ptr)
+{
+	uint32_t PRTDCB_RUP2TC;
+	uint32_t ret = -EINVAL;
+
+	if (conf_ptr->UserPriority > I40E_REGION_USERPRIORITY_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the queue region max index is 7");
+		return ret;
+	}
+
+	if (conf_ptr->TrafficClasses >= I40E_TCREGION_MAX_INDEX) {
+		PMD_INIT_LOG(ERR, "the TrafficClasses max index is 7");
+		return ret;
+	}
+
+	PRTDCB_RUP2TC  = I40E_READ_REG(hw, I40E_PRTDCB_RUP2TC);
+	if (conf_ptr->UserPriority == 0)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP0TC_SHIFT;
+	else if (conf_ptr->UserPriority == 1)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP1TC_SHIFT;
+	else if (conf_ptr->UserPriority == 2)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP2TC_SHIFT;
+	else if (conf_ptr->UserPriority == 3)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP3TC_SHIFT;
+	else if (conf_ptr->UserPriority == 4)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP4TC_SHIFT;
+	else if (conf_ptr->UserPriority == 5)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP5TC_SHIFT;
+	else if (conf_ptr->UserPriority == 6)
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP6TC_SHIFT;
+	else
+		PRTDCB_RUP2TC |= conf_ptr->TrafficClasses <<
+				I40E_PRTDCB_RUP2TC_UP7TC_SHIFT;
+
+	I40E_WRITE_REG(hw, I40E_PRTDCB_RUP2TC, PRTDCB_RUP2TC);
+
+	return 0;
+}
+
+static int
+i40e_flush_region_all_conf(struct i40e_hw *hw, struct i40e_pf *pf)
+{
+	uint32_t SET_ZERO = 0;
+	uint16_t i;
+	uint16_t main_vsi_seid = pf->main_vsi_seid;
+
+	I40E_WRITE_REG(hw, I40E_PRTDCB_RUP2TC, SET_ZERO);
+
+	for (i = 0; i < 4; i++)
+		i40e_write_rx_ctl(hw, I40E_VSIQF_TCREGION(i, main_vsi_seid),
+						SET_ZERO);
+
+	for (i = 0; i < I40E_PFQF_HREGION_MAX_INDEX; i++)
+		i40e_write_rx_ctl(hw, I40E_PFQF_HREGION(i), SET_ZERO);
+
+	for (i = 0; i < I40E_VFQF_HREGION_MAX_INDEX; i++)
+		i40e_write_rx_ctl(hw, I40E_VFQF_HREGION(i), SET_ZERO);
+
+	return 0;
+}
+
+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];
+	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;
+
+	if (!is_i40e_supported(dev))
+		return -ENOTSUP;
+
+	switch (op_type) {
+	case RTE_PMD_I40E_QUEUE_REGION_SET:
+		ret = i40e_set_queue_region(hw, pf, conf_ptr);
+		break;
+	case RTE_PMD_I40E_REGION_FLOWTYPE_PF_SET:
+		ret = i40e_set_region_flowtype_pf(hw, conf_ptr);
+		break;
+	case RTE_PMD_I40E_REGION_FLOWTYPE_VF_SET:
+		ret = i40e_set_region_flowtype_vf(hw, conf_ptr);
+		break;
+	case RTE_PMD_I40E_UP_TC_SET:
+		ret = i40e_set_up_tc(hw, conf_ptr);
+		break;
+
+	case RTE_PMD_I40E_REGION_ALL_FLUSH:
+		ret = i40e_flush_region_all_conf(hw, pf);
+		break;
+
+	default:
+		PMD_DRV_LOG(WARNING, "op type (%d) not supported",
+			    op_type);
+		ret = -EINVAL;
+		break;
+	}
+
+	I40E_WRITE_FLUSH(hw);
+
+	return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 356fa89..e85997a 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -91,6 +91,19 @@  enum rte_pmd_i40e_package_info {
 	RTE_PMD_I40E_PKG_INFO_MAX = 0xFFFFFFFF
 };
 
+/**
+ *  Option types of queue region.
+ */
+enum rte_pmd_i40e_queue_region_op {
+	RTE_PMD_I40E_REGION_UNDEFINED = 0,
+	RTE_PMD_I40E_QUEUE_REGION_SET,      /**< add queue region set*/
+	RTE_PMD_I40E_REGION_FLOWTYPE_PF_SET,   /**< add pf region pctype set */
+	RTE_PMD_I40E_REGION_FLOWTYPE_VF_SET,   /**< add vf region pctype set */
+	RTE_PMD_I40E_UP_TC_SET,   /**< add queue region pctype set */
+	RTE_PMD_I40E_REGION_ALL_FLUSH,   /**< flush all configuration */
+	RTE_PMD_I40E_QUEUE_REGION_OP_MAX
+};
+
 #define RTE_PMD_I40E_DDP_NAME_SIZE 32
 
 /**
@@ -146,6 +159,19 @@  struct rte_pmd_i40e_ptype_mapping {
 };
 
 /**
+ * Queue region information get from CLI.
+ */
+struct rte_i40e_rss_region_conf {
+	uint8_t region_id;
+	uint8_t flowtype;
+	uint8_t queue_startIndex;
+	uint8_t queue_num;
+	uint8_t UserPriority;
+	uint8_t TrafficClasses;
+	enum rte_pmd_i40e_queue_region_op  op;
+};
+
+/**
  * Notify VF when PF link status changes.
  *
  * @param port
@@ -637,4 +663,17 @@  int rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
 				       uint8_t mask,
 				       uint32_t pkt_type);
 
+/**
+ * Get RSS queue region info from CLI and do configuration for
+ * that port as the command otion type
+ *
+ * @param port
+ *    pointer to port identifier of the device
+ * @param conf_ptr
+ *    pointer to the struct that contain all the
+ *    region configuration parameters
+ */
+int rte_pmd_i40e_queue_region_conf(uint8_t port,
+		struct rte_i40e_rss_region_conf *conf_ptr);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 20cc980..77ac385 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -45,3 +45,10 @@  DPDK_17.08 {
 	rte_pmd_i40e_get_ddp_info;
 
 } DPDK_17.05;
+
+DPDK_17.11 {
+	global:
+
+	rte_pmd_i40e_queue_region_conf;
+
+} DPDK_17.08;