[dpdk-dev] [PATCH 0/7] some bugfixes for hns3

[dpdk-dev] [PATCH 0/7] some bugfixes for hns3

[Lijun Ou]

Here series fix some hns3 PMD bugs.

Huisong Li (3):
  net/hns3: fix xstats statistics with id
  net/hns3: fix abnormal return value in xstats
  net/hns3: fix Rx/Tx abnormal errors stats

Lijun Ou (2):
  net/hns3: fix incorrect interception with filter director
  net/hns3: remove unnecessary memset

Min Hu (Connor) (2):
  net/hns3: fix location of saving ethdev
  net/hns3: fix directly access with rte device

 drivers/net/hns3/hns3_ethdev.c       | 14 ++++----
 drivers/net/hns3/hns3_ethdev.h       |  6 ++++
 drivers/net/hns3/hns3_ethdev_vf.c    | 14 ++++----
 drivers/net/hns3/hns3_flow.c         |  5 ---
 drivers/net/hns3/hns3_rss.c          |  5 ---
 drivers/net/hns3/hns3_rxtx.c         | 13 ++++---
 drivers/net/hns3/hns3_rxtx_vec.c     |  4 ++-
 drivers/net/hns3/hns3_rxtx_vec_sve.c |  4 ++-
 drivers/net/hns3/hns3_stats.c        | 66 ++++++++++++++++++++++--------------
 9 files changed, 72 insertions(+), 59 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/7] net/hns3: fix incorrect interception with filter director

[Lijun Ou]

The rte_fdir_conf structure has deprecated and users need
to use the specified rule parameters of rte_flow structure
when configure a flow rule. As a result, it is incorrectly
used in the rte_flow API.

Fixes: fcba820d9b9e ("net/hns3: support flow director")
Cc: stable@dpdk.org

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_flow.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index ee6ec15..f303df4 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1208,11 +1208,6 @@ hns3_parse_fdir_filter(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 					  "Fdir not supported in VF");
 
-	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT)
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-					  "fdir_conf.mode isn't perfect");
-
 	step_mngr.items = first_items;
 	step_mngr.count = ARRAY_SIZE(first_items);
 	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-- 
2.7.4

[dpdk-dev] [PATCH 2/7] net/hns3: fix xstats statistics with id

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

Number of xstats item in rte_eth_xstats_get_by_id is obtained
by the eth_dev_get_xstats_count API, and the xstats_get_by_id
ops of the driver only needs to report the corresponding stats
item result.
However, a redundant code for reporting the number of stats items
in the hns3_dev_xstats_get_by_id API causes a problem. Namely, if
the ID range of the xstats stats item does not include the basic
stats item, the app can not obtain the corresponding xstats
statistics in hns3_dev_xstats_get_by_id.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 91168ac..b43143b 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -933,9 +933,6 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
 	uint32_t i;
 	int ret;
 
-	if (ids == NULL || size < cnt_stats)
-		return cnt_stats;
-
 	/* Update tqp stats by read register */
 	ret = hns3_update_tqp_stats(hw);
 	if (ret) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH 2/7] net/hns3: fix xstats statistics with id

[Ferruh Yigit]

On 12/13/2020 8:03 AM, Lijun Ou wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> Number of xstats item in rte_eth_xstats_get_by_id is obtained
> by the eth_dev_get_xstats_count API, and the xstats_get_by_id
> ops of the driver only needs to report the corresponding stats
> item result.
> However, a redundant code for reporting the number of stats items
> in the hns3_dev_xstats_get_by_id API causes a problem. Namely, if
> the ID range of the xstats stats item does not include the basic
> stats item, the app can not obtain the corresponding xstats
> statistics in hns3_dev_xstats_get_by_id.
> 
> Fixes: 8839c5e202f3 ("net/hns3: support device stats")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
>   drivers/net/hns3/hns3_stats.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
> index 91168ac..b43143b 100644
> --- a/drivers/net/hns3/hns3_stats.c
> +++ b/drivers/net/hns3/hns3_stats.c
> @@ -933,9 +933,6 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
>   	uint32_t i;
>   	int ret;
>   
> -	if (ids == NULL || size < cnt_stats)
> -		return cnt_stats;
> -

Hi Lijun,

Above check seems wrong, but just removing it also wrong.

Following checks should be there:
ids==NULL && values==NULL ? return cnt_stats
ids==NULL ? return all values

Also 'hns3_dev_xstats_get_names_by_id()' seems wrong in that manner.

Re: [dpdk-dev] [PATCH 2/7] net/hns3: fix xstats statistics with id

[oulijun]

在 2020/12/17 23:20, Ferruh Yigit 写道:
> On 12/13/2020 8:03 AM, Lijun Ou wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Number of xstats item in rte_eth_xstats_get_by_id is obtained
>> by the eth_dev_get_xstats_count API, and the xstats_get_by_id
>> ops of the driver only needs to report the corresponding stats
>> item result.
>> However, a redundant code for reporting the number of stats items
>> in the hns3_dev_xstats_get_by_id API causes a problem. Namely, if
>> the ID range of the xstats stats item does not include the basic
>> stats item, the app can not obtain the corresponding xstats
>> statistics in hns3_dev_xstats_get_by_id.
>>
>> Fixes: 8839c5e202f3 ("net/hns3: support device stats")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>>   drivers/net/hns3/hns3_stats.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/net/hns3/hns3_stats.c 
>> b/drivers/net/hns3/hns3_stats.c
>> index 91168ac..b43143b 100644
>> --- a/drivers/net/hns3/hns3_stats.c
>> +++ b/drivers/net/hns3/hns3_stats.c
>> @@ -933,9 +933,6 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, 
>> const uint64_t *ids,
>>       uint32_t i;
>>       int ret;
>> -    if (ids == NULL || size < cnt_stats)
>> -        return cnt_stats;
>> -
> 
> Hi Lijun,
> 
> Above check seems wrong, but just removing it also wrong.
> 
> Following checks should be there:
> ids==NULL && values==NULL ? return cnt_stats
> ids==NULL ? return all values
> 
> Also 'hns3_dev_xstats_get_names_by_id()' seems wrong in that manner.
> 
Thanks for your reviews. It should be deleted with the check in 
hns3_dev_xstats_get_by_id. Because the check have done in the API layer.
for example
   For rte_eth_xstats_get_by_id implementation：
   if ids == NULL and values == NULL, it returns the result of 
