[PATCH net-next v18 0/4] net: hinic3: Support ethtool ops

[PATCH net-next v18 0/4] net: hinic3: Support ethtool ops

[Fan Gong]

This is [3/3] part of hinic3 Ethernet driver second submission.
With this patch hinic3 becomes a complete Ethernet driver.

These patch-set will include all ethtool interfaces for hinic3. To facilitate
review, this interfaces will be Split into multiple submissions, with each
commit limited to approximately 1000 lines of code.

Add ethtool ops:
    .get_strings
    .get_sset_count
    .get_ethtool_stats
    .get_eth_phy_stats
    .get_eth_mac_stats
    .get_eth_ctrl_stats
    .get_rmon_stats
    .get_pause_stats
    .get_pauseparam
    .set_pauseparam
Add netdev ops:
    .get_queue_stats_rx
    .get_queue_stats_tx
    .get_base_stats

Changes:

PATCH 03 V01: https://lore.kernel.org/netdev/cover.1773387649.git.zhuyikai1@h-partners.com/
* Add rmon/pause/phy/mac/ctrl stats (Ioana Ciornei)

PATCH 03 V02: https://lore.kernel.org/netdev/cover.1774684571.git.zhuyikai1@h-partners.com/
* Remove OutOfRangeLengthField, FrameToolong and InRangeLengthErrors (Ioana Ciornei)
* Remove redundant mtu commit (Maxime Chevallier)
* Modify "return -EINVAL" intension problem (AI Review)
* Use le16_to_cpu for rss_indir pair.out->buf (AI Review)
* Use u32 instead of int in coalesce_limits to avoid overflow (AI Review)
* Remove redundant u64_stats_update_begin/end when reading stats without
  concurrent reader (AI Review)
* Modify nic_dev->stats.syncp logic (AI Review)
* Complete rxq/txq stats stats fileds in hinic3_rx/txq_get_stats (AI Review)
* Remove statistics values in rtnl_link_stats64 from ethtool statistics
  values (AI Review)
* Add channel_cfg_lock & channel_res_lock to protect resources access (AI Review)

PATCH 03 V03: https://lore.kernel.org/netdev/cover.1774940117.git.zhuyikai1@h-partners.com/
* Change unnedd to unneeded (AI Review)
* Remove packets,bytes,errors and dropped in hinic3_rx/tx_queue_stats (AI Review)
* Remove duplicated entried in hinic3_port_stats[] (AI Review)
* change stats_info.head.status to ps->head.status (AI Review)

PATCH 03 V04: https://lore.kernel.org/netdev/cover.1775618797.git.zhuyikai1@h-partners.com/
* Use old_q_param to store old config and use it in error handling (Mohsin Bashir)
* Add netdev_info to inform the user that depth is trimmed (Mohsin Bashir)
* Remove const in hinic3_get_qp_stats_strings parameters (Mohsin Bashir)
* Change EOPNOTSUPP to ERANGE in is_coalesce_exceed_limit (Mohsin Bashir)
* Update nic_dev->rss_type after hinic3_set_rss_type (Mohsin Bashir)
* Modify MGMT_STATUS_CMD_UNSUPPORTED to EOPNOTSUPP for complying with the
  error code specifications (Mohsin Bashir)
* Remove restore_drop_sge in hinic3_rx_queue_stats (AI Review)
* Remove hinic3_nic_stats (AI Review)

PATCH 03 V05: https://lore.kernel.org/netdev/cover.1775711066.git.zhuyikai1@h-partners.com/
* As tx and rx share interrupts, we only use ETHTOOL_COALESCE_RX_USECS for
  user setting to avoid user misunderstanding. So we do not add
  ETHTOOL_COALESCE_TX_USECS. (Mohsin Bashir & AI Review)
* Clear HINIC3_CHANGE_RES_INVALID bit in error handling (AI Review)
* Use low >= high to avoid low=high in is_coalesce_legal (AI Review)

PATCH 03 V06: https://lore.kernel.org/netdev/cover.1779867397.git.zhuyikai1@h-partners.com/
* Remove redundant rx_jumbo_pending and rx_mini_pending judgement (Jakub Kicinski)
* Remove redundant max tx_pending judgement when .get_ringparam already got the
   max value (Jakub Kicinski)
* Use extack instead of netdev_err/netdev_info/netdev_warning (Jakub Kicinski)
* Remove HINIC3_CHANNEL_RES_VALID and only use HINIC3_CHANGE_RES_INVALID
  bit (Jakub Kicinski)
* Deference freed pointers in hinic3_change_channel_settings error
  handling (Jakub Kicinski)
* Modify hinic3_open_channel (Jakub Kicinski)

PATCH 03 V07: https://lore.kernel.org/netdev/cover.1779940072.git.zhuyikai1@h-partners.com/
* Remove the trailing '\n' in NL_SET_ERR_MSG* (Jakub Kicinski)

PATCH 03 V08: https://lore.kernel.org/netdev/cover.1780907605.git.wudi234@huawei.com/
* Update patch commit to be more accurate for change information (AI Review)
* Modify <net/devlink.h> to <linux/netlink.h> (AI Review)
* Use sq/rq_depth instead of rx/txqs[0].q_depth (AI Review)
* hinic3_change_channel_settings() runs under change_res_mutex
  to protect dynamic channel updates (AI Review)
* hinic3_close() serializes close and channel reconfiguration paths (AI Review)
* Remove useless u64_stats_init in hinic3_get_drv_queue_stats() (AI Review)
* hinic3_get_drv_queue_stats() fills rxq stats at correct idx
  when txq is null (AI Review)
* Remove unnecessary semicolons after the closing bracket of
  static inline functions (AI Review)
* Disable preemption in hinic3_rx_fill_buffers() (AI Review)
* Add spinlock to protect per-queue coalesce parameters from
  concurrent access (AI Review)
* Reject static coalesce config when adaptive RX coalesce is
  enabled (AI Review)
* Validate interface state and queue ID before modifying coalesce
  parameters (AI Review)
* Differentiate get_coalesce output for adaptive/static modes and
  zero-fill the struct (AI Review)
* Return -ERANGE for invalid coalesce frame limits and simplify
  error message (AI Review)
* Remove unused watchdog timeout definition and assignment (AI Review)
* Return -EOPNOTSUPP when RSS type query is unsupported by firmware (AI Review)
* Reject L4 hash bits for pure L3 flow types (AI Review)
* Split RSS type conversion to correctly handle L3-only flows (AI Review)
* Reprogram RSS indir table and HW parameters after channel count
  changes (AI Review)
* Propagate exact error code from channel parameter validation (AI Review)
* Update SW RSS indir copy only after HW programming succeeds (AI Review)
* Program RSS hash type to HW before updating SW state to prevent
  inconsistency (AI Review)

PATCH 03 V09: https://lore.kernel.org/netdev/cover.1781062575.git.wudi234@huawei.com/
* Remove useless messages in hinic3_set_ringparam() (Jakub Kicinski)
* Solve the big-endian and little-endian issue (Jakub Kicinski)

PATCH 03 V10: https://lore.kernel.org/netdev/cover.1782718232.git.wudi234@huawei.com/
* Unify 3 mutex in struct hinic3_nic_dev: port_state_mutex, change_res_mutex,
  coal_lock.(Simon Horman)
* Use rxq->q_mask instead of rxq->q_depth - 1 in hinic3_configure_rxqs()
  (Dimitri Daskalakis)
* Implement dedicated get_rx_ring_count ethtool operation (Dimitri Daskalakis)
* Correct copy-paste error in hinic3_get_ringparam() (AI Review and
  Harshitha Ramamurthy)
* Clarify extack error message in hinic3_check_ringparam_valid()
  (Harshitha Ramamurthy)
* Use rounddown_pow_of_two() instead of 1U << ilog2() in hinic3_set_ringparam()
  for readability. (Harshitha Ramamurthy)
* hinic3_set_interrupt_moder() now compare q_id and num_qps inside mutex lock
  (AI Review)
* Error branches in hinic3_link_status_change() goto unlock mutex before return
  (AI Review)
* In hinic3_change_channel_settings(), hinic3_config_num_qps() and
  hinic3_alloc_channel_resources() now run inside mutex lock. (AI Review)
* Call hinic3_free_qp_ctxts() in error paths of hinic3_open_channel(),
  thus hinic3_change_channel_settings() don't call it in error path (AI Review)
* Unset HINIC3_INTF_UP during close and reopen in hinic3_change_channel_settings
* hinic3_change_channel_settings() restore previous resources before freeing
  new_qp_params (AI Review)
* Delete unused macro HINIC3_LRO_PKT_HDR_LEN (AI Review)
* Drop packets when skb_checksum_help() return error (AI Review)
* Protect interrupt coalesce parameter reads with state_lock to prevent data
  races(AI Review)
* Restore previous coalesce parameters on HW configuration failure (AI Review)
* Always report all coalesce parameters regardless of adaptive mode (AI Review)
* Safely stop DIM work when disabling adaptive RX coalescing (AI Review)
* Change coalesce disabled message from extack error to info log (AI Review)
* Pass new RSS hash type to HW instead of the old one (AI Review)
* Initialize default RSS indirection table before HW programming when not
  user-configured(AI Review)
* Revert queue count on RSS reprogramming failure (AI Review)
* Use sizeof for RSS indirection table copy to prevent size mismatch (AI Review)

PATCH 03 V11: https://lore.kernel.org/all/cover.1784538912.git.root@localhost.localdomain/
* Remove state_lock in hinic3_set_interrupt_moder() and
  hinic3_link_status_change() (Andrew Lunn and AI Review)
* In hinic3_change_channel_settings(), cur_trxq_params is read after taking lock,
  and get symmetrically restored when alloc resources failed. (AI Review)
* Remove preempt_disable()/preempt_enable() pairs in hinic3_rx_fill_buffers().
  (AI Review)
* Add a comment about csum_err, and rewrite its judgement in hinic3_rx_csum().
  (AI Review)
* Rollback hash function type to previous state if setting RSS indirection
  table or key fails (AI Review)
* Allow users to disable L3 hashing for IPV4/IPV6 flows by properly handling
  empty hash tuple in ethtool (AI Review)
* Update software queue coalesce state only after successful hardware
  configuration to avoid manual rollback (AI Review)
* Use READ_ONCE and WRITE_ONCE for adaptive_rx_coal to ensure safe
  concurrent memory access (AI Review)
* Reset RX queue DIM state to DIM_START_MEASURE when canceling
  adaptive RX coalescing work (AI Review)
* Remove unnecessary state_lock mutexes when reading queue coalesce
  parameters in IRQ and update paths (AI Review)
* Rename is_coalesce_legal to hinic3_validate_coalesce for better code
  readability and naming consistency (AI Review)

PATCH 03 V12: https://lore.kernel.org/all/cover.1785120952.git.root@localhost.localdomain/
* Remove get_val_of_ptr() which is unnecessary (Dimitri Daskalakis)
* Split the monolithic patch into multiple patches (Mohsin Bashir)
* Fix MSI-X leak and defer q_params publication and add error-path
  rollback to prevent regressions (AI Review)
* Rfactor hinic3_update_hash_func_type() (AI Review)
* Guard irq_cfg traversal with INTF_UP check to prevent NULL dereference
  and fully reinitialize DIM state on adaptive re-enable (AI Review)
* Snapshot per-queue coalesce configs before applying and rollback via
  backup array on partial failure to ensure atomic set_coalesce (AI Review)

PATCH 03 V13: https://lore.kernel.org/netdev/cover.1786774752.git.tengpeisen@huawei.com/
* Split previous version into 3 parts. The first 2 patches were sent to
  net tree, patch #5, #6 and #8 stayes in this series, and others will be sent
  in the future. (Larysa Zaremba and Jakub Kicinski)
* Remove dead size field from struct hinic3_stats and add
  HINIC3_STAT_SIZE_CHECK() with BUILD_BUG_ON_ZERO to enforce
  per-entry compile-time verification that all stats members are u64. (AI Review)
* Remove never-incremented frag_len_overflow and frag_size_err from
  hinic3_txq_stats, hinic3_txq_clean_stats(), hinic3_txq_get_stats()
  and the hinic3_tx_queue_stats ethtool table. (AI Review)
* Add missing txq_stats.busy increment in hinic3_send_one_skb() when
  returning NETDEV_TX_BUSY due to insufficient WQEBBs. (AI Review)
* Add missing txq_stats.offload_cow_skb_err increment when offload
  type is invalid in hinic3_send_one_skb(). (AI Review)
* Remove mac_tx_good_oct_num and mac_rx_good_oct_num from the
  hinic3_port_stats private table as they are already reported through
  the get_eth_mac_stats standard interface. (AI Review)
* Fix OctetsTransmittedOK/ReceivedOK mapping in
  hinic3_get_eth_mac_stats() to use mac_tx/rx_good_oct_num instead
  of mac_tx/rx_total_oct_num per RFC 2863 (error-free frames only). (AI Review)
* Add HINIC3_IS_VF guard to hinic3_get_eth_phy_stats() to prevent
  VF from reading physical port stats, consistent with the ethtool
  -S path which already gates hinic3_port_stats on PF-only. (AI Review)
