datapath: Always use generic stats for devices (vports)

Currently ovs is using device stats for Linux devices and count them itself in other situations. This leads to overlap with hardware stats, inconsistencies, etc. It's much better to just always count the packets flowing through the switch and let userspace do any merging that it wants. Following patch removes vport->get_stats() interface. vport-stat is changed to use new `struct ovs_vport_stat` rather than rtnl_link_stats64. Definitions of rtnl_link_stats64 is removed from OVS. dipf_port->stat is also removed as aggregate stats are only available at netdev layer. Signed-off-by: Pravin B Shelar <[email protected]> Acked-by: Jesse Gross <[email protected]>
eyoung-father · Sep 16, 2011 · f613a0d · f613a0d
1 parent 9197df7
commit f613a0d
Show file tree

Hide file tree

Showing 26 changed files with 303 additions and 436 deletions.
diff --git a/acinclude.m4 b/acinclude.m4
@@ -254,8 +254,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_get_be16])
   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_find_nested])
 
-  OVS_GREP_IFELSE([$KSRC/include/linux/if_link.h], [rtnl_link_stats64])
-
   OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [ADD_ALL_VLANS_CMD],
                   [OVS_DEFINE([HAVE_VLAN_BUG_WORKAROUND])])
 

diff --git a/configure.ac b/configure.ac
@@ -47,7 +47,6 @@ OVS_CHECK_NDEBUG
 OVS_CHECK_NETLINK
 OVS_CHECK_OPENSSL
 OVS_CHECK_LOGDIR
-OVS_CHECK_RTNL_LINK_STATS64
 OVS_CHECK_PYTHON
 OVS_CHECK_PYUIC4
 OVS_CHECK_OVSDBMONITOR

