[ovs-dev] [PATCH 2/6] vport: Allow offsets to be set for stats.

Jesse Gross jesse at nicira.com
Thu Jun 10 02:33:17 UTC 2010


Adds a method to set a group of stats to be added to the values
gathered normally.  This is needed for the fake bond device to
show the stats of its underlying slaves.  Also enables devices
that use the generic stats layer to define a get_stats() function
to provide additional error counts.
---
 datapath/datapath.c                     |    5 +
 datapath/vport.c                        |  200 ++++++++++++++++++++++++-------
 datapath/vport.h                        |   17 ++-
 include/openvswitch/datapath-protocol.h |    1 +
 4 files changed, 174 insertions(+), 49 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c968239..39bc190 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1634,6 +1634,10 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
 		err = vport_user_stats_get((struct odp_vport_stats_req __user *)argp);
 		goto exit;
 
+	case ODP_VPORT_STATS_SET:
+		err = vport_user_stats_set((struct odp_vport_stats_req __user *)argp);
+		goto exit;
+
 	case ODP_VPORT_ETHER_GET:
 		err = vport_user_ether_get((struct odp_vport_ether __user *)argp);
 		goto exit;
@@ -1998,6 +2002,7 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
 	case ODP_VPORT_MTU_GET:
 	case ODP_VPORT_ETHER_SET:
 	case ODP_VPORT_ETHER_GET:
+	case ODP_VPORT_STATS_SET:
 	case ODP_VPORT_STATS_GET:
 	case ODP_DP_STATS:
 	case ODP_GET_DROP_FRAGS:
diff --git a/datapath/vport.c b/datapath/vport.c
index 83b42d5..0ba0823 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -113,11 +113,6 @@ vport_init(void)
 	for (i = 0; i < ARRAY_SIZE(base_vport_ops_list); i++) {
 		struct vport_ops *new_ops = base_vport_ops_list[i];
 
-		if (new_ops->get_stats && new_ops->flags & VPORT_F_GEN_STATS) {
-			printk(KERN_INFO "openvswitch: both get_stats() and VPORT_F_GEN_STATS defined on vport %s, dropping VPORT_F_GEN_STATS\n", new_ops->type);
-			new_ops->flags &= ~VPORT_F_GEN_STATS;
-		}
-
 		if (new_ops->init)
 			err = new_ops->init();
 		else
@@ -403,56 +398,59 @@ vport_user_stats_get(struct odp_vport_stats_req __user *ustats_req)
 		goto out;
 	}
 
-	if (vport->ops->get_stats) {
-		rcu_read_lock();
-		err = vport->ops->get_stats(vport, &stats_req.stats);
-		rcu_read_unlock();
+	err = vport_get_stats(vport, &stats_req.stats);
 
-	} else if (vport->ops->flags & VPORT_F_GEN_STATS) {
-		int i;
+out:
+	vport_unlock();
 
-		memset(&stats_req.stats, 0, sizeof(struct odp_vport_stats));
+	if (!err)
+		if (copy_to_user(ustats_req, &stats_req, sizeof(struct odp_vport_stats_req)))
+			err = -EFAULT;
 
-		for_each_possible_cpu(i) {
-			const struct vport_percpu_stats *percpu_stats;
+	return err;
+}
 
-			percpu_stats = per_cpu_ptr(vport->percpu_stats, i);
-			stats_req.stats.rx_bytes	+= percpu_stats->rx_bytes;
-			stats_req.stats.rx_packets	+= percpu_stats->rx_packets;
-			stats_req.stats.tx_bytes	+= percpu_stats->tx_bytes;
-			stats_req.stats.tx_packets	+= percpu_stats->tx_packets;
-		}
+/**
+ *	vport_user_stats_set - sets offset device stats (for userspace callers)
+ *
+ * @ustats_req: Stats set parameters.
+ *
+ * Provides a set of transmit, receive, and error stats to be added as an
+ * offset to the collect data when stats are retreived.  Some devices may not
+ * support setting the stats, in which case the result will always be
+ * -EOPNOTSUPP.  This function is for userspace callers and assumes no locks
+ * are held.
+ */
+int
+vport_user_stats_set(struct odp_vport_stats_req __user *ustats_req)
+{
+	struct odp_vport_stats_req stats_req;
+	struct vport *vport;
+	int err;
 
-		spin_lock_bh(&vport->err_stats.lock);
+	if (copy_from_user(&stats_req, ustats_req, sizeof(struct odp_vport_stats_req)))
+		return -EFAULT;
 
-		stats_req.stats.rx_dropped	= vport->err_stats.rx_dropped;
-		stats_req.stats.rx_errors	= vport->err_stats.rx_errors
-						+ vport->err_stats.rx_frame_err
-						+ vport->err_stats.rx_over_err
-						+ vport->err_stats.rx_crc_err;
-		stats_req.stats.rx_frame_err	= vport->err_stats.rx_frame_err;
-		stats_req.stats.rx_over_err	= vport->err_stats.rx_over_err;
-		stats_req.stats.rx_crc_err	= vport->err_stats.rx_crc_err;
-		stats_req.stats.tx_dropped	= vport->err_stats.tx_dropped;
-		stats_req.stats.tx_errors	= vport->err_stats.tx_errors;
-		stats_req.stats.collisions	= vport->err_stats.collisions;
+	stats_req.devname[IFNAMSIZ - 1] = '\0';
 
-		spin_unlock_bh(&vport->err_stats.lock);
+	rtnl_lock();
+	vport_lock();
 
-		err = 0;
-	} else
-		err = -EOPNOTSUPP;
+	vport = vport_locate(stats_req.devname);
+	if (!vport) {
+		err = -ENODEV;
+		goto out;
+	}
+
+	err = vport_set_stats(vport, &stats_req.stats);
 
 out:
 	vport_unlock();