* Use u64_stats_update_begin_irqsave/end_irqrestore in
  hinic3_rx_fill_buffers() where the function can be called from
  preemptible process context via hinic3_configure_rxqs(), avoiding
  seqcount deadlock on 32-bit systems. (AI Review)
* Expand state_lock protection scope in hinic3_get_coalesce() and
  hinic3_set_coalesce() to cover intr_coalesce array accesses. (AI Review)
* Use standard net_dim_work_cancel() in hinic3_set_coalesce() to cancel
  DIM work instead of manually clearing struct fields, ensuring safe DIM
  state reset and simplifying error rollback. (AI Review)

PATCH 03 V14: https://lore.kernel.org/netdev/cover.1791858167.git.gongfan1@huawei.com/
* Remove if (!nic_dev->txqs) & if (!nic_dev->rxqs). (Vadim Fedorenko)
* All comments regarding this submission have been documented and will be
  addressed incrementally in subsequent patch-set.

PATCH 03 V15: https://lore.kernel.org/netdev/cover.1792053295.git.gongfan1@huawei.com/
* Break get_ethtool_stats into the standard statistic groups.
  Only retain the counters which are not part of standard groups.
  (Andrew Lunn and AI Review)
* Netdev netlink qstats (struct netdev_stat_ops) does not be implemented
  by hinic3, it will be implemented in future. So the per-queue stats of
  rxq%d_csum_errors etc. will be retained in ethtool -S.(AI Review)
* Change netdev_err to netdev_err_once and dev_err to dev_err_once.
  (AI Review)
* The stall of hinic3_rx_fill_buffers() and LRO in extreme condition will
  go in a separate patch.(AI Review)
* Use the same algorithms for calculating the number of bytes and packets
  in the TSO and LRO statistics.(AI Review)
* Replace skb_inner_transport_header() with skb_inner_tcp_all_headers()
  will go in a separate patch.(AI Review)
* Drops taken in hinic3_xmit_frame() are counted.(AI Review)
* u64_stats_update_begin() in hinic3_rxq_clean_stats() and
  hinic3_txq_clean_stats() will go in a separate patch.(AI Review)
* get_pauseparam callback will go in a separate patch.(AI Review)

PATCH 03 V16: https://lore.kernel.org/netdev/cover.1792149958.git.gongfan1@huawei.com/
* Change lro_packets = 1 to lro_packets++.(Mohsin Bashir)
* Init txq at the correct place.(Mohsin Bashir)
* Statistics in hinic3_function_stats and rtnl_link_stats64 originate from
  different sources: the chip records the former, while the driver records
  the latter. Chip statistics are readable via ethtool -S, and driver
  statistics are returned via rtnl_link_stats64.(AI Review)
* Change u64 to __le64 in l2nic_vport_stats and mag_cmd_port_stats.(AI Review)
* Reset lro_packets and gso_packets.(AI Review)

PATCH 03 V17: https://lore.kernel.org/netdev/cover.1792378301.git.gongfan1@huawei.com/
* Use ethtool_puts().(Dimitri Daskalakis)
* Remove the casting of char *p = (char *)data.(Dimitri Daskalakis)
* It was clarified in the last patch that tx_unicast_pkts_vport and
  per-queue metrics are not standard statistics.(AI Review)
* Use le64_to_cpu() to handle variables definded as __le64.(AI Review)
* Optimize the txq initialization position to prevent out-of-bounds
  memory access.(Mohsin Bashir & AI Review)
* The coexistence of hinic3_get_stats64() and hinic3_get_pkt_stats()
  will go in the separate patch.(AI Review)

PATCH 03 V18:
* Merge the alloc_skb_err and alloc_rx_buf_err counters into a standard
  alloc_fail counter.(Jakub Kicinski)
* Rename mac_rx/tx_1519_max_* to mac_rx/tx_jumbo_*.(Jakub Kicinski)
* Remove gso_packets and lro_packets.(Jakub Kicinski)
* Implement netdev netlink qstats (struct netdev_stat_ops) in hinic3 for
  per-queue standard counter.(AI Review)
* Remove some vport counters form ethtool -S. They will go in a separate
  patch.(AI Review)
* Remove mac_tx/rx_pfc_pri_* counter from ethtool -S. They will go in a
  separate patch for the dcbnl struct ieee_pfc.(AI Review)
* Move csum_errors to csum_bad in netdev netlink qstats.(AI Review)
* Solve the coexistence of hinic3_get_stats64() and hinic3_get_pkt_stats()
  on a 32-bit system.(AI Review)
* Detailed statistics of TSO and LRO will go in a separate patch. The problem
  in dim_update_sample() will be solved.(AI Review)
* Remove hinic3_get_pkt_stats().(AI Review)
* Implement .get_pauseparam and .set_pauseparam for hinic3_get_pause_stats().
  (AI Review)

Fan Gong (4):
  hinic3: Add ethtool statistic ops part1
  hinic3: Add ethtool statistic ops part2
  hinic3: Add Add per queue netdev-genl stats
  hinic3: Add ethtool pause param ops

 .../ethernet/huawei/hinic3/hinic3_ethtool.c   | 509 ++++++++++++++++++
 .../ethernet/huawei/hinic3/hinic3_hw_intf.h   |  11 +
 .../huawei/hinic3/hinic3_mgmt_interface.h     |  37 ++
 .../huawei/hinic3/hinic3_netdev_ops.c         | 103 +++-
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.c   |  71 +++
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.h   | 110 ++++
 .../net/ethernet/huawei/hinic3/hinic3_rx.c    |  59 +-
 .../net/ethernet/huawei/hinic3/hinic3_rx.h    |  10 +-
 .../net/ethernet/huawei/hinic3/hinic3_tx.c    |  67 ++-
 .../net/ethernet/huawei/hinic3/hinic3_tx.h    |   4 +-
 10 files changed, 946 insertions(+), 35 deletions(-)


base-commit: 73022f605c447f31610b94ed4a983d2b4bf2c721
-- 
2.43.0

[PATCH net-next v18 1/4] hinic3: Add ethtool statistic ops part1

[Fan Gong]

This commit adds per-queue TX/RX software counters and firmware-backed
vport and physical port statistics to the hinic3 driver. It implements
the .get_strings, .get_sset_count and .get_ethtool_stats callbacks so
these can be read with ethtool -S, and adds packet/byte accounting to
the RX and TX hot paths.

Co-developed-by: Wu Di <wudi234@huawei.com>
Signed-off-by: Wu Di <wudi234@huawei.com>
Co-developed-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Fan Gong <gongfan1@huawei.com>
---
 .../ethernet/huawei/hinic3/hinic3_ethtool.c   | 257 ++++++++++++++++++
 .../ethernet/huawei/hinic3/hinic3_hw_intf.h   |  11 +
 .../huawei/hinic3/hinic3_mgmt_interface.h     |  37 +++
 .../huawei/hinic3/hinic3_netdev_ops.c         |  29 +-
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.c   |  64 +++++
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.h   | 108 ++++++++
 .../net/ethernet/huawei/hinic3/hinic3_rx.c    |  59 +++-
 .../net/ethernet/huawei/hinic3/hinic3_rx.h    |  10 +-
 .../net/ethernet/huawei/hinic3/hinic3_tx.c    |  67 ++++-
 .../net/ethernet/huawei/hinic3/hinic3_tx.h    |   4 +-
 10 files changed, 611 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
index 90fc16288de9..43e0ba798cbf 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
@@ -13,8 +13,11 @@
 
 #include "hinic3_lld.h"
 #include "hinic3_hw_comm.h"
+#include "hinic3_hwif.h"
 #include "hinic3_nic_dev.h"
 #include "hinic3_nic_cfg.h"
+#include "hinic3_tx.h"
+#include "hinic3_rx.h"
 
 #define HINIC3_MGMT_VERSION_MAX_LEN     32
 
@@ -409,6 +412,257 @@ hinic3_get_link_ksettings(struct net_device *netdev,
 	return 0;
 }
 
