[ovs-dev] [PATCH v2] netdev-linux: Fix stats for ovs internal device.

Pravin B Shelar pshelar at nicira.com
Wed Feb 29 21:59:43 UTC 2012


Fixed according to comments from Ben.

v1-v2:
        - Fixed commit msg.
        - Added comments.
        - Fixed return type of get_stats_via_vport().

--8<--------------------------cut here-------------------------->8-

There is no need to retrieve linux system stats for internal devices
as all relevant stats for virtual device like internal device are
already reported by OVS over vport-stats. As a result it also fixes
error stats for internal-devices as they are not counted twice.

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
 lib/netdev-linux.c |   38 +++++++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 6ad0852..5c322ca 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -114,7 +114,7 @@ enum {
     VALID_IN6               = 1 << 3,
     VALID_MTU               = 1 << 4,
     VALID_POLICING          = 1 << 5,
-    VALID_HAVE_VPORT_STATS  = 1 << 6
+    VALID_VPORT_STAT_ERROR  = 1 << 6
 };
 
 struct tap_state {
@@ -372,7 +372,8 @@ struct netdev_dev_linux {
     long long int carrier_resets;
     uint32_t kbits_rate;        /* Policing data. */
     uint32_t kbits_burst;
-    bool have_vport_stats;
+    int vport_stats_error;      /* Cached error code from vport_get_stats().
+                                   0 or an errno value. */
     struct tc *tc;
 
     union {
@@ -1236,8 +1237,8 @@ get_stats_via_vport(const struct netdev *netdev_,
     struct netdev_dev_linux *netdev_dev =
                                 netdev_dev_linux_cast(netdev_get_dev(netdev_));
 
-    if (netdev_dev->have_vport_stats ||
-        !(netdev_dev->cache_valid & VALID_HAVE_VPORT_STATS)) {
+    if (!netdev_dev->vport_stats_error ||
+        !(netdev_dev->cache_valid & VALID_VPORT_STAT_ERROR)) {
         int error;
 
         error = netdev_vport_get_stats(netdev_, stats);
@@ -1245,8 +1246,8 @@ get_stats_via_vport(const struct netdev *netdev_,
             VLOG_WARN_RL(&rl, "%s: obtaining netdev stats via vport failed "
                          "(%s)", netdev_get_name(netdev_), strerror(error));
         }
-        netdev_dev->have_vport_stats = !error;
-        netdev_dev->cache_valid |= VALID_HAVE_VPORT_STATS;
+        netdev_dev->vport_stats_error = error;
+        netdev_dev->cache_valid |= VALID_VPORT_STAT_ERROR;
     }
 }
 
@@ -1295,14 +1296,14 @@ netdev_linux_get_stats(const struct netdev *netdev_,
     error = netdev_linux_sys_get_stats(netdev_, &dev_stats);
 
     if (error) {
-        if (!netdev_dev->have_vport_stats) {
+        if (netdev_dev->vport_stats_error) {
             return error;
         } else {
             return 0;
         }
     }
 
-    if (!netdev_dev->have_vport_stats) {
+    if (netdev_dev->vport_stats_error) {
         /* stats not available from OVS then use ioctl stats. */
         *stats = dev_stats;
     } else {
@@ -1330,7 +1331,7 @@ netdev_linux_get_stats(const struct netdev *netdev_,
 /* Retrieves current device stats for 'netdev-tap' netdev or
  * netdev-internal. */
 static int
-netdev_pseudo_get_stats(const struct netdev *netdev_,
+netdev_tap_get_stats(const struct netdev *netdev_,
                         struct netdev_stats *stats)
 {
     struct netdev_dev_linux *netdev_dev =
@@ -1342,7 +1343,7 @@ netdev_pseudo_get_stats(const struct netdev *netdev_,
 
     error = netdev_linux_sys_get_stats(netdev_, &dev_stats);
     if (error) {
-        if (!netdev_dev->have_vport_stats) {
+        if (netdev_dev->vport_stats_error) {
             return error;
         } else {
             return 0;
@@ -1355,7 +1356,7 @@ netdev_pseudo_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. */
-    if (!netdev_dev->have_vport_stats) {
+    if (netdev_dev->vport_stats_error) {
         *stats = dev_stats;
         swap_uint64(&stats->rx_packets, &stats->tx_packets);
         swap_uint64(&stats->rx_bytes, &stats->tx_bytes);
@@ -1385,6 +1386,17 @@ netdev_pseudo_get_stats(const struct netdev *netdev_,
     return 0;
 }
 
+static int
+netdev_internal_get_stats(const struct netdev *netdev_,
+                          struct netdev_stats *stats)
+{
+    struct netdev_dev_linux *netdev_dev =
+                                netdev_dev_linux_cast(netdev_get_dev(netdev_));
+
+    get_stats_via_vport(netdev_, stats);
+    return netdev_dev->vport_stats_error;
+}
+
 /* Stores the features supported by 'netdev' into each of '*current',
  * '*advertised', '*supported', and '*peer' that are non-null.  Each value is a
  * bitmap of "enum ofp_port_features" bits, in host byte order.  Returns 0 if
@@ -2302,14 +2314,14 @@ const struct netdev_class netdev_tap_class =
     NETDEV_LINUX_CLASS(
         "tap",
         netdev_linux_create_tap,
-        netdev_pseudo_get_stats,
+        netdev_tap_get_stats,
         NULL);                  /* set_stats */
 
 const struct netdev_class netdev_internal_class =
     NETDEV_LINUX_CLASS(
         "internal",
         netdev_linux_create,
-        netdev_pseudo_get_stats,
+        netdev_internal_get_stats,
         netdev_vport_set_stats);
 
 /* HTB traffic control class. */
-- 
1.7.1




More information about the dev mailing list