diff mbox series

[10/12] net/hns3: rename Rx burst API

Message ID 1618314611-47978-11-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Bugfix for hns3 PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

humin (Q) April 13, 2021, 11:50 a.m. UTC 
  From: Chengwen Feng <fengchengwen@huawei.com>

Currently, user could use runtime config "rx_func_hint=simple" to
select the hns3_recv_pkts API, but the API's name get from
rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".

So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
also change it's name which gets from rte_eth_rx_burst_mode_get to
"Scalar Simple" to maintain conceptual consistency.

Also changes the hns3_recv_scattered_pkts API's name which gets from
rte_eth_rx_burst_mode_get to "Scalar".

Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 20 +++++++++++---------
 drivers/net/hns3/hns3_rxtx.h |  4 ++--
 2 files changed, 13 insertions(+), 11 deletions(-)

Comments

Ferruh Yigit April 14, 2021, 5:41 p.m. UTC | #1 
On 4/13/2021 12:50 PM, Min Hu (Connor) wrote:
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> Currently, user could use runtime config "rx_func_hint=simple" to
> select the hns3_recv_pkts API, but the API's name get from
> rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
> 
> So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
> also change it's name which gets from rte_eth_rx_burst_mode_get to
> "Scalar Simple" to maintain conceptual consistency.
> 
> Also changes the hns3_recv_scattered_pkts API's name which gets from
> rte_eth_rx_burst_mode_get to "Scalar".
> 
> Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

<...>

> @@ -2743,10 +2745,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
>   		eth_rx_burst_t pkt_burst;
>   		const char *info;
>   	} burst_infos[] = {
> -		{ hns3_recv_pkts,		"Scalar" },
> -		{ hns3_recv_scattered_pkts,	"Scalar Scattered" },
> -		{ hns3_recv_pkts_vec,		"Vector Neon" },
> -		{ hns3_recv_pkts_vec_sve,	"Vector Sve" },
> +		{ hns3_recv_pkts_simple,	"Scalar Simple" },
> +		{ hns3_recv_scattered_pkts,	"Scalar"        },
> +		{ hns3_recv_pkts_vec,		"Vector Neon"   },
> +		{ hns3_recv_pkts_vec_sve,	"Vector Sve"    },

No concern on the burst function rename, that is driver internal, but related to 
the above change, I think new value "Scalar Simple" is not clear, what does 
'Simple' mean?
At least previously "Scalar Scattered" vs "Scalar" was more clear, one can 
easily say difference is scattered Rx support, but with "Scalar" vs "Scalar 
Simple" the difference is not clear.
humin (Q) April 15, 2021, 1:58 a.m. UTC | #2 
在 2021/4/15 1:41, Ferruh Yigit 写道:
> On 4/13/2021 12:50 PM, Min Hu (Connor) wrote:
>> From: Chengwen Feng <fengchengwen@huawei.com>
>>
>> Currently, user could use runtime config "rx_func_hint=simple" to
>> select the hns3_recv_pkts API, but the API's name get from
>> rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
>>
>> So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
>> also change it's name which gets from rte_eth_rx_burst_mode_get to
>> "Scalar Simple" to maintain conceptual consistency.
>>
>> Also changes the hns3_recv_scattered_pkts API's name which gets from
>> rte_eth_rx_burst_mode_get to "Scalar".
>>
>> Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> 
> <...>
> 
>> @@ -2743,10 +2745,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev 
>> *dev, __rte_unused uint16_t queue_id,
>>           eth_rx_burst_t pkt_burst;
>>           const char *info;
>>       } burst_infos[] = {
>> -        { hns3_recv_pkts,        "Scalar" },
>> -        { hns3_recv_scattered_pkts,    "Scalar Scattered" },
>> -        { hns3_recv_pkts_vec,        "Vector Neon" },
>> -        { hns3_recv_pkts_vec_sve,    "Vector Sve" },
>> +        { hns3_recv_pkts_simple,    "Scalar Simple" },
>> +        { hns3_recv_scattered_pkts,    "Scalar"        },
>> +        { hns3_recv_pkts_vec,        "Vector Neon"   },
>> +        { hns3_recv_pkts_vec_sve,    "Vector Sve"    },
> 
> No concern on the burst function rename, that is driver internal, but 
> related to the above change, I think new value "Scalar Simple" is not 
> clear, what does 'Simple' mean?
> At least previously "Scalar Scattered" vs "Scalar" was more clear, one 
> can easily say difference is scattered Rx support, but with "Scalar" vs 
> "Scalar Simple" the difference is not clear.
> 
Agreed to retain the hns3_recv_scattered_pkts name "Scalar Scattered",
but suggests changing the hns3_recv_pkts_simple name to "Scalar Simple"
for the following reasons:
1. Currently, the transmit and receive algorithms implemented in C language
only process single-BD algorithms. The Rx direction is Scalar, while the
Tx direction is Scalar Simple. The two do not correspond with each other.
2. The algorithm name selected by using rx_func_hint=simple does not contain
simple. The DPDK user may be confused.

BTW, v2 has been sent, please check it out, thanks.
> .
Ferruh Yigit April 15, 2021, 7:27 a.m. UTC | #3 
On 4/15/2021 2:58 AM, Min Hu (Connor) wrote:
> 
> 
> 在 2021/4/15 1:41, Ferruh Yigit 写道:
>> On 4/13/2021 12:50 PM, Min Hu (Connor) wrote:
>>> From: Chengwen Feng <fengchengwen@huawei.com>
>>>
>>> Currently, user could use runtime config "rx_func_hint=simple" to
>>> select the hns3_recv_pkts API, but the API's name get from
>>> rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
>>>
>>> So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
>>> also change it's name which gets from rte_eth_rx_burst_mode_get to
>>> "Scalar Simple" to maintain conceptual consistency.
>>>
>>> Also changes the hns3_recv_scattered_pkts API's name which gets from
>>> rte_eth_rx_burst_mode_get to "Scalar".
>>>
>>> Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>
>> <...>
>>
>>> @@ -2743,10 +2745,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev *dev, 
>>> __rte_unused uint16_t queue_id,
>>>           eth_rx_burst_t pkt_burst;
>>>           const char *info;
>>>       } burst_infos[] = {
>>> -        { hns3_recv_pkts,        "Scalar" },
>>> -        { hns3_recv_scattered_pkts,    "Scalar Scattered" },
>>> -        { hns3_recv_pkts_vec,        "Vector Neon" },
>>> -        { hns3_recv_pkts_vec_sve,    "Vector Sve" },
>>> +        { hns3_recv_pkts_simple,    "Scalar Simple" },
>>> +        { hns3_recv_scattered_pkts,    "Scalar"        },
>>> +        { hns3_recv_pkts_vec,        "Vector Neon"   },
>>> +        { hns3_recv_pkts_vec_sve,    "Vector Sve"    },
>>
>> No concern on the burst function rename, that is driver internal, but related 
>> to the above change, I think new value "Scalar Simple" is not clear, what does 
>> 'Simple' mean?
>> At least previously "Scalar Scattered" vs "Scalar" was more clear, one can 
>> easily say difference is scattered Rx support, but with "Scalar" vs "Scalar 
>> Simple" the difference is not clear.
>>
> Agreed to retain the hns3_recv_scattered_pkts name "Scalar Scattered",
> but suggests changing the hns3_recv_pkts_simple name to "Scalar Simple"
> for the following reasons:
> 1. Currently, the transmit and receive algorithms implemented in C language
> only process single-BD algorithms. The Rx direction is Scalar, while the
> Tx direction is Scalar Simple. The two do not correspond with each other.
> 2. The algorithm name selected by using rx_func_hint=simple does not contain
> simple. The DPDK user may be confused.
> 

ack

> BTW, v2 has been sent, please check it out, thanks.
>> .
diff mbox series

Patch

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 87d2df7..c524d5a 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -1992,7 +1992,7 @@  hns3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 		RTE_PTYPE_UNKNOWN
 	};
 