+struct hinic3_stats {
+	char name[ETH_GSTRING_LEN];
+	int  offset;
+};
+
+#define HINIC3_STAT_SIZE_CHECK(type, member) \
+	BUILD_BUG_ON_ZERO(sizeof_field(type, member) != sizeof(u64))
+
+#define HINIC3_FUNC_STAT(_stat_item) {	\
+	.name   = #_stat_item, \
+	.offset = offsetof(struct l2nic_vport_stats, _stat_item) \
+		  + 0 * HINIC3_STAT_SIZE_CHECK(struct l2nic_vport_stats, \
+					       _stat_item) \
+}
+
+static struct hinic3_stats hinic3_function_stats[] = {
+	HINIC3_FUNC_STAT(tx_unicast_pkts_vport),
+	HINIC3_FUNC_STAT(tx_unicast_bytes_vport),
+	HINIC3_FUNC_STAT(tx_broadcast_pkts_vport),
+	HINIC3_FUNC_STAT(tx_broadcast_bytes_vport),
+
+	HINIC3_FUNC_STAT(rx_unicast_pkts_vport),
+	HINIC3_FUNC_STAT(rx_unicast_bytes_vport),
+	HINIC3_FUNC_STAT(rx_broadcast_pkts_vport),
+	HINIC3_FUNC_STAT(rx_broadcast_bytes_vport),
+};
+
+#define HINIC3_PORT_STAT(_stat_item) { \
+	.name   = #_stat_item, \
+	.offset = offsetof(struct mag_cmd_port_stats, _stat_item) \
+		  + 0 * HINIC3_STAT_SIZE_CHECK(struct mag_cmd_port_stats, \
+					       _stat_item) \
+}
+
+static struct hinic3_stats hinic3_port_stats[] = {
+	HINIC3_PORT_STAT(mac_tx_fragment_pkts),
+	HINIC3_PORT_STAT(mac_tx_undersize_pkts),
+	HINIC3_PORT_STAT(mac_tx_undermin_pkts),
+	HINIC3_PORT_STAT(mac_tx_jumbo_bad_pkts),
+	HINIC3_PORT_STAT(mac_tx_jumbo_good_pkts),
+	HINIC3_PORT_STAT(mac_tx_oversize_pkts),
+	HINIC3_PORT_STAT(mac_tx_jabber_pkts),
+	HINIC3_PORT_STAT(mac_tx_bad_pkts),
+	HINIC3_PORT_STAT(mac_tx_bad_octs),
+	HINIC3_PORT_STAT(mac_tx_total_pkts),
+	HINIC3_PORT_STAT(mac_tx_uni_pkts),
+	HINIC3_PORT_STAT(mac_tx_err_all_pkts),
+	HINIC3_PORT_STAT(mac_tx_from_app_good_pkts),
+	HINIC3_PORT_STAT(mac_tx_from_app_bad_pkts),
+
+	HINIC3_PORT_STAT(mac_rx_undermin_pkts),
+	HINIC3_PORT_STAT(mac_rx_jumbo_bad_pkts),
+	HINIC3_PORT_STAT(mac_rx_jumbo_good_pkts),
+	HINIC3_PORT_STAT(mac_rx_bad_pkts),
+	HINIC3_PORT_STAT(mac_rx_bad_octs),
+	HINIC3_PORT_STAT(mac_rx_total_pkts),
+	HINIC3_PORT_STAT(mac_rx_uni_pkts),
+	HINIC3_PORT_STAT(mac_rx_send_app_good_pkts),
+	HINIC3_PORT_STAT(mac_rx_send_app_bad_pkts),
+	HINIC3_PORT_STAT(mac_rx_unfilter_pkts),
+};
+
+#define HINIC3_TXQ_STAT(_stat_item) { \
+	.name   = "txq%d_"#_stat_item, \
+	.offset = offsetof(struct hinic3_txq_stats, _stat_item) \
+		  + 0 * HINIC3_STAT_SIZE_CHECK(struct hinic3_txq_stats, \
+					       _stat_item) \
+}
+
+#define HINIC3_RXQ_STAT(_stat_item) { \
+	.name   = "rxq%d_"#_stat_item, \
+	.offset = offsetof(struct hinic3_rxq_stats, _stat_item) \
+		  + 0 * HINIC3_STAT_SIZE_CHECK(struct hinic3_rxq_stats, \
+					       _stat_item) \
+}
+
+static struct hinic3_stats hinic3_tx_queue_stats[] = {
+	HINIC3_TXQ_STAT(skb_pad_err),
+	HINIC3_TXQ_STAT(offload_cow_skb_err),
+	HINIC3_TXQ_STAT(map_frag_err),
+	HINIC3_TXQ_STAT(unknown_tunnel_pkt),
+};
+
+static struct hinic3_stats hinic3_rx_queue_stats[] = {
+	HINIC3_RXQ_STAT(other_errors),
+	HINIC3_RXQ_STAT(rx_buf_empty),
+};
+
+static u16 hinic3_get_hw_stats_strings(struct hinic3_nic_dev *nic_dev, u8 *p)
+{
+	u16 i, cnt = 0;
+
+	for (i = 0; i < ARRAY_SIZE(hinic3_function_stats); i++) {
+		ethtool_puts(&p, hinic3_function_stats[i].name);
+		cnt++;
+	}
+
+	if (!HINIC3_IS_VF(nic_dev->hwdev)) {
+		for (i = 0; i < ARRAY_SIZE(hinic3_port_stats); i++) {
+			ethtool_puts(&p, hinic3_port_stats[i].name);
+			cnt++;
+		}
+	}
+
+	return cnt;
+}
+
+static void hinic3_get_qp_stats_strings(struct hinic3_nic_dev *nic_dev, u8 *p)
+{
+	u16 i, j;
+
+	for (i = 0; i < nic_dev->q_params.num_qps; i++) {
+		for (j = 0; j < ARRAY_SIZE(hinic3_tx_queue_stats); j++)
+			ethtool_sprintf(&p,
+					hinic3_tx_queue_stats[j].name, i);
+	}
+
+	for (i = 0; i < nic_dev->q_params.num_qps; i++) {
+		for (j = 0; j < ARRAY_SIZE(hinic3_rx_queue_stats); j++)
+			ethtool_sprintf(&p,
+					hinic3_rx_queue_stats[j].name, i);
+	}
+}
+
+static void hinic3_get_strings(struct net_device *netdev,
+			       u32 stringset, u8 *data)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	u16 offset;
+
+	switch (stringset) {
+	case ETH_SS_STATS:
+		offset = hinic3_get_hw_stats_strings(nic_dev, data);
+		hinic3_get_qp_stats_strings(nic_dev,
+					    data + offset * ETH_GSTRING_LEN);
+
+		break;
+	}
+}
+
+static int hinic3_get_sset_count(struct net_device *netdev, int sset)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	int count, q_num;
+
+	switch (sset) {
+	case ETH_SS_STATS:
+		q_num = nic_dev->q_params.num_qps;
+		count = ARRAY_SIZE(hinic3_function_stats) +
+			(ARRAY_SIZE(hinic3_tx_queue_stats) +
+			 ARRAY_SIZE(hinic3_rx_queue_stats)) *
+			q_num;
+
+		if (!HINIC3_IS_VF(nic_dev->hwdev))
+			count += ARRAY_SIZE(hinic3_port_stats);
+
+		return count;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static u16 hinic3_get_ethtool_port_stats(struct net_device *netdev, u64 *data)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	u16 i = 0, j;
+	char *p;
+	int err;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		goto err_zero_stats;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev, "Failed to get port stats from fw\n");
+		goto err_zero_stats;
+	}
+
+	for (j = 0; j < ARRAY_SIZE(hinic3_port_stats); j++, i++) {
+		p = (char *)ps + hinic3_port_stats[j].offset;
+		data[i] = le64_to_cpu(*(__le64 *)p);
+	}
+
+	kfree(ps);
+
+	return i;
+
+err_zero_stats:
+	memset(&data[i], 0, ARRAY_SIZE(hinic3_port_stats) * sizeof(*data));
+
+	return i + ARRAY_SIZE(hinic3_port_stats);
+}
+
+static void hinic3_get_drv_queue_stats(struct net_device *netdev, u64 *data)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct hinic3_txq_stats txq_stats = {};
+	struct hinic3_rxq_stats rxq_stats = {};
+	u16 i = 0, j, qid;
+	char *p;
+
+	for (qid = 0; qid < nic_dev->q_params.num_qps; qid++) {
+		hinic3_txq_get_stats(&nic_dev->txqs[qid], &txq_stats);
+		for (j = 0; j < ARRAY_SIZE(hinic3_tx_queue_stats); j++, i++) {
+			p = (char *)&txq_stats +
+			    hinic3_tx_queue_stats[j].offset;
+			data[i] = *(u64 *)p;
+		}
+	}
+
+	i = nic_dev->q_params.num_qps * ARRAY_SIZE(hinic3_tx_queue_stats);
+	for (qid = 0; qid < nic_dev->q_params.num_qps; qid++) {
+		hinic3_rxq_get_stats(&nic_dev->rxqs[qid], &rxq_stats);
+		for (j = 0; j < ARRAY_SIZE(hinic3_rx_queue_stats); j++, i++) {
+			p = (char *)&rxq_stats +
+			    hinic3_rx_queue_stats[j].offset;
+			data[i] = *(u64 *)p;
+		}
+	}
+}
+
+static void hinic3_get_ethtool_stats(struct net_device *netdev,
+				     struct ethtool_stats *stats, u64 *data)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct l2nic_vport_stats vport_stats = {};
+	u16 i = 0, j;
+	char *p;
+	int err;
+
+	err = hinic3_get_vport_stats(nic_dev->hwdev,
+				     hinic3_global_func_id(nic_dev->hwdev),
+				     &vport_stats);
+	if (err)
+		netdev_err_once(netdev,
+				"Failed to get function stats from fw\n");
+
+	for (j = 0; j < ARRAY_SIZE(hinic3_function_stats); j++, i++) {
+		p = (char *)&vport_stats + hinic3_function_stats[j].offset;
+		data[i] = le64_to_cpu(*(__le64 *)p);
+	}
+
+	if (!HINIC3_IS_VF(nic_dev->hwdev))
+		i += hinic3_get_ethtool_port_stats(netdev, data + i);
+
+	hinic3_get_drv_queue_stats(netdev, data + i);
+}
+
 static const struct ethtool_ops hinic3_ethtool_ops = {
 	.supported_coalesce_params      = ETHTOOL_COALESCE_USECS |
 					  ETHTOOL_COALESCE_PKT_RATE_RX_USECS,
@@ -417,6 +671,9 @@ static const struct ethtool_ops hinic3_ethtool_ops = {
 	.get_msglevel                   = hinic3_get_msglevel,
 	.set_msglevel                   = hinic3_set_msglevel,
 	.get_link                       = ethtool_op_get_link,
+	.get_strings                    = hinic3_get_strings,
+	.get_sset_count                 = hinic3_get_sset_count,
+	.get_ethtool_stats              = hinic3_get_ethtool_stats,
 };
 
 void hinic3_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_intf.h b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_intf.h
index cfc9daa3034f..6b0f486ba590 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_hw_intf.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_hw_intf.h
@@ -53,6 +53,17 @@ static inline void mgmt_msg_params_init_default(struct mgmt_msg_params *msg_para
 	msg_params->timeout_ms = 0;
 }
 
+static inline void
+mgmt_msg_params_init_in_out(struct mgmt_msg_params *msg_params, void *in_buf,
+			    void *out_buf, u32 in_buf_size, u32 out_buf_size)
+{
+	msg_params->buf_in = in_buf;
+	msg_params->buf_out = out_buf;
+	msg_params->in_size = in_buf_size;
+	msg_params->expected_out_size = out_buf_size;
+	msg_params->timeout_ms = 0;
+}
+
 enum cfg_cmd {
 	CFG_CMD_GET_DEV_CAP = 0,
 };
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_mgmt_interface.h b/drivers/net/ethernet/huawei/hinic3/hinic3_mgmt_interface.h
index c5bca3c4af96..a2fe66a40b3f 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_mgmt_interface.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_mgmt_interface.h
@@ -143,6 +143,41 @@ struct l2nic_cmd_set_dcb_state {
 	u8                   rsvd[7];
 };
 
+struct l2nic_port_stats_info {
+	struct mgmt_msg_head msg_head;
+	u16                  func_id;
+	u16                  rsvd1;
+};
+
+struct l2nic_vport_stats {
+	__le64 tx_unicast_pkts_vport;
+	__le64 tx_unicast_bytes_vport;
+	__le64 tx_multicast_pkts_vport;
+	__le64 tx_multicast_bytes_vport;
+	__le64 tx_broadcast_pkts_vport;
+	__le64 tx_broadcast_bytes_vport;
+
+	__le64 rx_unicast_pkts_vport;
+	__le64 rx_unicast_bytes_vport;
+	__le64 rx_multicast_pkts_vport;
+	__le64 rx_multicast_bytes_vport;
+	__le64 rx_broadcast_pkts_vport;
+	__le64 rx_broadcast_bytes_vport;
+
+	__le64 tx_discard_vport;
+	__le64 rx_discard_vport;
+	__le64 tx_err_vport;
+	__le64 rx_err_vport;
+};
+
+struct l2nic_cmd_vport_stats {
+	struct mgmt_msg_head     msg_head;
+	u32                      stats_size;
+	u32                      rsvd1;
+	struct l2nic_vport_stats stats;
+	u64                      rsvd2[6];
+};
+
 struct l2nic_cmd_lro_config {
 	struct mgmt_msg_head msg_head;
 	u16                  func_id;
@@ -234,6 +269,7 @@ enum l2nic_cmd {
 	L2NIC_CMD_SET_VPORT_ENABLE    = 6,
 	L2NIC_CMD_SET_RX_MODE         = 7,
 	L2NIC_CMD_SET_SQ_CI_ATTR      = 8,
+	L2NIC_CMD_GET_VPORT_STAT      = 9,
 	L2NIC_CMD_CLEAR_QP_RESOURCE   = 11,
 	L2NIC_CMD_CFG_RX_LRO          = 13,
 	L2NIC_CMD_CFG_LRO_TIMER       = 14,
@@ -272,6 +308,7 @@ enum mag_cmd {
 	MAG_CMD_SET_PORT_ENABLE = 6,
 	MAG_CMD_GET_LINK_STATUS = 7,
 
+	MAG_CMD_GET_PORT_STAT   = 151,
 	MAG_CMD_GET_PORT_INFO   = 153,
 };
 
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
index da73811641a9..c71bb3dff39c 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
@@ -802,14 +802,14 @@ static void hinic3_get_stats64(struct net_device *netdev,
 		txq_stats = &txq->txq_stats;
 		do {
 			start = u64_stats_fetch_begin(&txq_stats->syncp);
-			bytes += txq_stats->bytes;
-			packets += txq_stats->packets;
-			dropped += txq_stats->dropped;
+			bytes = txq_stats->bytes;
+			packets = txq_stats->packets;
+			dropped = txq_stats->dropped;
 		} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
+		stats->tx_packets += packets;
+		stats->tx_bytes   += bytes;
+		stats->tx_dropped += dropped;
 	}
-	stats->tx_packets = packets;
-	stats->tx_bytes   = bytes;
-	stats->tx_dropped = dropped;
 
 	bytes = 0;
 	packets = 0;
@@ -823,17 +823,16 @@ static void hinic3_get_stats64(struct net_device *netdev,
 		rxq_stats = &rxq->rxq_stats;
 		do {
 			start = u64_stats_fetch_begin(&rxq_stats->syncp);
-			bytes += rxq_stats->bytes;
-			packets += rxq_stats->packets;
-			errors += rxq_stats->csum_errors +
-				rxq_stats->other_errors;
-			dropped += rxq_stats->dropped;
+			bytes = rxq_stats->bytes;
+			packets = rxq_stats->packets;
+			errors = rxq_stats->other_errors;
+			dropped = rxq_stats->dropped;
 		} while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
+		stats->rx_packets += packets;
+		stats->rx_bytes   += bytes;
+		stats->rx_errors  += errors;
+		stats->rx_dropped += dropped;
 	}
-	stats->rx_packets = packets;
-	stats->rx_bytes   = bytes;
-	stats->rx_errors  = errors;
-	stats->rx_dropped = dropped;
 }
 
 static void hinic3_nic_set_rx_mode(struct net_device *netdev)
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
index de5a7984d2cb..e4a3fa6fb8db 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
@@ -738,3 +738,67 @@ int hinic3_get_pause_info(struct hinic3_nic_dev *nic_dev,
 	return hinic3_cfg_hw_pause(nic_dev->hwdev, MGMT_MSG_CMD_OP_GET,
 				   nic_pause);
 }
+
+int hinic3_get_vport_stats(struct hinic3_hwdev *hwdev, u16 func_id,
+			   struct l2nic_vport_stats *stats)
+{
+	struct l2nic_cmd_vport_stats vport_stats = {};
+	struct l2nic_port_stats_info stats_info = {};
+	struct mgmt_msg_params msg_params = {};
+	int err;
+
+	stats_info.func_id = func_id;
+
+	mgmt_msg_params_init_in_out(&msg_params, &stats_info, &vport_stats,
+				    sizeof(stats_info), sizeof(vport_stats));
+
+	err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_L2NIC,
+				       L2NIC_CMD_GET_VPORT_STAT, &msg_params);
+
+	if (err || vport_stats.msg_head.status) {
+		dev_err_once(hwdev->dev,
+			     "Failed to get function statistics, err: %d, status: 0x%x\n",
+			     err, vport_stats.msg_head.status);
+		return -EFAULT;
+	}
+
+	memcpy(stats, &vport_stats.stats, sizeof(*stats));
+
+	return 0;
+}
+
+int hinic3_get_phy_port_stats(struct hinic3_hwdev *hwdev,
+			      struct mag_cmd_port_stats *stats)
+{
+	struct mag_cmd_get_port_stats_in ps_in = {};
+	struct mag_cmd_get_port_stats_out *ps_out;
+	struct mgmt_msg_params msg_params = {};
+	int err;
+
+	ps_out = kzalloc_obj(*ps_out);
+	if (!ps_out)
+		return -ENOMEM;
+
+	ps_in.port_id = hinic3_physical_port_id(hwdev);
+
+	mgmt_msg_params_init_in_out(&msg_params, &ps_in, ps_out,
+				    sizeof(ps_in), sizeof(*ps_out));
+
+	err = hinic3_send_mbox_to_mgmt(hwdev, MGMT_MOD_HILINK,
+				       MAG_CMD_GET_PORT_STAT, &msg_params);
+
+	if (err || ps_out->head.status) {
+		dev_err_once(hwdev->dev,
+			     "Failed to get port statistics, err: %d, status: 0x%x\n",
+			     err, ps_out->head.status);
+		err = -EFAULT;
+		goto out;
+	}
+
+	memcpy(stats, &ps_out->counter, sizeof(*stats));
+
+out:
+	kfree(ps_out);
+
+	return err;
+}
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
index 5d52202a8d4e..63382191446d 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
@@ -129,6 +129,110 @@ struct mag_cmd_get_xsfp_present {
 	u8                   rsvd[2];
 };
 
+struct mag_cmd_port_stats {
+	__le64 mac_tx_fragment_pkts;
+	__le64 mac_tx_undersize_pkts;
+	__le64 mac_tx_undermin_pkts;
+	__le64 mac_tx_64_oct_pkts;
+	__le64 mac_tx_65_127_oct_pkts;
+	__le64 mac_tx_128_255_oct_pkts;
+	__le64 mac_tx_256_511_oct_pkts;
+	__le64 mac_tx_512_1023_oct_pkts;
+	__le64 mac_tx_1024_1518_oct_pkts;
+	__le64 mac_tx_1519_2047_oct_pkts;
+	__le64 mac_tx_2048_4095_oct_pkts;
+	__le64 mac_tx_4096_8191_oct_pkts;
+	__le64 mac_tx_8192_9216_oct_pkts;
+	__le64 mac_tx_9217_12287_oct_pkts;
+	__le64 mac_tx_12288_16383_oct_pkts;
+	__le64 mac_tx_jumbo_bad_pkts;
+	__le64 mac_tx_jumbo_good_pkts;
+	__le64 mac_tx_oversize_pkts;
+	__le64 mac_tx_jabber_pkts;
+	__le64 mac_tx_bad_pkts;
+	__le64 mac_tx_bad_octs;
+	__le64 mac_tx_good_pkts;
+	__le64 mac_tx_good_octs;
+	__le64 mac_tx_total_pkts;
+	__le64 mac_tx_total_octs;
+	__le64 mac_tx_uni_pkts;
+	__le64 mac_tx_multi_pkts;
+	__le64 mac_tx_broad_pkts;
+	__le64 mac_tx_pause_pkts;
+	__le64 mac_tx_pfc_pkts;
+	__le64 mac_tx_pfc_pri0_pkts;
+	__le64 mac_tx_pfc_pri1_pkts;
+	__le64 mac_tx_pfc_pri2_pkts;
+	__le64 mac_tx_pfc_pri3_pkts;
+	__le64 mac_tx_pfc_pri4_pkts;
+	__le64 mac_tx_pfc_pri5_pkts;
+	__le64 mac_tx_pfc_pri6_pkts;
+	__le64 mac_tx_pfc_pri7_pkts;
+	__le64 mac_tx_control_pkts;
+	__le64 mac_tx_err_all_pkts;
+	__le64 mac_tx_from_app_good_pkts;
+	__le64 mac_tx_from_app_bad_pkts;
+
+	__le64 mac_rx_fragment_pkts;
+	__le64 mac_rx_undersize_pkts;
+	__le64 mac_rx_undermin_pkts;
+	__le64 mac_rx_64_oct_pkts;
+	__le64 mac_rx_65_127_oct_pkts;
+	__le64 mac_rx_128_255_oct_pkts;
+	__le64 mac_rx_256_511_oct_pkts;
+	__le64 mac_rx_512_1023_oct_pkts;
+	__le64 mac_rx_1024_1518_oct_pkts;
+	__le64 mac_rx_1519_2047_oct_pkts;
+	__le64 mac_rx_2048_4095_oct_pkts;
+	__le64 mac_rx_4096_8191_oct_pkts;
+	__le64 mac_rx_8192_9216_oct_pkts;
+	__le64 mac_rx_9217_12287_oct_pkts;
+	__le64 mac_rx_12288_16383_oct_pkts;
+	__le64 mac_rx_jumbo_bad_pkts;
+	__le64 mac_rx_jumbo_good_pkts;
+	__le64 mac_rx_oversize_pkts;
+	__le64 mac_rx_jabber_pkts;
+	__le64 mac_rx_bad_pkts;
+	__le64 mac_rx_bad_octs;
+	__le64 mac_rx_good_pkts;
+	__le64 mac_rx_good_octs;
+	__le64 mac_rx_total_pkts;
+	__le64 mac_rx_total_octs;
+	__le64 mac_rx_uni_pkts;
+	__le64 mac_rx_multi_pkts;
+	__le64 mac_rx_broad_pkts;
+	__le64 mac_rx_pause_pkts;
+	__le64 mac_rx_pfc_pkts;
+	__le64 mac_rx_pfc_pri0_pkts;
+	__le64 mac_rx_pfc_pri1_pkts;
+	__le64 mac_rx_pfc_pri2_pkts;
+	__le64 mac_rx_pfc_pri3_pkts;
+	__le64 mac_rx_pfc_pri4_pkts;
+	__le64 mac_rx_pfc_pri5_pkts;
+	__le64 mac_rx_pfc_pri6_pkts;
+	__le64 mac_rx_pfc_pri7_pkts;
+	__le64 mac_rx_control_pkts;
+	__le64 mac_rx_sym_err_pkts;
+	__le64 mac_rx_fcs_err_pkts;
+	__le64 mac_rx_send_app_good_pkts;
+	__le64 mac_rx_send_app_bad_pkts;
+	__le64 mac_rx_unfilter_pkts;
+};
+
+struct mag_cmd_get_port_stats_in {
+	struct mgmt_msg_head head;
+
+	u8                   port_id;
+	u8                   rsvd0[3];
+};
+
+struct mag_cmd_get_port_stats_out {
+	struct mgmt_msg_head      head;
+
+	struct mag_cmd_port_stats counter;
+	u64                       rsvd1[15];
+};
+
 enum link_err_type {
 	LINK_ERR_MODULE_UNRECOGENIZED,
 	LINK_ERR_NUM,
@@ -215,4 +319,8 @@ int hinic3_del_vlan(struct hinic3_hwdev *hwdev, u16 vlan_id, u16 func_id);
 int hinic3_get_pause_info(struct hinic3_nic_dev *nic_dev,
 			  struct hinic3_nic_pause_config *nic_pause);
 
+int hinic3_get_vport_stats(struct hinic3_hwdev *hwdev, u16 func_id,
+			   struct l2nic_vport_stats *stats);
+int hinic3_get_phy_port_stats(struct hinic3_hwdev *hwdev,
+			      struct mag_cmd_port_stats *stats);
 #endif
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
index 309ab5901379..ae4ccff4a09a 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
@@ -43,10 +43,7 @@ static void hinic3_rxq_clean_stats(struct hinic3_rxq_stats *rxq_stats)
 	rxq_stats->other_errors = 0;
 	rxq_stats->dropped = 0;
 	rxq_stats->rx_buf_empty = 0;
-
-	rxq_stats->alloc_skb_err = 0;
-	rxq_stats->alloc_rx_buf_err = 0;
-	rxq_stats->restore_drop_sge = 0;
+	rxq_stats->alloc_fail = 0;
 	u64_stats_update_end(&rxq_stats->syncp);
 }
 
@@ -147,6 +144,8 @@ static u32 hinic3_rx_fill_buffers(struct hinic3_rxq *rxq)
 {
 	u32 i, free_wqebbs = rxq->delta - 1;
 	struct hinic3_rx_info *rx_info;
+	struct u64_stats_sync *syncp;
+	unsigned long flags;
 	dma_addr_t dma_addr;
 	int err;
 
@@ -155,8 +154,13 @@ static u32 hinic3_rx_fill_buffers(struct hinic3_rxq *rxq)
 
 		err = rx_alloc_mapped_page(rxq->page_pool, rx_info,
 					   rxq->buf_len);
-		if (unlikely(err))
+		if (unlikely(err)) {
+			syncp = &rxq->rxq_stats.syncp;
+			flags = u64_stats_update_begin_irqsave(syncp);
+			rxq->rxq_stats.alloc_fail++;
+			u64_stats_update_end_irqrestore(syncp, flags);
 			break;
+		}
 
 		dma_addr = page_pool_get_dma_addr(rx_info->page) +
 			rx_info->page_offset;
@@ -170,6 +174,11 @@ static u32 hinic3_rx_fill_buffers(struct hinic3_rxq *rxq)
 				rxq->next_to_update << HINIC3_NORMAL_RQ_WQE);
 		rxq->delta -= i;
 		rxq->next_to_alloc = rxq->next_to_update;
+	} else if (free_wqebbs == rxq->q_depth - 1) {
+		syncp = &rxq->rxq_stats.syncp;
+		flags = u64_stats_update_begin_irqsave(syncp);
+		rxq->rxq_stats.rx_buf_empty++;
+		u64_stats_update_end_irqrestore(syncp, flags);
 	}
 
 	return i;