eth_dev_get_xstats_count and not run the hns3_dev_xstats_get_by_id

   if ids == NULL, it will not be able to run the hns3_dev_xstats_get_by_id.

However, Maybe the hns3_dev_xstats_get_names_by_id seems wrong and needs 
to fix it as flows:
diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 1d1f706..789ff6e 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -987,8 +987,8 @@ hns3_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
         uint64_t len;
         uint32_t i;

-       if (ids == NULL || xstats_names == NULL)
-               return cnt_stats;
+       if (ids == NULL)
+               return hns3_dev_xstats_get_names(dev, xstats_names, 
cnt_stats);

         len = cnt_stats * sizeof(struct rte_eth_xstat_name);
         names_copy = rte_zmalloc("hns3_xstats_names", len, 0);

What do you think?
> .
> 

Re: [dpdk-dev] [PATCH 2/7] net/hns3: fix xstats statistics with id

[Ferruh Yigit]

On 12/18/2020 7:06 AM, oulijun wrote:
> 
> 
> 在 2020/12/17 23:20, Ferruh Yigit 写道:
>> On 12/13/2020 8:03 AM, Lijun Ou wrote:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Number of xstats item in rte_eth_xstats_get_by_id is obtained
>>> by the eth_dev_get_xstats_count API, and the xstats_get_by_id
>>> ops of the driver only needs to report the corresponding stats
>>> item result.
>>> However, a redundant code for reporting the number of stats items
>>> in the hns3_dev_xstats_get_by_id API causes a problem. Namely, if
>>> the ID range of the xstats stats item does not include the basic
>>> stats item, the app can not obtain the corresponding xstats
>>> statistics in hns3_dev_xstats_get_by_id.
>>>
>>> Fixes: 8839c5e202f3 ("net/hns3: support device stats")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>> ---
>>>   drivers/net/hns3/hns3_stats.c | 3 ---
>>>   1 file changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
>>> index 91168ac..b43143b 100644
>>> --- a/drivers/net/hns3/hns3_stats.c
>>> +++ b/drivers/net/hns3/hns3_stats.c
>>> @@ -933,9 +933,6 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, const 
>>> uint64_t *ids,
>>>       uint32_t i;
>>>       int ret;
>>> -    if (ids == NULL || size < cnt_stats)
>>> -        return cnt_stats;
>>> -
>>
>> Hi Lijun,
>>
>> Above check seems wrong, but just removing it also wrong.
>>
>> Following checks should be there:
>> ids==NULL && values==NULL ? return cnt_stats
>> ids==NULL ? return all values
>>
>> Also 'hns3_dev_xstats_get_names_by_id()' seems wrong in that manner.
>>
> Thanks for your reviews. It should be deleted with the check in 
> hns3_dev_xstats_get_by_id. Because the check have done in the API layer.
> for example
>    For rte_eth_xstats_get_by_id implementation：
>    if ids == NULL and values == NULL, it returns the result of 
> eth_dev_get_xstats_count and not run the hns3_dev_xstats_get_by_id
> 

For the case "ids==NULL && values==NULL",
yes 'rte_eth_xstats_get_by_id()' is the wrapper of this dev_ops and it has the 
checks, but the dev_ops can be called from somewhere else, like being in 
'eth_dev_get_xstats_count()', currently 'xstats_get_names_by_id()' is used there 
but 'xstats_get_by_id()' may be used as well, please check how it is used in 
that function. dev_ops gets, "NULL, NULL, 0" arguments and it should be supported.

And for the "ids==NULL" case, it is not covered at all in 
'hns3_dev_xstats_get_names_by_id()' but it should, that is a valid usecase.
When dev_ops gets, "ids==NULL", that means all xstat values are requested.

>    if ids == NULL, it will not be able to run the hns3_dev_xstats_get_by_id.
> 
> However, Maybe the hns3_dev_xstats_get_names_by_id seems wrong and needs to fix 
> it as flows:
> diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
> index 1d1f706..789ff6e 100644
> --- a/drivers/net/hns3/hns3_stats.c
> +++ b/drivers/net/hns3/hns3_stats.c
> @@ -987,8 +987,8 @@ hns3_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
>          uint64_t len;
>          uint32_t i;
> 
> -       if (ids == NULL || xstats_names == NULL)
> -               return cnt_stats;
> +       if (ids == NULL)
> +               return hns3_dev_xstats_get_names(dev, xstats_names, cnt_stats);
> 
>          len = cnt_stats * sizeof(struct rte_eth_xstat_name);
>          names_copy = rte_zmalloc("hns3_xstats_names", len, 0);
> 
> What do you think?

I think both 'xstats_names' & 'ids' should be checked, please check above 
comment, like:

if (xstats_names == NULL)
	return cnt_stats;

if (ids == NULL)
	return hns3_dev_xstats_get_names(dev, xstats_names, cnt_stats);

[dpdk-dev] [PATCH 3/7] net/hns3: fix abnormal return value in xstats

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

