Skip to contentSkip to conversation

hns: Fix string set validation in ethtool string operations

PATCH New Ben Hutchings 2 participants 4 messages Mar 13, 2018 – Mar 15, 2018

Subsystems (3)
Networking Network drivers HiSilicon network subsystem driver
lore
Share & tools
lore

Ben Hutchings

author
#0 lore raw

PATCH 1/2 hns: Fix string set validation in ethtool string operations

The hns driver used to assume that it would only ever be asked about ETH_SS_TEST or ETH_SS_STATS, and for any other stringset it would claim a count of 0 but if asked for names would copy over all the statistic names. This resulted in a potential heap buffer overflow.

This was "fixed" some time ago by making it report the number of statistics when asked about the ETH_SS_PRIV_FLAGS stringset. This doesn't make any sense, and it left the bug still exploitable with other stringset numbers.

As a minimal but complete fix, stop calling the driver-internal string operations for any stringset other than ETH_SS_STATS.

Fixes 511e6bc071db ("net: add Hisilicon Network Subsystem DSAF support")
Fixes 412b65d15a7f ("net: hns: fix ethtool_get_strings overflow ...")
Signed-off-by Ben Hutchings <[email protected]>
---
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c +5 -12
index 7ea7f8a4aa2a..b836742f7ab6 100644
@@ -889,11 +889,6 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	struct hnae_handle *h = priv->ae_handle;
 	char *buff = (char *)data;
 
-	if (!h->dev->ops->get_strings) {
-		netdev_err(netdev, "h->dev->ops->get_strings is null!\n");
-		return;
-	}
-
 	if (stringset == ETH_SS_TEST) {
 		if (priv->ae_handle->phy_if != PHY_INTERFACE_MODE_XGMII) {
 			memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_MAC],

@@ -907,7 +902,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 			memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_PHY],
 			       ETH_GSTRING_LEN);
 
-	} else {
+	} else if (stringset == ETH_SS_STATS && h->dev->ops->get_strings) {
 		snprintf(buff, ETH_GSTRING_LEN, "rx_packets");
 		buff = buff + ETH_GSTRING_LEN;
 		snprintf(buff, ETH_GSTRING_LEN, "tx_packets");

@@ -969,7 +964,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 /**
  * nic_get_sset_count - get string set count witch returned by nic_get_strings.
  * @dev: net device
- * @stringset: string set index, 0: self test string; 1: statistics string.
+ * @stringset: string set index
  *
  * Return string set count.
  */

@@ -979,10 +974,6 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
 	struct hnae_handle *h = priv->ae_handle;
 	struct hnae_ae_ops *ops = h->dev->ops;
 
-	if (!ops->get_sset_count) {
-		netdev_err(netdev, "get_sset_count is null!\n");
-		return -EOPNOTSUPP;
-	}
 	if (stringset == ETH_SS_TEST) {
 		u32 cnt = (sizeof(hns_nic_test_strs) / ETH_GSTRING_LEN);
 

@@ -993,8 +984,10 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
 			cnt--;
 
 		return cnt;
+	} else if (stringset == ETH_SS_STATS && ops->get_sset_count) {
+		return HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset);
 	} else {
-		return (HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset));
+		return -EOPNOTSUPP;
 	}
 }
author
#1 lore raw

PATCH 2/2 hns: Clean up string operations

The driver-internal string operations are only ever used for statistics, so remove the stringset parameters and rename them accordingly.

Signed-off-by Ben Hutchings <[email protected]>
---
 drivers/net/ethernet/hisilicon/hns/hnae.h          | 13 ++++----
 drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c  | 37 +++++++++++-----------
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c | 16 +++-------
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c  |  9 +++---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h  | 10 +++---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 28 ++++++----------
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h |  6 ++--
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c  | 11 +++----
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h  |  4 +--
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c  | 20 ++++--------
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h  |  4 +--
 .../net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c    | 24 +++++---------
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c   |  9 +++---
 13 files changed, 78 insertions(+), 113 deletions(-)