@@ -330,10 +339,23 @@ static void hinic3_rx_csum(struct hinic3_rxq *rxq, u32 offload_type,
 	struct net_device *netdev = rxq->netdev;
 	bool l2_tunnel;
 
+	/* HW ensures that only 1 bit is set in csum_err. */
+	if (unlikely(csum_err == HINIC3_RX_CSUM_IPSU_OTHER_ERR)) {
+		u64_stats_update_begin(&rxq->rxq_stats.syncp);
+		rxq->rxq_stats.other_errors++;
+		u64_stats_update_end(&rxq->rxq_stats.syncp);
+	}
+
 	if (!(netdev->features & NETIF_F_RXCSUM))
 		return;
 
 	if (unlikely(csum_err)) {
+		if (csum_err != HINIC3_RX_CSUM_HW_CHECK_NONE &&
+		    csum_err != HINIC3_RX_CSUM_IPSU_OTHER_ERR) {
+			u64_stats_update_begin(&rxq->rxq_stats.syncp);
+			rxq->rxq_stats.csum_errors++;
+			u64_stats_update_end(&rxq->rxq_stats.syncp);
+		}
 		/* pkt type is recognized by HW, and csum is wrong */
 		skb->ip_summed = CHECKSUM_NONE;
 		return;
@@ -387,8 +409,12 @@ static int recv_one_pkt(struct hinic3_rxq *rxq, struct hinic3_rq_cqe *rx_cqe,
 	u16 num_lro;
 
 	skb = hinic3_fetch_rx_buffer(rxq, pkt_len);
-	if (unlikely(!skb))
+	if (unlikely(!skb)) {
+		u64_stats_update_begin(&rxq->rxq_stats.syncp);
+		rxq->rxq_stats.alloc_fail++;
+		u64_stats_update_end(&rxq->rxq_stats.syncp);
 		return -ENOMEM;
+	}
 
 	/* place header in linear portion of buffer */
 	if (skb_is_nonlinear(skb))
@@ -555,6 +581,7 @@ int hinic3_rx_poll(struct hinic3_rxq *rxq, int budget)
 	struct hinic3_nic_dev *nic_dev = netdev_priv(rxq->netdev);
 	u32 sw_ci, status, pkt_len, vlan_len;
 	struct hinic3_rq_cqe *rx_cqe;
+	u64 rx_bytes = 0;
 	u32 num_wqe = 0;
 	int nr_pkts = 0;
 	u16 num_lro;
@@ -575,6 +602,7 @@ int hinic3_rx_poll(struct hinic3_rxq *rxq, int budget)
 			break;
 
 		nr_pkts++;
+		rx_bytes += pkt_len;
 		num_lro = RQ_CQE_STATUS_GET(status, NUM_LRO);
 		if (num_lro)
 			num_wqe += hinic3_get_sge_num(rxq, pkt_len);
@@ -588,5 +616,24 @@ int hinic3_rx_poll(struct hinic3_rxq *rxq, int budget)
 	if (rxq->delta >= HINIC3_RX_BUFFER_WRITE)
 		hinic3_rx_fill_buffers(rxq);
 
+	u64_stats_update_begin(&rxq->rxq_stats.syncp);
+	rxq->rxq_stats.packets += (u64)nr_pkts;
+	rxq->rxq_stats.bytes += rx_bytes;
+	u64_stats_update_end(&rxq->rxq_stats.syncp);
 	return nr_pkts;
 }
+
+void hinic3_rxq_get_stats(struct hinic3_rxq *rxq,
+			  struct hinic3_rxq_stats *stats)
+{
+	struct hinic3_rxq_stats *rxq_stats = &rxq->rxq_stats;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin(&rxq_stats->syncp);
+		stats->csum_errors = rxq_stats->csum_errors;
+		stats->other_errors = rxq_stats->other_errors;
+		stats->rx_buf_empty = rxq_stats->rx_buf_empty;
+		stats->alloc_fail = rxq_stats->alloc_fail;
+	} while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
+}
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.h b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.h
index 06d1b3299e7c..a54184beedc9 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.h
@@ -8,6 +8,10 @@
 #include <linux/dim.h>
 #include <linux/netdevice.h>
 
+/* rx cqe checksum err */
+#define HINIC3_RX_CSUM_HW_CHECK_NONE    BIT(7)
+#define HINIC3_RX_CSUM_IPSU_OTHER_ERR   BIT(8)
+
 #define RQ_CQE_OFFOLAD_TYPE_PKT_TYPE_MASK           GENMASK(4, 0)
 #define RQ_CQE_OFFOLAD_TYPE_IP_TYPE_MASK            GENMASK(6, 5)
 #define RQ_CQE_OFFOLAD_TYPE_TUNNEL_PKT_FORMAT_MASK  GENMASK(11, 8)
@@ -37,9 +41,7 @@ struct hinic3_rxq_stats {
 	u64                   other_errors;
 	u64                   dropped;
 	u64                   rx_buf_empty;
-	u64                   alloc_skb_err;
-	u64                   alloc_rx_buf_err;
-	u64                   restore_drop_sge;
+	u64                   alloc_fail;
 	struct u64_stats_sync syncp;
 };
 
@@ -125,4 +127,6 @@ int hinic3_configure_rxqs(struct net_device *netdev, u16 num_rq,
 			  u32 rq_depth, struct hinic3_dyna_rxq_res *rxqs_res);
 int hinic3_rx_poll(struct hinic3_rxq *rxq, int budget);
 
+void hinic3_rxq_get_stats(struct hinic3_rxq *rxq,
+			  struct hinic3_rxq_stats *stats);
 #endif
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c
index f92568f0f00b..b0a0ffb1052b 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c
@@ -25,11 +25,9 @@ static void hinic3_txq_clean_stats(struct hinic3_txq_stats *txq_stats)
 	txq_stats->dropped = 0;
 
 	txq_stats->skb_pad_err = 0;
-	txq_stats->frag_len_overflow = 0;
 	txq_stats->offload_cow_skb_err = 0;
 	txq_stats->map_frag_err = 0;
 	txq_stats->unknown_tunnel_pkt = 0;
-	txq_stats->frag_size_err = 0;
 	u64_stats_update_end(&txq_stats->syncp);
 }
 
