All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next 0/4] return offloaded stats as default and expose original sw start
@ 2016-05-12  9:59 Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12  9:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Jiri Pirko <jiri@mellanox.com>

Until now we had stats functions return SW statistics. However, it makes
a lot of sense to return HW stats as default. The existing apps count with
having the defaults stats complete, but that is not true now as the offloaded
forward traffic is not visible there.

If user wants to know real SW stats, this patchset provides way to get
it as well.

Nogah Frankel (4):
  netdevice: add SW statistics ndo
  rtnetlink: add HW/SW stats distinction in rtnl_fill_stats
  net: core: add SW stats to if_stats_msg
  mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 65 ++++++++++++++++++++++----
 include/linux/netdevice.h                      | 12 +++++
 include/uapi/linux/if_link.h                   |  2 +
 net/core/dev.c                                 | 25 ++++++++++
 net/core/rtnetlink.c                           | 55 +++++++++++++++++++---
 5 files changed, 145 insertions(+), 14 deletions(-)

-- 
2.5.5

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

* [patch net-next 1/4] netdevice: add SW statistics ndo
  2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
@ 2016-05-12  9:59 ` Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats Jiri Pirko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12  9:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Nogah Frankel <nogahf@mellanox.com>

Till now we had a ndo statistics function that returned SW statistics.
We want to change the "basic" statistics to return HW statistics if
available.
In this case we need to expose a new ndo to return the SW statistics.
Add a new ndo declaration to get SW statistics
Add a function that gets SW statistics if a competible ndo exist

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/linux/netdevice.h | 12 ++++++++++++
 net/core/dev.c            | 25 +++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c2f5112..3ac3e8f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -891,6 +891,14 @@ struct tc_to_netdev {
  *	   field is written atomically.
  *	3. Update dev->stats asynchronously and atomically, and define
  *	   neither operation.
+ *	Driver should return HW statistics, if available.
+ *
+ * struct rtnl_link_stats64* (*ndo_get_sw_stats64)(struct net_device *dev,
+ *			struct rtnl_link_stats64 *storage);
+ *	Similar to rtnl_link_stats64 but used to get SW statistics,
+ *	if it is possible to get HW and SW statistics separately.
+ *	If this option isn't valid - driver doesn't need to define
+ *	this function.
  *
  * int (*ndo_vlan_rx_add_vid)(struct net_device *dev, __be16 proto, u16 vid);
  *	If device supports VLAN filtering this function is called when a
@@ -1133,6 +1141,9 @@ struct net_device_ops {
 
 	struct rtnl_link_stats64* (*ndo_get_stats64)(struct net_device *dev,
 						     struct rtnl_link_stats64 *storage);
+	struct rtnl_link_stats64* (*ndo_get_sw_stats64)(struct net_device *dev,
+							struct rtnl_link_stats64 *storage);
+
 	struct net_device_stats* (*ndo_get_stats)(struct net_device *dev);
 
 	int			(*ndo_vlan_rx_add_vid)(struct net_device *dev,
@@ -3755,6 +3766,7 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 					struct rtnl_link_stats64 *storage);
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats);
+int dev_get_sw_stats(struct net_device *dev, struct rtnl_link_stats64 *storage);
 
 extern int		netdev_max_backlog;
 extern int		netdev_tstamp_prequeue;
diff --git a/net/core/dev.c b/net/core/dev.c
index 12436d1..a69e418 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7376,6 +7376,8 @@ EXPORT_SYMBOL(netdev_stats_to_stats64);
  *	The device driver may provide its own method by setting
  *	dev->netdev_ops->get_stats64 or dev->netdev_ops->get_stats;
  *	otherwise the internal statistics structure is used.
+ *	If device supports both HW & SW statistics - this function should
+ *	return the HW statistics.
  */
 struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 					struct rtnl_link_stats64 *storage)
@@ -7397,6 +7399,29 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 }
 EXPORT_SYMBOL(dev_get_stats);
 
+/*	dev_get_sw_stats    - get network device SW statistics
+ *	(if it is possible to get HW & SW statistics separately)
+ *	@dev: device to get statistics from
+ *	@storage: place to store stats
+ *
+ *	if exist a function to query the netdev SW statistics get it to storage
+ *	return 0 if did, or -EINVAL if this function doesn't exist
+ */
+int dev_get_sw_stats(struct net_device *dev,
+		     struct rtnl_link_stats64 *storage)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (ops->ndo_get_sw_stats64) {
+		memset(storage, 0, sizeof(*storage));
+		ops->ndo_get_sw_stats64(dev, storage);
+	} else {
+		return -EINVAL;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(dev_get_sw_stats);
+
 struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
 {
 	struct netdev_queue *queue = dev_ingress_queue(dev);
-- 
2.5.5

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

* [patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats
  2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
@ 2016-05-12  9:59 ` Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 3/4] net: core: add SW stats to if_stats_msg Jiri Pirko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12  9:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Nogah Frankel <nogahf@mellanox.com>