diff --git a/datapath/datapath.c b/datapath/datapath.c
@@ -1504,10 +1504,10 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
 	[OVS_VPORT_ATTR_NAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ - 1 },
 	[OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
 	[OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
-	[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct rtnl_link_stats64) },
+	[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
 	[OVS_VPORT_ATTR_ADDRESS] = { .len = ETH_ALEN },
 #else
-	[OVS_VPORT_ATTR_STATS] = { .minlen = sizeof(struct rtnl_link_stats64) },
+	[OVS_VPORT_ATTR_STATS] = { .minlen = sizeof(struct ovs_vport_stats) },
 	[OVS_VPORT_ATTR_ADDRESS] = { .minlen = ETH_ALEN },
 #endif
 	[OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
@@ -1545,11 +1545,11 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 	NLA_PUT_U32(skb, OVS_VPORT_ATTR_TYPE, vport_get_type(vport));
 	NLA_PUT_STRING(skb, OVS_VPORT_ATTR_NAME, vport_get_name(vport));
 
-	nla = nla_reserve(skb, OVS_VPORT_ATTR_STATS, sizeof(struct rtnl_link_stats64));
+	nla = nla_reserve(skb, OVS_VPORT_ATTR_STATS, sizeof(struct ovs_vport_stats));
 	if (!nla)
 		goto nla_put_failure;
-	if (vport_get_stats(vport, nla_data(nla)))
-		__skb_trim(skb, skb->len - nla->nla_len);
+
+	vport_get_stats(vport, nla_data(nla));
 
 	NLA_PUT(skb, OVS_VPORT_ATTR_ADDRESS, ETH_ALEN, vport_get_addr(vport));
 
@@ -1628,10 +1628,13 @@ static struct vport *lookup_vport(struct ovs_header *ovs_header,
 static int change_vport(struct vport *vport, struct nlattr *a[OVS_VPORT_ATTR_MAX + 1])
 {
 	int err = 0;
+
 	if (a[OVS_VPORT_ATTR_STATS])
-		err = vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
-	if (!err && a[OVS_VPORT_ATTR_ADDRESS])
+		vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
+
+	if (a[OVS_VPORT_ATTR_ADDRESS])
 		err = vport_set_addr(vport, nla_data(a[OVS_VPORT_ATTR_ADDRESS]));
+
 	return err;
 }
 

diff --git a/datapath/linux/compat/include/linux/netdevice.h b/datapath/linux/compat/include/linux/netdevice.h
@@ -77,19 +77,6 @@ extern void unregister_netdevice_many(struct list_head *head);
 extern void dev_disable_lro(struct net_device *dev);
 #endif
 
-/* Linux 2.6.28 introduced dev_get_stats():
- * const struct net_device_stats *dev_get_stats(struct net_device *dev);
- *
- * Linux 2.6.36 changed dev_get_stats() to:
- * struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
- *                                         struct rtnl_link_stats64 *storage);
- */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)
-#define dev_get_stats(dev, storage) rpl_dev_get_stats(dev, storage)
-struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
-					struct rtnl_link_stats64 *storage);
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19)
 #define skb_checksum_help(skb) skb_checksum_help((skb), 0)
 #endif

diff --git a/datapath/linux/compat/netdevice.c b/datapath/linux/compat/netdevice.c
@@ -1,54 +1,6 @@
-#include <linux/if_link.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
 
-/* Linux 2.6.28 introduced dev_get_stats():
- * const struct net_device_stats *dev_get_stats(struct net_device *dev);
- *
- * Linux 2.6.36 changed dev_get_stats() to:
- * struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
- *                                         struct rtnl_link_stats64 *storage);
- */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)
-struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
-					struct rtnl_link_stats64 *storage)
-{
-	const struct net_device_stats *stats;
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,29)
-	stats = dev->get_stats(dev);
-#else  /* 2.6.28 < kernel version < 2.6.36 */
-	stats = (dev_get_stats)(dev);
-#endif /* 2.6.28 < kernel version < 2.6.36 */
-
-	storage->rx_packets = stats->rx_packets;
-	storage->tx_packets = stats->tx_packets;
-	storage->rx_bytes = stats->rx_bytes;
-	storage->tx_bytes = stats->tx_bytes;
-	storage->rx_errors = stats->rx_errors;
-	storage->tx_errors = stats->tx_errors;
-	storage->rx_dropped = stats->rx_dropped;
-	storage->tx_dropped = stats->tx_dropped;
-	storage->multicast = stats->multicast;
-	storage->collisions = stats->collisions;
-	storage->rx_length_errors = stats->rx_length_errors;
-	storage->rx_over_errors = stats->rx_over_errors;
-	storage->rx_crc_errors = stats->rx_crc_errors;
-	storage->rx_frame_errors = stats->rx_frame_errors;
-	storage->rx_fifo_errors = stats->rx_fifo_errors;
-	storage->rx_missed_errors = stats->rx_missed_errors;
-	storage->tx_aborted_errors = stats->tx_aborted_errors;
-	storage->tx_carrier_errors = stats->tx_carrier_errors;
-	storage->tx_fifo_errors = stats->tx_fifo_errors;
-	storage->tx_heartbeat_errors = stats->tx_heartbeat_errors;
-	storage->tx_window_errors = stats->tx_window_errors;
-	storage->rx_compressed = stats->rx_compressed;
-	storage->tx_compressed = stats->tx_compressed;
-
-	return storage;
-}
-#endif	/* kernel version < 2.6.36 */
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,38)
 static bool can_checksum_protocol(unsigned long features, __be16 protocol)
 {

diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
@@ -788,7 +788,7 @@ static void capwap_frag_expire(unsigned long ifq)
 
 const struct vport_ops capwap_vport_ops = {
 	.type		= OVS_VPORT_TYPE_CAPWAP,
-	.flags		= VPORT_F_GEN_STATS | VPORT_F_TUN_ID,
+	.flags		= VPORT_F_TUN_ID,
 	.init		= capwap_init,
 	.exit		= capwap_exit,
 	.create		= capwap_create,

diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
@@ -389,7 +389,7 @@ static void gre_exit(void)
 
 const struct vport_ops gre_vport_ops = {
 	.type		= OVS_VPORT_TYPE_GRE,
-	.flags		= VPORT_F_GEN_STATS | VPORT_F_TUN_ID,
+	.flags		= VPORT_F_TUN_ID,
 	.init		= gre_init,
 	.exit		= gre_exit,
 	.create		= gre_create,

diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
@@ -32,16 +32,14 @@ static inline struct internal_dev *internal_dev_priv(struct net_device *netdev)
 	return netdev_priv(netdev);
 }
 
-/* This function is only called by the kernel network layer.  It is not a vport
- * get_stats() function.  If a vport get_stats() function is defined that
- * results in this being called it will cause infinite recursion. */
+/* This function is only called by the kernel network layer.*/
 static struct net_device_stats *internal_dev_sys_stats(struct net_device *netdev)
 {
 	struct vport *vport = internal_dev_get_vport(netdev);
 	struct net_device_stats *stats = &internal_dev_priv(netdev)->stats;
 
 	if (vport) {
-		struct rtnl_link_stats64 vport_stats;
+		struct ovs_vport_stats vport_stats;
 
 		vport_get_stats(vport, &vport_stats);
 
@@ -55,7 +53,6 @@ static struct net_device_stats *internal_dev_sys_stats(struct net_device *netdev
 		stats->tx_errors	= vport_stats.rx_errors;
 		stats->rx_dropped	= vport_stats.tx_dropped;
 		stats->tx_dropped	= vport_stats.rx_dropped;
-		stats->collisions	= vport_stats.collisions;
 	}
 
 	return stats;
@@ -266,7 +263,7 @@ static int internal_dev_recv(struct vport *vport, struct sk_buff *skb)
 
 const struct vport_ops internal_vport_ops = {
 	.type		= OVS_VPORT_TYPE_INTERNAL,
-	.flags		= VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW,
+	.flags		= VPORT_F_REQUIRED | VPORT_F_FLOW,
 	.create		= internal_dev_create,
 	.destroy	= internal_dev_destroy,
 	.set_addr	= netdev_set_addr,

diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
@@ -35,9 +35,6 @@ MODULE_PARM_DESC(vlan_tso, "Enable TSO for VLAN packets");
 #define vlan_tso true
 #endif
 
-/* If the native device stats aren't 64 bit use the vport stats tracking instead. */
-#define USE_VPORT_STATS (sizeof(((struct net_device_stats *)0)->rx_bytes) < sizeof(u64))
-
 static void netdev_port_receive(struct vport *vport, struct sk_buff *skb);
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
@@ -144,16 +141,6 @@ static struct vport *netdev_create(const struct vport_parms *parms)
 		goto error_put;
 	}
 
-	/* If we are using the vport stats layer initialize it to the current
-	 * values so we are roughly consistent with the device stats. */
-	if (USE_VPORT_STATS) {
-		struct rtnl_link_stats64 stats;
-
-		err = netdev_get_stats(vport, &stats);
-		if (!err)
-			vport_set_stats(vport, &stats);
-	}
-
 	err = netdev_rx_handler_register(netdev_vport->dev, netdev_frame_hook,
 					 vport);
 	if (err)
@@ -218,13 +205,6 @@ struct kobject *netdev_get_kobj(const struct vport *vport)
 	return &netdev_vport->dev->NETDEV_DEV_MEMBER.kobj;
 }
 
-int netdev_get_stats(const struct vport *vport, struct rtnl_link_stats64 *stats)
-{
-	const struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
-	dev_get_stats(netdev_vport->dev, stats);
-	return 0;
-}
-
 unsigned netdev_get_dev_flags(const struct vport *vport)
 {
 	const struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
@@ -410,8 +390,7 @@ struct vport *netdev_get_vport(struct net_device *dev)
 
 const struct vport_ops netdev_vport_ops = {
 	.type		= OVS_VPORT_TYPE_NETDEV,
-	.flags          = (VPORT_F_REQUIRED |
-			  (USE_VPORT_STATS ? VPORT_F_GEN_STATS : 0)),
+	.flags          = VPORT_F_REQUIRED,
 	.init		= netdev_init,
 	.exit		= netdev_exit,
 	.create		= netdev_create,
@@ -420,7 +399,6 @@ const struct vport_ops netdev_vport_ops = {
 	.get_name	= netdev_get_name,
 	.get_addr	= netdev_get_addr,
 	.get_kobj	= netdev_get_kobj,
-	.get_stats	= netdev_get_stats,
 	.get_dev_flags	= netdev_get_dev_flags,
 	.is_running	= netdev_is_running,
 	.get_operstate	= netdev_get_operstate,

diff --git a/datapath/vport-netdev.h b/datapath/vport-netdev.h
@@ -30,7 +30,6 @@ const char *netdev_get_name(const struct vport *);
 const unsigned char *netdev_get_addr(const struct vport *);
 const char *netdev_get_config(const struct vport *);
 struct kobject *netdev_get_kobj(const struct vport *);
-int netdev_get_stats(const struct vport *, struct rtnl_link_stats64 *);
 unsigned netdev_get_dev_flags(const struct vport *);
 int netdev_is_running(const struct vport *);
 unsigned char netdev_get_operstate(const struct vport *);

diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c
@@ -287,7 +287,6 @@ static int patch_send(struct vport *vport, struct sk_buff *skb)
 
 const struct vport_ops patch_vport_ops = {
 	.type		= OVS_VPORT_TYPE_PATCH,
-	.flags		= VPORT_F_GEN_STATS,
 	.init		= patch_init,
 	.exit		= patch_exit,
 	.create		= patch_create,