-
-	if (!err)
-		if (copy_to_user(ustats_req, &stats_req, sizeof(struct odp_vport_stats_req)))
-			err = -EFAULT;
-
+	rtnl_unlock();
 	return err;
 }
 
+
 /**
  *	vport_user_ether_get - retrieve device Ethernet address (for userspace callers)
  *
@@ -699,7 +697,7 @@ vport_alloc(int priv_size, const struct vport_ops *ops)
 		if (!vport->percpu_stats)
 			return ERR_PTR(-ENOMEM);
 
-		spin_lock_init(&vport->err_stats.lock);
+		spin_lock_init(&vport->stats_lock);
 	}
 
 	return vport;
@@ -927,6 +925,34 @@ vport_set_addr(struct vport *vport, const unsigned char *addr)
 }
 
 /**
+ *	vport_set_stats - sets offset device stats (for kernel callers)
+ *
+ * @vport: vport on which to set stats
+ * @stats: stats to set
+ *
+ * Provides a set of transmit, receive, and error stats to be added as an
+ * offset to the collect data when stats are retreived.  Some devices may not
+ * support setting the stats, in which case the result will always be
+ * -EOPNOTSUPP.  RTNL lock must be held.
+ */
+int
+vport_set_stats(struct vport *vport, struct odp_vport_stats *stats)
+{
+	ASSERT_RTNL();
+
+	if (vport->ops->flags & VPORT_F_GEN_STATS) {
+		spin_lock_bh(&vport->stats_lock);
+		memcpy(&vport->offset_stats, stats, sizeof(struct odp_vport_stats));
+		spin_unlock_bh(&vport->stats_lock);
+
+		return 0;
+	} else if (vport->ops->set_stats)
+		return vport->ops->set_stats(vport, stats);
+	else
+		return -EOPNOTSUPP;
+}
+
+/**
  *	vport_get_name - retrieve device name
  *
  * @vport: vport from which to retrieve the name.
@@ -1002,6 +1028,92 @@ vport_get_kobj(const struct vport *vport)
 }
 
 /**
+ *	vport_get_stats - retrieve device stats (for kernel callers)
+ *
+ * @vport: vport from which to retrieve the stats
+ * @stats: location to store stats
+ *
+ * Retrieves transmit, receive, and error stats for the given device.
+ */
+int
+vport_get_stats(struct vport *vport, struct odp_vport_stats *stats)
+{
+	struct odp_vport_stats dev_stats;
+	struct odp_vport_stats *dev_statsp = NULL;
+	int err;
+
+	if (vport->ops->get_stats) {
+		if (vport->ops->flags & VPORT_F_GEN_STATS)
+			dev_statsp = &dev_stats;
+		else
+			dev_statsp = stats;
+
+		rcu_read_lock();
+		err = vport->ops->get_stats(vport, dev_statsp);
+		rcu_read_unlock();
+
+		if (err)
+			goto out;
+	}
+
+	if (vport->ops->flags & VPORT_F_GEN_STATS) {
+		int i;
+
+		/* 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). */
+
+		spin_lock_bh(&vport->stats_lock);
+
+		memcpy(stats, &vport->offset_stats, sizeof(struct odp_vport_stats));
+
+		stats->rx_errors	+= vport->err_stats.rx_errors
+						+ vport->err_stats.rx_frame_err
+						+ vport->err_stats.rx_over_err
+						+ vport->err_stats.rx_crc_err;
+		stats->tx_errors	+= vport->err_stats.tx_errors;
+		stats->tx_dropped	+= vport->err_stats.tx_dropped;
+		stats->rx_dropped	+= vport->err_stats.rx_dropped;
+		stats->rx_over_err	+= vport->err_stats.rx_over_err;
+		stats->rx_crc_err	+= vport->err_stats.rx_crc_err;
+		stats->rx_frame_err	+= vport->err_stats.rx_frame_err;
+		stats->collisions	+= vport->err_stats.collisions;
+
+		spin_unlock_bh(&vport->stats_lock);
+
+		if (dev_statsp) {
+			stats->rx_errors	+= dev_statsp->rx_errors;
+			stats->tx_errors	+= dev_statsp->tx_errors;
+			stats->rx_dropped	+= dev_statsp->rx_dropped;
+			stats->tx_dropped	+= dev_statsp->tx_dropped;
+			stats->rx_over_err	+= dev_statsp->rx_over_err;
+			stats->rx_crc_err	+= dev_statsp->rx_crc_err;
+			stats->rx_frame_err	+= dev_statsp->rx_frame_err;
+			stats->collisions	+= dev_statsp->collisions;
+		}
+
+		for_each_possible_cpu(i) {
+			const struct vport_percpu_stats *percpu_stats;
+
+			percpu_stats = per_cpu_ptr(vport->percpu_stats, i);
+			stats->rx_bytes		+= percpu_stats->rx_bytes;
+			stats->rx_packets	+= percpu_stats->rx_packets;
+			stats->tx_bytes		+= percpu_stats->tx_bytes;
+			stats->tx_packets	+= percpu_stats->tx_packets;
+		}
+
+		err = 0;
+	} else
+		err = -EOPNOTSUPP;
+
+out:
+	return err;
+}
+
+/**
  *	vport_get_flags - retrieve device flags
  *
  * @vport: vport from which to retrieve the flags
@@ -1191,7 +1303,7 @@ vport_record_error(struct vport *vport, enum vport_err_type err_type)
 {
 	if (vport->ops->flags & VPORT_F_GEN_STATS) {
 
-		spin_lock_bh(&vport->err_stats.lock);
+		spin_lock_bh(&vport->stats_lock);
 
 		switch (err_type) {
 		case VPORT_E_RX_DROPPED:
@@ -1227,6 +1339,6 @@ vport_record_error(struct vport *vport, enum vport_err_type err_type)
 			break;
 		};
 
-		spin_unlock_bh(&vport->err_stats.lock);
+		spin_unlock_bh(&vport->stats_lock);
 	}
 }
diff --git a/datapath/vport.h b/datapath/vport.h
index e84c4e3..80f12a9 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -32,6 +32,7 @@ int compat_vport_user_mod(struct compat_odp_vport_mod __user *);
 #endif
 
 int vport_user_stats_get(struct odp_vport_stats_req __user *);
+int vport_user_stats_set(struct odp_vport_stats_req __user *);
 int vport_user_ether_get(struct odp_vport_ether __user *);
 int vport_user_ether_set(struct odp_vport_ether __user *);
 int vport_user_mtu_get(struct odp_vport_mtu __user *);
@@ -54,14 +55,15 @@ int vport_detach(struct vport *);
 
 int vport_set_mtu(struct vport *, int mtu);
 int vport_set_addr(struct vport *, const unsigned char *);
+int vport_set_stats(struct vport *, struct odp_vport_stats *);
 
 const char *vport_get_name(const struct vport *);
 const char *vport_get_type(const struct vport *);
 const unsigned char *vport_get_addr(const struct vport *);
 
 struct dp_port *vport_get_dp_port(const struct vport *);
-
 struct kobject *vport_get_kobj(const struct vport *);
+int vport_get_stats(struct vport *, struct odp_vport_stats *);
 
 unsigned vport_get_flags(const struct vport *);
 int vport_is_running(const struct vport *);
@@ -84,8 +86,6 @@ struct vport_percpu_stats {
 };
 
 struct vport_err_stats {
-	spinlock_t lock;
-
 	u64 rx_dropped;
 	u64 rx_errors;
 	u64 rx_frame_err;
@@ -102,7 +102,10 @@ struct vport {
 	struct dp_port *dp_port;
 
 	struct vport_percpu_stats *percpu_stats;
+
+	spinlock_t stats_lock;
 	struct vport_err_stats err_stats;
+	struct odp_vport_stats offset_stats;
 };
 
 #define VPORT_F_REQUIRED	(1 << 0) /* If init fails, module loading fails. */
