[ovs-dev] [PATCH] netdev: Swap transmit and receive stats on internal ports.

Jesse Gross jesse at nicira.com
Sat Sep 12 00:40:14 UTC 2009


Internal ports appear to have their transmit and receive stats swapped
because from the kernel's point of view these ports are acting like
the machine connected to the switch, not the switch itself.  This swaps
the stats for consistency with other ports.
---
 lib/netdev-linux.c    |   58 +++++++++++++++++++++++++++++++++++++++++++++---
 lib/netdev-provider.h |    3 +-
 lib/netdev.c          |    5 ++-
 lib/netdev.h          |    5 +++-
 ofproto/ofproto.c     |    2 +-
 5 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 11d83e9..b2c45f2 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -625,25 +625,75 @@ check_for_working_netlink_stats(void)
  * XXX All of the members of struct netdev_stats are 64 bits wide, but on
  * 32-bit architectures the Linux network stats are only 32 bits. */
 static int
-netdev_linux_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
+netdev_linux_get_stats(const struct netdev *netdev_, struct dpif *dpif,
+                       struct netdev_stats *stats)
 {
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     static int use_netlink_stats = -1;
     int error;
+    bool internal_port;
+    struct netdev_stats raw_stats;
+    struct netdev_stats *collect_stats = stats;
 
     COVERAGE_INC(netdev_get_stats);
+
+    internal_port = (netdev->tap_fd != -1);
+    if (dpif && !internal_port) {
+        struct odp_port port;
+
+        error = dpif_port_query_by_name(dpif, netdev_get_name(&netdev->netdev),
+                                        &port);
+        if (!error) {
+            internal_port |= port.flags & ODP_PORT_INTERNAL;
+        }
+    }
+
+    if (internal_port) {
+        collect_stats = &raw_stats;
+    }
+
     if (use_netlink_stats < 0) {
         use_netlink_stats = check_for_working_netlink_stats();
     }
     if (use_netlink_stats) {
         int ifindex;
 
-        error = get_ifindex(netdev, &ifindex);
+        error = get_ifindex(&netdev->netdev, &ifindex);
         if (!error) {
-            error = get_stats_via_netlink(ifindex, stats);
+            error = get_stats_via_netlink(ifindex, collect_stats);
         }
     } else {
-        error = get_stats_via_proc(netdev->name, stats);
+        error = get_stats_via_proc(netdev->netdev.name, collect_stats);
+    }
+
+    /* If this port is an internal port then the transmit and receive stats
+     * will appear to be swapped relative to the other ports since we are the
+     * one sending the data, not a remote computer.  For consistency, we swap
+     * them back here. */
+    if (internal_port) {
+        stats->rx_packets = raw_stats.tx_packets;
+        stats->tx_packets = raw_stats.rx_packets;
+        stats->rx_bytes = raw_stats.tx_bytes;
+        stats->tx_bytes = raw_stats.rx_bytes;
+        stats->rx_errors = raw_stats.tx_errors;
+        stats->tx_errors = raw_stats.rx_errors;
+        stats->rx_dropped = raw_stats.tx_dropped;
+        stats->tx_dropped = raw_stats.rx_dropped;
+        stats->multicast = raw_stats.multicast;
+        stats->collisions = raw_stats.collisions;
+        stats->rx_length_errors = 0;
+        stats->rx_over_errors = 0;
+        stats->rx_crc_errors = 0;
+        stats->rx_frame_errors = 0;
+        stats->rx_fifo_errors = 0;
+        stats->rx_missed_errors = 0;
+        stats->tx_aborted_errors = 0;
+        stats->tx_carrier_errors = 0;
+        stats->tx_fifo_errors = 0;
+        stats->tx_heartbeat_errors = 0;
+        stats->tx_window_errors = 0;
     }
+
     return error;
 }
 
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index a573e24..d61a209 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -173,7 +173,8 @@ struct netdev_class {
      * A network device that supports some statistics but not others, it should
      * set the values of the unsupported statistics to all-1-bits
      * (UINT64_MAX). */
-    int (*get_stats)(const struct netdev *netdev, struct netdev_stats *stats);
+    int (*get_stats)(const struct netdev *netdev, struct dpif *dpif,
+                     struct netdev_stats *stats);
 
     /* Stores the features supported by 'netdev' into each of '*current',
      * '*advertised', '*supported', and '*peer'.  Each value is a bitmap of
diff --git a/lib/netdev.c b/lib/netdev.c
index 38610e1..d90a2f1 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -603,13 +603,14 @@ netdev_get_carrier(const struct netdev *netdev, bool *carrier)
 
 /* Retrieves current device stats for 'netdev'. */
 int
-netdev_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
+netdev_get_stats(const struct netdev *netdev, struct dpif *dpif,
+                 struct netdev_stats *stats)
 {
     int error;
 
     COVERAGE_INC(netdev_get_stats);
     error = (netdev->class->get_stats
-             ? netdev->class->get_stats(netdev, stats)
+             ? netdev->class->get_stats(netdev, dpif, stats)
              : EOPNOTSUPP);
     if (error) {
         memset(stats, 0xff, sizeof *stats);
diff --git a/lib/netdev.h b/lib/netdev.h
index 4a29cf3..3d02cd0 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -21,6 +21,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "dpif.h"
+
 /* Generic interface to network devices.
  *
  * Currently, there is a single implementation of this interface that supports
@@ -121,7 +123,8 @@ int netdev_set_flags(struct netdev *, enum netdev_flags, bool permanent);
 int netdev_turn_flags_on(struct netdev *, enum netdev_flags, bool permanent);
 int netdev_turn_flags_off(struct netdev *, enum netdev_flags, bool permanent);
 
-int netdev_get_stats(const struct netdev *, struct netdev_stats *);
+int netdev_get_stats(const struct netdev *, struct dpif *,
+                     struct netdev_stats *);
 int netdev_set_policing(struct netdev *, uint32_t kbits_rate, 
                         uint32_t kbits_burst);
 
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7650068..ee24565 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2366,7 +2366,7 @@ handle_port_stats_request(struct ofproto *p, struct ofconn *ofconn,
         /* Intentionally ignore return value, since errors will set 'stats' to
          * all-1s, which is correct for OpenFlow, and netdev_get_stats() will
          * log errors. */
-        netdev_get_stats(port->netdev, &stats);
+        netdev_get_stats(port->netdev, p->dpif, &stats);
 
         ops = append_stats_reply(sizeof *ops, ofconn, &msg);
         ops->port_no = htons(odp_port_to_ofp_port(port_no));
-- 
1.6.0.4





More information about the dev mailing list