netdev-vport: Use vport set_stats instead of internal dev.

In certain cases we require the ability to provide stats that are
added to the values collected by the kernel (currently only used
by bond fake devices).  Internal devices previously implemented
this directly but now that their stats are now handled by the vport
layer the functionality has been moved there.  This removes the
userspace code to set the stats and replaces it with a mechanism
to access the equivalent functionality in the vport layer.

diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index 3cc83d87988b0f19f7c13badc8ef18893dcc41cb..92e071884181568a8c4adbeb6ab84405b91e5368 100644 (file)
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -1,6 +1,5 @@
 noinst_HEADERS += \
        include/openvswitch/gre.h \
        include/openvswitch/brcompat-netlink.h \
-       include/openvswitch/internal_dev.h \
        include/openvswitch/datapath-protocol.h
 

diff --git a/include/openvswitch/internal_dev.h b/include/openvswitch/internal_dev.h
deleted file mode 100644 (file)
index 26c7359..0000000
--- a/include/openvswitch/internal_dev.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * Copyright (c) 2010 Nicira Networks.
- *
- * This file is offered under your choice of two licenses: Apache 2.0 or GNU
- * GPL 2.0 or later.  The permission statements for each of these licenses is
- * given below.  You may license your modifications to this file under either
- * of these licenses or both.  If you wish to license your modifications under
- * only one of these licenses, delete the permission text for the other
- * license.
- *
- * ----------------------------------------------------------------------
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ----------------------------------------------------------------------
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- * ----------------------------------------------------------------------
- */
-
-/* Ioctl for Open vSwitch "internal ports" to support XAPI, which does not
- * support summing statistics from bond slaves, but still needs to get bond
- * statistics.
- *
- * This is a nasty wart that needs removing. */
-
-#ifndef OPENVSWITCH_INTERNAL_DEV_H
-#define OPENVSWITCH_INTERNAL_DEV_H 1
-
-#ifdef __KERNEL__
-#include <linux/types.h>
-#else
-#include <sys/types.h>
-#endif
-
-#include <linux/sockios.h>
-
-struct internal_dev_stats {
-       __u64 rx_packets;
-       __u64 rx_bytes;
-       __u64 tx_packets;
-       __u64 tx_bytes;
-};
-
-#define INTERNAL_DEV_SET_STATS        SIOCDEVPRIVATE
-
-#endif /* openvswitch/internal_dev.h */

diff --git a/lib/netdev-gre.c b/lib/netdev-gre.c
index 55150d8e075a04409565e42e176aabff5aa2ac54..0aa587a25b02eebbb1fdaefd0ad61bde49c4cb5e 100644 (file)
--- a/lib/netdev-gre.c
+++ b/lib/netdev-gre.c
@@ -251,7 +251,7 @@ const struct netdev_class netdev_gre_class = {
     NULL,                       /* get_ifindex */
     netdev_vport_get_carrier,
     netdev_vport_get_stats,
-    NULL,                       /* set_stats */
+    netdev_vport_set_stats,
 
     NULL,                       /* get_features */
     NULL,                       /* set_advertisements */

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 31d01780d31c6a91916fe74b0648c58ea69eb484..c0bb2474c01999f17e5ebedaa8fb30c8d7496f67 100644 (file)
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -52,7 +52,6 @@
 #include "netlink.h"
 #include "ofpbuf.h"
 #include "openflow/openflow.h"
-#include "openvswitch/internal_dev.h"
 #include "openvswitch/gre.h"
 #include "packets.h"
 #include "poll-loop.h"
@@ -863,41 +862,6 @@ netdev_linux_get_stats(const struct netdev *netdev_,
     return error;
 }
 
-static int
-netdev_linux_set_stats(struct netdev *netdev,
-                       const struct netdev_stats *stats)
-{
-    struct netdev_dev_linux *netdev_dev =
-        netdev_dev_linux_cast(netdev_get_dev(netdev));
-    struct internal_dev_stats dp_dev_stats;
-    struct ifreq ifr;
-
-    /* We must reject this call if 'netdev' is not an Open vSwitch internal
-     * port, because the ioctl that we are about to execute is in the "device
-     * private ioctls" range, which means that executing it on a device that
-     * is not the type we expect could do any random thing.
-     *
-     * (Amusingly, these ioctl numbers are commented "THESE IOCTLS ARE
-     * _DEPRECATED_ AND WILL DISAPPEAR IN 2.5.X" in linux/sockios.h.  I guess
-     * DaveM is a little behind on that.) */
-    netdev_linux_update_is_pseudo(netdev_dev);
-    if (!netdev_dev->is_internal) {
-        return EOPNOTSUPP;
-    }
-
-    /* This actually only sets the *offset* that the dp_dev applies, but in our
-     * usage for fake bond devices the dp_dev never has any traffic of it own
-     * so it has the same effect. */
-    dp_dev_stats.rx_packets = stats->rx_packets;
-    dp_dev_stats.rx_bytes = stats->rx_bytes;
-    dp_dev_stats.tx_packets = stats->tx_packets;
-    dp_dev_stats.tx_bytes = stats->tx_bytes;
-    ifr.ifr_data = (void *) &dp_dev_stats;
-    return netdev_linux_do_ioctl(netdev_get_name(netdev), &ifr,
-                                 INTERNAL_DEV_SET_STATS,
-                                 "INTERNAL_DEV_SET_STATS");
-}
-
 /* 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
@@ -1633,7 +1597,7 @@ const struct netdev_class netdev_linux_class = {
     netdev_linux_get_ifindex,
     netdev_linux_get_carrier,
     netdev_linux_get_stats,
-    netdev_linux_set_stats,
+    netdev_vport_set_stats,
 
     netdev_linux_get_features,
     netdev_linux_set_advertisements,

diff --git a/lib/netdev-patch.c b/lib/netdev-patch.c
index 22353a196940f6769c38531863c4a686c47478dd..17c509f1d8d47e11d594d2d524c14e209ffca739 100644 (file)
--- a/lib/netdev-patch.c
+++ b/lib/netdev-patch.c
@@ -202,7 +202,7 @@ const struct netdev_class netdev_patch_class = {
     NULL,                       /* get_ifindex */
     netdev_vport_get_carrier,
     netdev_vport_get_stats,
-    NULL,                       /* set_stats */
+    netdev_vport_set_stats,
 
     NULL,                       /* get_features */
     NULL,                       /* set_advertisements */

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index a81262a6cae5caa92d028b45ce73709bf964298f..58858f90fe23bc2f03e05d6e5c6c35e50f043181 100644 (file)
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -158,6 +158,40 @@ netdev_vport_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
     return 0;
 }
 