drivers/net/ethernet/hisilicon/hns/hnae.h +6 -7
index 3e62692af011..480fa2b49be7 100644
@@ -457,10 +457,10 @@ enum hnae_media_type {
  *   update Old network device statistics
  * get_ethtool_stats()
  *   get ethtool network device statistics
- * get_strings()
- *   get a set of strings that describe the requested objects
- * get_sset_count()
- *   get number of strings that @get_strings will write
+ * get_stats_strings()
+ *   get a set of strings that describe the statistics
+ * get_stats_count()
+ *   get number of strings that @get_stats_strings will write
  * update_led_status()
  *   update the led status
  * set_led_id()

@@ -522,9 +522,8 @@ struct hnae_ae_ops {
 	void (*update_stats)(struct hnae_handle *handle,
 			     struct net_device_stats *net_stats);
 	void (*get_stats)(struct hnae_handle *handle, u64 *data);
-	void (*get_strings)(struct hnae_handle *handle,
-			    u32 stringset, u8 *data);
-	int (*get_sset_count)(struct hnae_handle *handle, int stringset);
+	void (*get_stats_strings)(struct hnae_handle *handle, u8 *data);
+	int (*get_stats_count)(struct hnae_handle *handle);
 	void (*update_led_status)(struct hnae_handle *handle);
 	int (*set_led_id)(struct hnae_handle *handle,
 			  enum hnae_led_state status);
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c +18 -19
index bd68379d2bea..638476aa6311 100644
@@ -679,21 +679,20 @@ void hns_ae_get_stats(struct hnae_handle *handle, u64 *data)
 
 	for (idx = 0; idx < handle->q_num; idx++) {
 		hns_rcb_get_stats(handle->qs[idx], p);
-		p += hns_rcb_get_ring_sset_count((int)ETH_SS_STATS);
+		p += hns_rcb_get_ring_stats_count();
 	}
 
 	hns_ppe_get_stats(ppe_cb, p);
-	p += hns_ppe_get_sset_count((int)ETH_SS_STATS);
+	p += hns_ppe_get_stats_count();
 
 	hns_mac_get_stats(mac_cb, p);
-	p += hns_mac_get_sset_count(mac_cb, (int)ETH_SS_STATS);
+	p += hns_mac_get_stats_count(mac_cb);
 
 	if (mac_cb->mac_type == HNAE_PORT_SERVICE)
 		hns_dsaf_get_stats(vf_cb->dsaf_dev, p, vf_cb->port_index);
 }
 
-void hns_ae_get_strings(struct hnae_handle *handle,
-			u32 stringset, u8 *data)
+void hns_ae_get_stats_strings(struct hnae_handle *handle, u8 *data)
 {
 	int port;
 	int idx;

@@ -711,21 +710,21 @@ void hns_ae_get_strings(struct hnae_handle *handle,
 	ppe_cb = hns_get_ppe_cb(handle);
 
 	for (idx = 0; idx < handle->q_num; idx++) {
-		hns_rcb_get_strings(stringset, p, idx);
-		p += ETH_GSTRING_LEN * hns_rcb_get_ring_sset_count(stringset);
+		hns_rcb_get_stats_strings(p, idx);
+		p += ETH_GSTRING_LEN * hns_rcb_get_ring_stats_count();
 	}
 
-	hns_ppe_get_strings(ppe_cb, stringset, p);
-	p += ETH_GSTRING_LEN * hns_ppe_get_sset_count(stringset);
+	hns_ppe_get_stats_strings(ppe_cb, p);
+	p += ETH_GSTRING_LEN * hns_ppe_get_stats_count();
 
-	hns_mac_get_strings(mac_cb, stringset, p);
-	p += ETH_GSTRING_LEN * hns_mac_get_sset_count(mac_cb, stringset);
+	hns_mac_get_stats_strings(mac_cb, p);
+	p += ETH_GSTRING_LEN * hns_mac_get_stats_count(mac_cb);
 
 	if (mac_cb->mac_type == HNAE_PORT_SERVICE)
-		hns_dsaf_get_strings(stringset, p, port, dsaf_dev);
+		hns_dsaf_get_stats_strings(p, port, dsaf_dev);
 }
 
-int hns_ae_get_sset_count(struct hnae_handle *handle, int stringset)
+int hns_ae_get_stats_count(struct hnae_handle *handle)
 {
 	u32 sset_count = 0;
 	struct hns_mac_cb *mac_cb;

@@ -735,12 +734,12 @@ int hns_ae_get_sset_count(struct hnae_handle *handle, int stringset)
 
 	mac_cb = hns_get_mac_cb(handle);
 
-	sset_count += hns_rcb_get_ring_sset_count(stringset) * handle->q_num;
-	sset_count += hns_ppe_get_sset_count(stringset);
-	sset_count += hns_mac_get_sset_count(mac_cb, stringset);
+	sset_count += hns_rcb_get_ring_stats_count() * handle->q_num;
+	sset_count += hns_ppe_get_stats_count();
+	sset_count += hns_mac_get_stats_count(mac_cb);
 
 	if (mac_cb->mac_type == HNAE_PORT_SERVICE)
-		sset_count += hns_dsaf_get_sset_count(dsaf_dev, stringset);
+		sset_count += hns_dsaf_get_stats_count(dsaf_dev);
 
 	return sset_count;
 }

@@ -923,8 +922,8 @@ static struct hnae_ae_ops hns_dsaf_ops = {
 	.update_stats = hns_ae_update_stats,
 	.set_tso_stats = hns_ae_set_tso_stats,
 	.get_stats = hns_ae_get_stats,
-	.get_strings = hns_ae_get_strings,
-	.get_sset_count = hns_ae_get_sset_count,
+	.get_stats_strings = hns_ae_get_stats_strings,
+	.get_stats_count = hns_ae_get_stats_count,
 	.update_led_status = hns_ae_update_led_status,
 	.set_led_id = hns_ae_cpld_set_led_id,
 	.get_regs = hns_ae_get_regs,
drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c +5 -11
index 86944bc3b273..7525b03fd3d8 100644
@@ -649,14 +649,11 @@ static void hns_gmac_get_stats(void *mac_drv, u64 *data)
 	}
 }
 
-static void hns_gmac_get_strings(u32 stringset, u8 *data)
+static void hns_gmac_get_stats_strings(u8 *data)
 {
 	char *buff = (char *)data;
 	u32 i;
 
-	if (stringset != ETH_SS_STATS)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(g_gmac_stats_string); i++) {
 		snprintf(buff, ETH_GSTRING_LEN, "%s",
 			 g_gmac_stats_string[i].desc);

@@ -664,12 +661,9 @@ static void hns_gmac_get_strings(u32 stringset, u8 *data)
 	}
 }
 