Since hardware stats are now returned by default, add a way to
query only software stats.
They are saved in IFLA_SW_STATS64.
(This option is valid only if the driver return HW stats in the
default ndo stats)

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index bb36bd5..98175e7 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -156,6 +156,7 @@ enum {
 	IFLA_GSO_MAX_SEGS,
 	IFLA_GSO_MAX_SIZE,
 	IFLA_PAD,
+	IFLA_SW_STATS64,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d69c464..a127d67 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1046,20 +1046,40 @@ static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
+static void copy_rtnl_link_stats64(void *v, const struct rtnl_link_stats64 *b)
+{
+	memcpy(v, b, sizeof(*b));
+}
+
 static noinline_for_stack int rtnl_fill_stats(struct sk_buff *skb,
 					      struct net_device *dev)
 {
+	struct rtnl_link_stats64 sw_stats;
 	struct rtnl_link_stats64 *sp;
 	struct nlattr *attr;
+	int err;
 
 	attr = nla_reserve_64bit(skb, IFLA_STATS64,
 				 sizeof(struct rtnl_link_stats64), IFLA_PAD);
+
 	if (!attr)
 		return -EMSGSIZE;
 
 	sp = nla_data(attr);
 	dev_get_stats(dev, sp);
 
+	err = dev_get_sw_stats(dev, &sw_stats);
+	if (!err) {
+		attr = nla_reserve_64bit(skb, IFLA_SW_STATS64,
+					 sizeof(struct rtnl_link_stats64),
+					 IFLA_PAD);
+
+		if (!attr)
+			return -EMSGSIZE;
+
+		copy_rtnl_link_stats64(nla_data(attr), &sw_stats);
+	}
+
 	attr = nla_reserve(skb, IFLA_STATS,
 			   sizeof(struct rtnl_link_stats));
 	if (!attr)
-- 
2.5.5

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

* [patch net-next 3/4] net: core: add SW stats to if_stats_msg
  2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
  2016-05-12  9:59 ` [patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats Jiri Pirko
@ 2016-05-12  9:59 ` Jiri Pirko
  2016-05-12 11:33   ` Nikolay Aleksandrov
  2016-05-12  9:59 ` [patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default Jiri Pirko
  2016-05-12 14:36 ` [patch net-next 0/4] return offloaded stats as default and expose original sw start Andrew Lunn
  4 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12  9:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Nogah Frankel <nogahf@mellanox.com>

If there is a dedicated ndo to return SW stats - use
it. Otherwise (indicates that there is no HW stats) use
the default stats ndo.
Return results under IFLA_STATS_LINK_SW_64.

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 35 +++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 98175e7..fcfb944 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -823,6 +823,7 @@ enum {
 	IFLA_STATS_UNSPEC, /* also used as 64bit pad attribute */
 	IFLA_STATS_LINK_64,
 	IFLA_STATS_LINK_XSTATS,
+	IFLA_STATS_LINK_SW_64,
 	__IFLA_STATS_MAX,
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a127d67..f8b12e4 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3481,6 +3481,9 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
 	struct nlmsghdr *nlh;
 	struct nlattr *attr;
 	int s_prividx = *prividx;
+	struct rtnl_link_stats64 *sp;
+	struct rtnl_link_stats64 *stats64_sp = 0;
+	int err;
 
 	ASSERT_RTNL();
 
@@ -3493,24 +3496,20 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
 	ifsm->filter_mask = filter_mask;
 
 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, *idxattr)) {
-		struct rtnl_link_stats64 *sp;
-
 		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_64,
 					 sizeof(struct rtnl_link_stats64),
 					 IFLA_STATS_UNSPEC);
 		if (!attr)
 			goto nla_put_failure;
 
-		sp = nla_data(attr);
-		dev_get_stats(dev, sp);
+		stats64_sp = nla_data(attr);
+		dev_get_stats(dev, stats64_sp);
 	}
 
 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) {
 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
 
 		if (ops && ops->fill_linkxstats) {
-			int err;
-
 			*idxattr = IFLA_STATS_LINK_XSTATS;
 			attr = nla_nest_start(skb,
 					      IFLA_STATS_LINK_XSTATS);
@@ -3525,6 +3524,27 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
 		}
 	}
 
+	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, *idxattr)) {
+		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_SW_64,
+					 sizeof(struct rtnl_link_stats64),
+					 IFLA_STATS_UNSPEC);
+		if (!attr)
+			goto nla_put_failure;
+
+		sp = nla_data(attr);
+		err = dev_get_sw_stats(dev, sp);
+		if (err) {
+			/* if err it means there is no dedicated ndo to
+			 * get SW stats - so it is returned by the default
+			 * stats ndo
+			 */
+			if (stats64_sp)
+				copy_rtnl_link_stats64(sp, stats64_sp);
+			else
+				dev_get_stats(dev, sp);
+		}
+	}
+
 	nlmsg_end(skb, nlh);
 
 	return 0;
@@ -3541,6 +3561,7 @@ nla_put_failure:
 
 static const struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
 	[IFLA_STATS_LINK_64]	= { .len = sizeof(struct rtnl_link_stats64) },
+	[IFLA_STATS_LINK_SW_64] = { .len = sizeof(struct rtnl_link_stats64) },
 };
 
 static size_t if_nlmsg_stats_size(const struct net_device *dev,
@@ -3550,6 +3571,8 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
 
 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
 		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
+	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, 0))
+		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
 
 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, 0)) {
 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
-- 
2.5.5

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

* [patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default
  2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
                   ` (2 preceding siblings ...)
  2016-05-12  9:59 ` [patch net-next 3/4] net: core: add SW stats to if_stats_msg Jiri Pirko
@ 2016-05-12  9:59 ` Jiri Pirko
  2016-05-12 14:36 ` [patch net-next 0/4] return offloaded stats as default and expose original sw start Andrew Lunn
  4 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12  9:59 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Nogah Frankel <nogahf@mellanox.com>

Add a function to get the SW statistics with an ndo
Change the default statistics ndo to return HW statistics
(like the one returned by ethtool_ops)

Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 65 ++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4a72737..956e724 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -535,8 +535,8 @@ err_port_mtu_set:
 }
 
 static struct rtnl_link_stats64 *
-mlxsw_sp_port_get_stats64(struct net_device *dev,
-			  struct rtnl_link_stats64 *stats)
+mlxsw_sp_port_get_sw_stats64(struct net_device *dev,
+			     struct rtnl_link_stats64 *stats)
 {
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	struct mlxsw_sp_port_pcpu_stats *p;
@@ -566,6 +566,59 @@ mlxsw_sp_port_get_stats64(struct net_device *dev,
 	return stats;
 }
 
+static int mlxsw_sp_port_get_stats_raw(struct net_device *dev,
+				       char *ppcnt_pl)
+{
+	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
+	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+	int err;
+
+	mlxsw_reg_ppcnt_pack(ppcnt_pl, mlxsw_sp_port->local_port,
+			     MLXSW_REG_PPCNT_IEEE_8023_CNT, 0);
+	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ppcnt), ppcnt_pl);
+	return err;
+}
+
+static struct rtnl_link_stats64 *
+mlxsw_sp_port_get_stats64(struct net_device *dev,
+			  struct rtnl_link_stats64 *stats)
+{
+	char ppcnt_pl[MLXSW_REG_PPCNT_LEN];
+	int err;
+
+	err =  mlxsw_sp_port_get_stats_raw(dev, ppcnt_pl);
+	if (err)
+		goto out;
+
+	stats->tx_packets =
+		mlxsw_reg_ppcnt_a_frames_transmitted_ok_get(ppcnt_pl);
+	stats->rx_packets =
+		mlxsw_reg_ppcnt_a_frames_received_ok_get(ppcnt_pl);
+	stats->tx_bytes =
+		mlxsw_reg_ppcnt_a_octets_transmitted_ok_get(ppcnt_pl);
+	stats->rx_bytes =
+		mlxsw_reg_ppcnt_a_octets_received_ok_get(ppcnt_pl);
+	stats->multicast =
+		mlxsw_reg_ppcnt_a_multicast_frames_received_ok_get(ppcnt_pl);
+	stats->multicast +=
+		mlxsw_reg_ppcnt_a_broadcast_frames_received_ok_get(ppcnt_pl);
+
+	stats->rx_crc_errors =
+		mlxsw_reg_ppcnt_a_frame_check_sequence_errors_get(ppcnt_pl);
+	stats->rx_frame_errors =
+		mlxsw_reg_ppcnt_a_alignment_errors_get(ppcnt_pl);
+
+	stats->rx_length_errors = (
+		mlxsw_reg_ppcnt_a_in_range_length_errors_get(ppcnt_pl) +
+		mlxsw_reg_ppcnt_a_out_of_range_length_field_get(ppcnt_pl) +
+		mlxsw_reg_ppcnt_a_frame_too_long_errors_get(ppcnt_pl));
+
+	stats->rx_errors = (stats->rx_crc_errors +
+		stats->rx_frame_errors + stats->rx_length_errors);
+out:
+	return stats;
+}
+
 int mlxsw_sp_port_vlan_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid_begin,
 			   u16 vid_end, bool is_member, bool untagged)
 {
@@ -980,6 +1033,7 @@ static const struct net_device_ops mlxsw_sp_port_netdev_ops = {
 	.ndo_set_mac_address	= mlxsw_sp_port_set_mac_address,
 	.ndo_change_mtu		= mlxsw_sp_port_change_mtu,
 	.ndo_get_stats64	= mlxsw_sp_port_get_stats64,
+	.ndo_get_sw_stats64     = mlxsw_sp_port_get_sw_stats64,
 	.ndo_vlan_rx_add_vid	= mlxsw_sp_port_add_vid,
 	.ndo_vlan_rx_kill_vid	= mlxsw_sp_port_kill_vid,
 	.ndo_fdb_add		= switchdev_port_fdb_add,
@@ -1200,15 +1254,10 @@ static int mlxsw_sp_port_set_phys_id(struct net_device *dev,
 static void mlxsw_sp_port_get_stats(struct net_device *dev,
 				    struct ethtool_stats *stats, u64 *data)
 {
-	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
-	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	char ppcnt_pl[MLXSW_REG_PPCNT_LEN];
 	int i;
 	int err;
-
-	mlxsw_reg_ppcnt_pack(ppcnt_pl, mlxsw_sp_port->local_port,
-			     MLXSW_REG_PPCNT_IEEE_8023_CNT, 0);
-	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ppcnt), ppcnt_pl);
+	err = mlxsw_sp_port_get_stats_raw(dev, ppcnt_pl);
 	for (i = 0; i < MLXSW_SP_PORT_HW_STATS_LEN; i++)
 		data[i] = !err ? mlxsw_sp_port_hw_stats[i].getter(ppcnt_pl) : 0;
 }
-- 
2.5.5

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

* Re: [patch net-next 3/4] net: core: add SW stats to if_stats_msg
  2016-05-12  9:59 ` [patch net-next 3/4] net: core: add SW stats to if_stats_msg Jiri Pirko
@ 2016-05-12 11:33   ` Nikolay Aleksandrov
  2016-05-12 11:44     ` Jiri Pirko
  0 siblings, 1 reply; 10+ messages in thread
From: Nikolay Aleksandrov @ 2016-05-12 11:33 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, David S . Miller, nogahf, Ido Schimmel, eladr, yotamg,
	ogerlitz, Roopa Prabhu, linville, tgraf, Andy Gospodarek,
	sfeldma, sd, eranbe, ast, edumazet, hannes


> On May 12, 2016, at 12:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> 
> From: Nogah Frankel <nogahf@mellanox.com>
> 
> If there is a dedicated ndo to return SW stats - use
> it. Otherwise (indicates that there is no HW stats) use
> the default stats ndo.
> Return results under IFLA_STATS_LINK_SW_64.
> 
> Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/uapi/linux/if_link.h |  1 +
> net/core/rtnetlink.c         | 35 +++++++++++++++++++++++++++++------
> 2 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 98175e7..fcfb944 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -823,6 +823,7 @@ enum {
> 	IFLA_STATS_UNSPEC, /* also used as 64bit pad attribute */
> 	IFLA_STATS_LINK_64,
> 	IFLA_STATS_LINK_XSTATS,
> +	IFLA_STATS_LINK_SW_64,
> 	__IFLA_STATS_MAX,
> };
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index a127d67..f8b12e4 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3481,6 +3481,9 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
> 	struct nlmsghdr *nlh;
> 	struct nlattr *attr;
> 	int s_prividx = *prividx;
> +	struct rtnl_link_stats64 *sp;
> +	struct rtnl_link_stats64 *stats64_sp = 0;

s/0/NULL/
net/core//rtnetlink.c:3485:48: warning: Using plain integer as NULL pointer

> +	int err;
> 
> 	ASSERT_RTNL();
> 
> @@ -3493,24 +3496,20 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
> 	ifsm->filter_mask = filter_mask;
> 
> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, *idxattr)) {
> -		struct rtnl_link_stats64 *sp;
> -
> 		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_64,
> 					 sizeof(struct rtnl_link_stats64),
> 					 IFLA_STATS_UNSPEC);
> 		if (!attr)
> 			goto nla_put_failure;
> 
> -		sp = nla_data(attr);
> -		dev_get_stats(dev, sp);
> +		stats64_sp = nla_data(attr);
> +		dev_get_stats(dev, stats64_sp);
> 	}
> 
> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) {
> 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
> 
> 		if (ops && ops->fill_linkxstats) {
> -			int err;
> -
> 			*idxattr = IFLA_STATS_LINK_XSTATS;
> 			attr = nla_nest_start(skb,
> 					      IFLA_STATS_LINK_XSTATS);
> @@ -3525,6 +3524,27 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
> 		}
> 	}
> 
> +	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, *idxattr)) {
> +		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_SW_64,
> +					 sizeof(struct rtnl_link_stats64),
> +					 IFLA_STATS_UNSPEC);
> +		if (!attr)
> +			goto nla_put_failure;
> +
> +		sp = nla_data(attr);
> +		err = dev_get_sw_stats(dev, sp);
> +		if (err) {
> +			/* if err it means there is no dedicated ndo to
> +			 * get SW stats - so it is returned by the default
> +			 * stats ndo
> +			 */
> +			if (stats64_sp)
> +				copy_rtnl_link_stats64(sp, stats64_sp);
> +			else
> +				dev_get_stats(dev, sp);
> +		}
> +	}
> +
> 	nlmsg_end(skb, nlh);
> 
> 	return 0;
> @@ -3541,6 +3561,7 @@ nla_put_failure:
> 
> static const struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
> 	[IFLA_STATS_LINK_64]	= { .len = sizeof(struct rtnl_link_stats64) },
> +	[IFLA_STATS_LINK_SW_64] = { .len = sizeof(struct rtnl_link_stats64) },
> };
> 
> static size_t if_nlmsg_stats_size(const struct net_device *dev,
> @@ -3550,6 +3571,8 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
> 
> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
> 		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
> +	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, 0))
> +		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
> 
> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, 0)) {
> 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
> -- 
> 2.5.5
> 

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

* Re: [patch net-next 3/4] net: core: add SW stats to if_stats_msg
  2016-05-12 11:33   ` Nikolay Aleksandrov
@ 2016-05-12 11:44     ` Jiri Pirko
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12 11:44 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: netdev, David S . Miller, nogahf, Ido Schimmel, eladr, yotamg,
	ogerlitz, Roopa Prabhu, linville, tgraf, Andy Gospodarek,
	sfeldma, sd, eranbe, ast, edumazet, hannes

Thu, May 12, 2016 at 01:33:11PM CEST, nikolay@cumulusnetworks.com wrote:
>
>> On May 12, 2016, at 12:59 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> 
>> From: Nogah Frankel <nogahf@mellanox.com>
>> 
>> If there is a dedicated ndo to return SW stats - use
>> it. Otherwise (indicates that there is no HW stats) use
>> the default stats ndo.
>> Return results under IFLA_STATS_LINK_SW_64.
>> 
>> Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
>> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/uapi/linux/if_link.h |  1 +
>> net/core/rtnetlink.c         | 35 +++++++++++++++++++++++++++++------
>> 2 files changed, 30 insertions(+), 6 deletions(-)
>> 
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index 98175e7..fcfb944 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -823,6 +823,7 @@ enum {
>> 	IFLA_STATS_UNSPEC, /* also used as 64bit pad attribute */
>> 	IFLA_STATS_LINK_64,
>> 	IFLA_STATS_LINK_XSTATS,
>> +	IFLA_STATS_LINK_SW_64,
>> 	__IFLA_STATS_MAX,
>> };
>> 
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index a127d67..f8b12e4 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -3481,6 +3481,9 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
>> 	struct nlmsghdr *nlh;
>> 	struct nlattr *attr;
>> 	int s_prividx = *prividx;
>> +	struct rtnl_link_stats64 *sp;
>> +	struct rtnl_link_stats64 *stats64_sp = 0;
>
>s/0/NULL/
>net/core//rtnetlink.c:3485:48: warning: Using plain integer as NULL pointer

Will fix and send v2. Thanks.


>
>> +	int err;
>> 
>> 	ASSERT_RTNL();
>> 
>> @@ -3493,24 +3496,20 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
>> 	ifsm->filter_mask = filter_mask;
>> 
>> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, *idxattr)) {
>> -		struct rtnl_link_stats64 *sp;
>> -
>> 		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_64,
>> 					 sizeof(struct rtnl_link_stats64),
>> 					 IFLA_STATS_UNSPEC);
>> 		if (!attr)
>> 			goto nla_put_failure;
>> 
>> -		sp = nla_data(attr);
>> -		dev_get_stats(dev, sp);
>> +		stats64_sp = nla_data(attr);
>> +		dev_get_stats(dev, stats64_sp);
>> 	}
>> 
>> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) {
>> 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
>> 
>> 		if (ops && ops->fill_linkxstats) {
>> -			int err;
>> -
>> 			*idxattr = IFLA_STATS_LINK_XSTATS;
>> 			attr = nla_nest_start(skb,
>> 					      IFLA_STATS_LINK_XSTATS);
>> @@ -3525,6 +3524,27 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
>> 		}
>> 	}
>> 
>> +	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, *idxattr)) {
>> +		attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_SW_64,
>> +					 sizeof(struct rtnl_link_stats64),
>> +					 IFLA_STATS_UNSPEC);
>> +		if (!attr)
>> +			goto nla_put_failure;
>> +
>> +		sp = nla_data(attr);
>> +		err = dev_get_sw_stats(dev, sp);
>> +		if (err) {
>> +			/* if err it means there is no dedicated ndo to
>> +			 * get SW stats - so it is returned by the default
>> +			 * stats ndo
>> +			 */
>> +			if (stats64_sp)
>> +				copy_rtnl_link_stats64(sp, stats64_sp);
>> +			else
>> +				dev_get_stats(dev, sp);
>> +		}
>> +	}
>> +
>> 	nlmsg_end(skb, nlh);
>> 
>> 	return 0;
>> @@ -3541,6 +3561,7 @@ nla_put_failure:
>> 
>> static const struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
>> 	[IFLA_STATS_LINK_64]	= { .len = sizeof(struct rtnl_link_stats64) },
>> +	[IFLA_STATS_LINK_SW_64] = { .len = sizeof(struct rtnl_link_stats64) },
>> };
>> 
>> static size_t if_nlmsg_stats_size(const struct net_device *dev,
>> @@ -3550,6 +3571,8 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
>> 
>> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
>> 		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
>> +	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_SW_64, 0))
>> +		size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
>> 
>> 	if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, 0)) {
>> 		const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
>> -- 
>> 2.5.5
>> 
>

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

* Re: [patch net-next 0/4] return offloaded stats as default and expose original sw start
  2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
                   ` (3 preceding siblings ...)
  2016-05-12  9:59 ` [patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default Jiri Pirko
@ 2016-05-12 14:36 ` Andrew Lunn
  2016-05-12 14:48   ` Jiri Pirko
  4 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2016-05-12 14:36 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa,
	nikolay, linville, tgraf, gospo, sfeldma, sd, eranbe, ast,
	edumazet, hannes

On Thu, May 12, 2016 at 11:59:54AM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Until now we had stats functions return SW statistics. However, it makes
> a lot of sense to return HW stats as default.

Hi Jiri

DSA has always returned the HW stats, i.e, ethtool -S shows the
counters the switch has for the port.

How do you expect ethtool to handle your change? ethtool -S will print
both sets? Or are you thinking of adding a new flag?

I'm just trying to understand the big picture, and how it affects DSA,
if at all.

   Andrew

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

* Re: [patch net-next 0/4] return offloaded stats as default and expose original sw start
  2016-05-12 14:36 ` [patch net-next 0/4] return offloaded stats as default and expose original sw start Andrew Lunn
@ 2016-05-12 14:48   ` Jiri Pirko
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12 14:48 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa,
	nikolay, linville, tgraf, gospo, sfeldma, sd, eranbe, ast,
	edumazet, hannes

Thu, May 12, 2016 at 04:36:45PM CEST, andrew@lunn.ch wrote:
>On Thu, May 12, 2016 at 11:59:54AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Until now we had stats functions return SW statistics. However, it makes
>> a lot of sense to return HW stats as default.
>
>Hi Jiri
>
>DSA has always returned the HW stats, i.e, ethtool -S shows the
>counters the switch has for the port.
>
>How do you expect ethtool to handle your change? ethtool -S will print
>both sets? Or are you thinking of adding a new flag?

Ethtool still continues to provide the hw counters. We do the same for
mlxsw driver. No change there.

>
>I'm just trying to understand the big picture, and how it affects DSA,
>if at all.

I didn't check the code, but if dsa provides sw stats by default, it
should be changed to provide hwstats instead. The goal is to keep the
existing applications to see what is going on.

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

* [patch net-next 0/4] return offloaded stats as default and expose original sw start
@ 2016-05-12 11:48 Jiri Pirko
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2016-05-12 11:48 UTC (permalink / raw)
  To: netdev
  Cc: davem, nogahf, idosch, eladr, yotamg, ogerlitz, roopa, nikolay,
	linville, tgraf, gospo, sfeldma, sd, eranbe, ast, edumazet,
	hannes

From: Jiri Pirko <jiri@mellanox.com>

Until now we had stats functions return SW statistics. However, it makes
a lot of sense to return HW stats as default. The existing apps count with
having the defaults stats complete, but that is not true now as the offloaded
forward traffic is not visible there.

If user wants to know real SW stats, this patchset provides way to get
it as well.

---
v1->v2:
- patch3/4: 
  - fixed NULL initialization

Nogah Frankel (4):
  netdevice: add SW statistics ndo
  rtnetlink: add HW/SW stats distinction in rtnl_fill_stats
  net: core: add SW stats to if_stats_msg
  mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 65 ++++++++++++++++++++++----
 include/linux/netdevice.h                      | 12 +++++
 include/uapi/linux/if_link.h                   |  2 +
 net/core/dev.c                                 | 25 ++++++++++
 net/core/rtnetlink.c                           | 55 +++++++++++++++++++---
 5 files changed, 145 insertions(+), 14 deletions(-)

-- 
2.5.5

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

end of thread, other threads:[~2016-05-12 14:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-12  9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
2016-05-12  9:59 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
2016-05-12  9:59 ` [patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats Jiri Pirko
2016-05-12  9:59 ` [patch net-next 3/4] net: core: add SW stats to if_stats_msg Jiri Pirko
2016-05-12 11:33   ` Nikolay Aleksandrov
2016-05-12 11:44     ` Jiri Pirko
2016-05-12  9:59 ` [patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default Jiri Pirko
2016-05-12 14:36 ` [patch net-next 0/4] return offloaded stats as default and expose original sw start Andrew Lunn
2016-05-12 14:48   ` Jiri Pirko
2016-05-12 11:48 Jiri Pirko

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.