[patch net-next 0/4] return offloaded stats as default and expose original sw start

STALE3739d

9 messages, 3 authors, 2016-05-12 · open the first message on its own page

[patch net-next 0/4] return offloaded stats as default and expose original sw start

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 10:00:01

From: Jiri Pirko <redacted>

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

[patch net-next 1/4] netdevice: add SW statistics ndo

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 10:00:02

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
 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

[patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 10:00:03

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
 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

[patch net-next 3/4] net: core: add SW stats to if_stats_msg

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 10:00:05

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
 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

[patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 10:00:06

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
 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

Re: [patch net-next 3/4] net: core: add SW stats to if_stats_msg

From: Nikolay Aleksandrov <hidden>
Date: 2016-05-12 11:33:16

quoted hunk

On May 12, 2016, at 12:59 PM, Jiri Pirko [off-list ref] wrote:

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
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

quoted hunk

+	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

Re: [patch net-next 3/4] net: core: add SW stats to if_stats_msg

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 11:44:08

Thu, May 12, 2016 at 01:33:11PM CEST, nikolay@cumulusnetworks.com wrote:

quoted

On May 12, 2016, at 12:59 PM, Jiri Pirko [off-list ref] wrote:

From: Nogah Frankel <redacted>

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 <redacted>
Reviewed-by: Ido Schimmel <redacted>
Signed-off-by: Jiri Pirko <redacted>
---
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.

quoted

+	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

Re: [patch net-next 0/4] return offloaded stats as default and expose original sw start

From: Andrew Lunn <andrew@lunn.ch>
Date: 2016-05-12 14:36:52

On Thu, May 12, 2016 at 11:59:54AM +0200, Jiri Pirko wrote:

From: Jiri Pirko <redacted>

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

Re: [patch net-next 0/4] return offloaded stats as default and expose original sw start

From: Jiri Pirko <jiri@resnulli.us>
Date: 2016-05-12 14:48:12

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:

quoted

From: Jiri Pirko <redacted>

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.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help