@@ -97,8 +95,12 @@ static int hinic3_tx_map_skb(struct net_device *netdev, struct sk_buff *skb,
 
 	dma_info[0].dma = dma_map_single(&pdev->dev, skb->data,
 					 skb_headlen(skb), DMA_TO_DEVICE);
-	if (dma_mapping_error(&pdev->dev, dma_info[0].dma))
+	if (dma_mapping_error(&pdev->dev, dma_info[0].dma)) {
+		u64_stats_update_begin(&txq->txq_stats.syncp);
+		txq->txq_stats.map_frag_err++;
+		u64_stats_update_end(&txq->txq_stats.syncp);
 		return -EFAULT;
+	}
 
 	dma_info[0].len = skb_headlen(skb);
 
@@ -117,6 +119,9 @@ static int hinic3_tx_map_skb(struct net_device *netdev, struct sk_buff *skb,
 						     skb_frag_size(frag),
 						     DMA_TO_DEVICE);
 		if (dma_mapping_error(&pdev->dev, dma_info[idx].dma)) {
+			u64_stats_update_begin(&txq->txq_stats.syncp);
+			txq->txq_stats.map_frag_err++;
+			u64_stats_update_end(&txq->txq_stats.syncp);
 			err = -EFAULT;
 			goto err_unmap_page;
 		}
@@ -260,6 +265,9 @@ static int hinic3_tx_csum(struct hinic3_txq *txq, struct hinic3_sq_task *task,
 		if (l4_proto != IPPROTO_UDP ||
 		    ((struct udphdr *)skb_transport_header(skb))->dest !=
 		    VXLAN_OFFLOAD_PORT_LE) {
+			u64_stats_update_begin(&txq->txq_stats.syncp);
+			txq->txq_stats.unknown_tunnel_pkt++;
+			u64_stats_update_end(&txq->txq_stats.syncp);
 			/* Unsupported tunnel packet, disable csum offload */
 			return skb_checksum_help(skb);
 		}
@@ -541,8 +549,12 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
 	u32 offload;
 	int err;
 
-	if (skb_put_padto(skb, MIN_SKB_LEN))
+	if (skb_put_padto(skb, MIN_SKB_LEN)) {
+		u64_stats_update_begin(&txq->txq_stats.syncp);
+		txq->txq_stats.skb_pad_err++;
+		u64_stats_update_end(&txq->txq_stats.syncp);
 		goto err_out;
+	}
 
 	offload = hinic3_tx_offload(skb, &task, &queue_info, txq);
 	num_sge = skb_shinfo(skb)->nr_frags + 1;
@@ -557,10 +569,16 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
 					hinic3_wq_free_wqebbs(&txq->sq->wq),
 					txq->tx_start_thrs);
 
+		u64_stats_update_begin(&txq->txq_stats.syncp);
+		txq->txq_stats.busy++;
+		u64_stats_update_end(&txq->txq_stats.syncp);
 		return NETDEV_TX_BUSY;
 	}
 
 	if (unlikely(offload == HINIC3_TX_OFFLOAD_INVALID)) {
+		u64_stats_update_begin(&txq->txq_stats.syncp);
+		txq->txq_stats.offload_cow_skb_err++;
+		u64_stats_update_end(&txq->txq_stats.syncp);
 		goto err_drop_pkt;
 	} else if (!offload) {
 		wqebb_cnt -= 1;
@@ -599,11 +617,19 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
 	hinic3_write_db(txq->sq, 0, DB_CFLAG_DP_SQ,
 			hinic3_get_sq_local_pi(txq->sq));
 
+	u64_stats_update_begin(&txq->txq_stats.syncp);
+	txq->txq_stats.bytes += skb->len;
+	txq->txq_stats.packets += 1;
+	u64_stats_update_end(&txq->txq_stats.syncp);
+
 	return NETDEV_TX_OK;
 
 err_drop_pkt:
 	dev_kfree_skb_any(skb);
 err_out:
+	u64_stats_update_begin(&txq->txq_stats.syncp);
+	txq->txq_stats.dropped++;
+	u64_stats_update_end(&txq->txq_stats.syncp);
 	return NETDEV_TX_OK;
 }
 
@@ -611,18 +637,25 @@ netdev_tx_t hinic3_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
 	u16 q_id = skb_get_queue_mapping(skb);
-
-	if (unlikely(!netif_carrier_ok(netdev)))
-		goto err_drop_pkt;
+	struct hinic3_txq *txq;
 
 	if (unlikely(q_id >= nic_dev->q_params.num_qps))
+		goto err_qid_out;
+
+	txq = &nic_dev->txqs[q_id];
+
+	if (unlikely(!netif_carrier_ok(netdev)))
 		goto err_drop_pkt;
 
-	return hinic3_send_one_skb(skb, netdev, &nic_dev->txqs[q_id]);
+	return hinic3_send_one_skb(skb, netdev, txq);
 
 err_drop_pkt:
-	dev_kfree_skb_any(skb);
+	u64_stats_update_begin(&txq->txq_stats.syncp);
+	txq->txq_stats.dropped++;
+	u64_stats_update_end(&txq->txq_stats.syncp);
 
+err_qid_out:
+	dev_kfree_skb_any(skb);
 	return NETDEV_TX_OK;
 }
 
@@ -795,3 +828,19 @@ bool hinic3_tx_poll(struct hinic3_txq *txq, int budget)
 
 	return pkts == HINIC3_TX_POLL_WEIGHT;
 }
+
+void hinic3_txq_get_stats(struct hinic3_txq *txq,
+			  struct hinic3_txq_stats *stats)
+{
+	struct hinic3_txq_stats *txq_stats = &txq->txq_stats;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin(&txq_stats->syncp);
+		stats->busy = txq_stats->busy;
+		stats->skb_pad_err = txq_stats->skb_pad_err;
+		stats->offload_cow_skb_err = txq_stats->offload_cow_skb_err;
+		stats->map_frag_err = txq_stats->map_frag_err;
+		stats->unknown_tunnel_pkt = txq_stats->unknown_tunnel_pkt;
+	} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
+}
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.h b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.h
index 00194f2a1bcc..9e429362b936 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.h
@@ -106,11 +106,9 @@ struct hinic3_txq_stats {
 	u64                   busy;
 	u64                   dropped;
 	u64                   skb_pad_err;
-	u64                   frag_len_overflow;
 	u64                   offload_cow_skb_err;
 	u64                   map_frag_err;
 	u64                   unknown_tunnel_pkt;
-	u64                   frag_size_err;
 	struct u64_stats_sync syncp;
 };
 
@@ -160,4 +158,6 @@ netdev_tx_t hinic3_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
 bool hinic3_tx_poll(struct hinic3_txq *txq, int budget);
 void hinic3_flush_txqs(struct net_device *netdev);
 
+void hinic3_txq_get_stats(struct hinic3_txq *txq,
+			  struct hinic3_txq_stats *stats);
 #endif
-- 
2.43.0

[PATCH net-next v18 2/4] hinic3: Add ethtool statistic ops part2

[Fan Gong]

This commit adds five standard ethtool statistics callbacks to the hinic3
driver: get_eth_phy_stats, get_eth_mac_stats, get_eth_ctrl_stats,
get_rmon_stats and get_pause_stats. Each callback reads the physical port
counters from firmware via hinic3_get_phy_port_stats() and copies them
into the standard ethtool structures.

Co-developed-by: Wu Di <wudi234@huawei.com>
Signed-off-by: Wu Di <wudi234@huawei.com>
Co-developed-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Fan Gong <gongfan1@huawei.com>
---
 .../ethernet/huawei/hinic3/hinic3_ethtool.c   | 211 ++++++++++++++++++
 1 file changed, 211 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
index 43e0ba798cbf..e1f23e2477fa 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
@@ -663,6 +663,212 @@ static void hinic3_get_ethtool_stats(struct net_device *netdev,
 	hinic3_get_drv_queue_stats(netdev, data + i);
 }
 