-static int hns_gmac_get_sset_count(int stringset)
+static int hns_gmac_get_stats_count(void)
 {
-	if (stringset == ETH_SS_STATS || stringset == ETH_SS_PRIV_FLAGS)
-		return ARRAY_SIZE(g_gmac_stats_string);
-
-	return 0;
+	return ARRAY_SIZE(g_gmac_stats_string);
 }
 
 static int hns_gmac_get_regs_count(void)

@@ -713,8 +707,8 @@ void *hns_gmac_config(struct hns_mac_cb *mac_cb, struct mac_params *mac_param)
 	mac_drv->get_regs = hns_gmac_get_regs;
 	mac_drv->get_regs_count = hns_gmac_get_regs_count;
 	mac_drv->get_ethtool_stats = hns_gmac_get_stats;
-	mac_drv->get_sset_count = hns_gmac_get_sset_count;
-	mac_drv->get_strings = hns_gmac_get_strings;
+	mac_drv->get_stats_count = hns_gmac_get_stats_count;
+	mac_drv->get_stats_strings = hns_gmac_get_stats_strings;
 	mac_drv->update_stats = hns_gmac_update_stats;
 	mac_drv->set_promiscuous = hns_gmac_set_promisc;
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c +4 -5
index cac86e9ae0dd..094476b27c00 100644
@@ -1109,19 +1109,18 @@ void hns_mac_get_stats(struct hns_mac_cb *mac_cb, u64 *data)
 	mac_ctrl_drv->get_ethtool_stats(mac_ctrl_drv, data);
 }
 
-void hns_mac_get_strings(struct hns_mac_cb *mac_cb,
-			 int stringset, u8 *data)
+void hns_mac_get_stats_strings(struct hns_mac_cb *mac_cb, u8 *data)
 {
 	struct mac_driver *mac_ctrl_drv = hns_mac_get_drv(mac_cb);
 
-	mac_ctrl_drv->get_strings(stringset, data);
+	mac_ctrl_drv->get_stats_strings(data);
 }
 
