[V5,2/2] ethdev: change data type in TC rxq and TC txq

Message ID 1601176596-29900-3-git-send-email-humin29@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series change data type in TC queue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

humin (Q) Sept. 27, 2020, 3:16 a.m. UTC
  From: Huisong Li <lihuisong@huawei.com>

Currently, base and nb_queue in the tc_rxq and tc_txq information
of queue and TC mapping on both TX and RX paths are uint8_t.
However, these data will be truncated when queue number under a TC
is greater than 256. So it is necessary for base and nb_queue to
change from uint8_t to uint16_t.

Fixes: 01eb53eefeb40e8 ("ethdev: rename folder to library name")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
---
V4 -> V5:
add release notes updated.

---
 doc/guides/rel_notes/release_20_11.rst | 5 +++++
 lib/librte_ethdev/rte_ethdev.h         | 8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)
  

Comments

Ferruh Yigit Sept. 28, 2020, 9:04 a.m. UTC | #1
On 9/27/2020 4:16 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> Currently, base and nb_queue in the tc_rxq and tc_txq information
> of queue and TC mapping on both TX and RX paths are uint8_t.
> However, these data will be truncated when queue number under a TC
> is greater than 256. So it is necessary for base and nb_queue to
> change from uint8_t to uint16_t.
> 
> Fixes: 01eb53eefeb40e8 ("ethdev: rename folder to library name")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
> V4 -> V5:
> add release notes updated.
> 
> ---
>   doc/guides/rel_notes/release_20_11.rst | 5 +++++
>   lib/librte_ethdev/rte_ethdev.h         | 8 ++++----
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
> index 1ef6f0e..ad14fd7 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -183,6 +183,11 @@ API Changes
>     function will be changed from ``uint8_t`` to ``uint16_t``, which supports for
>     using 256 or more than 256 queues and displaying all statistics of rx/tx queue.
>   
> +* ethdev: Modified field type of base and nb_queue in struct
> +  ``rte_eth_dcb_tc_queue_mapping`` from ``uint8_t`` to ``uint16_t``.
> +  As the data of uint8_t will be truncated when queue number under
> +  a TC is greater than 256.
> +
>   
>   ABI Changes
>   -----------
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index ff3a616..2270460 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1522,13 +1522,13 @@ struct rte_eth_xstat_name {
>   struct rte_eth_dcb_tc_queue_mapping {
>   	/** rx queues assigned to tc per Pool */
>   	struct {
> -		uint8_t base;
> -		uint8_t nb_queue;
> +		uint16_t base;
> +		uint16_t nb_queue;
>   	} tc_rxq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>   	/** rx queues assigned to tc per Pool */
>   	struct {
> -		uint8_t base;
> -		uint8_t nb_queue;
> +		uint16_t base;
> +		uint16_t nb_queue;
>   	} tc_txq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>   };
>   
> 

cc'ed tech-board,

The patch breaks the ethdev ABI without a deprecation notice from previous 
release(s).

It is increasing the storage size of the fields to support more than 255 queues.

Since the ethdev library already heavily breaks the ABI this release, I am for 
getting this patch, instead of waiting for one more year for the update.

Can you please review the patch, is there any objection to proceed with it?
  
Thomas Monjalon Sept. 28, 2020, 9:21 a.m. UTC | #2
28/09/2020 11:04, Ferruh Yigit:
> On 9/27/2020 4:16 AM, Min Hu (Connor) wrote:
> > From: Huisong Li <lihuisong@huawei.com>
> > 
> > Currently, base and nb_queue in the tc_rxq and tc_txq information
> > of queue and TC mapping on both TX and RX paths are uint8_t.
> > However, these data will be truncated when queue number under a TC
> > is greater than 256. So it is necessary for base and nb_queue to
> > change from uint8_t to uint16_t.
[...]
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> >   struct rte_eth_dcb_tc_queue_mapping {
> >   	/** rx queues assigned to tc per Pool */
> >   	struct {
> > -		uint8_t base;
> > -		uint8_t nb_queue;
> > +		uint16_t base;
> > +		uint16_t nb_queue;
> >   	} tc_rxq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
> >   	/** rx queues assigned to tc per Pool */
> >   	struct {
> > -		uint8_t base;
> > -		uint8_t nb_queue;
> > +		uint16_t base;
> > +		uint16_t nb_queue;
> >   	} tc_txq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
> >   };
> >   
> > 
> 
> cc'ed tech-board,
> 
> The patch breaks the ethdev ABI without a deprecation notice from previous 
> release(s).
> 
> It is increasing the storage size of the fields to support more than 255 queues.

Yes queues are in 16-bit range.

> Since the ethdev library already heavily breaks the ABI this release, I am for 
> getting this patch, instead of waiting for one more year for the update.
> 
> Can you please review the patch, is there any objection to proceed with it?

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  
Ferruh Yigit Oct. 5, 2020, 12:26 p.m. UTC | #3
On 9/28/2020 10:21 AM, Thomas Monjalon wrote:
> 28/09/2020 11:04, Ferruh Yigit:
>> On 9/27/2020 4:16 AM, Min Hu (Connor) wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Currently, base and nb_queue in the tc_rxq and tc_txq information
>>> of queue and TC mapping on both TX and RX paths are uint8_t.
>>> However, these data will be truncated when queue number under a TC
>>> is greater than 256. So it is necessary for base and nb_queue to
>>> change from uint8_t to uint16_t.
> [...]
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>    struct rte_eth_dcb_tc_queue_mapping {
>>>    	/** rx queues assigned to tc per Pool */
>>>    	struct {
>>> -		uint8_t base;
>>> -		uint8_t nb_queue;
>>> +		uint16_t base;
>>> +		uint16_t nb_queue;
>>>    	} tc_rxq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>>>    	/** rx queues assigned to tc per Pool */
>>>    	struct {
>>> -		uint8_t base;
>>> -		uint8_t nb_queue;
>>> +		uint16_t base;
>>> +		uint16_t nb_queue;
>>>    	} tc_txq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>>>    };
>>>    
>>>
>>
>> cc'ed tech-board,
>>
>> The patch breaks the ethdev ABI without a deprecation notice from previous
>> release(s).
>>
>> It is increasing the storage size of the fields to support more than 255 queues.
> 
> Yes queues are in 16-bit range.
> 
>> Since the ethdev library already heavily breaks the ABI this release, I am for
>> getting this patch, instead of waiting for one more year for the update.
>>
>> Can you please review the patch, is there any objection to proceed with it?
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> 

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

I will continue with this patch (not patchset) if there is no objection.
  
Ferruh Yigit Oct. 6, 2020, 12:04 p.m. UTC | #4
On 10/5/2020 1:26 PM, Ferruh Yigit wrote:
> On 9/28/2020 10:21 AM, Thomas Monjalon wrote:
>> 28/09/2020 11:04, Ferruh Yigit:
>>> On 9/27/2020 4:16 AM, Min Hu (Connor) wrote:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> Currently, base and nb_queue in the tc_rxq and tc_txq information
>>>> of queue and TC mapping on both TX and RX paths are uint8_t.
>>>> However, these data will be truncated when queue number under a TC
>>>> is greater than 256. So it is necessary for base and nb_queue to
>>>> change from uint8_t to uint16_t.
>> [...]
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>    struct rte_eth_dcb_tc_queue_mapping {
>>>>        /** rx queues assigned to tc per Pool */
>>>>        struct {
>>>> -        uint8_t base;
>>>> -        uint8_t nb_queue;
>>>> +        uint16_t base;
>>>> +        uint16_t nb_queue;
>>>>        } tc_rxq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>>>>        /** rx queues assigned to tc per Pool */
>>>>        struct {
>>>> -        uint8_t base;
>>>> -        uint8_t nb_queue;
>>>> +        uint16_t base;
>>>> +        uint16_t nb_queue;
>>>>        } tc_txq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
>>>>    };
>>>>
>>>
>>> cc'ed tech-board,
>>>
>>> The patch breaks the ethdev ABI without a deprecation notice from previous
>>> release(s).
>>>
>>> It is increasing the storage size of the fields to support more than 255 queues.
>>
>> Yes queues are in 16-bit range.
>>
>>> Since the ethdev library already heavily breaks the ABI this release, I am for
>>> getting this patch, instead of waiting for one more year for the update.
>>>
>>> Can you please review the patch, is there any objection to proceed with it?
>>
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>
>>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> I will continue with this patch (not patchset) if there is no objection.
 >

Applied to dpdk-next-net/main, thanks.

Only this patch in the patchset merged, discussion is going on the 1/2 one, 
since the issues are separate, it can continue on its own.
  

Patch

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 1ef6f0e..ad14fd7 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -183,6 +183,11 @@  API Changes
   function will be changed from ``uint8_t`` to ``uint16_t``, which supports for
   using 256 or more than 256 queues and displaying all statistics of rx/tx queue.
 
+* ethdev: Modified field type of base and nb_queue in struct
+  ``rte_eth_dcb_tc_queue_mapping`` from ``uint8_t`` to ``uint16_t``.
+  As the data of uint8_t will be truncated when queue number under
+  a TC is greater than 256.
+
 
 ABI Changes
 -----------
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index ff3a616..2270460 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1522,13 +1522,13 @@  struct rte_eth_xstat_name {
 struct rte_eth_dcb_tc_queue_mapping {
 	/** rx queues assigned to tc per Pool */
 	struct {
-		uint8_t base;
-		uint8_t nb_queue;
+		uint16_t base;
+		uint16_t nb_queue;
 	} tc_rxq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
 	/** rx queues assigned to tc per Pool */
 	struct {
-		uint8_t base;
-		uint8_t nb_queue;
+		uint16_t base;
+		uint16_t nb_queue;
 	} tc_txq[ETH_MAX_VMDQ_POOL][ETH_DCB_NUM_TCS];
 };