+static void hinic3_get_eth_phy_stats(struct net_device *netdev,
+				     struct ethtool_eth_phy_stats *phy_stats)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		return;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		return;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev,
+				"Failed to get eth phy stats from fw\n");
+		return;
+	}
+
+	phy_stats->SymbolErrorDuringCarrier =
+		le64_to_cpu(ps->mac_rx_sym_err_pkts);
+
+	kfree(ps);
+}
+
+static void hinic3_get_eth_mac_stats(struct net_device *netdev,
+				     struct ethtool_eth_mac_stats *mac_stats)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		return;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		return;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev,
+				"Failed to get eth mac stats from fw\n");
+		return;
+	}
+
+	mac_stats->FramesTransmittedOK =
+		le64_to_cpu(ps->mac_tx_good_pkts);
+	mac_stats->FramesReceivedOK =
+		le64_to_cpu(ps->mac_rx_good_pkts);
+	mac_stats->FrameCheckSequenceErrors =
+		le64_to_cpu(ps->mac_rx_fcs_err_pkts);
+	mac_stats->OctetsTransmittedOK =
+		le64_to_cpu(ps->mac_tx_good_octs);
+	mac_stats->OctetsReceivedOK =
+		le64_to_cpu(ps->mac_rx_good_octs);
+	mac_stats->MulticastFramesXmittedOK =
+		le64_to_cpu(ps->mac_tx_multi_pkts);
+	mac_stats->BroadcastFramesXmittedOK =
+		le64_to_cpu(ps->mac_tx_broad_pkts);
+	mac_stats->MulticastFramesReceivedOK =
+		le64_to_cpu(ps->mac_rx_multi_pkts);
+	mac_stats->BroadcastFramesReceivedOK =
+		le64_to_cpu(ps->mac_rx_broad_pkts);
+
+	kfree(ps);
+}
+
+static void hinic3_get_eth_ctrl_stats(struct net_device *netdev,
+				      struct ethtool_eth_ctrl_stats *ctrl_stats)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		return;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		return;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev,
+				"Failed to get eth ctrl stats from fw\n");
+		return;
+	}
+
+	ctrl_stats->MACControlFramesTransmitted =
+		le64_to_cpu(ps->mac_tx_control_pkts);
+	ctrl_stats->MACControlFramesReceived =
+		le64_to_cpu(ps->mac_rx_control_pkts);
+
+	kfree(ps);
+}
+
+static const struct ethtool_rmon_hist_range hinic3_rmon_ranges[] = {
+	{     0,    64 },
+	{    65,   127 },
+	{   128,   255 },
+	{   256,   511 },
+	{   512,  1023 },
+	{  1024,  1518 },
+	{  1519,  2047 },
+	{  2048,  4095 },
+	{  4096,  8191 },
+	{  8192,  9216 },
+	{  9217, 12287 },
+	{}
+};
+
+static void hinic3_get_rmon_stats(struct net_device *netdev,
+				  struct ethtool_rmon_stats *rmon_stats,
+				  const struct ethtool_rmon_hist_range **ranges)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		goto get_rmon_stats_err;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		goto get_rmon_stats_err;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev,
+				"Failed to get eth rmon stats from fw\n");
+		goto get_rmon_stats_err;
+	}
+
+	rmon_stats->undersize_pkts = le64_to_cpu(ps->mac_rx_undersize_pkts);
+	rmon_stats->oversize_pkts  = le64_to_cpu(ps->mac_rx_oversize_pkts);
+	rmon_stats->fragments      = le64_to_cpu(ps->mac_rx_fragment_pkts);
+	rmon_stats->jabbers        = le64_to_cpu(ps->mac_rx_jabber_pkts);
+
+	rmon_stats->hist[0]  = le64_to_cpu(ps->mac_rx_64_oct_pkts);
+	rmon_stats->hist[1]  = le64_to_cpu(ps->mac_rx_65_127_oct_pkts);
+	rmon_stats->hist[2]  = le64_to_cpu(ps->mac_rx_128_255_oct_pkts);
+	rmon_stats->hist[3]  = le64_to_cpu(ps->mac_rx_256_511_oct_pkts);
+	rmon_stats->hist[4]  = le64_to_cpu(ps->mac_rx_512_1023_oct_pkts);
+	rmon_stats->hist[5]  = le64_to_cpu(ps->mac_rx_1024_1518_oct_pkts);
+	rmon_stats->hist[6]  = le64_to_cpu(ps->mac_rx_1519_2047_oct_pkts);
+	rmon_stats->hist[7]  = le64_to_cpu(ps->mac_rx_2048_4095_oct_pkts);
+	rmon_stats->hist[8]  = le64_to_cpu(ps->mac_rx_4096_8191_oct_pkts);
+	rmon_stats->hist[9]  = le64_to_cpu(ps->mac_rx_8192_9216_oct_pkts);
+	rmon_stats->hist[10] = le64_to_cpu(ps->mac_rx_9217_12287_oct_pkts);
+
+	rmon_stats->hist_tx[0]  = le64_to_cpu(ps->mac_tx_64_oct_pkts);
+	rmon_stats->hist_tx[1]  = le64_to_cpu(ps->mac_tx_65_127_oct_pkts);
+	rmon_stats->hist_tx[2]  = le64_to_cpu(ps->mac_tx_128_255_oct_pkts);
+	rmon_stats->hist_tx[3]  = le64_to_cpu(ps->mac_tx_256_511_oct_pkts);
+	rmon_stats->hist_tx[4]  = le64_to_cpu(ps->mac_tx_512_1023_oct_pkts);
+	rmon_stats->hist_tx[5]  = le64_to_cpu(ps->mac_tx_1024_1518_oct_pkts);
+	rmon_stats->hist_tx[6]  = le64_to_cpu(ps->mac_tx_1519_2047_oct_pkts);
+	rmon_stats->hist_tx[7]  = le64_to_cpu(ps->mac_tx_2048_4095_oct_pkts);
+	rmon_stats->hist_tx[8]  = le64_to_cpu(ps->mac_tx_4096_8191_oct_pkts);
+	rmon_stats->hist_tx[9]  = le64_to_cpu(ps->mac_tx_8192_9216_oct_pkts);
+	rmon_stats->hist_tx[10] = le64_to_cpu(ps->mac_tx_9217_12287_oct_pkts);
+
+	*ranges = hinic3_rmon_ranges;
+
+	kfree(ps);
+	return;
+
+get_rmon_stats_err:
+	*ranges = NULL;
+}
+
+static void hinic3_get_pause_stats(struct net_device *netdev,
+				   struct ethtool_pause_stats *pause_stats)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct mag_cmd_port_stats *ps;
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		return;
+
+	ps = kmalloc_obj(*ps);
+	if (!ps)
+		return;
+
+	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
+	if (err) {
+		kfree(ps);
+		netdev_err_once(netdev,
+				"Failed to get eth pause stats from fw\n");
+		return;
+	}
+
+	pause_stats->tx_pause_frames = le64_to_cpu(ps->mac_tx_pause_pkts);
+	pause_stats->rx_pause_frames = le64_to_cpu(ps->mac_rx_pause_pkts);
+
+	kfree(ps);
+}
+
 static const struct ethtool_ops hinic3_ethtool_ops = {
 	.supported_coalesce_params      = ETHTOOL_COALESCE_USECS |
 					  ETHTOOL_COALESCE_PKT_RATE_RX_USECS,
@@ -674,6 +880,11 @@ static const struct ethtool_ops hinic3_ethtool_ops = {
 	.get_strings                    = hinic3_get_strings,
 	.get_sset_count                 = hinic3_get_sset_count,
 	.get_ethtool_stats              = hinic3_get_ethtool_stats,
+	.get_eth_phy_stats              = hinic3_get_eth_phy_stats,
+	.get_eth_mac_stats              = hinic3_get_eth_mac_stats,
+	.get_eth_ctrl_stats             = hinic3_get_eth_ctrl_stats,
+	.get_rmon_stats                 = hinic3_get_rmon_stats,
+	.get_pause_stats                = hinic3_get_pause_stats,
 };
 
 void hinic3_set_ethtool_ops(struct net_device *netdev)
-- 
2.43.0

[PATCH net-next v18 3/4] hinic3: Add Add per queue netdev-genl stats

[Fan Gong]

Co-developed-by: Wu Di <wudi234@huawei.com>
Signed-off-by: Wu Di <wudi234@huawei.com>
Co-developed-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Fan Gong <gongfan1@huawei.com>
---
 .../huawei/hinic3/hinic3_netdev_ops.c         | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
index c71bb3dff39c..10364c4dc11f 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
@@ -5,6 +5,7 @@
 #include <linux/if_vlan.h>
 #include <linux/netdevice.h>
 #include <net/vxlan.h>
+#include <net/netdev_queues.h>
 
 #include "hinic3_hwif.h"
 #include "hinic3_nic_cfg.h"
@@ -866,7 +867,80 @@ static const struct net_device_ops hinic3_netdev_ops = {
 	.ndo_start_xmit       = hinic3_xmit_frame,
 };
 
+static void hinic3_get_queue_stats_rx(struct net_device *netdev, int i,
+				      struct netdev_queue_stats_rx *stats_rx)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct hinic3_rxq_stats *rxq_stats;
+	struct hinic3_rxq *rxq;
+	unsigned int start;
+
+	rxq = &nic_dev->rxqs[i];
+	rxq_stats = &rxq->rxq_stats;
+	do {
+		start = u64_stats_fetch_begin(&rxq_stats->syncp);
+		stats_rx->bytes = rxq_stats->bytes;
+		stats_rx->packets = rxq_stats->packets;
+		stats_rx->csum_bad = rxq_stats->csum_errors;
+		stats_rx->alloc_fail = rxq_stats->alloc_fail;
+	} while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
+}
+
+static void hinic3_get_queue_stats_tx(struct net_device *netdev, int i,
+				      struct netdev_queue_stats_tx *stats_tx)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct hinic3_txq_stats *txq_stats;
+	struct hinic3_txq *txq;
+	unsigned int start;
+
+	txq = &nic_dev->txqs[i];
+	txq_stats = &txq->txq_stats;
+	do {
+		start = u64_stats_fetch_begin(&txq_stats->syncp);
+		stats_tx->bytes = txq_stats->bytes;
+		stats_tx->packets = txq_stats->packets;
+		stats_tx->stop = txq_stats->busy;
+	} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
+}
+
+static void hinic3_get_base_stats(struct net_device *netdev,
+				  struct netdev_queue_stats_rx *stats_rx,
+				  struct netdev_queue_stats_tx *stats_tx)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	int i;
+
+	memset(stats_rx, 0, sizeof(*stats_rx));
+	memset(stats_tx, 0, sizeof(*stats_tx));
+
+	for (i = nic_dev->q_params.num_qps; i < nic_dev->max_qps; i++) {
+		struct netdev_queue_stats_rx rx = {0};
+
+		hinic3_get_queue_stats_rx(netdev, i, &rx);
+		stats_rx->bytes += rx.bytes;
+		stats_rx->packets += rx.packets;
+		stats_rx->csum_bad += rx.csum_bad;
+	}
+
+	for (i = nic_dev->q_params.num_qps; i < nic_dev->max_qps; i++) {
+		struct netdev_queue_stats_tx tx = {0};
+
+		hinic3_get_queue_stats_tx(netdev, i, &tx);
+		stats_tx->bytes += tx.bytes;
+		stats_tx->packets += tx.packets;
+		stats_tx->stop += tx.stop;
+	}
+}
+
+static const struct netdev_stat_ops hinic3_stat_ops = {
+	.get_queue_stats_rx  = hinic3_get_queue_stats_rx,
+	.get_queue_stats_tx  = hinic3_get_queue_stats_tx,
+	.get_base_stats      = hinic3_get_base_stats,
+};
+
 void hinic3_set_netdev_ops(struct net_device *netdev)
 {
 	netdev->netdev_ops = &hinic3_netdev_ops;
+	netdev->stat_ops = &hinic3_stat_ops;
 }
-- 
2.43.0

[PATCH net-next v18 4/4] hinic3: Add ethtool pause param ops

[Fan Gong]

This commit adds port pause ops to the hinic3 driver. It implements
the .get_pauseparam and .set_pauseparam callbacks so these can be
read and write with ethtool -a/A.

Co-developed-by: Wu Di <wudi234@huawei.com>
Signed-off-by: Wu Di <wudi234@huawei.com>
Co-developed-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Teng Peisen <tengpeisen@huawei.com>
Signed-off-by: Fan Gong <gongfan1@huawei.com>
---
 .../ethernet/huawei/hinic3/hinic3_ethtool.c   | 41 +++++++++++++++++++
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.c   |  7 ++++
 .../ethernet/huawei/hinic3/hinic3_nic_cfg.h   |  2 +
 3 files changed, 50 insertions(+)

diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
index e1f23e2477fa..3005441056f9 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
@@ -869,6 +869,45 @@ static void hinic3_get_pause_stats(struct net_device *netdev,
 	kfree(ps);
 }
 
+static void hinic3_get_pauseparam(struct net_device *netdev,
+				  struct ethtool_pauseparam *pause)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct hinic3_nic_pause_config nic_pause = {};
+	int err;
+
+	err = hinic3_get_pause_info(nic_dev, &nic_pause);
+	if (err) {
+		netdev_err_once(netdev, "Failed to get pauseparam\n");
+	} else {
+		pause->autoneg = AUTONEG_DISABLE;
+		pause->rx_pause = nic_pause.rx_pause;
+		pause->tx_pause = nic_pause.tx_pause;
+	}
+}
+
+static int hinic3_set_pauseparam(struct net_device *netdev,
+				 struct ethtool_pauseparam *pause)
+{
+	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
+	struct hinic3_nic_pause_config nic_pause = {};
+	int err;
+
+	if (HINIC3_IS_VF(nic_dev->hwdev))
+		return -EOPNOTSUPP;
+
+	if (pause->autoneg)
+		return -EOPNOTSUPP;
+
+	nic_pause.rx_pause = (u8)pause->rx_pause;
+	nic_pause.tx_pause = (u8)pause->tx_pause;
+
+	err = hinic3_set_pause_info(nic_dev, &nic_pause);
+	if (err)
+		netdev_err_once(netdev, "Failed to set pauseparam\n");
+	return err;
+}
+
 static const struct ethtool_ops hinic3_ethtool_ops = {
 	.supported_coalesce_params      = ETHTOOL_COALESCE_USECS |
 					  ETHTOOL_COALESCE_PKT_RATE_RX_USECS,
@@ -885,6 +924,8 @@ static const struct ethtool_ops hinic3_ethtool_ops = {
 	.get_eth_ctrl_stats             = hinic3_get_eth_ctrl_stats,
 	.get_rmon_stats                 = hinic3_get_rmon_stats,
 	.get_pause_stats                = hinic3_get_pause_stats,
+	.get_pauseparam                 = hinic3_get_pauseparam,
+	.set_pauseparam                 = hinic3_set_pauseparam,
 };
 
 void hinic3_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
index e4a3fa6fb8db..b5c696fe6cdf 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.c
@@ -739,6 +739,13 @@ int hinic3_get_pause_info(struct hinic3_nic_dev *nic_dev,
 				   nic_pause);
 }
 
+int hinic3_set_pause_info(struct hinic3_nic_dev *nic_dev,
+			  struct hinic3_nic_pause_config *nic_pause)
+{
+	return hinic3_cfg_hw_pause(nic_dev->hwdev, MGMT_MSG_CMD_OP_SET,
+				   nic_pause);
+}
+
 int hinic3_get_vport_stats(struct hinic3_hwdev *hwdev, u16 func_id,
 			   struct l2nic_vport_stats *stats)
 {
diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
index 63382191446d..beb491d66738 100644
--- a/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
+++ b/drivers/net/ethernet/huawei/hinic3/hinic3_nic_cfg.h
@@ -318,6 +318,8 @@ int hinic3_del_vlan(struct hinic3_hwdev *hwdev, u16 vlan_id, u16 func_id);
 
 int hinic3_get_pause_info(struct hinic3_nic_dev *nic_dev,
 			  struct hinic3_nic_pause_config *nic_pause);
+int hinic3_set_pause_info(struct hinic3_nic_dev *nic_dev,
+			  struct hinic3_nic_pause_config *nic_pause);
 
 int hinic3_get_vport_stats(struct hinic3_hwdev *hwdev, u16 func_id,
 			   struct l2nic_vport_stats *stats);
-- 
2.43.0

Re: [PATCH net-next v18 0/4] net: hinic3: Support ethtool ops

[Jakub Kicinski]

On Thu, 22 Oct 2026 20:25:14 +0800 Fan Gong wrote:
> This is [3/3] part of hinic3 Ethernet driver second submission.
> With this patch hinic3 becomes a complete Ethernet driver.

## Form letter - net-next-closed

The merge window for Linux v7.4 has begun, and the net-next
tree is now closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens. There is some uncertainty
about how long this merge window will last, so please check
https://netdev.bots.linux.dev/net-next.html
before reposting. net-next _may_ reopen on Nov 2nd.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer
pv-bot: closed

Re: [PATCH net-next v18 1/4] hinic3: Add ethtool statistic ops part1

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 3 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] hinic3_send_one_skb()
  (drivers/net/ethernet/huawei/hinic3/hinic3_tx.c) reads skb->len for…
