Continuous pfc frames may cause tx timeout.
Therefore, pfc frame statistics are added to logs.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h             | 2 ++
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c         | 6 ++++--
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 ++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index ff475b0eac22..bf1e386617bc 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -209,6 +209,8 @@ struct hnae3_queue {
 struct hns3_mac_stats {
 	u64 tx_pause_cnt;
 	u64 rx_pause_cnt;
+	u64 tx_pfc_cnt;
+	u64 rx_pfc_cnt;
 };
 
 /* hnae3 loop mode */
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index b618797a7e8d..8e237f0f4fc9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2871,8 +2871,10 @@ static bool hns3_get_tx_timeo_queue_info(struct net_device *ndev)
 		struct hns3_mac_stats mac_stats;
 
 		h->ae_algo->ops->get_mac_stats(h, &mac_stats);
-		netdev_info(ndev, "tx_pause_cnt: %llu, rx_pause_cnt: %llu\n",
-			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt);
+		netdev_info(ndev,
+			    "tx_pause_cnt: %llu, rx_pause_cnt: %llu, tx_pfc_cnt: %llu, rx_pfc_cnt: %llu\n",
+			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt,
+			    mac_stats.tx_pfc_cnt, mac_stats.rx_pfc_cnt);
 	}
 
 	hns3_dump_queue_reg(ndev, tx_ring);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index cf85ef55a0f4..f70a1159de40 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -775,6 +775,8 @@ static void hclge_get_mac_stat(struct hnae3_handle *handle,
 
 	mac_stats->tx_pause_cnt = hdev->mac_stats.mac_tx_mac_pause_num;
 	mac_stats->rx_pause_cnt = hdev->mac_stats.mac_rx_mac_pause_num;
+	mac_stats->tx_pfc_cnt = hdev->mac_stats.mac_tx_pfc_pause_pkt_num;
+	mac_stats->rx_pfc_cnt = hdev->mac_stats.mac_rx_pfc_pause_pkt_num;
 }
 
 static int hclge_parse_func_status(struct hclge_dev *hdev,
-- 
2.30.0

Fri, Jan 05, 2024 at 02:01:19AM CET, shaojijie@huawei.com wrote:
>Continuous pfc frames may cause tx timeout.
>Therefore, pfc frame statistics are added to logs.
>
>Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>Reviewed-by: Simon Horman <horms@kernel.org>
>---
> drivers/net/ethernet/hisilicon/hns3/hnae3.h             | 2 ++
> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c         | 6 ++++--
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 ++
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
>index ff475b0eac22..bf1e386617bc 100644
>--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
>+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
>@@ -209,6 +209,8 @@ struct hnae3_queue {
> struct hns3_mac_stats {
> 	u64 tx_pause_cnt;
> 	u64 rx_pause_cnt;
>+	u64 tx_pfc_cnt;
>+	u64 rx_pfc_cnt;
> };
> 
> /* hnae3 loop mode */
>diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>index b618797a7e8d..8e237f0f4fc9 100644
>--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>@@ -2871,8 +2871,10 @@ static bool hns3_get_tx_timeo_queue_info(struct net_device *ndev)
> 		struct hns3_mac_stats mac_stats;
> 
> 		h->ae_algo->ops->get_mac_stats(h, &mac_stats);
>-		netdev_info(ndev, "tx_pause_cnt: %llu, rx_pause_cnt: %llu\n",
>-			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt);
>+		netdev_info(ndev,
>+			    "tx_pause_cnt: %llu, rx_pause_cnt: %llu, tx_pfc_cnt: %llu, rx_pfc_cnt: %llu\n",
>+			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt,
>+			    mac_stats.tx_pfc_cnt, mac_stats.rx_pfc_cnt);

Don't we have a better way to expose this? I mean, whenever there is a
patch that extends the amount of text written in dmesg, it smells.
We should rather reduce it.


> 	}
> 
> 	hns3_dump_queue_reg(ndev, tx_ring);
>diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>index cf85ef55a0f4..f70a1159de40 100644
>--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>@@ -775,6 +775,8 @@ static void hclge_get_mac_stat(struct hnae3_handle *handle,
> 
> 	mac_stats->tx_pause_cnt = hdev->mac_stats.mac_tx_mac_pause_num;
> 	mac_stats->rx_pause_cnt = hdev->mac_stats.mac_rx_mac_pause_num;
>+	mac_stats->tx_pfc_cnt = hdev->mac_stats.mac_tx_pfc_pause_pkt_num;
>+	mac_stats->rx_pfc_cnt = hdev->mac_stats.mac_rx_pfc_pause_pkt_num;
> }
> 
> static int hclge_parse_func_status(struct hclge_dev *hdev,
>-- 
>2.30.0
>
>

on 2024/1/5 17:55, Jiri Pirko wrote:
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>> @@ -2871,8 +2871,10 @@ static bool hns3_get_tx_timeo_queue_info(struct net_device *ndev)
>> 		struct hns3_mac_stats mac_stats;
>>
>> 		h->ae_algo->ops->get_mac_stats(h, &mac_stats);
>> -		netdev_info(ndev, "tx_pause_cnt: %llu, rx_pause_cnt: %llu\n",
>> -			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt);
>> +		netdev_info(ndev,
>> +			    "tx_pause_cnt: %llu, rx_pause_cnt: %llu, tx_pfc_cnt: %llu, rx_pfc_cnt: %llu\n",
>> +			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt,
>> +			    mac_stats.tx_pfc_cnt, mac_stats.rx_pfc_cnt);
> Don't we have a better way to expose this? I mean, whenever there is a
> patch that extends the amount of text written in dmesg, it smells.
> We should rather reduce it.
>
In fact, we include this part of the statistics in the ethtool -S 
statistics. However, if tx timeout occurs,the driver performs a reset 
attempt to recover it. And the statistics are cleared after the reset. 
Therefore, pfc statistics are added to tx timeout log to determine the 
timeout cause.

Tue, Jan 09, 2024 at 09:19:48AM CET, shaojijie@huawei.com wrote:
>
>on 2024/1/5 17:55, Jiri Pirko wrote:
>> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>> > @@ -2871,8 +2871,10 @@ static bool hns3_get_tx_timeo_queue_info(struct net_device *ndev)
>> > 		struct hns3_mac_stats mac_stats;
>> > 
>> > 		h->ae_algo->ops->get_mac_stats(h, &mac_stats);
>> > -		netdev_info(ndev, "tx_pause_cnt: %llu, rx_pause_cnt: %llu\n",
>> > -			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt);
>> > +		netdev_info(ndev,
>> > +			    "tx_pause_cnt: %llu, rx_pause_cnt: %llu, tx_pfc_cnt: %llu, rx_pfc_cnt: %llu\n",
>> > +			    mac_stats.tx_pause_cnt, mac_stats.rx_pause_cnt,
>> > +			    mac_stats.tx_pfc_cnt, mac_stats.rx_pfc_cnt);
>> Don't we have a better way to expose this? I mean, whenever there is a
>> patch that extends the amount of text written in dmesg, it smells.
>> We should rather reduce it.
>> 
>In fact, we include this part of the statistics in the ethtool -S statistics.
>However, if tx timeout occurs,the driver performs a reset attempt to recover
>it. And the statistics are cleared after the reset. Therefore, pfc statistics
>are added to tx timeout log to determine the timeout cause.

Does not sound correct at all. You are basically forcing user to check
the dmesg to understand the behaviour of stats he gets from ethtool. You
can expose "reset"/"recover" counter through ethtool to expose this fact
rather than dmesg print. Please don't add dmesg print.

>
>