-	if (dev->rx_pkt_burst == hns3_recv_pkts ||
+	if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
 	    dev->rx_pkt_burst == hns3_recv_scattered_pkts ||
 	    dev->rx_pkt_burst == hns3_recv_pkts_vec ||
 	    dev->rx_pkt_burst == hns3_recv_pkts_vec_sve)
@@ -2360,7 +2360,9 @@  hns3_rx_ptp_timestamp_handle(struct hns3_rx_queue *rxq, struct rte_mbuf *mbuf,
 }
 
 uint16_t
-hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
+hns3_recv_pkts_simple(void *rx_queue,
+		      struct rte_mbuf **rx_pkts,
+		      uint16_t nb_pkts)
 {
 	volatile struct hns3_desc *rx_ring;  /* RX ring (desc) */
 	volatile struct hns3_desc *rxdp;     /* pointer of the current desc */
@@ -2743,10 +2745,10 @@  hns3_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
 		eth_rx_burst_t pkt_burst;
 		const char *info;
 	} burst_infos[] = {
-		{ hns3_recv_pkts,		"Scalar" },
-		{ hns3_recv_scattered_pkts,	"Scalar Scattered" },
-		{ hns3_recv_pkts_vec,		"Vector Neon" },
-		{ hns3_recv_pkts_vec_sve,	"Vector Sve" },
+		{ hns3_recv_pkts_simple,	"Scalar Simple" },
+		{ hns3_recv_scattered_pkts,	"Scalar"        },
+		{ hns3_recv_pkts_vec,		"Vector Neon"   },
+		{ hns3_recv_pkts_vec_sve,	"Vector Sve"    },
 	};
 
 	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
@@ -2804,14 +2806,14 @@  hns3_get_rx_function(struct rte_eth_dev *dev)
 	if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SVE && sve_allowed)
 		return hns3_recv_pkts_vec_sve;
 	if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SIMPLE && simple_allowed)
-		return hns3_recv_pkts;
+		return hns3_recv_pkts_simple;
 	if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_COMMON)
 		return hns3_recv_scattered_pkts;
 
 	if (vec_allowed)
 		return hns3_recv_pkts_vec;
 	if (simple_allowed)
-		return hns3_recv_pkts;
+		return hns3_recv_pkts_simple;
 
 	return hns3_recv_scattered_pkts;
 }
@@ -4488,7 +4490,7 @@  hns3_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 	rxdp = &rxq->rx_ring[desc_id];
 	bd_base_info = rte_le_to_cpu_32(rxdp->rx.bd_base_info);
 	dev = &rte_eth_devices[rxq->port_id];
-	if (dev->rx_pkt_burst == hns3_recv_pkts ||
+	if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
 	    dev->rx_pkt_burst == hns3_recv_scattered_pkts) {
 		if (offset >= rxq->nb_rx_desc - rxq->rx_free_hold)
 			return RTE_ETH_RX_DESC_UNAVAIL;
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index 10a6c64..ad85d0d 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -683,8 +683,8 @@  int hns3_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int hns3_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int hns3_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
 int hns3_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
-uint16_t hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
-			uint16_t nb_pkts);
+uint16_t hns3_recv_pkts_simple(void *rx_queue, struct rte_mbuf **rx_pkts,
+				uint16_t nb_pkts);
 uint16_t hns3_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				  uint16_t nb_pkts);
 uint16_t hns3_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,