@@ -133,12 +136,15 @@ struct vport {
  * @detach: Detach a vport from a datapath.  May be null if not needed.
  * @set_mtu: Set the device's MTU.  May be null if not supported.
  * @set_addr: Set the device's MAC address.  May be null if not supported.
+ * @set_stats: Provides stats as an offset to be added to the device stats.
+ * May be null if not supported.
  * @get_name: Get the device's name.
  * @get_addr: Get the device's MAC address.
  * @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 overrides
- * VPORT_F_GEN_STATS.
+ * 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.
@@ -168,6 +174,7 @@ struct vport_ops {
 
 	int (*set_mtu)(struct vport *, int mtu);
 	int (*set_addr)(struct vport *, const unsigned char *);
+	int (*set_stats)(const struct vport *, struct odp_vport_stats *);
 
 	/* Called with rcu_read_lock or RTNL lock. */
 	const char *(*get_name)(const struct vport *);
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index f443000..4b2168c 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -101,6 +101,7 @@
 #define ODP_VPORT_ETHER_SET     _IOW('O', 26, struct odp_vport_ether)
 #define ODP_VPORT_MTU_GET       _IOWR('O', 27, struct odp_vport_mtu)
 #define ODP_VPORT_MTU_SET       _IOW('O', 28, struct odp_vport_mtu)
+#define ODP_VPORT_STATS_SET     _IOWR('O', 29, struct odp_vport_stats_req)
 
 struct odp_stats {
     /* Flows. */
-- 
1.7.0.4





More information about the dev mailing list