-int hns_mac_get_sset_count(struct hns_mac_cb *mac_cb, int stringset)
+int hns_mac_get_stats_count(struct hns_mac_cb *mac_cb)
 {
 	struct mac_driver *mac_ctrl_drv = hns_mac_get_drv(mac_cb);
 
-	return mac_ctrl_drv->get_sset_count(stringset);
+	return mac_ctrl_drv->get_stats_count();
 }
 
 void hns_mac_set_promisc(struct hns_mac_cb *mac_cb, u8 en)
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h +5 -5
index bbc0a98e7ca3..495462d38cb2 100644
@@ -383,9 +383,9 @@ struct mac_driver {
 	void (*get_regs)(void *mac_drv, void *data);
 	int (*get_regs_count)(void);
 	/* get strings name for ethtool statistic */
-	void (*get_strings)(u32 stringset, u8 *data);
-	/* get the number of strings*/
-	int (*get_sset_count)(int stringset);
+	void (*get_stats_strings)(u8 *data);
+	/* get the number of statistics */
+	int (*get_stats_count)(void);
 
 	/* get the statistic by ethtools*/
 	void (*get_ethtool_stats)(void *mac_drv, u64 *data);

@@ -448,8 +448,8 @@ int hns_mac_config_mac_loopback(struct hns_mac_cb *mac_cb,
 				enum hnae_loop loop, int en);
 void hns_mac_update_stats(struct hns_mac_cb *mac_cb);
 void hns_mac_get_stats(struct hns_mac_cb *mac_cb, u64 *data);
-void hns_mac_get_strings(struct hns_mac_cb *mac_cb, int stringset, u8 *data);
-int hns_mac_get_sset_count(struct hns_mac_cb *mac_cb, int stringset);
+void hns_mac_get_stats_strings(struct hns_mac_cb *mac_cb, u8 *data);
+int hns_mac_get_stats_count(struct hns_mac_cb *mac_cb);
 void hns_mac_get_regs(struct hns_mac_cb *mac_cb, void *data);
 int hns_mac_get_regs_count(struct hns_mac_cb *mac_cb);
 void hns_set_led_opt(struct hns_mac_cb *mac_cb);
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c +10 -18
index e0bc79ea3d88..0ba28a29b738 100644
@@ -2617,38 +2617,30 @@ void hns_dsaf_get_stats(struct dsaf_device *ddev, u64 *data, int port)
 }
 
 /**
- *hns_dsaf_get_sset_count - get dsaf string set count
- *@stringset: type of values in data
- *return dsaf string name count
+ *hns_dsaf_get_stats_count - get dsaf statistic count
+ *return dsaf statistic count
  */
-int hns_dsaf_get_sset_count(struct dsaf_device *dsaf_dev, int stringset)
+int hns_dsaf_get_stats_count(struct dsaf_device *dsaf_dev)
 {
 	bool is_ver1 = AE_IS_VER1(dsaf_dev->dsaf_ver);
 
-	if (stringset == ETH_SS_STATS) {
-		if (is_ver1)
-			return DSAF_STATIC_NUM;
-		else
-			return DSAF_V2_STATIC_NUM;
-	}
-	return 0;
+	if (is_ver1)
+		return DSAF_STATIC_NUM;
+	else
+		return DSAF_V2_STATIC_NUM;
 }
 
 /**
- *hns_dsaf_get_strings - get dsaf string set
- *@stringset:srting set index
+ *hns_dsaf_get_stats_strings - get dsaf statistic strings
  *@data:strings name value
  *@port:port index
  */