- [Medium] The commit is described as a feature (ethtool -S support).
- [Medium] The new TX byte (and packet) accounting in
  hinic3_send_one_skb() adds only skb->len and 1 per skb.
- [Medium] The new RX accounting in hinic3_rx_poll() adds the CQE pkt_len
  to rx_bytes and counts nr_pkts once per completion.

--

hinic3: Add ethtool statistic ops part1

This commit adds per-queue TX/RX software counters plus firmware-backed
vport and physical port statistics to hinic3. It implements the
.get_strings, .get_sset_count and .get_ethtool_stats callbacks for
ethtool -S, and adds packet/byte accounting to the RX and TX hot paths.

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
> index da73811641a9d..c71bb3dff39c3 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
> @@ -802,14 +802,14 @@ static void hinic3_get_stats64(struct net_device *netdev,
>  		txq_stats = &txq->txq_stats;
>  		do {
>  			start = u64_stats_fetch_begin(&txq_stats->syncp);
> -			bytes += txq_stats->bytes;
> -			packets += txq_stats->packets;
> -			dropped += txq_stats->dropped;
> +			bytes = txq_stats->bytes;
> +			packets = txq_stats->packets;
> +			dropped = txq_stats->dropped;
>  		} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
> +		stats->tx_packets += packets;
> +		stats->tx_bytes   += bytes;
> +		stats->tx_dropped += dropped;
>  	}
> -	stats->tx_packets = packets;
> -	stats->tx_bytes   = bytes;
> -	stats->tx_dropped = dropped;

[Severity: Medium]
The commit message says this patch adds ethtool -S support. This hunk
also changes hinic3_get_stats64() so it no longer accumulates with +=
inside the u64_stats_fetch_begin()/u64_stats_fetch_retry() loop. Before,
every retry added the values again.

The new increments in hinic3_rx_poll() and hinic3_send_one_skb() also
look like the first writes to rxq_stats/txq_stats packets and bytes,
apart from hinic3_rxq_clean_stats() and hinic3_txq_clean_stats().

At the baseline, did hinic3_get_stats64() then always report 0 for RX/TX
packets, bytes and dropped in ip -s link and /proc/net/dev?

If so, hinic3_net_dim() was only passing zeros to DIM:

hinic3_irq.c:hinic3_net_dim() {
    ...
	dim_update_sample(irq_cfg->total_events, rxq->rxq_stats.packets,
			  rxq->rxq_stats.bytes, &sample);
    ...
}

That would mean adaptive RX coalescing never saw any traffic.

The commit message doesn't mention any of this. Could these fixes go to
net as separate patches, with Fixes: tags for the commits that added
.ndo_get_stats64 and DIM, so stable kernels get them?

[ ... ]

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
> index 309ab59013797..ae4ccff4a09a9 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_rx.c

[ ... ]

> @@ -575,6 +602,7 @@ int hinic3_rx_poll(struct hinic3_rxq *rxq, int budget)
>  			break;
>  
>  		nr_pkts++;
> +		rx_bytes += pkt_len;
>  		num_lro = RQ_CQE_STATUS_GET(status, NUM_LRO);
>  		if (num_lro)
>  			num_wqe += hinic3_get_sge_num(rxq, pkt_len);

[Severity: Medium]
When hardware LRO is on (ethtool -K <dev> lro on), one CQE stands for
num_lro coalesced wire packets. recv_one_pkt() already passes num_lro to
hinic3_lro_set_gso_params(), which sets gso_segs = num_lro.

Are rx_packets and rx_bytes undercounted for LRO aggregates? nr_pkts goes
up by one per completion. pkt_len holds only one copy of the headers, so
the headers of the other num_lro - 1 packets are not counted.

hinic3_get_stats64() reports these values as rx_packets/rx_bytes as they
are. Later in the series they are also reported as qstats
rx-packets/rx-bytes, which netdev.yaml defines as wire packets.

num_lro is already read here for the replenish threshold. Could it be
used for the counters too?

[ ... ]

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c
> index f92568f0f00b7..b0a0ffb1052b3 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_tx.c

[ ... ]

> @@ -599,11 +617,19 @@ static netdev_tx_t hinic3_send_one_skb(struct sk_buff *skb,
>  	hinic3_write_db(txq->sq, 0, DB_CFLAG_DP_SQ,
>  			hinic3_get_sq_local_pi(txq->sq));
>  
> +	u64_stats_update_begin(&txq->txq_stats.syncp);
> +	txq->txq_stats.bytes += skb->len;
> +	txq->txq_stats.packets += 1;
> +	u64_stats_update_end(&txq->txq_stats.syncp);
> +
>  	return NETDEV_TX_OK;

[Severity: High]
Can the skb already be freed when skb->len is read here?

At this point tx_info->skb is published and hinic3_write_db() has given
the WQE to the device. TX completion runs from NAPI on whichever CPU
handles the queue interrupt. It takes no lock that the xmit path also
takes:

hinic3_poll()
  hinic3_tx_poll()
    hinic3_tx_unmap_skb(netdev, tx_info->skb, tx_info->dma_info);
    napi_consume_skb(tx_info->skb, budget);

Suppose the device completes the packet and another CPU runs
hinic3_tx_poll() before this CPU reaches the stats update. That can
happen if this CPU takes a hardirq, if the vCPU is preempted, or under
PREEMPT_RT. The read of skb->len is then a use-after-free, and a garbage
value is added to tx_bytes.

Before this patch the skb was not touched after the doorbell, because
netif_subqueue_sent() reads skb->len earlier. The code is unchanged at
the end of the series ("hinic3: Add ethtool pause param ops").

Would saving skb->len in a local before the doorbell, next to
netif_subqueue_sent(), and using that local for the stats fix this?

[Severity: Medium]
How are TSO skbs counted here? hinic3_tso() passes gso_size to the
hardware via hinic3_set_tso_info(), so one GSO skb goes out as several
Ethernet frames. skb->len includes the L2/L3/L4 headers only once.

So tx_packets goes up by one per GSO skb rather than once per wire frame.
tx_bytes also misses the headers that are copied into each extra segment.

hinic3_get_stats64() reports these as rtnl_link_stats64
tx_packets/tx_bytes. The UAPI says tx_bytes should count the length of
Ethernet frames excluding the FCS.

Later in the series, "hinic3: Add Add per queue netdev-genl stats" also
reports these counters as qstats tx-packets/tx-bytes, which are defined
as wire packets.

Should this take gso_segs and the copied header length into account, for
example skb->len + (gso_segs - 1) * hdr_len?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792671176.git.gongfan1%40huawei.com

Re: [PATCH net-next v18 3/4] hinic3: Add Add per queue netdev-genl stats

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 5 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] hinic3_get_queue_stats_rx() and hinic3_get_queue_stats_tx()
  index &nic_dev->rxqs[i] and &nic_dev->txqs[i] with the core-supplied…
- [Medium] hinic3_get_base_stats() calls memset(stats_rx, 0, ...) and
  memset(stats_tx, 0, ...) on the whole netdev_queue_stats_rx/tx structs.
- [Medium] hinic3_get_queue_stats_tx() reports txq_stats->busy as the
  qstat `stop` counter (stats_tx->stop = txq_stats->busy).
- [Medium] hinic3_get_base_stats() sums the inactive-queue history over
  [nic_dev->q_params.num_qps, nic_dev->max_qps).
- [Medium] hinic3_get_queue_stats_rx() reports stats_rx->alloc_fail =
  rxq_stats->alloc_fail per queue.
- [Medium] The per-queue qstats packets/bytes exported by
  hinic3_get_queue_stats_tx() and hinic3_get_queue_stats_rx() count skbs…
- [Low] The commit message has no body, and the subject has a typo:
  "hinic3: Add Add per queue netdev-genl stats".

--

hinic3: Add Add per queue netdev-genl stats

This commit adds netdev_stat_ops to the hinic3 driver. It implements
get_queue_stats_rx, get_queue_stats_tx and get_base_stats, so per-queue
and device-level counters are exported through the netdev-genl qstats
interface.

> hinic3: Add Add per queue netdev-genl stats

[Severity: Low]
This isn't a bug, but "Add" appears twice in the subject line.

The commit message also has no body. This patch adds new uAPI-visible
qstats output. Could the message describe how the driver counters map onto
the standard attributes (busy to tx-stop, csum_errors to rx-csum-bad,
alloc_fail to rx-alloc-fail)?

It could also say that get_base_stats treats queues in
[q_params.num_qps, max_qps) as the inactive range and zero-fills every
qstat field.

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
> index c71bb3dff39c3..10364c4dc11f9 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c

[ ... ]

> @@ -866,7 +867,80 @@ static const struct net_device_ops hinic3_netdev_ops = {
>  	.ndo_start_xmit       = hinic3_xmit_frame,
>  };
>  
> +static void hinic3_get_queue_stats_rx(struct net_device *netdev, int i,
> +				      struct netdev_queue_stats_rx *stats_rx)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_rxq_stats *rxq_stats;
> +	struct hinic3_rxq *rxq;
> +	unsigned int start;
> +
> +	rxq = &nic_dev->rxqs[i];

[Severity: High]
Can i go past the end of nic_dev->rxqs[] here?

hinic3_alloc_rxqs() allocates nic_dev->rxqs with max_qps entries. The core,
however, calls this callback for every i below
netdev->real_num_rx_queues.

At probe, real_num_*_queues comes from hinic3_func_max_qnum():

hinic3_main.c:hinic3_nic_probe() {
    ...
    max_qps = hinic3_func_max_qnum(hwdev);
    netdev = alloc_etherdev_mq(sizeof(*nic_dev), max_qps);
    ...
}

After that, hinic3_sw_init()->hinic3_try_to_enable_rss() can shrink
max_qps to 1 when alloc_rss_resource() or hinic3_set_hw_rss_parameters()
fails:

hinic3_rss.c:hinic3_try_to_enable_rss() {
    ...
    err = alloc_rss_resource(netdev);
    if (err) {
        nic_dev->max_qps = 1;
        goto err_reset_q_params;
    }
    ...
}

Only hinic3_vport_up() lowers real_num_*_queues, and only on a successful
open. A device-scope dump before the first successful open (or after opens
that fail before hinic3_vport_up()) still does this:

netdev_nl_stats_by_netdev()
    netdev_stat_queue_sum(netdev, 0, netdev->real_num_rx_queues, &rx_sum,
                          0, netdev->real_num_tx_queues, &tx_sum);

There is no netdev->up check on this path, so i runs from 1 to
func_max_qnum - 1 against a one-entry array.

Would this read past the end of the rxqs and txqs slab objects and return
those values to user space? NETDEV_CMD_QSTATS_GET does not require
GENL_ADMIN_PERM, so any local user can issue it.

The same applies to nic_dev->txqs[i] in hinic3_get_queue_stats_tx().
Before this patch hinic3 had no stat_ops, and hinic3_get_stats64() bounds
its loops by max_qps.

> +	rxq_stats = &rxq->rxq_stats;
> +	do {
> +		start = u64_stats_fetch_begin(&rxq_stats->syncp);
> +		stats_rx->bytes = rxq_stats->bytes;
> +		stats_rx->packets = rxq_stats->packets;

[Severity: Medium]
netdev.yaml defines rx-packets as "Number of wire packets successfully
received and passed to the stack." Do these counters still match that when
hardware LRO is active?

hinic3_rx_poll() counts one packet per CQE:

hinic3_rx.c:hinic3_rx_poll() {
    ...
        nr_pkts++;
        rx_bytes += pkt_len;
        num_lro = RQ_CQE_STATUS_GET(status, NUM_LRO);
    ...
}

NUM_LRO gives the number of wire packets that were merged, and the skb gets
gso_segs = num_lro.

The tx side in hinic3_get_queue_stats_tx() looks similar. It counts once
per skb, right after the doorbell rather than at DMA completion:

hinic3_tx.c:hinic3_send_one_skb() {
    ...
    txq->txq_stats.bytes += skb->len;
    txq->txq_stats.packets += 1;
    ...
}

TSO/TSO6 is enabled by default when HINIC3_NIC_F_TSO is reported. One skb
then usually covers gso_segs wire frames, and the replicated headers are
missing from the byte count.

Would rx-packets/rx-bytes and tx-packets/tx-bytes undercount wire traffic
whenever TSO or LRO is active? The existing ndo_get_stats64 use did not
have this wire-packet requirement.

> +		stats_rx->csum_bad = rxq_stats->csum_errors;
> +		stats_rx->alloc_fail = rxq_stats->alloc_fail;
> +	} while (u64_stats_fetch_retry(&rxq_stats->syncp, start));
> +}
> +
> +static void hinic3_get_queue_stats_tx(struct net_device *netdev, int i,
> +				      struct netdev_queue_stats_tx *stats_tx)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_txq_stats *txq_stats;
> +	struct hinic3_txq *txq;
> +	unsigned int start;
> +
> +	txq = &nic_dev->txqs[i];
> +	txq_stats = &txq->txq_stats;
> +	do {
> +		start = u64_stats_fetch_begin(&txq_stats->syncp);
> +		stats_tx->bytes = txq_stats->bytes;
> +		stats_tx->packets = txq_stats->packets;
> +		stats_tx->stop = txq_stats->busy;

[Severity: Medium]
Is busy the right counter to report as tx-stop?

In hinic3_send_one_skb(), busy is only incremented in the NETDEV_TX_BUSY
fallback, when hinic3_wq_free_wqebbs() < wqebb_cnt:

hinic3_tx.c:hinic3_send_one_skb() {
    ...
        netif_subqueue_try_stop(netdev, txq->sq->q_id, ...);
        u64_stats_update_begin(&txq->txq_stats.syncp);
        txq->txq_stats.busy++;
        u64_stats_update_end(&txq->txq_stats.syncp);
        return NETDEV_TX_BUSY;
    ...
}

The normal stop happens on the success path, and no counter is updated
there:

    netif_subqueue_maybe_stop(netdev, txq->sq->q_id,
                              hinic3_wq_free_wqebbs(&txq->sq->wq),
                              txq->tx_stop_thrs,
                              txq->tx_start_thrs);

The fallback also increments busy even when netif_subqueue_try_stop() has
re-woken the queue.

netdev.yaml defines tx-stop as "Number of times driver paused accepting new
tx packets from the stack to this queue, because the queue was full."
Would tx-stop miss most real queue stops here, and count some events that
were not stops? tx-wake is not reported either.

> +	} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
> +}
> +
> +static void hinic3_get_base_stats(struct net_device *netdev,
> +				  struct netdev_queue_stats_rx *stats_rx,
> +				  struct netdev_queue_stats_tx *stats_tx)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	int i;
> +
> +	memset(stats_rx, 0, sizeof(*stats_rx));
> +	memset(stats_tx, 0, sizeof(*stats_tx));

