[ovs-dev] [PATCH] Always use generic stats for devices (vports)

Pravin Shelar pshelar at nicira.com
Tue Sep 13 00:39:38 UTC 2011


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.

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
 datapath/datapath.c                             |    8 +-
 datapath/linux/compat/include/linux/netdevice.h |   13 --
 datapath/linux/compat/netdevice.c               |   47 -------
 datapath/vport-capwap.c                         |    2 +-
 datapath/vport-gre.c                            |    2 +-
 datapath/vport-internal_dev.c                   |    6 +-
 datapath/vport-netdev.c                         |   24 +----
 datapath/vport-netdev.h                         |    1 -
 datapath/vport-patch.c                          |    1 -
 datapath/vport.c                                |  148 ++++++---------------
 datapath/vport.h                                |   16 +--
 lib/netdev-linux.c                              |  161 +++++++++++++++++------
 12 files changed, 178 insertions(+), 251 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 79df5f8..082701c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1589,8 +1589,8 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 	nla = nla_reserve(skb, OVS_VPORT_ATTR_STATS, sizeof(struct rtnl_link_stats64));
 	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));
 
@@ -1669,8 +1669,10 @@ 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]));
+		vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
+
 	if (!err && 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
index 1ac3831..8d24cd9 100644
--- 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
index 91855c4..21d595a 100644
--- a/datapath/linux/compat/netdevice.c
+++ b/datapath/linux/compat/netdevice.c
@@ -2,53 +2,6 @@
 #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
index 68a168b..76db8a7 100644
--- 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
index 24c53f8..5beae42 100644
--- 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
index 82079bd..220d124 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -32,9 +32,7 @@ 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);
@@ -271,7 +269,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_mtu	= netdev_set_mtu,
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index 478f271..2076f40 100644
--- 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)
@@ -224,13 +211,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);
@@ -416,8 +396,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,
@@ -427,7 +406,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
index 3f64767..aa38bfe 100644
--- a/datapath/vport-netdev.h
+++ b/datapath/vport-netdev.h
@@ -31,7 +31,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
index 1ec3a4c..9554f12 100644
--- 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,
diff --git a/datapath/vport.c b/datapath/vport.c
index b1418a4..acf84ef 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -184,13 +184,11 @@ struct vport *vport_alloc(int priv_size, const struct vport_ops *ops, const stru
 	vport->kobj.kset = NULL;
 	kobject_init(&vport->kobj, &brport_ktype);
 
-	if (vport->ops->flags & VPORT_F_GEN_STATS) {
-		vport->percpu_stats = alloc_percpu(struct vport_percpu_stats);
-		if (!vport->percpu_stats)
-			return ERR_PTR(-ENOMEM);
+	vport->percpu_stats = alloc_percpu(struct vport_percpu_stats);
+	if (!vport->percpu_stats)
+		return ERR_PTR(-ENOMEM);
 
-		spin_lock_init(&vport->stats_lock);
-	}
+	spin_lock_init(&vport->stats_lock);
 
 	return vport;
 }