The ethdev API has processed the failure to obtain
xstats statistics. Therefore, driver should return
an error code instead of 0 in 'hns3_dev_xstats_get'
API.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index b43143b..94d34bc 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -739,9 +739,9 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	if (!hns->is_vf) {
 		/* Update Mac stats */
 		ret = hns3_query_update_mac_stats(dev);
-		if (ret) {
+		if (ret < 0) {
 			hns3_err(hw, "Update Mac stats fail : %d", ret);
-			return 0;
+			return ret;
 		}
 
 		/* Get MAC stats from hw->hw_xstats.mac_stats struct */
-- 
2.7.4

[dpdk-dev] [PATCH 4/7] net/hns3: fix Rx/Tx abnormal errors stats

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

Abnormal errors stats in Rx/Tx datapath are statistics
items in driver, and displayed in xstats. They should
be cleared by the rte_eth_xstats_reset api, instead of
the rte_eth_stats_reset.

Fixes: c4b7d6761d01 ("net/hns3: get Tx abnormal errors in xstats")
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 94d34bc..1d1f706 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -551,7 +551,6 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_cmd_desc desc_reset;
 	struct hns3_rx_queue *rxq;
-	struct hns3_tx_queue *txq;
 	uint16_t i;
 	int ret;
 
@@ -581,29 +580,15 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 		}
 	}
 
-	/* Clear the Rx BD errors stats */
-	for (i = 0; i != eth_dev->data->nb_rx_queues; ++i) {
+	/*
+	 * Clear soft stats of rx error packet which will be dropped
+	 * in driver.
+	 */
+	for (i = 0; i < eth_dev->data->nb_rx_queues; ++i) {
 		rxq = eth_dev->data->rx_queues[i];
 		if (rxq) {
 			rxq->pkt_len_errors = 0;
 			rxq->l2_errors = 0;
-			rxq->l3_csum_errors = 0;
-			rxq->l4_csum_errors = 0;
-			rxq->ol3_csum_errors = 0;
-			rxq->ol4_csum_errors = 0;
-		}
-	}
-
-	/* Clear the Tx errors stats */
-	for (i = 0; i != eth_dev->data->nb_tx_queues; ++i) {
-		txq = eth_dev->data->tx_queues[i];
-		if (txq) {
-			txq->over_length_pkt_cnt = 0;
-			txq->exceed_limit_bd_pkt_cnt = 0;
-			txq->exceed_limit_bd_reassem_fail = 0;
-			txq->unsupported_tunnel_pkt_cnt = 0;
-			txq->queue_full_cnt = 0;
-			txq->pkt_padding_fail_cnt = 0;
 		}
 	}
 
@@ -1030,6 +1015,38 @@ hns3_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
 	return size;
 }
 