[Severity: Medium]
Does zeroing the whole structs here report counters that hinic3 never
collects?

netdev_nl_stats_by_netdev() pre-fills the sums with the NOT_SET sentinel.
It relies on get_base_stats to touch only the fields the driver supports:

    memset(&rx_sum, 0xff, sizeof(rx_sum));
    memset(&tx_sum, 0xff, sizeof(tx_sum));

    netdev->stat_ops->get_base_stats(netdev, &rx_sum, &tx_sum);

The kdoc for struct netdev_stat_ops in include/net/netdev_queues.h says:

 * Only statistics which are set in @get_base_stats will be reported
 * at the device level, ...
 * This is because @get_base_stats has a second function of designating which
 * statistics are in fact correct for the entire device ...

With this memset, fields such as rx-hw-drops, rx-csum-complete,
rx-hw-gro-*, tx-hw-drops, tx-needs-csum, tx-hw-gso-* and tx-wake become
valid zeros. Per-queue NOT_SET values are skipped when summing, so every
device-scope dump reports these fields as a constant 0.

Could this function set only the fields hinic3 actually collects (bytes,
packets, csum_bad, alloc_fail, stop), as other drivers do?

> +
> +	for (i = nic_dev->q_params.num_qps; i < nic_dev->max_qps; i++) {

[Severity: Medium]
Can this range overlap the live-queue range that the core sums?

The core sums [0, netdev->real_num_*x_queues) itself, and
netdev_stat_queue_sum() documents the base range as
[real_num_*x_queues, num_*x_queues). The lower bound here is
q_params.num_qps instead, and it can drift away from real_num_*_queues.

hinic3_open()->hinic3_setup_num_qps()->hinic3_config_num_qps() can lower
num_qps when there are not enough IRQs:

    if (alloc_num_irq < q_params->num_qps) {
        q_params->num_qps = alloc_num_irq;

real_num_*_queues is only synced later, in hinic3_vport_up():

    err = netif_set_real_num_queues(netdev, nic_dev->q_params.num_qps,
                                    nic_dev->q_params.num_qps);

Suppose one of these fails in between: hinic3_alloc_channel_resources(),
hinic3_open_channel(), hinic3_set_vport_enable() or
hinic3_maybe_set_port_state(). No error path restores num_qps, so it stays
at the lower value M while real_num_*_queues keeps the old value N.

Would queues [M, N) then be counted twice in every device-level query,
once here and once in the core sum? Those queues still hold counters from
earlier opens, since stats are only reset at probe. That would inflate
bytes, packets, csum_bad and stop.

> +		struct netdev_queue_stats_rx rx = {0};
> +
> +		hinic3_get_queue_stats_rx(netdev, i, &rx);
> +		stats_rx->bytes += rx.bytes;
> +		stats_rx->packets += rx.packets;
> +		stats_rx->csum_bad += rx.csum_bad;
> +	}

[Severity: Medium]
Is rx.alloc_fail meant to be dropped here?

hinic3_get_queue_stats_rx() reports alloc_fail per queue, and the memset
above marks stats_rx->alloc_fail as a valid device total. This loop,
however, only adds up bytes, packets and csum_bad.

When num_qps is lowered (for example by hinic3_config_num_qps() on an IRQ
shortage), allocation failures recorded on the now-inactive queues would
drop out of the device-level rx-alloc-fail. Bytes, packets and csum_bad
keep their history, because rxq_stats is only cleared at probe by
hinic3_rxq_stats_init().

Should this loop also do stats_rx->alloc_fail += rx.alloc_fail?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792671176.git.gongfan1%40huawei.com

Re: [PATCH net-next v18 4/4] hinic3: Add ethtool pause param ops

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] hinic3_get_pauseparam() always sets `pause->autoneg =
  AUTONEG_DISABLE` (hinic3_ethtool.c:883) and ignores…
- [Medium] hinic3: hinic3_get_pauseparam() lacks VF check and exposes
  pause ops on VFs
- [Low] hinic3_set_pauseparam() (hinic3_ethtool.c:902-903) turns the
  __u32 uAPI flags into firmware bytes with `nic_pause.rx_pause =…
- [Low] When hinic3_get_pause_info() fails (mailbox timeout, -EPERM when
  !chip_present_flag in hinic3_send_mbox_to_mgmt(), or firmware status…

--

hinic3: Add ethtool pause param ops

This commit adds .get_pauseparam and .set_pauseparam ethtool callbacks to
the hinic3 driver. A new hinic3_set_pause_info() helper sends the
L2NIC_CMD_CFG_PAUSE_INFO SET command to firmware through
hinic3_cfg_hw_pause().

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
> index e1f23e2477fa0..3005441056f92 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_ethtool.c
> @@ -869,6 +869,45 @@ static void hinic3_get_pause_stats(struct net_device *netdev,
>  	kfree(ps);
>  }
>  
> +static void hinic3_get_pauseparam(struct net_device *netdev,
> +				  struct ethtool_pauseparam *pause)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_nic_pause_config nic_pause = {};
> +	int err;
> +
> +	err = hinic3_get_pause_info(nic_dev, &nic_pause);
> +	if (err) {
> +		netdev_err_once(netdev, "Failed to get pauseparam\n");
> +	} else {

[Severity: Medium]
Is hinic3_get_pauseparam() missing a VF check? The other pause paths
in this driver treat physical port pause as PF-only:
hinic3_get_link_settings() skips hinic3_get_link_pause_settings() on
VFs, hinic3_get_pause_stats() returns early for HINIC3_IS_VF(), and the
new hinic3_set_pauseparam() returns -EOPNOTSUPP for VFs. The getter has
no such check. On a VF netdev, "ethtool -a" therefore sends
L2NIC_CMD_CFG_PAUSE_INFO to the management CPU through
hinic3_get_pause_info() and hinic3_cfg_hw_pause(). When that command
fails, as it presumably does for a VF, the driver logs the dev_err()
from hinic3_cfg_hw_pause() plus the netdev_err_once() here.

hinic3_set_ethtool_ops() installs the same hinic3_ethtool_ops table
on both PF and VF netdevs, so the core treats VFs as supporting pause.
Because get_pauseparam returns void, neither ethtool_get_pauseparam()
nor pause_prepare_data() can report -EOPNOTSUPP. The user sees a
zeroed or bogus pause state instead of an unsupported error.

The set side is affected as well. ethnl_set_pause() calls
get_pauseparam first and only calls set_pauseparam if a value
changed. Take a VF where the query fails and leaves pause zeroed.
"ethtool -A ethX rx off" sees mod == false and returns 0 without ever
reaching the HINIC3_IS_VF() check in hinic3_set_pauseparam(). The VF
then reports success for a request it can't support.

Would a separate ethtool_ops table for VFs, without .get_pauseparam
and .set_pauseparam, be the cleaner fix? That lets the core return
-EOPNOTSUPP for both ethtool -a and ethtool -A on VFs. Failing that,
should the getter at least bail out on HINIC3_IS_VF() before sending
anything to firmware?

[Severity: Low]

If hinic3_get_pause_info() fails here (mailbox timeout, -EPERM when
chip_present_flag is clear, or a non-zero firmware status), pause is left
zeroed. What happens when this runs as the read step of ethnl_set_pause()?

net/ethtool/pause.c:ethnl_set_pause() {
    ...
	dev->ethtool_ops->get_pauseparam(dev, &params);

	ethnl_update_bool32(&params.autoneg, tb[ETHTOOL_A_PAUSE_AUTONEG], &mod);
	ethnl_update_bool32(&params.rx_pause, tb[ETHTOOL_A_PAUSE_RX], &mod);
	ethnl_update_bool32(&params.tx_pause, tb[ETHTOOL_A_PAUSE_TX], &mod);
	if (!mod)
		return 0;
    ...
}

Take a partial request like "ethtool -A ethX rx on". rx goes from 0 to 1,
and hinic3_set_pauseparam() then sends both directions to firmware. That
includes tx_pause = 0, which came from the failed query and not from the
user. Can this turn off TX pause that was previously on?

Now take a request that matches the zero fallback, such as "rx off" while
RX pause is actually on. mod stays false, and ethnl_set_pause() returns
success without calling the setter. Is the requested change silently
dropped in that case?

Because netdev_err_once() is used, later failures also leave nothing in
the log.

get_pauseparam returns void, so the driver can't report the error. The
older hinic driver caches the last configured pause state in nic_cfg. Would
something similar work here?

> +		pause->autoneg = AUTONEG_DISABLE;

[Severity: Medium]

Should this report the firmware's pause autoneg state instead of
hardcoding AUTONEG_DISABLE? hinic3_cfg_hw_pause() already copies it from
the firmware response:

	if (opcode == MGMT_MSG_CMD_OP_GET) {
		nic_pause->auto_neg = pause_info.auto_neg;
		...
	}

Here nic_pause.auto_neg is dropped. As a result, ethtool -a shows
"Autonegotiate: off" even on ports where hinic3_get_link_settings()
reports autoneg as on:

	link_settings->autoneg = port_info.autoneg_state == PORT_CFG_AN_ON ?
				 AUTONEG_ENABLE : AUTONEG_DISABLE;

On the set side, hinic3_set_pauseparam() rejects any autoneg request and
never checks the port's AN state. The zero-initialized nic_pause.auto_neg
stays 0, and hinic3_cfg_hw_pause() copies it into the SET command:

	if (opcode == MGMT_MSG_CMD_OP_SET) {
		pause_info.auto_neg = nic_pause->auto_neg;
		...

On a port with PORT_CFG_AN_ON, does every "ethtool -A" then write
auto_neg = 0 to firmware and overwrite its pause autoneg state? It's not
clear how the firmware reacts to that on an AN link. It might keep forced
pause settings that disagree with the advertisement, or reject the set
with -EFAULT.

The older hinic driver handles this differently. hinic_get_pauseparam()
reports pause_info.auto_neg from firmware. hinic_set_pauseparam() requires
pause->autoneg to match port_cap.autoneg_state and then passes it through
to firmware.

> +		pause->rx_pause = nic_pause.rx_pause;
> +		pause->tx_pause = nic_pause.tx_pause;
> +	}
> +}
> +
> +static int hinic3_set_pauseparam(struct net_device *netdev,
> +				 struct ethtool_pauseparam *pause)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct hinic3_nic_pause_config nic_pause = {};
> +	int err;
> +
> +	if (HINIC3_IS_VF(nic_dev->hwdev))
> +		return -EOPNOTSUPP;
> +
> +	if (pause->autoneg)
> +		return -EOPNOTSUPP;
> +
> +	nic_pause.rx_pause = (u8)pause->rx_pause;
> +	nic_pause.tx_pause = (u8)pause->tx_pause;

[Severity: Low]

Can these u8 casts turn a non-zero request into 0?

The netlink path normalizes these values with ethnl_update_bool32(). The
legacy ETHTOOL_SPAUSEPARAM ioctl path does not: ethtool_set_pauseparam()
copies struct ethtool_pauseparam from userspace and calls
->set_pauseparam() with the raw values.

With rx_pause = 0x100, nic_pause.rx_pause becomes 0. Pause is disabled
while the call returns success. Values 2 to 255 go to firmware unchanged.

Would !!pause->rx_pause and !!pause->tx_pause be safer here?

> +
> +	err = hinic3_set_pause_info(nic_dev, &nic_pause);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792671176.git.gongfan1%40huawei.com