-void hns_dsaf_get_strings(int stringset, u8 *data, int port,
-			  struct dsaf_device *dsaf_dev)
+void hns_dsaf_get_stats_strings(u8 *data, int port,
+				struct dsaf_device *dsaf_dev)
 {
 	char *buff = (char *)data;
 	int node = port;
 
-	if (stringset != ETH_SS_STATS)
-		return;
-
 	/* for ge/xge node info */
 	buff = hns_dsaf_get_node_stats_strings(buff, node, dsaf_dev);
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h +3 -3
index 4507e8222683..275dec595dc7 100644
@@ -442,10 +442,10 @@ void hns_dsaf_ae_uninit(struct dsaf_device *dsaf_dev);
 
 void hns_dsaf_update_stats(struct dsaf_device *dsaf_dev, u32 inode_num);
 
-int hns_dsaf_get_sset_count(struct dsaf_device *dsaf_dev, int stringset);
+int hns_dsaf_get_stats_count(struct dsaf_device *dsaf_dev);
 void hns_dsaf_get_stats(struct dsaf_device *ddev, u64 *data, int port);
-void hns_dsaf_get_strings(int stringset, u8 *data, int port,
-			  struct dsaf_device *dsaf_dev);
+void hns_dsaf_get_stats_strings(u8 *data, int port,
+				struct dsaf_device *dsaf_dev);
 
 void hns_dsaf_get_regs(struct dsaf_device *ddev, u32 port, void *data);
 int hns_dsaf_get_regs_count(void);
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c +4 -7
index b62816c1574e..a08471441285 100644
@@ -420,11 +420,9 @@ void hns_ppe_update_stats(struct hns_ppe_cb *ppe_cb)
 		+= dsaf_read_dev(ppe_cb, PPE_HIS_TX_PKT_CS_FAIL_CNT_REG);
 }
 
-int hns_ppe_get_sset_count(int stringset)
+int hns_ppe_get_stats_count(void)
 {
-	if (stringset == ETH_SS_STATS || stringset == ETH_SS_PRIV_FLAGS)
-		return ETH_PPE_STATIC_NUM;
-	return 0;
+	return ETH_PPE_STATIC_NUM;
 }
 
 int hns_ppe_get_regs_count(void)

@@ -433,12 +431,11 @@ int hns_ppe_get_regs_count(void)
 }
 
 /**
- * ppe_get_strings - get ppe srting
+ * ppe_get_strings - get ppe statistic strings
  * @ppe_device: ppe device
- * @stringset: string set type
  * @data: output string
  */
