[PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[Guangbin Huang]

From: Hao Chen <chenhao418@huawei.com>

The hns3 driver header file uses the structure of other files, but does
not include corresponding file, which causes a check warning that the
header file is not self-contained.

Therefore, the required header file is included in the header file, and
the structure declaration is added to the header file to avoid cyclic
dependency of the header file.

Signed-off-by: Hao Chen <chenhao418@huawei.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h              | 4 +++-
 .../hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h        | 2 ++
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h              | 3 +++
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h       | 5 ++++-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
index abcd7877f7d2..487216aeae50 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
@@ -7,6 +7,8 @@
 #include <linux/mutex.h>
 #include <linux/types.h>
 
+struct hclgevf_dev;
+
 enum HCLGE_MBX_OPCODE {
 	HCLGE_MBX_RESET = 0x01,		/* (VF -> PF) assert reset */
 	HCLGE_MBX_ASSERTING_RESET,	/* (PF -> VF) PF is asserting reset */
@@ -233,7 +235,7 @@ struct hclgevf_mbx_arq_ring {
 	__le16 msg_q[HCLGE_MBX_MAX_ARQ_MSG_NUM][HCLGE_MBX_MAX_ARQ_MSG_SIZE];
 };
 
-struct hclge_dev;
+struct hclge_vport;
 
 #define HCLGE_MBX_OPCODE_MAX 256
 struct hclge_mbx_ops_param {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
index a46350162ee8..7aff1a544cf4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
@@ -7,6 +7,8 @@
 #include <linux/etherdevice.h>
 #include "hnae3.h"
 
+struct hclge_comm_hw;
+
 /* each tqp has TX & RX two queues */
 #define HCLGE_COMM_QUEUE_PAIR_SIZE 2
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 133a054af6b7..557a5fa70d0a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -13,6 +13,9 @@
 
 struct iphdr;
 struct ipv6hdr;
+struct gre_base_hdr;
+struct tcphdr;
+struct udphdr;
 
 enum hns3_nic_state {
 	HNS3_NIC_STATE_TESTING,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
index bbee74cd8404..bceb61c791a1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
@@ -8,8 +8,11 @@
 #include <linux/net_tstamp.h>
 #include <linux/types.h>
 
-struct hclge_dev;
 struct ifreq;
+struct ethtool_ts_info;
+
+struct hnae3_handle;
+struct hclge_dev;
 
 #define HCLGE_PTP_REG_OFFSET	0x29000
 
-- 
2.33.0

[PATCH net-next 0/4] net: hns3: There are some cleanup for the HNS3 ethernet driver

[Hao Lan]

There are some cleanup for the HNS3 ethernet driver.

Hao Chen (2):
  net: hns3: fix hns3 driver header file not self-contained issue
  net: hns3: fix strncpy() not using dest-buf length as length issue

Jian Shen (1):
  net: hns3: refine the tcam key convert handle

Peiyang Wang (1):
  net: hns3: clear hns unused parameter alarm

 .../net/ethernet/hisilicon/hns3/hclge_mbx.h   |  4 +-
 drivers/net/ethernet/hisilicon/hns3/hnae3.h   |  3 +-
 .../hns3/hns3_common/hclge_comm_rss.c         |  3 +-
 .../hns3/hns3_common/hclge_comm_rss.h         |  3 +-
 .../hns3/hns3_common/hclge_comm_tqp_stats.h   |  2 +
 .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 36 ++++++++++----
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   |  2 +-
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  5 +-
 .../ethernet/hisilicon/hns3/hns3_ethtool.c    |  7 ++-
 .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++--
 .../hisilicon/hns3/hns3pf/hclge_main.c        | 47 ++++++++-----------
 .../hisilicon/hns3/hns3pf/hclge_main.h        | 11 ++---
 .../hisilicon/hns3/hns3pf/hclge_ptp.h         |  5 +-
 .../hisilicon/hns3/hns3vf/hclgevf_main.c      | 14 ++----
 14 files changed, 97 insertions(+), 74 deletions(-)

-- 
2.30.0

[PATCH net-next 1/4] net: hns3: refine the tcam key convert handle

[Hao Lan]

From: Jian Shen <shenjian15@huawei.com>

The expression '(k ^ ~v)' is exaclty '(k & v)', and
'(k & v) & k' is exaclty 'k & v'. So simplify the
expression for tcam key convert.
(k ^ ~v) & k ==  (k & v) & k ==  k & k & v == k & v

It also add necessary brackets for them.

Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Hao Lan <lanhao@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h   | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 81aa6b0facf5..6a43d1515585 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg {
  * Then for input key(k) and mask(v), we can calculate the value by
  * the formulae:
  *	x = (~k) & v
- *	y = (k ^ ~v) & k
+ *	y = k & v
  */
-#define calc_x(x, k, v) (x = ~(k) & (v))
-#define calc_y(y, k, v) \
-	do { \
-		const typeof(k) _k_ = (k); \
-		const typeof(v) _v_ = (v); \
-		(y) = (_k_ ^ ~_v_) & (_k_); \
-	} while (0)
+#define calc_x(x, k, v) ((x) = ~(k) & (v))
+#define calc_y(y, k, v) ((y) = (k) & (v))
 
 #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f))
 #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))
-- 
2.30.0

[PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[Hao Lan]

From: Hao Chen <chenhao418@huawei.com>

Hns3 driver header file uses the structure of other files, but does
not include corresponding file, which causes a check warning that the
header file is not self-contained by clang-tidy checker.

For example,
Header file 'hclge_mbx.h' is not self contained.
It should include following headers: (1) 'hclgevf_main.h'
due to symbols 'struct hclgevf_dev'. The main source file is hns3_enet.c

Therefore, the required header file is included in the header file, and
the structure declaration is added to the header file to avoid cyclic
dependency of the header file.

Signed-off-by: Hao Chen <chenhao418@huawei.com>
Signed-off-by: Hao Lan <lanhao@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h              | 4 +++-
 .../hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h        | 2 ++
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h              | 3 +++
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h       | 5 ++++-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
index abcd7877f7d2..487216aeae50 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
@@ -7,6 +7,8 @@
 #include <linux/mutex.h>
 #include <linux/types.h>
 
+struct hclgevf_dev;
+
 enum HCLGE_MBX_OPCODE {
 	HCLGE_MBX_RESET = 0x01,		/* (VF -> PF) assert reset */
 	HCLGE_MBX_ASSERTING_RESET,	/* (PF -> VF) PF is asserting reset */
@@ -233,7 +235,7 @@ struct hclgevf_mbx_arq_ring {
 	__le16 msg_q[HCLGE_MBX_MAX_ARQ_MSG_NUM][HCLGE_MBX_MAX_ARQ_MSG_SIZE];
 };
 
-struct hclge_dev;
+struct hclge_vport;
 
 #define HCLGE_MBX_OPCODE_MAX 256
 struct hclge_mbx_ops_param {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
index a46350162ee8..7aff1a544cf4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_tqp_stats.h
@@ -7,6 +7,8 @@
 #include <linux/etherdevice.h>
 #include "hnae3.h"
 
+struct hclge_comm_hw;
+
 /* each tqp has TX & RX two queues */
 #define HCLGE_COMM_QUEUE_PAIR_SIZE 2
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 88af34bbee34..1b360aa52e5d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -13,6 +13,9 @@
 
 struct iphdr;
 struct ipv6hdr;
+struct gre_base_hdr;
+struct tcphdr;
+struct udphdr;
 
 enum hns3_nic_state {
 	HNS3_NIC_STATE_TESTING,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
index bbee74cd8404..bceb61c791a1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h
@@ -8,8 +8,11 @@
 #include <linux/net_tstamp.h>
 #include <linux/types.h>
 
-struct hclge_dev;
 struct ifreq;
+struct ethtool_ts_info;
+
+struct hnae3_handle;
+struct hclge_dev;
 
 #define HCLGE_PTP_REG_OFFSET	0x29000
 
-- 
2.30.0

[PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Hao Lan]

From: Hao Chen <chenhao418@huawei.com>

Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
it may result in dest-buf overflow.

This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
compiler.

The warning reports as below:

hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
the length of the source argument [-Wstringop-truncation]

strncpy(pos, items[i].name, strlen(items[i].name));

hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
terminating nul copying as many bytes from a string as its length
[-Wstringop-truncation]

strncpy(pos, result[i], strlen(result[i]));

strncpy() use src-length as copy-length, it may result in
dest-buf overflow.

So,this patch add some values check to avoid this issue.

Signed-off-by: Hao Chen <chenhao418@huawei.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
Signed-off-by: Hao Lan <lanhao@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
 .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index 4c3e90a1c4d0..cf415cb37685 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
 				  const struct hns3_dbg_item *items,
 				  const char **result, u16 size)
 {
+#define HNS3_DBG_LINE_END_LEN	2
 	char *pos = content;
+	u16 item_len;
 	u16 i;
 
+	if (!len) {
+		return;
+	} else if (len <= HNS3_DBG_LINE_END_LEN) {
+		*pos++ = '\0';
+		return;
+	}
+
 	memset(content, ' ', len);
-	for (i = 0; i < size; i++) {
-		if (result)
-			strncpy(pos, result[i], strlen(result[i]));
-		else
-			strncpy(pos, items[i].name, strlen(items[i].name));
+	len -= HNS3_DBG_LINE_END_LEN;
 
-		pos += strlen(items[i].name) + items[i].interval;
+	for (i = 0; i < size; i++) {
+		item_len = strlen(items[i].name) + items[i].interval;
+		if (len < item_len)
+			break;
+
+		if (result) {
+			if (item_len < strlen(result[i]))
+				break;
+			memcpy(pos, result[i], strlen(result[i]));
+		} else {
+			memcpy(pos, items[i].name, strlen(items[i].name));
+		}
+		pos += item_len;
+		len -= item_len;
 	}
-
 	*pos++ = '\n';
 	*pos++ = '\0';
 }
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
index a0b46e7d863e..1354fd0461f7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
@@ -88,16 +88,35 @@ static void hclge_dbg_fill_content(char *content, u16 len,
 				   const struct hclge_dbg_item *items,
 				   const char **result, u16 size)
 {
+#define HCLGE_DBG_LINE_END_LEN	2
 	char *pos = content;
+	u16 item_len;
 	u16 i;
 
+	if (!len) {
+		return;
+	} else if (len <= HCLGE_DBG_LINE_END_LEN) {
+		*pos++ = '\0';
+		return;
+	}
+
 	memset(content, ' ', len);
+	len -= HCLGE_DBG_LINE_END_LEN;
+
 	for (i = 0; i < size; i++) {
-		if (result)
-			strncpy(pos, result[i], strlen(result[i]));
-		else
-			strncpy(pos, items[i].name, strlen(items[i].name));
-		pos += strlen(items[i].name) + items[i].interval;
+		item_len = strlen(items[i].name) + items[i].interval;
+		if (len < item_len)
+			break;
+
+		if (result) {
+			if (item_len < strlen(result[i]))
+				break;
+			memcpy(pos, result[i], strlen(result[i]));
+		} else {
+			memcpy(pos, items[i].name, strlen(items[i].name));
+		}
+		pos += item_len;
+		len -= item_len;
 	}
 	*pos++ = '\n';
 	*pos++ = '\0';
-- 
2.30.0

[PATCH net-next 4/4] net: hns3: clear hns unused parameter alarm

[Hao Lan]

From: Peiyang Wang <wangpeiyang1@huawei.com>

Several functions in the hns3 driver have unused parameters.
The compiler will warn about them when building
with -Wunused-parameter option of hns3.

Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com>
Signed-off-by: Hao Lan <lanhao@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h   |  3 +-
 .../hns3/hns3_common/hclge_comm_rss.c         |  3 +-
 .../hns3/hns3_common/hclge_comm_rss.h         |  3 +-
 .../ethernet/hisilicon/hns3/hns3_debugfs.c    |  5 +-
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   |  2 +-
 .../net/ethernet/hisilicon/hns3/hns3_enet.h   |  2 +-
 .../ethernet/hisilicon/hns3/hns3_ethtool.c    |  7 ++-
 .../hisilicon/hns3/hns3pf/hclge_main.c        | 47 ++++++++-----------
 .../hisilicon/hns3/hns3vf/hclgevf_main.c      | 14 ++----
 9 files changed, 34 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 9c9c72dc57e0..b99d75260d59 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -647,8 +647,7 @@ struct hnae3_ae_ops {
 	int (*rm_mc_addr)(struct hnae3_handle *handle,
 			  const unsigned char *addr);
 	void (*set_tso_stats)(struct hnae3_handle *handle, int enable);
-	void (*update_stats)(struct hnae3_handle *handle,
-			     struct net_device_stats *net_stats);
+	void (*update_stats)(struct hnae3_handle *handle);
 	void (*get_stats)(struct hnae3_handle *handle, u64 *data);
 	void (*get_mac_stats)(struct hnae3_handle *handle,
 			      struct hns3_mac_stats *mac_stats);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.c b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.c
index ae2736549526..b4ae2160aff4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.c
@@ -305,8 +305,7 @@ int hclge_comm_set_rss_indir_table(struct hnae3_ae_dev *ae_dev,
 	return 0;
 }
 
-int hclge_comm_set_rss_input_tuple(struct hnae3_handle *nic,
-				   struct hclge_comm_hw *hw, bool is_pf,
+int hclge_comm_set_rss_input_tuple(struct hclge_comm_hw *hw,
 				   struct hclge_comm_rss_cfg *rss_cfg)
 {
 	struct hclge_comm_rss_input_tuple_cmd *req;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.h b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.h
index 92af3d2980d3..cdafa63fe38b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_common/hclge_comm_rss.h
@@ -112,8 +112,7 @@ int hclge_comm_init_rss_tuple_cmd(struct hclge_comm_rss_cfg *rss_cfg,
 				  struct hnae3_ae_dev *ae_dev,
 				  struct hclge_comm_rss_input_tuple_cmd *req);
 u64 hclge_comm_convert_rss_tuple(u8 tuple_sets);
-int hclge_comm_set_rss_input_tuple(struct hnae3_handle *nic,
-				   struct hclge_comm_hw *hw, bool is_pf,
+int hclge_comm_set_rss_input_tuple(struct hclge_comm_hw *hw,
 				   struct hclge_comm_rss_cfg *rss_cfg);
 int hclge_comm_set_rss_indir_table(struct hnae3_ae_dev *ae_dev,
 				   struct hclge_comm_hw *hw, const u16 *indir);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
index cf415cb37685..8679dd91a8a2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
@@ -958,8 +958,7 @@ static const struct hns3_dbg_item tx_bd_info_items[] = {
 	{ "MSS_HW_CSUM", 0 },
 };
 
-static void hns3_dump_tx_bd_info(struct hns3_nic_priv *priv,
-				 struct hns3_desc *desc, char **result, int idx)
+static void hns3_dump_tx_bd_info(struct hns3_desc *desc, char **result, int idx)
 {
 	unsigned int j = 0;
 
@@ -1008,7 +1007,7 @@ static int hns3_dbg_tx_bd_info(struct hns3_dbg_data *d, char *buf, int len)
 	for (i = 0; i < ring->desc_num; i++) {
 		desc = &ring->desc[i];
 
-		hns3_dump_tx_bd_info(priv, desc, result, i);
+		hns3_dump_tx_bd_info(desc, result, i);
 		hns3_dbg_fill_content(content, sizeof(content),
 				      tx_bd_info_items, (const char **)result,
 				      ARRAY_SIZE(tx_bd_info_items));
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index b676496ec6d7..9f6890059666 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2538,7 +2538,7 @@ static void hns3_nic_get_stats64(struct net_device *netdev,
 	if (test_bit(HNS3_NIC_STATE_DOWN, &priv->state))
 		return;
 
-	handle->ae_algo->ops->update_stats(handle, &netdev->stats);
+	handle->ae_algo->ops->update_stats(handle);
 
 	memset(&ring_total_stats, 0, sizeof(ring_total_stats));
 	for (idx = 0; idx < queue_num; idx++) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 1b360aa52e5d..8c0016b359b7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -693,7 +693,7 @@ static inline unsigned int hns3_page_order(struct hns3_enet_ring *ring)
 
 /* iterator for handling rings in ring group */
 #define hns3_for_each_ring(pos, head) \
-	for (pos = (head).ring; (pos); pos = (pos)->next)
+	for ((pos) = (head).ring; (pos); (pos) = (pos)->next)
 
 #define hns3_get_handle(ndev) \
 	(((struct hns3_nic_priv *)netdev_priv(ndev))->ae_handle)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 51d1278b18f6..407d30ee55d2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -228,7 +228,7 @@ static u32 hns3_lb_check_rx_ring(struct hns3_nic_priv *priv, u32 budget)
 }
 
 static void hns3_lb_clear_tx_ring(struct hns3_nic_priv *priv, u32 start_ringid,
-				  u32 end_ringid, u32 budget)
+				  u32 end_ringid)
 {
 	u32 i;
 
@@ -295,8 +295,7 @@ static int hns3_lp_run_test(struct net_device *ndev, enum hnae3_loop mode)
 
 out:
 	hns3_lb_clear_tx_ring(priv, HNS3_NIC_LB_TEST_RING_ID,
-			      HNS3_NIC_LB_TEST_RING_ID,
-			      HNS3_NIC_LB_TEST_PKT_NUM);
+			      HNS3_NIC_LB_TEST_RING_ID);
 
 	kfree_skb(skb);
 	return ret_val;
@@ -618,7 +617,7 @@ static void hns3_get_stats(struct net_device *netdev,
 		return;
 	}
 
-	h->ae_algo->ops->update_stats(h, &netdev->stats);
+	h->ae_algo->ops->update_stats(h);
 
 	/* get per-queue stats */
 	p = hns3_get_stats_tqps(h, p);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 4fb5406c1951..1bbf18a96bf1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -689,8 +689,7 @@ static void hclge_update_stats_for_all(struct hclge_dev *hdev)
 			"Update MAC stats fail, status = %d.\n", status);
 }
 
-static void hclge_update_stats(struct hnae3_handle *handle,
-			       struct net_device_stats *net_stats)
+static void hclge_update_stats(struct hnae3_handle *handle)
 {
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_dev *hdev = vport->back;
@@ -824,7 +823,7 @@ static void hclge_get_mac_stat(struct hnae3_handle *handle,
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_dev *hdev = vport->back;
 
-	hclge_update_stats(handle, NULL);
+	hclge_update_stats(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;
@@ -4965,9 +4964,7 @@ int hclge_rss_init_hw(struct hclge_dev *hdev)
 	if (ret)
 		return ret;
 
-	ret = hclge_comm_set_rss_input_tuple(&hdev->vport[0].nic,
-					     &hdev->hw.hw, true,
-					     &hdev->rss_cfg);
+	ret = hclge_comm_set_rss_input_tuple(&hdev->hw.hw, &hdev->rss_cfg);
 	if (ret)
 		return ret;
 
@@ -6243,8 +6240,7 @@ static int hclge_fd_check_spec(struct hclge_dev *hdev,
 	return hclge_fd_check_ext_tuple(hdev, fs, unused_tuple);
 }
 
-static void hclge_fd_get_tcpip4_tuple(struct hclge_dev *hdev,
-				      struct ethtool_rx_flow_spec *fs,
+static void hclge_fd_get_tcpip4_tuple(struct ethtool_rx_flow_spec *fs,
 				      struct hclge_fd_rule *rule, u8 ip_proto)
 {
 	rule->tuples.src_ip[IPV4_INDEX] =
@@ -6273,8 +6269,7 @@ static void hclge_fd_get_tcpip4_tuple(struct hclge_dev *hdev,
 	rule->tuples_mask.ip_proto = 0xFF;
 }
 
-static void hclge_fd_get_ip4_tuple(struct hclge_dev *hdev,
-				   struct ethtool_rx_flow_spec *fs,
+static void hclge_fd_get_ip4_tuple(struct ethtool_rx_flow_spec *fs,
 				   struct hclge_fd_rule *rule)
 {
 	rule->tuples.src_ip[IPV4_INDEX] =
@@ -6297,8 +6292,7 @@ static void hclge_fd_get_ip4_tuple(struct hclge_dev *hdev,
 	rule->tuples_mask.ether_proto = 0xFFFF;
 }
 
-static void hclge_fd_get_tcpip6_tuple(struct hclge_dev *hdev,
-				      struct ethtool_rx_flow_spec *fs,
+static void hclge_fd_get_tcpip6_tuple(struct ethtool_rx_flow_spec *fs,
 				      struct hclge_fd_rule *rule, u8 ip_proto)
 {
 	be32_to_cpu_array(rule->tuples.src_ip, fs->h_u.tcp_ip6_spec.ip6src,
@@ -6327,8 +6321,7 @@ static void hclge_fd_get_tcpip6_tuple(struct hclge_dev *hdev,
 	rule->tuples_mask.ip_proto = 0xFF;
 }
 
-static void hclge_fd_get_ip6_tuple(struct hclge_dev *hdev,
-				   struct ethtool_rx_flow_spec *fs,
+static void hclge_fd_get_ip6_tuple(struct ethtool_rx_flow_spec *fs,
 				   struct hclge_fd_rule *rule)
 {
 	be32_to_cpu_array(rule->tuples.src_ip, fs->h_u.usr_ip6_spec.ip6src,
@@ -6351,8 +6344,7 @@ static void hclge_fd_get_ip6_tuple(struct hclge_dev *hdev,
 	rule->tuples_mask.ether_proto = 0xFFFF;
 }
 
-static void hclge_fd_get_ether_tuple(struct hclge_dev *hdev,
-				     struct ethtool_rx_flow_spec *fs,
+static void hclge_fd_get_ether_tuple(struct ethtool_rx_flow_spec *fs,
 				     struct hclge_fd_rule *rule)
 {
 	ether_addr_copy(rule->tuples.src_mac, fs->h_u.ether_spec.h_source);
@@ -6388,8 +6380,7 @@ static void hclge_fd_get_user_def_tuple(struct hclge_fd_user_def_info *info,
 	rule->ep.user_def = *info;
 }
 
-static int hclge_fd_get_tuple(struct hclge_dev *hdev,
-			      struct ethtool_rx_flow_spec *fs,
+static int hclge_fd_get_tuple(struct ethtool_rx_flow_spec *fs,
 			      struct hclge_fd_rule *rule,
 			      struct hclge_fd_user_def_info *info)
 {
@@ -6397,31 +6388,31 @@ static int hclge_fd_get_tuple(struct hclge_dev *hdev,
 
 	switch (flow_type) {
 	case SCTP_V4_FLOW:
-		hclge_fd_get_tcpip4_tuple(hdev, fs, rule, IPPROTO_SCTP);
+		hclge_fd_get_tcpip4_tuple(fs, rule, IPPROTO_SCTP);
 		break;
 	case TCP_V4_FLOW:
-		hclge_fd_get_tcpip4_tuple(hdev, fs, rule, IPPROTO_TCP);
+		hclge_fd_get_tcpip4_tuple(fs, rule, IPPROTO_TCP);
 		break;
 	case UDP_V4_FLOW:
-		hclge_fd_get_tcpip4_tuple(hdev, fs, rule, IPPROTO_UDP);
+		hclge_fd_get_tcpip4_tuple(fs, rule, IPPROTO_UDP);
 		break;
 	case IP_USER_FLOW:
-		hclge_fd_get_ip4_tuple(hdev, fs, rule);
+		hclge_fd_get_ip4_tuple(fs, rule);
 		break;
 	case SCTP_V6_FLOW:
-		hclge_fd_get_tcpip6_tuple(hdev, fs, rule, IPPROTO_SCTP);
+		hclge_fd_get_tcpip6_tuple(fs, rule, IPPROTO_SCTP);
 		break;
 	case TCP_V6_FLOW:
-		hclge_fd_get_tcpip6_tuple(hdev, fs, rule, IPPROTO_TCP);
+		hclge_fd_get_tcpip6_tuple(fs, rule, IPPROTO_TCP);
 		break;
 	case UDP_V6_FLOW:
-		hclge_fd_get_tcpip6_tuple(hdev, fs, rule, IPPROTO_UDP);
+		hclge_fd_get_tcpip6_tuple(fs, rule, IPPROTO_UDP);
 		break;
 	case IPV6_USER_FLOW:
-		hclge_fd_get_ip6_tuple(hdev, fs, rule);
+		hclge_fd_get_ip6_tuple(fs, rule);
 		break;
 	case ETHER_FLOW:
-		hclge_fd_get_ether_tuple(hdev, fs, rule);
+		hclge_fd_get_ether_tuple(fs, rule);
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -6578,7 +6569,7 @@ static int hclge_add_fd_entry(struct hnae3_handle *handle,
 	if (!rule)
 		return -ENOMEM;
 
-	ret = hclge_fd_get_tuple(hdev, fs, rule, &info);
+	ret = hclge_fd_get_tuple(fs, rule, &info);
 	if (ret) {
 		kfree(rule);
 		return ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index f24046250341..a84a15b7f645 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -121,8 +121,7 @@ static struct hclgevf_dev *hclgevf_ae_get_hdev(struct hnae3_handle *handle)
 		return container_of(handle, struct hclgevf_dev, nic);
 }
 
-static void hclgevf_update_stats(struct hnae3_handle *handle,
-				 struct net_device_stats *net_stats)
+static void hclgevf_update_stats(struct hnae3_handle *handle)
 {
 	struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
 	int status;
@@ -1642,8 +1641,7 @@ static void hclgevf_reset(struct hclgevf_dev *hdev)
 	hclgevf_reset_err_handle(hdev);
 }
 
-static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
-						     unsigned long *addr)
+static enum hnae3_reset_type hclgevf_get_reset_level(unsigned long *addr)
 {
 	enum hnae3_reset_type rst_level = HNAE3_NONE_RESET;
 
@@ -1682,8 +1680,7 @@ static void hclgevf_reset_event(struct pci_dev *pdev,
 
 	if (hdev->default_reset_request)
 		hdev->reset_level =
-			hclgevf_get_reset_level(hdev,
-						&hdev->default_reset_request);
+			hclgevf_get_reset_level(&hdev->default_reset_request);
 	else
 		hdev->reset_level = HNAE3_VF_FUNC_RESET;
 
@@ -1825,7 +1822,7 @@ static void hclgevf_reset_service_task(struct hclgevf_dev *hdev)
 
 		hdev->last_reset_time = jiffies;
 		hdev->reset_type =
-			hclgevf_get_reset_level(hdev, &hdev->reset_pending);
+			hclgevf_get_reset_level(&hdev->reset_pending);
 		if (hdev->reset_type != HNAE3_NONE_RESET)
 			hclgevf_reset(hdev);
 	} else if (test_and_clear_bit(HCLGEVF_RESET_REQUESTED,
@@ -2157,8 +2154,7 @@ static int hclgevf_rss_init_hw(struct hclgevf_dev *hdev)
 		if (ret)
 			return ret;
 
-		ret = hclge_comm_set_rss_input_tuple(&hdev->nic, &hdev->hw.hw,
-						     false, rss_cfg);
+		ret = hclge_comm_set_rss_input_tuple(&hdev->hw.hw, rss_cfg);
 		if (ret)
 			return ret;
 	}
-- 
2.30.0

Re: [PATCH net-next 1/4] net: hns3: refine the tcam key convert handle

[Simon Horman]

On Mon, May 15, 2023 at 09:46:40PM +0800, Hao Lan wrote:
> From: Jian Shen <shenjian15@huawei.com>
> 
> The expression '(k ^ ~v)' is exaclty '(k & v)', and

This part doesn't seem correct to me.

Suppose both k and v are 0.
For simplicity, consider only one bit.

Then: (k ^ ~v) == (0 ^ ~0) == (0 ^ 1) = 1
But   (k & v)  == (0 & 0)  == 0

> '(k & v) & k' is exaclty 'k & v'. So simplify the
> expression for tcam key convert.

This I follow.

> (k ^ ~v) & k ==  (k & v) & k ==  k & k & v == k & v

Looking at the truth table (in non table form), this seems correct.

k == 0, v == 0:

  (k ^ ~v) & k == (0 ^ ~0) & 0 == (0 ^ 1) & 0 == 1 & 0 == 0
  k & v == 0 & 0 == 0
  Good!

k == 0, v == 1:

  (k ^ ~v) & k == (0 ^ ~1) & 0 == (0 ^ 0) & 0 == 1 & 0 == 0
  k & v == 0 & 1 == 0
  Good!

k == 1, v == 0:

  (k ^ ~v) & k == (1 ^ ~0) & 1 == (1 ^ 1) & 1 == 0 & 1 == 0
  k & v == 1 & 0 == 0
  Good!

k == 1, v == 1:

  (k ^ ~v) & k == (1 ^ ~1) & 1 == (1 ^ 0) & 1 == 1 & 1 == 1
  k & v == 1 & 1 == 1
  Good!

> 
> It also add necessary brackets for them.
> 
> Signed-off-by: Jian Shen <shenjian15@huawei.com>
> Signed-off-by: Hao Lan <lanhao@huawei.com>
> ---
>  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h   | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> index 81aa6b0facf5..6a43d1515585 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> @@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg {
>   * Then for input key(k) and mask(v), we can calculate the value by
>   * the formulae:
>   *	x = (~k) & v
> - *	y = (k ^ ~v) & k
> + *	y = k & v
>   */
> -#define calc_x(x, k, v) (x = ~(k) & (v))
> -#define calc_y(y, k, v) \
> -	do { \
> -		const typeof(k) _k_ = (k); \
> -		const typeof(v) _v_ = (v); \
> -		(y) = (_k_ ^ ~_v_) & (_k_); \
> -	} while (0)
> +#define calc_x(x, k, v) ((x) = ~(k) & (v))
> +#define calc_y(y, k, v) ((y) = (k) & (v))

This also looks good to me.

>  
>  #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f))
>  #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))

Re: [PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Simon Horman]

On Mon, May 15, 2023 at 09:46:42PM +0800, Hao Lan wrote:
> From: Hao Chen <chenhao418@huawei.com>
> 
> Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
> it may result in dest-buf overflow.
> 
> This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
> compiler.
> 
> The warning reports as below:
> 
> hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
> the length of the source argument [-Wstringop-truncation]
> 
> strncpy(pos, items[i].name, strlen(items[i].name));
> 
> hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
> terminating nul copying as many bytes from a string as its length
> [-Wstringop-truncation]
> 
> strncpy(pos, result[i], strlen(result[i]));
> 
> strncpy() use src-length as copy-length, it may result in
> dest-buf overflow.
> 
> So,this patch add some values check to avoid this issue.
> 
> Signed-off-by: Hao Chen <chenhao418@huawei.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
> Signed-off-by: Hao Lan <lanhao@huawei.com>
> ---
>  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
>  .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
>  2 files changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> index 4c3e90a1c4d0..cf415cb37685 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> @@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
>  				  const struct hns3_dbg_item *items,
>  				  const char **result, u16 size)
>  {
> +#define HNS3_DBG_LINE_END_LEN	2
>  	char *pos = content;
> +	u16 item_len;
>  	u16 i;
>  
> +	if (!len) {
> +		return;
> +	} else if (len <= HNS3_DBG_LINE_END_LEN) {
> +		*pos++ = '\0';
> +		return;
> +	}
> +
>  	memset(content, ' ', len);
> -	for (i = 0; i < size; i++) {
> -		if (result)
> -			strncpy(pos, result[i], strlen(result[i]));
> -		else
> -			strncpy(pos, items[i].name, strlen(items[i].name));
> +	len -= HNS3_DBG_LINE_END_LEN;
>  
> -		pos += strlen(items[i].name) + items[i].interval;
> +	for (i = 0; i < size; i++) {
> +		item_len = strlen(items[i].name) + items[i].interval;
> +		if (len < item_len)
> +			break;
> +
> +		if (result) {
> +			if (item_len < strlen(result[i]))
> +				break;
> +			memcpy(pos, result[i], strlen(result[i]));
> +		} else {
> +			memcpy(pos, items[i].name, strlen(items[i].name));

Hi,

The above memcpy() calls share the same property as the warning that
is being addressed: the length copied depends on the source not the
destination.

With the reworked code this seems safe. Which is good. But I wonder if,
given all the checking done, it makes sense to simply call strcpy() here.
Using strlen() as a length argument seems odd to me.

> +		}
> +		pos += item_len;
> +		len -= item_len;
>  	}
> -
>  	*pos++ = '\n';
>  	*pos++ = '\0';
>  }
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> index a0b46e7d863e..1354fd0461f7 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
> @@ -88,16 +88,35 @@ static void hclge_dbg_fill_content(char *content, u16 len,
>  				   const struct hclge_dbg_item *items,
>  				   const char **result, u16 size)
>  {
> +#define HCLGE_DBG_LINE_END_LEN	2
>  	char *pos = content;
> +	u16 item_len;
>  	u16 i;
>  
> +	if (!len) {
> +		return;
> +	} else if (len <= HCLGE_DBG_LINE_END_LEN) {
> +		*pos++ = '\0';
> +		return;
> +	}
> +
>  	memset(content, ' ', len);
> +	len -= HCLGE_DBG_LINE_END_LEN;
> +
>  	for (i = 0; i < size; i++) {
> -		if (result)
> -			strncpy(pos, result[i], strlen(result[i]));
> -		else
> -			strncpy(pos, items[i].name, strlen(items[i].name));
> -		pos += strlen(items[i].name) + items[i].interval;
> +		item_len = strlen(items[i].name) + items[i].interval;
> +		if (len < item_len)
> +			break;
> +
> +		if (result) {
> +			if (item_len < strlen(result[i]))
> +				break;
> +			memcpy(pos, result[i], strlen(result[i]));
> +		} else {
> +			memcpy(pos, items[i].name, strlen(items[i].name));
> +		}
> +		pos += item_len;
> +		len -= item_len;
>  	}
>  	*pos++ = '\n';
>  	*pos++ = '\0';
> -- 
> 2.30.0
> 
> 

Re: [PATCH net-next 4/4] net: hns3: clear hns unused parameter alarm

[Simon Horman]

On Mon, May 15, 2023 at 09:46:43PM +0800, Hao Lan wrote:
> From: Peiyang Wang <wangpeiyang1@huawei.com>
> 
> Several functions in the hns3 driver have unused parameters.
> The compiler will warn about them when building
> with -Wunused-parameter option of hns3.
> 
> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com>
> Signed-off-by: Hao Lan <lanhao@huawei.com>

...

> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
> index 1b360aa52e5d..8c0016b359b7 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
> @@ -693,7 +693,7 @@ static inline unsigned int hns3_page_order(struct hns3_enet_ring *ring)
>  
>  /* iterator for handling rings in ring group */
>  #define hns3_for_each_ring(pos, head) \
> -	for (pos = (head).ring; (pos); pos = (pos)->next)
> +	for ((pos) = (head).ring; (pos); (pos) = (pos)->next)

Hi,

A minor nit from my side.

This hunk does not seem related to the topic of this patch.

>  
>  #define hns3_get_handle(ndev) \
>  	(((struct hns3_nic_priv *)netdev_priv(ndev))->ae_handle)

...

Re: [PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[Simon Horman]

On Mon, May 15, 2023 at 09:46:41PM +0800, Hao Lan wrote:
> From: Hao Chen <chenhao418@huawei.com>
> 
> Hns3 driver header file uses the structure of other files, but does
> not include corresponding file, which causes a check warning that the
> header file is not self-contained by clang-tidy checker.
> 
> For example,
> Header file 'hclge_mbx.h' is not self contained.
> It should include following headers: (1) 'hclgevf_main.h'
> due to symbols 'struct hclgevf_dev'. The main source file is hns3_enet.c
> 
> Therefore, the required header file is included in the header file, and
> the structure declaration is added to the header file to avoid cyclic
> dependency of the header file.
> 
> Signed-off-by: Hao Chen <chenhao418@huawei.com>
> Signed-off-by: Hao Lan <lanhao@huawei.com>

Hi,

out of curiosity I'm wondering if you could provide some
more information on how you generated the warnings that you are
addressing here.

Re: [PATCH net-next 1/4] net: hns3: refine the tcam key convert handle

[Simon Horman]

On Mon, May 15, 2023 at 05:51:03PM +0200, Simon Horman wrote:
> On Mon, May 15, 2023 at 09:46:40PM +0800, Hao Lan wrote:
> > From: Jian Shen <shenjian15@huawei.com>
> > 
> > The expression '(k ^ ~v)' is exaclty '(k & v)', and

Sorry, one more thing: s/exaclty/exactly/
Also one more time below.

> 
> This part doesn't seem correct to me.
> 
> Suppose both k and v are 0.
> For simplicity, consider only one bit.
> 
> Then: (k ^ ~v) == (0 ^ ~0) == (0 ^ 1) = 1
> But   (k & v)  == (0 & 0)  == 0
> 
> > '(k & v) & k' is exaclty 'k & v'. So simplify the
> > expression for tcam key convert.
> 
> This I follow.
> 
> > (k ^ ~v) & k ==  (k & v) & k ==  k & k & v == k & v
> 
> Looking at the truth table (in non table form), this seems correct.
> 
> k == 0, v == 0:
> 
>   (k ^ ~v) & k == (0 ^ ~0) & 0 == (0 ^ 1) & 0 == 1 & 0 == 0
>   k & v == 0 & 0 == 0
>   Good!
> 
> k == 0, v == 1:
> 
>   (k ^ ~v) & k == (0 ^ ~1) & 0 == (0 ^ 0) & 0 == 1 & 0 == 0
>   k & v == 0 & 1 == 0
>   Good!
> 
> k == 1, v == 0:
> 
>   (k ^ ~v) & k == (1 ^ ~0) & 1 == (1 ^ 1) & 1 == 0 & 1 == 0
>   k & v == 1 & 0 == 0
>   Good!
> 
> k == 1, v == 1:
> 
>   (k ^ ~v) & k == (1 ^ ~1) & 1 == (1 ^ 0) & 1 == 1 & 1 == 1
>   k & v == 1 & 1 == 1
>   Good!
> 
> > 
> > It also add necessary brackets for them.
> > 
> > Signed-off-by: Jian Shen <shenjian15@huawei.com>
> > Signed-off-by: Hao Lan <lanhao@huawei.com>
> > ---
> >  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h   | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> > index 81aa6b0facf5..6a43d1515585 100644
> > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
> > @@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg {
> >   * Then for input key(k) and mask(v), we can calculate the value by
> >   * the formulae:
> >   *	x = (~k) & v
> > - *	y = (k ^ ~v) & k
> > + *	y = k & v
> >   */
> > -#define calc_x(x, k, v) (x = ~(k) & (v))
> > -#define calc_y(y, k, v) \
> > -	do { \
> > -		const typeof(k) _k_ = (k); \
> > -		const typeof(v) _v_ = (v); \
> > -		(y) = (_k_ ^ ~_v_) & (_k_); \
> > -	} while (0)
> > +#define calc_x(x, k, v) ((x) = ~(k) & (v))
> > +#define calc_y(y, k, v) ((y) = (k) & (v))
> 
> This also looks good to me.
> 
> >  
> >  #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f))
> >  #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))
> 

Re: [PATCH net-next 4/4] net: hns3: clear hns unused parameter alarm

[Hao Lan]

On 2023/5/16 4:01, Simon Horman wrote:
> On Mon, May 15, 2023 at 09:46:43PM +0800, Hao Lan wrote:
>> From: Peiyang Wang <wangpeiyang1@huawei.com>
>>
>> Several functions in the hns3 driver have unused parameters.
>> The compiler will warn about them when building
>> with -Wunused-parameter option of hns3.
>>
>> Signed-off-by: Peiyang Wang <wangpeiyang1@huawei.com>
>> Signed-off-by: Hao Lan <lanhao@huawei.com>
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
>> index 1b360aa52e5d..8c0016b359b7 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
>> @@ -693,7 +693,7 @@ static inline unsigned int hns3_page_order(struct hns3_enet_ring *ring)
>>  
>>  /* iterator for handling rings in ring group */
>>  #define hns3_for_each_ring(pos, head) \
>> -	for (pos = (head).ring; (pos); pos = (pos)->next)
>> +	for ((pos) = (head).ring; (pos); (pos) = (pos)->next)
> 
> Hi,
> 
> A minor nit from my side.
> 
> This hunk does not seem related to the topic of this patch.
> 

Hi,
Thank you for the review. It's really an oversight. We'll delete this hunk from this patch.

>>  
>>  #define hns3_get_handle(ndev) \
>>  	(((struct hns3_nic_priv *)netdev_priv(ndev))->ae_handle)
> 
> ...
> .
> 

Re: [PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Hao Lan]

On 2023/5/16 3:57, Simon Horman wrote:
> On Mon, May 15, 2023 at 09:46:42PM +0800, Hao Lan wrote:
>> From: Hao Chen <chenhao418@huawei.com>
>>
>> Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
>> it may result in dest-buf overflow.
>>
>> This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
>> compiler.
>>
>> The warning reports as below:
>>
>> hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
>> the length of the source argument [-Wstringop-truncation]
>>
>> strncpy(pos, items[i].name, strlen(items[i].name));
>>
>> hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
>> terminating nul copying as many bytes from a string as its length
>> [-Wstringop-truncation]
>>
>> strncpy(pos, result[i], strlen(result[i]));
>>
>> strncpy() use src-length as copy-length, it may result in
>> dest-buf overflow.
>>
>> So,this patch add some values check to avoid this issue.
>>
>> Signed-off-by: Hao Chen <chenhao418@huawei.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
>> Signed-off-by: Hao Lan <lanhao@huawei.com>
>> ---
>>  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
>>  .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
>>  2 files changed, 48 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>> index 4c3e90a1c4d0..cf415cb37685 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>> @@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
>>  				  const struct hns3_dbg_item *items,
>>  				  const char **result, u16 size)
>>  {
>> +#define HNS3_DBG_LINE_END_LEN	2
>>  	char *pos = content;
>> +	u16 item_len;
>>  	u16 i;
>>  
>> +	if (!len) {
>> +		return;
>> +	} else if (len <= HNS3_DBG_LINE_END_LEN) {
>> +		*pos++ = '\0';
>> +		return;
>> +	}
>> +
>>  	memset(content, ' ', len);
>> -	for (i = 0; i < size; i++) {
>> -		if (result)
>> -			strncpy(pos, result[i], strlen(result[i]));
>> -		else
>> -			strncpy(pos, items[i].name, strlen(items[i].name));
>> +	len -= HNS3_DBG_LINE_END_LEN;
>>  
>> -		pos += strlen(items[i].name) + items[i].interval;
>> +	for (i = 0; i < size; i++) {
>> +		item_len = strlen(items[i].name) + items[i].interval;
>> +		if (len < item_len)
>> +			break;
>> +
>> +		if (result) {
>> +			if (item_len < strlen(result[i]))
>> +				break;
>> +			memcpy(pos, result[i], strlen(result[i]));
>> +		} else {
>> +			memcpy(pos, items[i].name, strlen(items[i].name));
> 
> Hi,
> 
> The above memcpy() calls share the same property as the warning that
> is being addressed: the length copied depends on the source not the
> destination.
> 
> With the reworked code this seems safe. Which is good. But I wonder if,
> given all the checking done, it makes sense to simply call strcpy() here.
> Using strlen() as a length argument seems odd to me.
> 
Hi,
Thanks for your review.
1. We think the memcpy is correct, our length copied depends on the source,
or do I not understand you?
void *memcpy(void *dest, const void *src, size_t count)
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/string.c#n619

2. We don't know any other way to replace strlen. Do you have a better way for us?

Thank you
>> +		}
>> +		pos += item_len;
>> +		len -= item_len;
>>  	}
>> -
>>  	*pos++ = '\n';
>>  	*pos++ = '\0';
>>  }
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
>> index a0b46e7d863e..1354fd0461f7 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c
>> @@ -88,16 +88,35 @@ static void hclge_dbg_fill_content(char *content, u16 len,
>>  				   const struct hclge_dbg_item *items,
>>  				   const char **result, u16 size)
>>  {
>> +#define HCLGE_DBG_LINE_END_LEN	2
>>  	char *pos = content;
>> +	u16 item_len;
>>  	u16 i;
>>  
>> +	if (!len) {
>> +		return;
>> +	} else if (len <= HCLGE_DBG_LINE_END_LEN) {
>> +		*pos++ = '\0';
>> +		return;
>> +	}
>> +
>>  	memset(content, ' ', len);
>> +	len -= HCLGE_DBG_LINE_END_LEN;
>> +
>>  	for (i = 0; i < size; i++) {
>> -		if (result)
>> -			strncpy(pos, result[i], strlen(result[i]));
>> -		else
>> -			strncpy(pos, items[i].name, strlen(items[i].name));
>> -		pos += strlen(items[i].name) + items[i].interval;
>> +		item_len = strlen(items[i].name) + items[i].interval;
>> +		if (len < item_len)
>> +			break;
>> +
>> +		if (result) {
>> +			if (item_len < strlen(result[i]))
>> +				break;
>> +			memcpy(pos, result[i], strlen(result[i]));
>> +		} else {
>> +			memcpy(pos, items[i].name, strlen(items[i].name));
>> +		}
>> +		pos += item_len;
>> +		len -= item_len;
>>  	}
>>  	*pos++ = '\n';
>>  	*pos++ = '\0';
>> -- 
>> 2.30.0
>>
>>
> .
> 

Re: [PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[Hao Lan]

On 2023/5/16 4:02, Simon Horman wrote:
> Hi,
> 
> out of curiosity I'm wondering if you could provide some
> more information on how you generated the warnings that you are
> addressing here.
> .
Hi,
Thanks for your review.
We will attach the warning details in the next patch.

Re: [PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Simon Horman]

On Tue, May 16, 2023 at 09:09:45PM +0800, Hao Lan wrote:
> 
> 
> On 2023/5/16 3:57, Simon Horman wrote:
> > On Mon, May 15, 2023 at 09:46:42PM +0800, Hao Lan wrote:
> >> From: Hao Chen <chenhao418@huawei.com>
> >>
> >> Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
> >> it may result in dest-buf overflow.
> >>
> >> This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
> >> compiler.
> >>
> >> The warning reports as below:
> >>
> >> hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
> >> the length of the source argument [-Wstringop-truncation]
> >>
> >> strncpy(pos, items[i].name, strlen(items[i].name));
> >>
> >> hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
> >> terminating nul copying as many bytes from a string as its length
> >> [-Wstringop-truncation]
> >>
> >> strncpy(pos, result[i], strlen(result[i]));
> >>
> >> strncpy() use src-length as copy-length, it may result in
> >> dest-buf overflow.
> >>
> >> So,this patch add some values check to avoid this issue.
> >>
> >> Signed-off-by: Hao Chen <chenhao418@huawei.com>
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
> >> Signed-off-by: Hao Lan <lanhao@huawei.com>
> >> ---
> >>  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
> >>  .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
> >>  2 files changed, 48 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >> index 4c3e90a1c4d0..cf415cb37685 100644
> >> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >> @@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
> >>  				  const struct hns3_dbg_item *items,
> >>  				  const char **result, u16 size)
> >>  {
> >> +#define HNS3_DBG_LINE_END_LEN	2
> >>  	char *pos = content;
> >> +	u16 item_len;
> >>  	u16 i;
> >>  
> >> +	if (!len) {
> >> +		return;
> >> +	} else if (len <= HNS3_DBG_LINE_END_LEN) {
> >> +		*pos++ = '\0';
> >> +		return;
> >> +	}
> >> +
> >>  	memset(content, ' ', len);
> >> -	for (i = 0; i < size; i++) {
> >> -		if (result)
> >> -			strncpy(pos, result[i], strlen(result[i]));
> >> -		else
> >> -			strncpy(pos, items[i].name, strlen(items[i].name));
> >> +	len -= HNS3_DBG_LINE_END_LEN;
> >>  
> >> -		pos += strlen(items[i].name) + items[i].interval;
> >> +	for (i = 0; i < size; i++) {
> >> +		item_len = strlen(items[i].name) + items[i].interval;
> >> +		if (len < item_len)
> >> +			break;
> >> +
> >> +		if (result) {
> >> +			if (item_len < strlen(result[i]))
> >> +				break;
> >> +			memcpy(pos, result[i], strlen(result[i]));
> >> +		} else {
> >> +			memcpy(pos, items[i].name, strlen(items[i].name));
> > 
> > Hi,
> > 
> > The above memcpy() calls share the same property as the warning that
> > is being addressed: the length copied depends on the source not the
> > destination.
> > 
> > With the reworked code this seems safe. Which is good. But I wonder if,
> > given all the checking done, it makes sense to simply call strcpy() here.
> > Using strlen() as a length argument seems odd to me.
> > 
> Hi,
> Thanks for your review.
> 1. We think the memcpy is correct, our length copied depends on the source,
> or do I not understand you?
> void *memcpy(void *dest, const void *src, size_t count)
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/string.c#n619
> 
> 2. We don't know any other way to replace strlen. Do you have a better way for us?

My point is that strcpy() could be used here.
Because the source is a string, and the length of the copy
is the length of the source (string).

f.e., aren't the following functionally equivalent?

	memcpy(pos, result[i], strlen(result[i]));

	strcpy(pos, result[i])

In my view using strcpy here seems a bit simpler and therefore nicer.
But if you don't think so, that is fine.

...

Re: [PATCH net-next 2/4] net: hns3: fix hns3 driver header file not self-contained issue

[Simon Horman]

On Tue, May 16, 2023 at 09:12:57PM +0800, Hao Lan wrote:
> 
> 
> On 2023/5/16 4:02, Simon Horman wrote:
> > Hi,
> > 
> > out of curiosity I'm wondering if you could provide some
> > more information on how you generated the warnings that you are
> > addressing here.
> > .
> Hi,
> Thanks for your review.
> We will attach the warning details in the next patch.

Great, thanks!

Re: [PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Hao Lan]

On 2023/5/16 22:11, Simon Horman wrote:
> On Tue, May 16, 2023 at 09:09:45PM +0800, Hao Lan wrote:
>>
>>
>> On 2023/5/16 3:57, Simon Horman wrote:
>>> On Mon, May 15, 2023 at 09:46:42PM +0800, Hao Lan wrote:
>>>> From: Hao Chen <chenhao418@huawei.com>
>>>>
>>>> Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
>>>> it may result in dest-buf overflow.
>>>>
>>>> This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
>>>> compiler.
>>>>
>>>> The warning reports as below:
>>>>
>>>> hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
>>>> the length of the source argument [-Wstringop-truncation]
>>>>
>>>> strncpy(pos, items[i].name, strlen(items[i].name));
>>>>
>>>> hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
>>>> terminating nul copying as many bytes from a string as its length
>>>> [-Wstringop-truncation]
>>>>
>>>> strncpy(pos, result[i], strlen(result[i]));
>>>>
>>>> strncpy() use src-length as copy-length, it may result in
>>>> dest-buf overflow.
>>>>
>>>> So,this patch add some values check to avoid this issue.
>>>>
>>>> Signed-off-by: Hao Chen <chenhao418@huawei.com>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>> Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
>>>> Signed-off-by: Hao Lan <lanhao@huawei.com>
>>>> ---
>>>>  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
>>>>  .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
>>>>  2 files changed, 48 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>>>> index 4c3e90a1c4d0..cf415cb37685 100644
>>>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>>>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
>>>> @@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
>>>>  				  const struct hns3_dbg_item *items,
>>>>  				  const char **result, u16 size)
>>>>  {
>>>> +#define HNS3_DBG_LINE_END_LEN	2
>>>>  	char *pos = content;
>>>> +	u16 item_len;
>>>>  	u16 i;
>>>>  
>>>> +	if (!len) {
>>>> +		return;
>>>> +	} else if (len <= HNS3_DBG_LINE_END_LEN) {
>>>> +		*pos++ = '\0';
>>>> +		return;
>>>> +	}
>>>> +
>>>>  	memset(content, ' ', len);
>>>> -	for (i = 0; i < size; i++) {
>>>> -		if (result)
>>>> -			strncpy(pos, result[i], strlen(result[i]));
>>>> -		else
>>>> -			strncpy(pos, items[i].name, strlen(items[i].name));
>>>> +	len -= HNS3_DBG_LINE_END_LEN;
>>>>  
>>>> -		pos += strlen(items[i].name) + items[i].interval;
>>>> +	for (i = 0; i < size; i++) {
>>>> +		item_len = strlen(items[i].name) + items[i].interval;
>>>> +		if (len < item_len)
>>>> +			break;
>>>> +
>>>> +		if (result) {
>>>> +			if (item_len < strlen(result[i]))
>>>> +				break;
>>>> +			memcpy(pos, result[i], strlen(result[i]));
>>>> +		} else {
>>>> +			memcpy(pos, items[i].name, strlen(items[i].name));
>>>
>>> Hi,
>>>
>>> The above memcpy() calls share the same property as the warning that
>>> is being addressed: the length copied depends on the source not the
>>> destination.
>>>
>>> With the reworked code this seems safe. Which is good. But I wonder if,
>>> given all the checking done, it makes sense to simply call strcpy() here.
>>> Using strlen() as a length argument seems odd to me.
>>>
>> Hi,
>> Thanks for your review.
>> 1. We think the memcpy is correct, our length copied depends on the source,
>> or do I not understand you?
>> void *memcpy(void *dest, const void *src, size_t count)
>> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/string.c#n619
>>
>> 2. We don't know any other way to replace strlen. Do you have a better way for us?
> 
> My point is that strcpy() could be used here.
> Because the source is a string, and the length of the copy
> is the length of the source (string).
> 
> f.e., aren't the following functionally equivalent?
> 
> 	memcpy(pos, result[i], strlen(result[i]));
> 
> 	strcpy(pos, result[i])
> 
> In my view using strcpy here seems a bit simpler and therefore nicer.
> But if you don't think so, that is fine.
> 
Hi,
Thanks for your review.
Here is a warning report about using strcpy. This patch is used to fix the warning.
Link: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/

All warnings (new ones prefixed by >>):

   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_vlan_filter_config.constprop' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2080:2:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_vlan_filter_config.constprop' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2102:3:
>> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
      90 |                         strncpy(pos, result[i], strlen(result[i]));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_mac_list' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1872:2:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_mac_list' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1887:4:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      90 |                         strncpy(pos, result[i], strlen(result[i]));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_tm_pg' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:735:2:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_tm_pg' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:775:3:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      90 |                         strncpy(pos, result[i], strlen(result[i]));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_tm_qset' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1027:2:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_tm_qset' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1059:3:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      90 |                         strncpy(pos, result[i], strlen(result[i]));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_vlan_offload_config' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2123:2:
   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
      92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In function 'hclge_dbg_fill_content',
       inlined from 'hclge_dbg_dump_vlan_offload_config' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2154:3:
>> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
      90 |                         strncpy(pos, result[i], strlen(result[i]));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> ...
> .
> 

Re: [PATCH net-next 3/4] net: hns3: fix strncpy() not using dest-buf length as length issue

[Simon Horman]

On Tue, May 16, 2023 at 11:35:55PM +0800, Hao Lan wrote:
> 
> 
> On 2023/5/16 22:11, Simon Horman wrote:
> > On Tue, May 16, 2023 at 09:09:45PM +0800, Hao Lan wrote:
> >>
> >>
> >> On 2023/5/16 3:57, Simon Horman wrote:
> >>> On Mon, May 15, 2023 at 09:46:42PM +0800, Hao Lan wrote:
> >>>> From: Hao Chen <chenhao418@huawei.com>
> >>>>
> >>>> Now, strncpy() in hns3_dbg_fill_content() use src-length as copy-length,
> >>>> it may result in dest-buf overflow.
> >>>>
> >>>> This patch is to fix intel compile warning for csky-linux-gcc (GCC) 12.1.0
> >>>> compiler.
> >>>>
> >>>> The warning reports as below:
> >>>>
> >>>> hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on
> >>>> the length of the source argument [-Wstringop-truncation]
> >>>>
> >>>> strncpy(pos, items[i].name, strlen(items[i].name));
> >>>>
> >>>> hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before
> >>>> terminating nul copying as many bytes from a string as its length
> >>>> [-Wstringop-truncation]
> >>>>
> >>>> strncpy(pos, result[i], strlen(result[i]));
> >>>>
> >>>> strncpy() use src-length as copy-length, it may result in
> >>>> dest-buf overflow.
> >>>>
> >>>> So,this patch add some values check to avoid this issue.
> >>>>
> >>>> Signed-off-by: Hao Chen <chenhao418@huawei.com>
> >>>> Reported-by: kernel test robot <lkp@intel.com>
> >>>> Closes: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/
> >>>> Signed-off-by: Hao Lan <lanhao@huawei.com>
> >>>> ---
> >>>>  .../ethernet/hisilicon/hns3/hns3_debugfs.c    | 31 ++++++++++++++-----
> >>>>  .../hisilicon/hns3/hns3pf/hclge_debugfs.c     | 29 ++++++++++++++---
> >>>>  2 files changed, 48 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >>>> index 4c3e90a1c4d0..cf415cb37685 100644
> >>>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >>>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c
> >>>> @@ -438,19 +438,36 @@ static void hns3_dbg_fill_content(char *content, u16 len,
> >>>>  				  const struct hns3_dbg_item *items,
> >>>>  				  const char **result, u16 size)
> >>>>  {
> >>>> +#define HNS3_DBG_LINE_END_LEN	2
> >>>>  	char *pos = content;
> >>>> +	u16 item_len;
> >>>>  	u16 i;
> >>>>  
> >>>> +	if (!len) {
> >>>> +		return;
> >>>> +	} else if (len <= HNS3_DBG_LINE_END_LEN) {
> >>>> +		*pos++ = '\0';
> >>>> +		return;
> >>>> +	}
> >>>> +
> >>>>  	memset(content, ' ', len);
> >>>> -	for (i = 0; i < size; i++) {
> >>>> -		if (result)
> >>>> -			strncpy(pos, result[i], strlen(result[i]));
> >>>> -		else
> >>>> -			strncpy(pos, items[i].name, strlen(items[i].name));
> >>>> +	len -= HNS3_DBG_LINE_END_LEN;
> >>>>  
> >>>> -		pos += strlen(items[i].name) + items[i].interval;
> >>>> +	for (i = 0; i < size; i++) {
> >>>> +		item_len = strlen(items[i].name) + items[i].interval;
> >>>> +		if (len < item_len)
> >>>> +			break;
> >>>> +
> >>>> +		if (result) {
> >>>> +			if (item_len < strlen(result[i]))
> >>>> +				break;
> >>>> +			memcpy(pos, result[i], strlen(result[i]));
> >>>> +		} else {
> >>>> +			memcpy(pos, items[i].name, strlen(items[i].name));
> >>>
> >>> Hi,
> >>>
> >>> The above memcpy() calls share the same property as the warning that
> >>> is being addressed: the length copied depends on the source not the
> >>> destination.
> >>>
> >>> With the reworked code this seems safe. Which is good. But I wonder if,
> >>> given all the checking done, it makes sense to simply call strcpy() here.
> >>> Using strlen() as a length argument seems odd to me.
> >>>
> >> Hi,
> >> Thanks for your review.
> >> 1. We think the memcpy is correct, our length copied depends on the source,
> >> or do I not understand you?
> >> void *memcpy(void *dest, const void *src, size_t count)
> >> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/string.c#n619
> >>
> >> 2. We don't know any other way to replace strlen. Do you have a better way for us?
> > 
> > My point is that strcpy() could be used here.
> > Because the source is a string, and the length of the copy
> > is the length of the source (string).
> > 
> > f.e., aren't the following functionally equivalent?
> > 
> > 	memcpy(pos, result[i], strlen(result[i]));
> > 
> > 	strcpy(pos, result[i])
> > 
> > In my view using strcpy here seems a bit simpler and therefore nicer.
> > But if you don't think so, that is fine.
> > 
> Hi,
> Thanks for your review.
> Here is a warning report about using strcpy. This patch is used to fix the warning.
> Link: https://lore.kernel.org/lkml/202207170606.7WtHs9yS-lkp@intel.com/T/

Thanks.

Just to clarify, I was suggesting strcpy() rather than strncpy().
But I didn't try my suggestion.

In any case, all these ideas, including memcpy() have the underlying
issue that is being flagged below: that the length of the copy is based on
the source not the destination.

As I said earlier, I agree that your changes make this safe,
regardless of what function is used to actually copy the data.
And my suggestion is cosmetic rather than functional.

> All warnings (new ones prefixed by >>):
> 
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_vlan_filter_config.constprop' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2080:2:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_vlan_filter_config.constprop' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2102:3:
> >> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
>       90 |                         strncpy(pos, result[i], strlen(result[i]));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_mac_list' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1872:2:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_mac_list' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1887:4:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       90 |                         strncpy(pos, result[i], strlen(result[i]));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_tm_pg' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:735:2:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_tm_pg' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:775:3:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       90 |                         strncpy(pos, result[i], strlen(result[i]));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_tm_qset' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1027:2:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_tm_qset' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:1059:3:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       90 |                         strncpy(pos, result[i], strlen(result[i]));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_vlan_offload_config' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2123:2:
>    drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:92:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-truncation]
>       92 |                         strncpy(pos, items[i].name, strlen(items[i].name));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    In function 'hclge_dbg_fill_content',
>        inlined from 'hclge_dbg_dump_vlan_offload_config' at drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:2154:3:
> >> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c:90:25: warning: 'strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
>       90 |                         strncpy(pos, result[i], strlen(result[i]));
>          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> > ...
> > .
> >