+int
+netdev_vport_set_stats(struct netdev *netdev, const struct netdev_stats *stats)
+{
+    struct odp_vport_stats_req ovsr;
+    int err;
+
+    ovs_strlcpy(ovsr.devname, netdev_get_name(netdev), sizeof ovsr.devname);
+
+    ovsr.stats.rx_packets = stats->rx_packets;
+    ovsr.stats.tx_packets = stats->tx_packets;
+    ovsr.stats.rx_bytes = stats->rx_bytes;
+    ovsr.stats.tx_bytes = stats->tx_bytes;
+    ovsr.stats.rx_errors = stats->rx_errors;
+    ovsr.stats.tx_errors = stats->tx_errors;
+    ovsr.stats.rx_dropped = stats->rx_dropped;
+    ovsr.stats.tx_dropped = stats->tx_dropped;
+    ovsr.stats.collisions = stats->collisions;
+    ovsr.stats.rx_over_err = stats->rx_over_errors;
+    ovsr.stats.rx_crc_err = stats->rx_crc_errors;
+    ovsr.stats.rx_frame_err = stats->rx_frame_errors;
+
+    err = netdev_vport_do_ioctl(ODP_VPORT_STATS_SET, &ovsr);
+
+    /* If the vport layer doesn't know about the device, that doesn't mean it
+     * doesn't exist (after all were able to open it when netdev_open() was
+     * called), it just means that it isn't attached and we'll be getting
+     * stats a different way. */
+    if (err == ENODEV) {
+        err = EOPNOTSUPP;
+    }
+
+    return err;
+}
+
 int
 netdev_vport_update_flags(struct netdev *netdev OVS_UNUSED,
                         enum netdev_flags off, enum netdev_flags on OVS_UNUSED,

diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index cb467f077f98ea463de0b7ac7a83cc7b08a52438..b4016606f79a4379ab923d782d0cc44bd843ede1 100644 (file)
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -29,6 +29,7 @@ int netdev_vport_get_etheraddr(const struct netdev *,
 int netdev_vport_get_mtu(const struct netdev *, int *mtup);
 int netdev_vport_get_carrier(const struct netdev *, bool *carrier);
 int netdev_vport_get_stats(const struct netdev *, struct netdev_stats *);
+int netdev_vport_set_stats(struct netdev *, const struct netdev_stats *);
 int netdev_vport_update_flags(struct netdev *, enum netdev_flags off,
                               enum netdev_flags on,
                               enum netdev_flags *old_flagsp);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index b43a9bf6614924ccb83eeb0c94da41860df0acc6..feb1c5d83d70c65d210f4d760964de943dec8c9a 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1927,10 +1927,19 @@ bond_update_fake_iface_stats(struct port *port)
         struct netdev_stats slave_stats;
 
         if (!netdev_get_stats(port->ifaces[i]->netdev, &slave_stats)) {
-            bond_stats.rx_packets += slave_stats.rx_packets;
-            bond_stats.rx_bytes += slave_stats.rx_bytes;
-            bond_stats.tx_packets += slave_stats.tx_packets;
-            bond_stats.tx_bytes += slave_stats.tx_bytes;
+            /* XXX: We swap the stats here because they are swapped back when
+             * reported by the internal device.  The reason for this is
+             * internal devices normally represent packets going into the system
+             * but when used as fake bond device they represent packets leaving
+             * the system.  We really should do this in the internal device
+             * itself because changing it here reverses the counts from the
+             * perspective of the switch.  However, the internal device doesn't
+             * know what type of device it represents so we have to do it here
+             * for now. */
+            bond_stats.tx_packets += slave_stats.rx_packets;
+            bond_stats.tx_bytes += slave_stats.rx_bytes;
+            bond_stats.rx_packets += slave_stats.tx_packets;
+            bond_stats.rx_bytes += slave_stats.tx_bytes;
         }
     }