@@ -207,8 +205,7 @@ struct vport *vport_alloc(int priv_size, const struct vport_ops *ops, const stru
  */
 void vport_free(struct vport *vport)
 {
-	if (vport->ops->flags & VPORT_F_GEN_STATS)
-		free_percpu(vport->percpu_stats);
+	free_percpu(vport->percpu_stats);
 
 	kobject_put(&vport->kobj);
 }
@@ -350,18 +347,13 @@ int vport_set_addr(struct vport *vport, const unsigned char *addr)
  *
  * Must be called with RTNL lock.
  */
-int vport_set_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
+void vport_set_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
 {
 	ASSERT_RTNL();
 
-	if (vport->ops->flags & VPORT_F_GEN_STATS) {
-		spin_lock_bh(&vport->stats_lock);
-		vport->offset_stats = *stats;
-		spin_unlock_bh(&vport->stats_lock);
-
-		return 0;
-	} else
-		return -EOPNOTSUPP;
+	spin_lock_bh(&vport->stats_lock);
+	vport->offset_stats = *stats;
+	spin_unlock_bh(&vport->stats_lock);
 }
 
 /**
@@ -419,17 +411,6 @@ struct kobject *vport_get_kobj(const struct vport *vport)
 		return NULL;
 }
 
-static int vport_call_get_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
-{
-	int err;
-
-	rcu_read_lock();
-	err = vport->ops->get_stats(vport, stats);
-	rcu_read_unlock();
-
-	return err;
-}
-
 /**
  *	vport_get_stats - retrieve device stats
  *
@@ -440,19 +421,17 @@ static int vport_call_get_stats(struct vport *vport, struct rtnl_link_stats64 *s
  *
  * Must be called with RTNL lock or rcu_read_lock.
  */
-int vport_get_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
+void vport_get_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
 {
 	int i;
 
-	if (!(vport->ops->flags & VPORT_F_GEN_STATS))
-		return vport_call_get_stats(vport, stats);
-
 	/* We potentially have 3 sources of stats that need to be
 	 * combined: those we have collected (split into err_stats and
 	 * percpu_stats), offset_stats from set_stats(), and device
 	 * error stats from get_stats() (for errors that happen
 	 * downstream and therefore aren't reported through our
-	 * vport_record_error() function). */
+	 * vport_record_error() function). Merging with device error stats
+	 * is moved to userspace. */
 
 	spin_lock_bh(&vport->stats_lock);
 
@@ -465,35 +444,6 @@ int vport_get_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
 
 	spin_unlock_bh(&vport->stats_lock);
 
-	if (vport->ops->get_stats) {
-		struct rtnl_link_stats64 dev_stats;
-		int err;
-
-		err = vport_call_get_stats(vport, &dev_stats);
-		if (err)
-			return err;
-
-		stats->rx_errors           += dev_stats.rx_errors;
-		stats->tx_errors           += dev_stats.tx_errors;
-		stats->rx_dropped          += dev_stats.rx_dropped;
-		stats->tx_dropped          += dev_stats.tx_dropped;
-		stats->multicast           += dev_stats.multicast;
-		stats->collisions          += dev_stats.collisions;
-		stats->rx_length_errors    += dev_stats.rx_length_errors;
-		stats->rx_over_errors      += dev_stats.rx_over_errors;
-		stats->rx_crc_errors       += dev_stats.rx_crc_errors;
-		stats->rx_frame_errors     += dev_stats.rx_frame_errors;
-		stats->rx_fifo_errors      += dev_stats.rx_fifo_errors;
-		stats->rx_missed_errors    += dev_stats.rx_missed_errors;
-		stats->tx_aborted_errors   += dev_stats.tx_aborted_errors;
-		stats->tx_carrier_errors   += dev_stats.tx_carrier_errors;
-		stats->tx_fifo_errors      += dev_stats.tx_fifo_errors;
-		stats->tx_heartbeat_errors += dev_stats.tx_heartbeat_errors;
-		stats->tx_window_errors    += dev_stats.tx_window_errors;
-		stats->rx_compressed       += dev_stats.rx_compressed;
-		stats->tx_compressed       += dev_stats.tx_compressed;
-	}
-
 	for_each_possible_cpu(i) {
 		const struct vport_percpu_stats *percpu_stats;
 		struct vport_percpu_stats local_stats;
@@ -511,8 +461,6 @@ int vport_get_stats(struct vport *vport, struct rtnl_link_stats64 *stats)
 		stats->tx_bytes		+= local_stats.tx_bytes;
 		stats->tx_packets	+= local_stats.tx_packets;
 	}
-
-	return 0;
 }
 
 /**
@@ -640,19 +588,17 @@ int vport_get_options(const struct vport *vport, struct sk_buff *skb)
  */
 void vport_receive(struct vport *vport, struct sk_buff *skb)
 {
-	if (vport->ops->flags & VPORT_F_GEN_STATS) {
-		struct vport_percpu_stats *stats;
+	struct vport_percpu_stats *stats;
 
-		local_bh_disable();
-		stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
+	local_bh_disable();
+	stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
 
-		write_seqcount_begin(&stats->seqlock);
-		stats->rx_packets++;
-		stats->rx_bytes += skb->len;
-		write_seqcount_end(&stats->seqlock);
+	write_seqcount_begin(&stats->seqlock);
+	stats->rx_packets++;
+	stats->rx_bytes += skb->len;
+	write_seqcount_end(&stats->seqlock);
 
-		local_bh_enable();
-	}
+	local_bh_enable();
 
 	if (!(vport->ops->flags & VPORT_F_FLOW))
 		OVS_CB(skb)->flow = NULL;
@@ -674,21 +620,18 @@ void vport_receive(struct vport *vport, struct sk_buff *skb)
  */
 int vport_send(struct vport *vport, struct sk_buff *skb)
 {
+	struct vport_percpu_stats *stats;
 	int sent = vport->ops->send(vport, skb);
 
-	if (vport->ops->flags & VPORT_F_GEN_STATS && sent > 0) {
-		struct vport_percpu_stats *stats;
-
-		local_bh_disable();
-		stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
+	local_bh_disable();
+	stats = per_cpu_ptr(vport->percpu_stats, smp_processor_id());
 
-		write_seqcount_begin(&stats->seqlock);
-		stats->tx_packets++;
-		stats->tx_bytes += sent;
-		write_seqcount_end(&stats->seqlock);
+	write_seqcount_begin(&stats->seqlock);
+	stats->tx_packets++;
+	stats->tx_bytes += sent;
+	write_seqcount_end(&stats->seqlock);
 
-		local_bh_enable();
-	}
+	local_bh_enable();
 
 	return sent;
 }
@@ -704,28 +647,25 @@ int vport_send(struct vport *vport, struct sk_buff *skb)
  */
 void vport_record_error(struct vport *vport, enum vport_err_type err_type)
 {
-	if (vport->ops->flags & VPORT_F_GEN_STATS) {
-
-		spin_lock_bh(&vport->stats_lock);
+	spin_lock_bh(&vport->stats_lock);
 
-		switch (err_type) {
-		case VPORT_E_RX_DROPPED:
-			vport->err_stats.rx_dropped++;
-			break;
+	switch (err_type) {
+	case VPORT_E_RX_DROPPED:
+		vport->err_stats.rx_dropped++;
+		break;
 
-		case VPORT_E_RX_ERROR:
-			vport->err_stats.rx_errors++;
-			break;
+	case VPORT_E_RX_ERROR:
+		vport->err_stats.rx_errors++;
+		break;
 
-		case VPORT_E_TX_DROPPED:
-			vport->err_stats.tx_dropped++;
-			break;
+	case VPORT_E_TX_DROPPED:
+		vport->err_stats.tx_dropped++;
+		break;
 
-		case VPORT_E_TX_ERROR:
-			vport->err_stats.tx_errors++;
-			break;
-		};
+	case VPORT_E_TX_ERROR:
+		vport->err_stats.tx_errors++;
+		break;
+	};
 
-		spin_unlock_bh(&vport->stats_lock);
-	}
+	spin_unlock_bh(&vport->stats_lock);
 }
diff --git a/datapath/vport.h b/datapath/vport.h
index bbbb214..08e424e 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -32,14 +32,14 @@ struct vport *vport_locate(const char *name);
 
 int vport_set_mtu(struct vport *, int mtu);
 int vport_set_addr(struct vport *, const unsigned char *);
-int vport_set_stats(struct vport *, struct rtnl_link_stats64 *);
+void vport_set_stats(struct vport *, struct rtnl_link_stats64 *);
 
 const char *vport_get_name(const struct vport *);
 enum ovs_vport_type vport_get_type(const struct vport *);
 const unsigned char *vport_get_addr(const struct vport *);
 
 struct kobject *vport_get_kobj(const struct vport *);
-int vport_get_stats(struct vport *, struct rtnl_link_stats64 *);
+void vport_get_stats(struct vport *, struct rtnl_link_stats64 *);
 
 unsigned vport_get_flags(const struct vport *);
 int vport_is_running(const struct vport *);
@@ -85,11 +85,9 @@ struct vport_err_stats {
  * regardless of whether they were actually chosen and sent down to userspace.
  * @hash_node: Element in @dev_table hash table in vport.c.
  * @ops: Class structure.
- * @percpu_stats: Points to per-CPU statistics used and maintained by the vport
- * code if %VPORT_F_GEN_STATS is set to 1 in @ops flags, otherwise unused.
+ * @percpu_stats: Points to per-CPU statistics used and maintained by vport
  * @stats_lock: Protects @err_stats and @offset_stats.
- * @err_stats: Points to error statistics used and maintained by the vport code
- * if %VPORT_F_GEN_STATS is set to 1 in @ops flags, otherwise unused.
+ * @err_stats: Points to error statistics used and maintained by vport
  * @offset_stats: Added to actual statistics as a sop to compatibility with
  * XAPI for Citrix XenServer.  Deprecated.
  */
@@ -113,7 +111,6 @@ struct vport {
 };
 
 #define VPORT_F_REQUIRED	(1 << 0) /* If init fails, module loading fails. */
-#define VPORT_F_GEN_STATS	(1 << 1) /* Track stats at the generic layer. */
 #define VPORT_F_FLOW		(1 << 2) /* Sets OVS_CB(skb)->flow. */
 #define VPORT_F_TUN_ID		(1 << 3) /* Sets OVS_CB(skb)->tun_id. */
 
@@ -162,10 +159,6 @@ struct vport_parms {
  * @get_addr: Get the device's MAC address.
  * @get_config: Get the device's configuration.
  * @get_kobj: Get the kobj associated with the device (may return null).
- * @get_stats: Fill in the transmit/receive stats.  May be null if stats are
- * not supported or if generic stats are in use.  If defined and
- * VPORT_F_GEN_STATS is also set, the error stats are added to those already
- * collected.
  * @get_dev_flags: Get the device's flags.
  * @is_running: Checks whether the device is running.
  * @get_operstate: Get the device's operating state.
@@ -198,7 +191,6 @@ struct vport_ops {
 	const unsigned char *(*get_addr)(const struct vport *);
 	void (*get_config)(const struct vport *, void *);
 	struct kobject *(*get_kobj)(const struct vport *);
-	int (*get_stats)(const struct vport *, struct rtnl_link_stats64 *);
 
 	unsigned (*get_dev_flags)(const struct vport *);
 	int (*is_running)(const struct vport *);
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index fc61d45..6553906 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -115,7 +115,6 @@ enum {
     VALID_IN6               = 1 << 3,
     VALID_MTU               = 1 << 4,
     VALID_CARRIER           = 1 << 5,
-    VALID_IS_PSEUDO         = 1 << 6, /* Represents is_internal and is_tap. */
     VALID_POLICING          = 1 << 7,
     VALID_HAVE_VPORT_STATS  = 1 << 8
 };
@@ -369,8 +368,6 @@ struct netdev_dev_linux {
     struct in6_addr in6;
     int mtu;
     int carrier;
-    bool is_internal;           /* Is this an openvswitch internal device? */
-    bool is_tap;                /* Is this a tuntap device? */
     uint32_t kbits_rate;        /* Policing data. */
     uint32_t kbits_burst;
     bool have_vport_stats;
@@ -1246,21 +1243,6 @@ check_for_working_netlink_stats(void)
     }
 }
 
-/* Brings the 'is_internal' and 'is_tap' members of 'netdev_dev' up-to-date. */
-static void
-netdev_linux_update_is_pseudo(struct netdev_dev_linux *netdev_dev)
-{
-    if (!(netdev_dev->cache_valid & VALID_IS_PSEUDO)) {
-        const char *name = netdev_dev_get_name(&netdev_dev->netdev_dev);
-        const char *type = netdev_dev_get_type(&netdev_dev->netdev_dev);
-
-        netdev_dev->is_tap = !strcmp(type, "tap");
-        netdev_dev->is_internal = (!netdev_dev->is_tap
-                                   && dpif_linux_is_internal_device(name));
-        netdev_dev->cache_valid |= VALID_IS_PSEUDO;
-    }
-}
-
 static void
 swap_uint64(uint64_t *a, uint64_t *b)
 {
@@ -1269,38 +1251,125 @@ swap_uint64(uint64_t *a, uint64_t *b)
     *b = tmp;
 }
 
-/* Retrieves current device stats for 'netdev'. */
-static int
-netdev_linux_get_stats(const struct netdev *netdev_,
-                       struct netdev_stats *stats)
+static void
+ovs_get_stats(const struct netdev *netdev_,
+              struct netdev_stats *stats)
 {
     struct netdev_dev_linux *netdev_dev =
                                 netdev_dev_linux_cast(netdev_get_dev(netdev_));
-    static int use_netlink_stats = -1;
-    int error;
 
     if (netdev_dev->have_vport_stats ||
         !(netdev_dev->cache_valid & VALID_HAVE_VPORT_STATS)) {
+        int error;
 
         error = netdev_vport_get_stats(netdev_, stats);
+        if (error) {
+            VLOG_WARN_RL(&rl, "%s: ovs get stats failed %d",
+                         netdev_get_name(netdev_), error);
+        }
         netdev_dev->have_vport_stats = !error;
         netdev_dev->cache_valid |= VALID_HAVE_VPORT_STATS;
     }
+}
 
-    if (!netdev_dev->have_vport_stats) {
-        if (use_netlink_stats < 0) {
-            use_netlink_stats = check_for_working_netlink_stats();
-        }
-        if (use_netlink_stats) {
-            int ifindex;
+static int
+netdev_linux_sys_get_stats(const struct netdev *netdev_,
+                         struct netdev_stats *stats)
+{
+    static int use_netlink_stats = -1;
 
-            error = get_ifindex(netdev_, &ifindex);
-            if (!error) {
-                error = get_stats_via_netlink(ifindex, stats);
-            }
+    if (use_netlink_stats < 0) {
+        use_netlink_stats = check_for_working_netlink_stats();
+    }
+
+    if (use_netlink_stats) {
+        int error;
+        int ifindex;
+
+        error = get_ifindex(netdev_, &ifindex);
+        if (!error) {
+            error = get_stats_via_netlink(ifindex, stats);
         } else {
             error = get_stats_via_proc(netdev_get_name(netdev_), stats);
         }
+
+        if (error) {
+            VLOG_WARN_RL(&rl, "%s: linux-sys get stats failed %d",
+                         netdev_get_name(netdev_), error);
+        }
+        return error;
+    } else {
+        return -EOPNOTSUPP;
+    }
+}
+
+/* Retrieves current device stats for 'netdev-linux'. */
+static int
+netdev_linux_get_stats(const struct netdev *netdev_,
+                       struct netdev_stats *stats)
+{
+    struct netdev_dev_linux *netdev_dev =
+                                netdev_dev_linux_cast(netdev_get_dev(netdev_));
+    struct netdev_stats dev_stats;
+    int error;
+
+    ovs_get_stats(netdev_, stats);
+
+    error = netdev_linux_sys_get_stats(netdev_, &dev_stats);
+
+    if (error) {
+        if (!netdev_dev->have_vport_stats) {
+            return error;
+        } else {
+            return 0;
+        }
+    }
+
+    if (!netdev_dev->have_vport_stats) {
+        /* stats not available from OVS then use ioctl stats. */
+        *stats = dev_stats;
+    } else {
+        stats->rx_errors           += dev_stats.rx_errors;
+        stats->tx_errors           += dev_stats.tx_errors;
+        stats->rx_dropped          += dev_stats.rx_dropped;
+        stats->tx_dropped          += dev_stats.tx_dropped;
+        stats->multicast           += dev_stats.multicast;
+        stats->collisions          += dev_stats.collisions;
+        stats->rx_length_errors    += dev_stats.rx_length_errors;
+        stats->rx_over_errors      += dev_stats.rx_over_errors;
+        stats->rx_crc_errors       += dev_stats.rx_crc_errors;
+        stats->rx_frame_errors     += dev_stats.rx_frame_errors;
+        stats->rx_fifo_errors      += dev_stats.rx_fifo_errors;
+        stats->rx_missed_errors    += dev_stats.rx_missed_errors;
+        stats->tx_aborted_errors   += dev_stats.tx_aborted_errors;
+        stats->tx_carrier_errors   += dev_stats.tx_carrier_errors;
+        stats->tx_fifo_errors      += dev_stats.tx_fifo_errors;
+        stats->tx_heartbeat_errors += dev_stats.tx_heartbeat_errors;
+        stats->tx_window_errors    += dev_stats.tx_window_errors;
+    }
+    return 0;
+}
+
+/* Retrieves current device stats for 'netdev-tap' netdev or
+ * netdev-internal. */
+static int
+netdev_pseudo_get_stats(const struct netdev *netdev_,
+                        struct netdev_stats *stats)
+{
+    struct netdev_dev_linux *netdev_dev =
+                                netdev_dev_linux_cast(netdev_get_dev(netdev_));
+    struct netdev_stats dev_stats;
+    int error;
+
+    ovs_get_stats(netdev_, stats);
+
+    error = netdev_linux_sys_get_stats(netdev_, &dev_stats);
+    if (error) {
+        if (!netdev_dev->have_vport_stats) {
+            return error;
+        } else {
+            return 0;
+        }
     }
 
     /* If this port is an internal port then the transmit and receive stats
@@ -1309,9 +1378,8 @@ netdev_linux_get_stats(const struct netdev *netdev_,
      * them back here. This does not apply if we are getting stats from the
      * vport layer because it always tracks stats from the perspective of the
      * switch. */
-    netdev_linux_update_is_pseudo(netdev_dev);
-    if (!error && !netdev_dev->have_vport_stats &&
-        (netdev_dev->is_internal || netdev_dev->is_tap)) {
+    if (!netdev_dev->have_vport_stats) {
+        *stats = dev_stats;
         swap_uint64(&stats->rx_packets, &stats->tx_packets);
         swap_uint64(&stats->rx_bytes, &stats->tx_bytes);
         swap_uint64(&stats->rx_errors, &stats->tx_errors);
@@ -1327,9 +1395,17 @@ netdev_linux_get_stats(const struct netdev *netdev_,
         stats->tx_fifo_errors = 0;
         stats->tx_heartbeat_errors = 0;
         stats->tx_window_errors = 0;
-    }
+    } else {
+        stats->rx_dropped          += dev_stats.tx_dropped;
+        stats->tx_dropped          += dev_stats.rx_dropped;
 
-    return error;
+        stats->rx_errors           += dev_stats.tx_errors;
+        stats->tx_errors           += dev_stats.rx_errors;
+
+        stats->multicast           += dev_stats.multicast;
+        stats->collisions          += dev_stats.collisions;
+    }
+    return 0;
 }
 
 /* Stores the features supported by 'netdev' into each of '*current',
@@ -2263,7 +2339,7 @@ netdev_linux_change_seq(const struct netdev *netdev)
     return netdev_dev_linux_cast(netdev_get_dev(netdev))->change_seq;
 }
 
-#define NETDEV_LINUX_CLASS(NAME, CREATE, ENUMERATE, SET_STATS)  \
+#define NETDEV_LINUX_CLASS(NAME, CREATE, ENUMERATE, GET_STATS, SET_STATS)  \
 {                                                               \
     NAME,                                                       \
                                                                 \
@@ -2296,7 +2372,7 @@ netdev_linux_change_seq(const struct netdev *netdev)
     netdev_linux_get_ifindex,                                   \
     netdev_linux_get_carrier,                                   \
     netdev_linux_set_miimon_interval,                           \
-    netdev_linux_get_stats,                                     \
+    GET_STATS,                                                  \
     SET_STATS,                                                  \
                                                                 \
     netdev_linux_get_features,                                  \
@@ -2333,6 +2409,7 @@ const struct netdev_class netdev_linux_class =
         "system",
         netdev_linux_create,
         netdev_linux_enumerate,
+        netdev_linux_get_stats,
         NULL);                  /* set_stats */
 
 const struct netdev_class netdev_tap_class =
@@ -2340,6 +2417,7 @@ const struct netdev_class netdev_tap_class =
         "tap",
         netdev_linux_create_tap,
         NULL,                   /* enumerate */
+        netdev_pseudo_get_stats,
         NULL);                  /* set_stats */
 
 const struct netdev_class netdev_internal_class =
@@ -2347,6 +2425,7 @@ const struct netdev_class netdev_internal_class =
         "internal",
         netdev_linux_create,
         NULL,                    /* enumerate */
+        netdev_pseudo_get_stats,
         netdev_vport_set_stats);
 
 /* HTB traffic control class. */
-- 
1.7.1




More information about the dev mailing list