-void hns_ppe_get_strings(struct hns_ppe_cb *ppe_cb, int stringset, u8 *data)
+void hns_ppe_get_stats_strings(struct hns_ppe_cb *ppe_cb, u8 *data)
 {
 	char *buff = (char *)data;
 	int index = ppe_cb->index;
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h +2 -2
index 9d8e643e8aa6..1baf04caf4bf 100644
@@ -108,11 +108,11 @@ void hns_ppe_reset_common(struct dsaf_device *dsaf_dev, u8 ppe_common_index);
 
 void hns_ppe_update_stats(struct hns_ppe_cb *ppe_cb);
 
-int hns_ppe_get_sset_count(int stringset);
+int hns_ppe_get_stats_count(void);
 int hns_ppe_get_regs_count(void);
 void hns_ppe_get_regs(struct hns_ppe_cb *ppe_cb, void *data);
 
-void hns_ppe_get_strings(struct hns_ppe_cb *ppe_cb, int stringset, u8 *data);
+void hns_ppe_get_stats_strings(struct hns_ppe_cb *ppe_cb, u8 *data);
 void hns_ppe_get_stats(struct hns_ppe_cb *ppe_cb, u64 *data);
 void hns_ppe_set_tso_enable(struct hns_ppe_cb *ppe_cb, u32 value);
 void hns_ppe_set_rss_key(struct hns_ppe_cb *ppe_cb,
drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c +6 -14
index 6f3570cfb501..7d4ecd8c8d6e 100644
@@ -870,16 +870,12 @@ void hns_rcb_get_stats(struct hnae_queue *queue, u64 *data)
 }
 
 /**
- *hns_rcb_get_ring_sset_count - rcb string set count
- *@stringset:ethtool cmd
- *return rcb ring string set count
+ *hns_rcb_get_ring_stats_count - rcb statistic count
+ *return rcb ring statistic count
  */
-int hns_rcb_get_ring_sset_count(int stringset)
+int hns_rcb_get_ring_stats_count(void)
 {
-	if (stringset == ETH_SS_STATS || stringset == ETH_SS_PRIV_FLAGS)
-		return HNS_RING_STATIC_REG_NUM;
-
-	return 0;
+	return HNS_RING_STATIC_REG_NUM;
 }
 
 /**

@@ -901,18 +897,14 @@ int hns_rcb_get_ring_regs_count(void)
 }
 
 /**
- *hns_rcb_get_strings - get rcb string set
- *@stringset:string set index
+ *hns_rcb_get_strings - get rcb statistic strings
  *@data:strings name value
  *@index:queue index
  */
-void hns_rcb_get_strings(int stringset, u8 *data, int index)
+void hns_rcb_get_stats_strings(u8 *data, int index)
 {
 	char *buff = (char *)data;
 
-	if (stringset != ETH_SS_STATS)
-		return;
-
 	snprintf(buff, ETH_GSTRING_LEN, "tx_ring%d_rcb_pkt_num", index);
 	buff = buff + ETH_GSTRING_LEN;
 	snprintf(buff, ETH_GSTRING_LEN, "tx_ring%d_ppe_tx_pkt_num", index);
drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h +2 -2
index 602816498c8d..95cd0111e802 100644
@@ -154,13 +154,13 @@ void hns_rcb_get_stats(struct hnae_queue *queue, u64 *data);
 
 void hns_rcb_get_common_regs(struct rcb_common_cb *rcb_common, void *data);
 
-int hns_rcb_get_ring_sset_count(int stringset);
+int hns_rcb_get_ring_stats_count(void);
 int hns_rcb_get_common_regs_count(void);
 int hns_rcb_get_ring_regs_count(void);
 
 void hns_rcb_get_ring_regs(struct hnae_queue *queue, void *data);
 
-void hns_rcb_get_strings(int stringset, u8 *data, int index);
+void hns_rcb_get_stats_strings(u8 *data, int index);
 void hns_rcb_set_rx_ring_bs(struct hnae_queue *q, u32 buf_size);
 void hns_rcb_set_tx_ring_bs(struct hnae_queue *q, u32 buf_size);
drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c +8 -16
index 51e7e9f5af49..0e8fd285bfd7 100644
@@ -756,18 +756,14 @@ static void hns_xgmac_get_stats(void *mac_drv, u64 *data)
 }
 
 /**
- *hns_xgmac_get_strings - get xgmac strings name
- *@stringset: type of values in data
+ *hns_xgmac_get_stats_strings - get xgmac statistic strings
  *@data:data for value of string name
  */
-static void hns_xgmac_get_strings(u32 stringset, u8 *data)
+static void hns_xgmac_get_stats_strings(u8 *data)
 {
 	char *buff = (char *)data;
 	u32 i;
 
-	if (stringset != ETH_SS_STATS)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(g_xgmac_stats_string); i++) {
 		snprintf(buff, ETH_GSTRING_LEN, g_xgmac_stats_string[i].desc);
 		buff = buff + ETH_GSTRING_LEN;

@@ -775,16 +771,12 @@ static void hns_xgmac_get_strings(u32 stringset, u8 *data)
 }
 
 /**
- *hns_xgmac_get_sset_count - get xgmac string set count
- *@stringset: type of values in data
- *return xgmac string set count
+ *hns_xgmac_get_stats_count - get xgmac statistic count
+ *return xgmac statistic count
  */
-static int hns_xgmac_get_sset_count(int stringset)
+static int hns_xgmac_get_stats_count(void)
 {
-	if (stringset == ETH_SS_STATS || stringset == ETH_SS_PRIV_FLAGS)
-		return ARRAY_SIZE(g_xgmac_stats_string);
-
-	return 0;
+	return ARRAY_SIZE(g_xgmac_stats_string);
 }
 
 /**

@@ -832,9 +824,9 @@ void *hns_xgmac_config(struct hns_mac_cb *mac_cb, struct mac_params *mac_param)
 	mac_drv->get_link_status = hns_xgmac_get_link_status;
 	mac_drv->get_regs = hns_xgmac_get_regs;
 	mac_drv->get_ethtool_stats = hns_xgmac_get_stats;
-	mac_drv->get_sset_count = hns_xgmac_get_sset_count;
+	mac_drv->get_stats_count = hns_xgmac_get_stats_count;
 	mac_drv->get_regs_count = hns_xgmac_get_regs_count;
-	mac_drv->get_strings = hns_xgmac_get_strings;
+	mac_drv->get_stats_strings = hns_xgmac_get_stats_strings;
 	mac_drv->update_stats = hns_xgmac_update_stats;
 
 	return (void *)mac_drv;
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c +5 -4
index b836742f7ab6..71075f69122c 100644
@@ -902,7 +902,8 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 			memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_PHY],
 			       ETH_GSTRING_LEN);
 
-	} else if (stringset == ETH_SS_STATS && h->dev->ops->get_strings) {
+	} else if (stringset == ETH_SS_STATS &&
+		   h->dev->ops->get_stats_strings) {
 		snprintf(buff, ETH_GSTRING_LEN, "rx_packets");
 		buff = buff + ETH_GSTRING_LEN;
 		snprintf(buff, ETH_GSTRING_LEN, "tx_packets");

@@ -957,7 +958,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 		snprintf(buff, ETH_GSTRING_LEN, "netdev_tx_timeout");
 		buff = buff + ETH_GSTRING_LEN;
 
-		h->dev->ops->get_strings(h, stringset, (u8 *)buff);
+		h->dev->ops->get_stats_strings(h, (u8 *)buff);
 	}
 }
 

@@ -984,8 +985,8 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
 			cnt--;
 
 		return cnt;
-	} else if (stringset == ETH_SS_STATS && ops->get_sset_count) {
-		return HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset);
+	} else if (stringset == ETH_SS_STATS && ops->get_stats_count) {
+		return HNS_NET_STATS_CNT + ops->get_stats_count(h);
 	} else {
 		return -EOPNOTSUPP;
 	}
build bot
#2 lore raw

Re: [PATCH 2/2] hns: Clean up string operations

Thank you for the patch! Perhaps something to improve:

Hi Ben,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180314]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Ben-Hutchings/hns-Fix-string-set-validation-in-ethtool-string-operations/20180315-082407
reproduce:

        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:73:20: sparse: symbol 'hns_ae_get_handle' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:332:6: sparse: symbol 'hns_ae_stop' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:360:6: sparse: symbol 'hns_ae_toggle_ring_irq' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:580:6: sparse: symbol 'hns_ae_update_stats' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:663:6: sparse: symbol 'hns_ae_get_stats' was not declared. Should it be static?

>> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:695:6: sparse: symbol 'hns_ae_get_stats_strings' was not declared. Should it be static?
>> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:727:5: sparse: symbol 'hns_ae_get_stats_count' was not declared. Should it be static?

drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:773:6: sparse: symbol 'hns_ae_update_led_status' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:785:5: sparse: symbol 'hns_ae_cpld_set_led_id' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:797:6: sparse: symbol 'hns_ae_get_regs' was not declared. Should it be static? drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c:822:5: sparse: symbol 'hns_ae_get_regs_len' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#3 lore raw

[RFC PATCH] hns: hns_ae_get_stats_strings() can be static

In reply to Ben Hutchings
Fixes: 1316dd7b461c ("hns: Clean up string operations")
Fixes 1316dd7b461c ("hns: Clean up string operations")
Signed-off-by Fengguang Wu <[email protected]>
---
 hns_ae_adapt.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c +2 -2
index 638476aa..aa89e46 100644
@@ -692,7 +692,7 @@ void hns_ae_get_stats(struct hnae_handle *handle, u64 *data)
 		hns_dsaf_get_stats(vf_cb->dsaf_dev, p, vf_cb->port_index);
 }
 
-void hns_ae_get_stats_strings(struct hnae_handle *handle, u8 *data)
+static void hns_ae_get_stats_strings(struct hnae_handle *handle, u8 *data)
 {
 	int port;
 	int idx;

@@ -724,7 +724,7 @@ void hns_ae_get_stats_strings(struct hnae_handle *handle, u8 *data)
 		hns_dsaf_get_stats_strings(p, port, dsaf_dev);
 }
 
-int hns_ae_get_stats_count(struct hnae_handle *handle)
+static int hns_ae_get_stats_count(struct hnae_handle *handle)
 {
 	u32 sset_count = 0;
 	struct hns_mac_cb *mac_cb;