All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2] hns: Fix string set validation in ethtool string operations
@ 2018-03-13 23:39 Ben Hutchings
  2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2018-03-13 23:39 UTC (permalink / raw)
  To: Yisen Zhuang, Salil Mehta; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 3201 bytes --]

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 <ben@decadent.org.uk>
---
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 7ea7f8a4aa2a..b836742f7ab6 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -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;
 	}
 }
 


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] hns: Clean up string operations
  2018-03-13 23:39 [PATCH net 1/2] hns: Fix string set validation in ethtool string operations Ben Hutchings
@ 2018-03-13 23:41 ` Ben Hutchings
  2018-03-15 10:10   ` kbuild test robot
  2018-03-15 10:10   ` [RFC PATCH] hns: hns_ae_get_stats_strings() can be static kbuild test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Ben Hutchings @ 2018-03-13 23:41 UTC (permalink / raw)
  To: Yisen Zhuang, Salil Mehta; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 20972 bytes --]

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 <ben@decadent.org.uk>
---
 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(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.h b/drivers/net/ethernet/hisilicon/hns/hnae.h
index 3e62692af011..480fa2b49be7 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.h
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.h
@@ -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);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index bd68379d2bea..638476aa6311 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -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,
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
index 86944bc3b273..7525b03fd3d8 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
@@ -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;
 
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index cac86e9ae0dd..094476b27c00 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -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)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
index bbc0a98e7ca3..495462d38cb2 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h
@@ -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);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index e0bc79ea3d88..0ba28a29b738 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -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);
 
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
index 4507e8222683..275dec595dc7 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h
@@ -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);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index b62816c1574e..a08471441285 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -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;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h
index 9d8e643e8aa6..1baf04caf4bf 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.h
@@ -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,
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
index 6f3570cfb501..7d4ecd8c8d6e 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
@@ -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);
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
index 602816498c8d..95cd0111e802 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.h
@@ -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);
 
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
index 51e7e9f5af49..0e8fd285bfd7 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
@@ -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;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index b836742f7ab6..71075f69122c 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -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;
 	}

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [RFC PATCH] hns: hns_ae_get_stats_strings() can be static
  2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
  2018-03-15 10:10   ` kbuild test robot
@ 2018-03-15 10:10   ` kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-03-15 10:10 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: kbuild-all, Yisen Zhuang, Salil Mehta, netdev


Fixes: 1316dd7b461c ("hns: Clean up string operations")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 hns_ae_adapt.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
index 638476aa..aa89e46 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c
@@ -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;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] hns: Clean up string operations
  2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
@ 2018-03-15 10:10   ` kbuild test robot
  2018-03-15 10:10   ` [RFC PATCH] hns: hns_ae_get_stats_strings() can be static kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-03-15 10:10 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: kbuild-all, Yisen Zhuang, Salil Mehta, netdev

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-15 10:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 23:39 [PATCH net 1/2] hns: Fix string set validation in ethtool string operations Ben Hutchings
2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
2018-03-15 10:10   ` kbuild test robot
2018-03-15 10:10   ` [RFC PATCH] hns: hns_ae_get_stats_strings() can be static kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.