+static void
+hns3_tqp_dfx_stats_clear(struct rte_eth_dev *dev)
+{
+	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
+	int i;
+
+	/* Clear Rx dfx stats */
+	for (i = 0; i < dev->data->nb_rx_queues; ++i) {
+		rxq = dev->data->rx_queues[i];
+		if (rxq) {
+			rxq->l3_csum_errors = 0;
+			rxq->l4_csum_errors = 0;
+			rxq->ol3_csum_errors = 0;
+			rxq->ol4_csum_errors = 0;
+		}
+	}
+
+	/* Clear Tx dfx stats */
+	for (i = 0; i < dev->data->nb_tx_queues; ++i) {
+		txq = dev->data->tx_queues[i];
+		if (txq) {
+			txq->over_length_pkt_cnt = 0;
+			txq->exceed_limit_bd_pkt_cnt = 0;
+			txq->exceed_limit_bd_reassem_fail = 0;
+			txq->unsupported_tunnel_pkt_cnt = 0;
+			txq->queue_full_cnt = 0;
+			txq->pkt_padding_fail_cnt = 0;
+		}
+	}
+}
+
 int
 hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 {
@@ -1045,6 +1062,8 @@ hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 	/* Clear reset stats */
 	memset(&hns->hw.reset.stats, 0, sizeof(struct hns3_reset_stats));
 
+	hns3_tqp_dfx_stats_clear(dev);
+
 	if (hns->is_vf)
 		return 0;
 
-- 
2.7.4

[dpdk-dev] [PATCH 5/7] net/hns3: fix location of saving ethdev

[Lijun Ou]

From: "Min Hu (Connor)" <humin29@huawei.com>

In current version, procedure of saving eth_dev in
hns3 PMD init will be called more than twice, one
for primary, the other for secondary. That will cause
segmentation fault in Multi-process as eth_dev will
be changed in secondary process, which is different
from one in primary process.
This patch saved eth_dev in dev_private only in
primary process, as dev_private is shared by primary
process and secondary process.

Fixes: 8929efbc1c46 ("net/hns3: fix FEC state query")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 3 +--
 drivers/net/hns3/hns3_ethdev_vf.c | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 7c34e38..f49b779 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6106,8 +6106,6 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	hns->eth_dev = eth_dev;
-
 	eth_dev->process_private = (struct hns3_process_private *)
 	    rte_zmalloc_socket("hns3_filter_list",
 			       sizeof(struct hns3_process_private),
@@ -6134,6 +6132,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
+	hns->eth_dev = eth_dev;
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	ret = hns3_mp_init_primary();
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index f09cabc..9c382bf 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2753,6 +2753,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
+	hns->eth_dev = eth_dev;
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	ret = hns3_mp_init_primary();
-- 
2.7.4

Re: [dpdk-dev] [PATCH 5/7] net/hns3: fix location of saving ethdev

[Ferruh Yigit]

On 12/13/2020 8:03 AM, Lijun Ou wrote:
> From: "Min Hu (Connor)" <humin29@huawei.com>
> 
> In current version, procedure of saving eth_dev in
> hns3 PMD init will be called more than twice, one
> for primary, the other for secondary. That will cause
> segmentation fault in Multi-process as eth_dev will
> be changed in secondary process, which is different
> from one in primary process.
> This patch saved eth_dev in dev_private only in
> primary process, as dev_private is shared by primary
> process and secondary process.
> 
> Fixes: 8929efbc1c46 ("net/hns3: fix FEC state query")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
>   drivers/net/hns3/hns3_ethdev.c    | 3 +--
>   drivers/net/hns3/hns3_ethdev_vf.c | 1 +
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 7c34e38..f49b779 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -6106,8 +6106,6 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>   
>   	PMD_INIT_FUNC_TRACE();
>   
> -	hns->eth_dev = eth_dev;
> -
>   	eth_dev->process_private = (struct hns3_process_private *)
>   	    rte_zmalloc_socket("hns3_filter_list",
>   			       sizeof(struct hns3_process_private),
> @@ -6134,6 +6132,7 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>   		return 0;
>   	}
>   
> +	hns->eth_dev = eth_dev;
>   	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>   

The definition of the problem is correct, but with this update the 
'hns->eth_dev' will be wrong for the secondary process.

The initial problem was access to 'rte_eth_devices' global variable, which is 
wrong. But current approach can cause problem for the secondaries, moving 
'eth_dev' to process private can work but before making things more complex,
I would like to check why it is needed.

The usage is as following:

hns3_query_dev_fec_info(struct hns3_hw *hw)
	struct rte_eth_dev *eth_dev = hns->eth_dev;
	ret = hns3_fec_get(eth_dev, &pf->fec_mode);

hns3_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);


Here since 'hns3_fec_get()' is dev_ops, it needs to get 'eth_dev' as parameter. 
Trying to use it internally forces to know 'eth_dev', but instead it can be 
possible to create an internal function that gets "struct hns3_hw" as parameter 
and it can be called internally without knowing 'eth_dev', and the .dev_ops can 
be wrapper to this,
I mean something like this, what do you think:

hns3_fec_get_internal(struct hns3_hw *hw, uint32_t *fec_capa);

hns3_query_dev_fec_info(struct hns3_hw *hw)
	ret = hns3_fec_get_internal(hw, &pf->fec_mode);

hns3_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
	hns3_fec_get_internal(hw, fec_capa)


There is other patch to switch to 'hns->eth_dev', I wonder can same approach be 
applied to those usages?

[dpdk-dev] [PATCH 6/7] net/hns3: fix directly access with rte device

[Lijun Ou]

From: "Min Hu (Connor)" <humin29@huawei.com>

In current version, there exists the way of access
global 'rte_eth_devices' array directly, and this
is not a good way.
Better way is to store the 'eth_dev' in the device
private data in the probe(). Then driver could use
it later.

Fixes: ab2e2e344163 ("net/hns3: get device capability in primary process")
Fixes: c4ae39b2cfc5 ("net/hns3: fix Rx interrupt after reset")
Fixes: 2790c6464725 ("net/hns3: support device reset")
Fixes: c203571b3602 ("net/hns3: register and add log interface")
Fixes: 1265b5372d9d ("net/hns3: add some definitions for data structure and macro")
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Fixes: a3d4f4d291d7 ("net/hns3: support NEON Rx")
Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c       | 11 +++++------
 drivers/net/hns3/hns3_ethdev.h       |  6 ++++++
 drivers/net/hns3/hns3_ethdev_vf.c    | 13 ++++++-------
 drivers/net/hns3/hns3_rxtx.c         | 13 ++++++-------
 drivers/net/hns3/hns3_rxtx_vec.c     |  4 +++-
 drivers/net/hns3/hns3_rxtx_vec_sve.c |  4 +++-
 6 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index f49b779..1093be1 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2992,15 +2992,14 @@ hns3_query_dev_specifications(struct hns3_hw *hw)
 static int
 hns3_get_capability(struct hns3_hw *hw)
 {
+	struct rte_eth_dev *eth_dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
 	struct rte_pci_device *pci_dev;
 	struct hns3_pf *pf = &hns->pf;
-	struct rte_eth_dev *eth_dev;
 	uint16_t device_id;
 	uint8_t revision;
 	int ret;
 
-	eth_dev = &rte_eth_devices[hw->data->port_id];
 	pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	device_id = pci_dev->id.device_id;
 
@@ -4836,7 +4835,7 @@ hns3_map_rx_interrupt(struct rte_eth_dev *dev)
 static int
 hns3_restore_rx_interrupt(struct hns3_hw *hw)
 {
-	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	struct rte_eth_dev *dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	uint16_t q_id;
@@ -5526,7 +5525,7 @@ hns3_stop_service(struct hns3_adapter *hns)
 	struct hns3_hw *hw = &hns->hw;
 	struct rte_eth_dev *eth_dev;
 
-	eth_dev = &rte_eth_devices[hw->data->port_id];
+	eth_dev = hns->eth_dev;
 	if (hw->adapter_state == HNS3_NIC_STARTED)
 		rte_eal_alarm_cancel(hns3_service_handler, eth_dev);
 	hw->mac.link_status = ETH_LINK_DOWN;
@@ -5567,7 +5566,7 @@ hns3_start_service(struct hns3_adapter *hns)
 	if (hw->reset.level == HNS3_IMP_RESET ||
 	    hw->reset.level == HNS3_GLOBAL_RESET)
 		hns3_set_rst_done(hw);
-	eth_dev = &rte_eth_devices[hw->data->port_id];
+	eth_dev = hns->eth_dev;
 	hns3_set_rxtx_function(eth_dev);
 	hns3_mp_req_start_rxtx(eth_dev);
 	if (hw->adapter_state == HNS3_NIC_STARTED) {
@@ -5668,7 +5667,7 @@ hns3_reset_service(void *param)
 	if (rte_atomic16_read(&hns->hw.reset.schedule) == SCHEDULE_DEFERRED) {
 		rte_atomic16_set(&hns->hw.reset.schedule, SCHEDULE_REQUESTED);
 		hns3_err(hw, "Handling interrupts in delayed tasks");
-		hns3_interrupt_handler(&rte_eth_devices[hw->data->port_id]);
+		hns3_interrupt_handler(hns->eth_dev);
 		reset_level = hns3_get_reset_level(hns, &hw->reset.pending);
 		if (reset_level == HNS3_NONE_RESET) {
 			hns3_err(hw, "No reset level is set, try IMP reset");
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 8d6b8cd..51e450a 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -798,6 +798,12 @@ struct hns3_adapter {
 #define HNS3_DEV_HW_TO_ADAPTER(hw) \
 	container_of(hw, struct hns3_adapter, hw)
 
+static inline struct rte_eth_dev *HNS3_DEV_HW_TO_ETH_DEV(struct hns3_hw *hw)
+{
+	struct hns3_adapter *adapter = HNS3_DEV_HW_TO_ADAPTER(hw);
+	return adapter->eth_dev;
+}
+
 #define hns3_set_field(origin, mask, shift, val) \
 	do { \
 		(origin) &= (~(mask)); \
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 9c382bf..890d33d 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1180,12 +1180,11 @@ hns3vf_query_dev_specifications(struct hns3_hw *hw)
 static int
 hns3vf_get_capability(struct hns3_hw *hw)
 {
+	struct rte_eth_dev *eth_dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	struct rte_pci_device *pci_dev;
-	struct rte_eth_dev *eth_dev;
 	uint8_t revision;
 	int ret;
 
-	eth_dev = &rte_eth_devices[hw->data->port_id];
 	pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 
 	/* Get PCI revision id */
@@ -2152,7 +2151,7 @@ hns3vf_map_rx_interrupt(struct rte_eth_dev *dev)
 static int
 hns3vf_restore_rx_interrupt(struct hns3_hw *hw)
 {
-	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	struct rte_eth_dev *dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	uint16_t q_id;
@@ -2377,7 +2376,7 @@ hns3vf_stop_service(struct hns3_adapter *hns)
 	struct hns3_hw *hw = &hns->hw;
 	struct rte_eth_dev *eth_dev;
 
-	eth_dev = &rte_eth_devices[hw->data->port_id];
+	eth_dev = hns->eth_dev;
 	if (hw->adapter_state == HNS3_NIC_STARTED)
 		rte_eal_alarm_cancel(hns3vf_service_handler, eth_dev);
 	hw->mac.link_status = ETH_LINK_DOWN;
@@ -2415,7 +2414,7 @@ hns3vf_start_service(struct hns3_adapter *hns)
 	struct hns3_hw *hw = &hns->hw;
 	struct rte_eth_dev *eth_dev;
 
-	eth_dev = &rte_eth_devices[hw->data->port_id];
+	eth_dev = hns->eth_dev;
 	hns3_set_rxtx_function(eth_dev);
 	hns3_mp_req_start_rxtx(eth_dev);
 	if (hw->adapter_state == HNS3_NIC_STARTED) {
@@ -2577,7 +2576,7 @@ hns3vf_reset_service(void *param)
 	if (rte_atomic16_read(&hns->hw.reset.schedule) == SCHEDULE_DEFERRED) {
 		rte_atomic16_set(&hns->hw.reset.schedule, SCHEDULE_REQUESTED);
 		hns3_err(hw, "Handling interrupts in delayed tasks");
-		hns3vf_interrupt_handler(&rte_eth_devices[hw->data->port_id]);
+		hns3vf_interrupt_handler(hns->eth_dev);
 		reset_level = hns3vf_get_reset_level(hw, &hw->reset.pending);
 		if (reset_level == HNS3_NONE_RESET) {
 			hns3_err(hw, "No reset level is set, try global reset");
@@ -2608,7 +2607,7 @@ hns3vf_reset_service(void *param)
 static int
 hns3vf_reinit_dev(struct hns3_adapter *hns)
 {
-	struct rte_eth_dev *eth_dev = &rte_eth_devices[hns->hw.data->port_id];
+	struct rte_eth_dev *eth_dev = hns->eth_dev;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	struct hns3_hw *hw = &hns->hw;
 	int ret;
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 88d3bab..b7a28e8 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -912,7 +912,7 @@ hns3_queue_intr_enable(struct hns3_hw *hw, uint16_t queue_id, bool en)
 void
 hns3_dev_all_rx_queue_intr_enable(struct hns3_hw *hw, bool en)
 {
-	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	struct rte_eth_dev *dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	uint16_t nb_rx_q = hw->data->nb_rx_queues;
 	int i;
 
@@ -1620,7 +1620,7 @@ static int
 hns3_rxq_conf_runtime_check(struct hns3_hw *hw, uint16_t buf_size,
 				uint16_t nb_desc)
 {
-	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
+	struct rte_eth_dev *dev = HNS3_DEV_HW_TO_ETH_DEV(hw);
 	struct rte_eth_rxmode *rxmode = &hw->data->dev_conf.rxmode;
 	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
 	uint16_t min_vec_bds;
@@ -2078,6 +2078,7 @@ hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	volatile struct hns3_desc *rxdp;     /* pointer of the current desc */
 	struct hns3_rx_queue *rxq;      /* RX queue */
 	struct hns3_entry *sw_ring;
+	struct rte_eth_dev *dev;
 	struct hns3_entry *rxe;
 	struct hns3_desc rxd;
 	struct rte_mbuf *nmb;           /* pointer of the new mbuf */
@@ -2110,10 +2111,8 @@ hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 
 		nmb = hns3_rx_alloc_buffer(rxq);
 		if (unlikely(nmb == NULL)) {
-			uint16_t port_id;
-
-			port_id = rxq->port_id;
-			rte_eth_devices[port_id].data->rx_mbuf_alloc_failed++;
+			dev = rxq->hns->eth_dev;
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
 		}
 
@@ -2289,7 +2288,7 @@ hns3_recv_scattered_pkts(void *rx_queue,
 
 		nmb = hns3_rx_alloc_buffer(rxq);
 		if (unlikely(nmb == NULL)) {
-			dev = &rte_eth_devices[rxq->port_id];
+			dev = rxq->hns->eth_dev;
 			dev->data->rx_mbuf_alloc_failed++;
 			break;
 		}
diff --git a/drivers/net/hns3/hns3_rxtx_vec.c b/drivers/net/hns3/hns3_rxtx_vec.c
index a26c83d..5d02588 100644
--- a/drivers/net/hns3/hns3_rxtx_vec.c
+++ b/drivers/net/hns3/hns3_rxtx_vec.c
@@ -52,12 +52,14 @@ hns3_rxq_rearm_mbuf(struct hns3_rx_queue *rxq)
 #define REARM_LOOP_STEP_NUM	4
 	struct hns3_entry *rxep = &rxq->sw_ring[rxq->rx_rearm_start];
 	struct hns3_desc *rxdp = rxq->rx_ring + rxq->rx_rearm_start;
+	struct rte_eth_dev *dev;
 	uint64_t dma_addr;
 	int i;
 
 	if (unlikely(rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
 					  HNS3_DEFAULT_RXQ_REARM_THRESH) < 0)) {
-		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
+		dev = rxq->hns->eth_dev;
+		dev->data->rx_mbuf_alloc_failed++;
 		return;
 	}
 
diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c b/drivers/net/hns3/hns3_rxtx_vec_sve.c
index 8c2c8f6..603d726 100644
--- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
+++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
@@ -239,11 +239,13 @@ hns3_rxq_rearm_mbuf_sve(struct hns3_rx_queue *rxq)
 	struct hns3_entry *rxep = &rxq->sw_ring[rxq->rx_rearm_start];
 	struct hns3_desc *rxdp = rxq->rx_ring + rxq->rx_rearm_start;
 	struct hns3_entry *rxep_tmp = rxep;
+	struct rte_eth_dev *dev;
 	int i;
 
 	if (unlikely(rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep,
 					  HNS3_DEFAULT_RXQ_REARM_THRESH) < 0)) {
-		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
+		dev = rxq->hns->eth_dev;
+		dev->data->rx_mbuf_alloc_failed++;
 		return;
 	}
 
-- 
2.7.4

[dpdk-dev] [PATCH 7/7] net/hns3: remove unnecessary memset

[Lijun Ou]

The hns3_cmd_desc has memset when setup and the memset
for req is unnecessary.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index e2f0468..b5df374 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -633,16 +633,11 @@ hns3_set_rss_tc_mode(struct hns3_hw *hw)
 static void
 hns3_rss_tuple_uninit(struct hns3_hw *hw)
 {
-	struct hns3_rss_input_tuple_cmd *req;
 	struct hns3_cmd_desc desc;
 	int ret;
 
 	hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_RSS_INPUT_TUPLE, false);
 
-	req = (struct hns3_rss_input_tuple_cmd *)desc.data;
-
-	memset(req, 0, sizeof(struct hns3_rss_tuple_cfg));
-
 	ret = hns3_cmd_send(hw, &desc, 1);
 	if (ret) {
 		hns3_err(hw, "RSS uninit tuple failed %d", ret);
-- 
2.7.4

[dpdk-dev] [PATCH v2 0/6] some bugfixes for hns3

[Lijun Ou]

Here series fix some hns3 PMD bugs.

Change frome V1:
1. remove the fixes patch[6/7] from V1
2. fix ferruh Yigit's comments.

Huisong Li (3):
  net/hns3: fix xstats statistics with id and names
  net/hns3: fix abnormal return value in xstats
  net/hns3: fix Rx/Tx abnormal errors stats

Lijun Ou (2):
  net/hns3: fix incorrect interception with filter director
  net/hns3: remove unnecessary memset

Min Hu (Connor) (1):
  net/hns3: fix crash with multi-process

 drivers/net/hns3/hns3_ethdev.c | 16 +++++---
 drivers/net/hns3/hns3_ethdev.h |  2 -
 drivers/net/hns3/hns3_flow.c   |  5 ---
 drivers/net/hns3/hns3_rss.c    |  5 ---
 drivers/net/hns3/hns3_stats.c  | 87 ++++++++++++++++++++++++++++++------------
 5 files changed, 73 insertions(+), 42 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v2 1/6] net/hns3: fix incorrect interception with filter director

[Lijun Ou]

The rte_fdir_conf structure has deprecated and users need
to use the specified rule parameters of rte_flow structure
when configure a flow rule. As a result, it is incorrectly
used in the rte_flow API.

Fixes: fcba820d9b9e ("net/hns3: support flow director")
Cc: stable@dpdk.org

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_flow.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index ee6ec15..f303df4 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1208,11 +1208,6 @@ hns3_parse_fdir_filter(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
 					  "Fdir not supported in VF");
 
-	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT)
-		return rte_flow_error_set(error, ENOTSUP,
-					  RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-					  "fdir_conf.mode isn't perfect");
-
 	step_mngr.items = first_items;
 	step_mngr.count = ARRAY_SIZE(first_items);
 	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/6] net/hns3: fix incorrect interception with filter director

[Thomas Monjalon]

06/01/2021 04:46, Lijun Ou:
> The rte_fdir_conf structure has deprecated and users need
> to use the specified rule parameters of rte_flow structure
> when configure a flow rule. As a result, it is incorrectly
> used in the rte_flow API.

Note: it should be "flow director" in the title.

I really don't understand why the hns3 PMD is based
on flow director concept, because:
1/ this is an Intel design
2/ it was already deprecated when hns3 started his development

Please could you explain if there is a HW constraint
or something else I'm missing?

[dpdk-dev] [PATCH v2 2/6] net/hns3: fix xstats statistics with id and names

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

Currently, validity check for ids and values in the
hns3_dev_xstats_get_by_id API is incorrect, which will
cause a problem. Namely, if the ID range of the xstats
stats item does not include the basic stats item, the
app can not obtain the corresponding xstats statistics
in hns3_dev_xstats_get_by_id.

Similarly, the hns3_dev_xstats_get_names_by_id interface
also has a problem.

Although the input parameter verification code cannot be
executed due to the implementation of the ethdev framework
interface, the driver needs to ensure the correctness of
the input parameters.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
V1->V2:
-fix ids and values check
-add hns3_dev_xstats_get_names_by_id check
---
 drivers/net/hns3/hns3_stats.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 91168ac..1597af3 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -933,9 +933,13 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
 	uint32_t i;
 	int ret;
 
-	if (ids == NULL || size < cnt_stats)
+	if (ids == NULL && values == NULL)
 		return cnt_stats;
 
+	if (ids == NULL)
+		if (size < cnt_stats)
+			return cnt_stats;
+
 	/* Update tqp stats by read register */
 	ret = hns3_update_tqp_stats(hw);
 	if (ret) {
@@ -957,6 +961,15 @@ hns3_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
 		return -EINVAL;
 	}
 
+	if (ids == NULL && values != NULL) {
+		for (i = 0; i < cnt_stats; i++)
+			memcpy(&values[i], &values_copy[i].value,
+			       sizeof(values[i]));
+
+		rte_free(values_copy);
+		return cnt_stats;
+	}
+
 	for (i = 0; i < size; i++) {
 		if (ids[i] >= cnt_stats) {
 			hns3_err(hw, "ids[%u] (%" PRIx64 ") is invalid, "
@@ -1005,9 +1018,16 @@ hns3_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
 	uint64_t len;
 	uint32_t i;
 
-	if (ids == NULL || xstats_names == NULL)
+	if (xstats_names == NULL)
 		return cnt_stats;
 
+	if (ids == NULL) {
+		if (size < cnt_stats)
+			return cnt_stats;
+
+		return hns3_dev_xstats_get_names(dev, xstats_names, cnt_stats);
+	}
+
 	len = cnt_stats * sizeof(struct rte_eth_xstat_name);
 	names_copy = rte_zmalloc("hns3_xstats_names", len, 0);
 	if (names_copy == NULL) {
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/6] net/hns3: fix abnormal return value in xstats

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

The ethdev API has processed the failure to obtain
xstats statistics. Therefore, driver should return
an error code instead of 0 in 'hns3_dev_xstats_get'
API.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 1597af3..42ec9b8 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -739,9 +739,9 @@ hns3_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	if (!hns->is_vf) {
 		/* Update Mac stats */
 		ret = hns3_query_update_mac_stats(dev);
-		if (ret) {
+		if (ret < 0) {
 			hns3_err(hw, "Update Mac stats fail : %d", ret);
-			return 0;
+			return ret;
 		}
 
 		/* Get MAC stats from hw->hw_xstats.mac_stats struct */
-- 
2.7.4

[dpdk-dev] [PATCH v2 4/6] net/hns3: fix Rx/Tx abnormal errors stats

[Lijun Ou]

From: Huisong Li <lihuisong@huawei.com>

Abnormal errors stats in Rx/Tx datapath are statistics
items in driver, and displayed in xstats. They should
be cleared by the rte_eth_xstats_reset api, instead of
the rte_eth_stats_reset.

Fixes: c4b7d6761d01 ("net/hns3: get Tx abnormal errors in xstats")
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_stats.c | 59 ++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index 42ec9b8..62a712b 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -551,7 +551,6 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 	struct hns3_hw *hw = &hns->hw;
 	struct hns3_cmd_desc desc_reset;
 	struct hns3_rx_queue *rxq;
-	struct hns3_tx_queue *txq;
 	uint16_t i;
 	int ret;
 
@@ -581,29 +580,15 @@ hns3_stats_reset(struct rte_eth_dev *eth_dev)
 		}
 	}
 
-	/* Clear the Rx BD errors stats */
-	for (i = 0; i != eth_dev->data->nb_rx_queues; ++i) {
+	/*
+	 * Clear soft stats of rx error packet which will be dropped
+	 * in driver.
+	 */
+	for (i = 0; i < eth_dev->data->nb_rx_queues; ++i) {
 		rxq = eth_dev->data->rx_queues[i];
 		if (rxq) {
 			rxq->pkt_len_errors = 0;
 			rxq->l2_errors = 0;
-			rxq->l3_csum_errors = 0;
-			rxq->l4_csum_errors = 0;
-			rxq->ol3_csum_errors = 0;
-			rxq->ol4_csum_errors = 0;
-		}
-	}
-
-	/* Clear the Tx errors stats */
-	for (i = 0; i != eth_dev->data->nb_tx_queues; ++i) {
-		txq = eth_dev->data->tx_queues[i];
-		if (txq) {
-			txq->over_length_pkt_cnt = 0;
-			txq->exceed_limit_bd_pkt_cnt = 0;
-			txq->exceed_limit_bd_reassem_fail = 0;
-			txq->unsupported_tunnel_pkt_cnt = 0;
-			txq->queue_full_cnt = 0;
-			txq->pkt_padding_fail_cnt = 0;
 		}
 	}
 
@@ -1053,6 +1038,38 @@ hns3_dev_xstats_get_names_by_id(struct rte_eth_dev *dev,
 	return size;
 }
 
+static void
+hns3_tqp_dfx_stats_clear(struct rte_eth_dev *dev)
+{
+	struct hns3_rx_queue *rxq;
+	struct hns3_tx_queue *txq;
+	int i;
+
+	/* Clear Rx dfx stats */
+	for (i = 0; i < dev->data->nb_rx_queues; ++i) {
+		rxq = dev->data->rx_queues[i];
+		if (rxq) {
+			rxq->l3_csum_errors = 0;
+			rxq->l4_csum_errors = 0;
+			rxq->ol3_csum_errors = 0;
+			rxq->ol4_csum_errors = 0;
+		}
+	}
+
+	/* Clear Tx dfx stats */
+	for (i = 0; i < dev->data->nb_tx_queues; ++i) {
+		txq = dev->data->tx_queues[i];
+		if (txq) {
+			txq->over_length_pkt_cnt = 0;
+			txq->exceed_limit_bd_pkt_cnt = 0;
+			txq->exceed_limit_bd_reassem_fail = 0;
+			txq->unsupported_tunnel_pkt_cnt = 0;
+			txq->queue_full_cnt = 0;
+			txq->pkt_padding_fail_cnt = 0;
+		}
+	}
+}
+
 int
 hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 {
@@ -1068,6 +1085,8 @@ hns3_dev_xstats_reset(struct rte_eth_dev *dev)
 	/* Clear reset stats */
 	memset(&hns->hw.reset.stats, 0, sizeof(struct hns3_reset_stats));
 
+	hns3_tqp_dfx_stats_clear(dev);
+
 	if (hns->is_vf)
 		return 0;
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 5/6] net/hns3: fix crash with multi-process

[Lijun Ou]

From: "Min Hu (Connor)" <humin29@huawei.com>

In current version, procedure of saving eth_dev in
hns3 PMD init will be called more than twice, one
for primary, the other for secondary. That will cause
segmentation fault in Multi-process as eth_dev will
be changed in secondary process, which is different
from one in primary process.

The initial problem was access to 'rte_eth_devices'
global variable, which is wrong. But current approach
can cause problem for the secondaries, moving 'eth_dev'
to process private can work but before making things
more complex.

This patch deserted the procedure of saving eth_dev in
hns3 PMD init. Instead, it creates an internal function
that gets "struct hns3_hw" as parameter and it can be
called internally without knowing 'eth_dev'and the
.dev_ops can be wrapper to this.

Fixes: 5e8d3de4ca36 ("net/hns3: fix FEC state query")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 16 ++++++++++------
 drivers/net/hns3/hns3_ethdev.h |  2 --
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 7c34e38..90544fe 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5821,10 +5821,9 @@ get_current_fec_auto_state(struct hns3_hw *hw, uint8_t *state)
 }
 
 static int
-hns3_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+hns3_fec_get_internal(struct hns3_hw *hw, uint32_t *fec_capa)
 {
 #define QUERY_ACTIVE_SPEED	1
-	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct hns3_sfp_speed_cmd *resp;
 	uint32_t tmp_fec_capa;
 	uint8_t auto_state;
@@ -5885,6 +5884,14 @@ hns3_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
 }
 
 static int
+hns3_fec_get(struct rte_eth_dev *dev, uint32_t *fec_capa)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	return hns3_fec_get_internal(hw, fec_capa);
+}
+
+static int
 hns3_set_fec_hw(struct hns3_hw *hw, uint32_t mode)
 {
 	struct hns3_config_fec_cmd *req;
@@ -6017,10 +6024,9 @@ hns3_query_dev_fec_info(struct hns3_hw *hw)
 {
 	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
 	struct hns3_pf *pf = HNS3_DEV_PRIVATE_TO_PF(hns);
-	struct rte_eth_dev *eth_dev = hns->eth_dev;
 	int ret;
 
-	ret = hns3_fec_get(eth_dev, &pf->fec_mode);
+	ret = hns3_fec_get_internal(hw, &pf->fec_mode);
 	if (ret)
 		hns3_err(hw, "query device FEC info failed, ret = %d", ret);
 
@@ -6106,8 +6112,6 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	hns->eth_dev = eth_dev;
-
 	eth_dev->process_private = (struct hns3_process_private *)
 	    rte_zmalloc_socket("hns3_filter_list",
 			       sizeof(struct hns3_process_private),
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 8d6b8cd..31f78a1 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -743,8 +743,6 @@ struct hns3_adapter {
 		struct hns3_vf vf;
 	};
 
-	struct rte_eth_dev *eth_dev;
-
 	bool rx_simple_allowed;
 	bool rx_vec_allowed;
 	bool tx_simple_allowed;
-- 
2.7.4

[dpdk-dev] [PATCH v2 6/6] net/hns3: remove unnecessary memset

[Lijun Ou]

The hns3_cmd_desc has memset when setup and the memset
for req is unnecessary.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index e2f0468..b5df374 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -633,16 +633,11 @@ hns3_set_rss_tc_mode(struct hns3_hw *hw)
 static void
 hns3_rss_tuple_uninit(struct hns3_hw *hw)
 {
-	struct hns3_rss_input_tuple_cmd *req;
 	struct hns3_cmd_desc desc;
 	int ret;
 
 	hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_RSS_INPUT_TUPLE, false);
 
-	req = (struct hns3_rss_input_tuple_cmd *)desc.data;
-
-	memset(req, 0, sizeof(struct hns3_rss_tuple_cfg));
-
 	ret = hns3_cmd_send(hw, &desc, 1);
 	if (ret) {
 		hns3_err(hw, "RSS uninit tuple failed %d", ret);
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 0/6] some bugfixes for hns3

[Ferruh Yigit]

On 1/6/2021 3:46 AM, Lijun Ou wrote:
> Here series fix some hns3 PMD bugs.
> 
> Change frome V1:
> 1. remove the fixes patch[6/7] from V1
> 2. fix ferruh Yigit's comments.
> 
> Huisong Li (3):
>    net/hns3: fix xstats statistics with id and names
>    net/hns3: fix abnormal return value in xstats
>    net/hns3: fix Rx/Tx abnormal errors stats
> 
> Lijun Ou (2):
>    net/hns3: fix incorrect interception with filter director
>    net/hns3: remove unnecessary memset
> 
> Min Hu (Connor) (1):
>    net/hns3: fix crash with multi-process
> 

Series applied to dpdk-next-net/main, thanks.