datapath: Drop datapath index and port number from Ethtool output.

I introduced this a long time ago as an efficient way for userspace to find
out whether and where an internal device was attached, but I've always
considered it an ugly kluge.  Now that ODP_VPORT_QUERY can fetch a vport's
info regardless of datapath, it is no longer necessary.  This commit
stops using Ethtool for this purpose and drops the feature.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 74a7dfdb236d77b729b4fe0f64e8287637dc508f..b9e54ab1f9a98ee36db2088d4ef72407f34a270a 100644 (file)
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -93,10 +93,7 @@ static int internal_dev_stop(struct net_device *netdev)
 static void internal_dev_getinfo(struct net_device *netdev,
                                 struct ethtool_drvinfo *info)
 {
-       struct vport *vport = internal_dev_get_vport(netdev);
-
        strcpy(info->driver, "openvswitch");
-       sprintf(info->bus_info, "%d.%d", vport->dp->dp_idx, vport->port_no);
 }
 
 static const struct ethtool_ops internal_dev_ethtool_ops = {

diff --git a/lib/automake.mk b/lib/automake.mk
index fee55b2f352bc2f86348dec73eed54a4cc45d248..ba02d41119b360ee19819c9ca0b2d4ee800b8426 100644 (file)
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -184,6 +184,7 @@ endif
 if HAVE_NETLINK
 lib_libopenvswitch_a_SOURCES += \
        lib/dpif-linux.c \
+       lib/dpif-linux.h \
        lib/netdev-linux.c \
        lib/netdev-vport.c \
        lib/netdev-vport.h \

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 530784ce49a256afe7099fd7d7abf1790dd6f943..f7314a0b2b059b82d89f7c10a7811233b369131d 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -15,7 +15,8 @@
  */
 
 #include <config.h>
-#include "dpif.h"
+
+#include "dpif-linux.h"
 
 #include <assert.h>
 #include <ctype.h>
@@ -69,6 +70,7 @@ struct dpif_linux {
 static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(9999, 5);
 
 static int do_ioctl(const struct dpif *, int cmd, const void *arg);
+static int lookup_internal_device(const char *name, int *dp_idx, int *port_no);
 static int lookup_minor(const char *name, int *minor);
 static int finish_open(struct dpif *, const char *local_ifname);
 static int get_openvswitch_major(void);
@@ -638,60 +640,74 @@ do_ioctl(const struct dpif *dpif_, int cmd, const void *arg)
 }
 
 static int
-lookup_minor(const char *name, int *minorp)
+lookup_internal_device(const char *name, int *dp_idx, int *port_no)
 {
-    struct ethtool_drvinfo drvinfo;
-    int minor, port_no;
-    struct ifreq ifr;
-    int error;
-    int sock;
+    struct odp_port odp_port;
+    static int dp0_fd = -1;
 
-    sock = socket(AF_INET, SOCK_DGRAM, 0);
-    if (sock < 0) {
-        VLOG_WARN("socket(AF_INET) failed: %s", strerror(errno));
-        error = errno;
-        goto error;
-    }
+    if (dp0_fd < 0) {
+        int error;
+        char *fn;
 
-    memset(&ifr, 0, sizeof ifr);
-    strncpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
-    ifr.ifr_data = (caddr_t) &drvinfo;
+        error = make_openvswitch_device(0, &fn);
+        if (error) {
+            return error;
+        }
 
-    memset(&drvinfo, 0, sizeof drvinfo);
-    drvinfo.cmd = ETHTOOL_GDRVINFO;
-    if (ioctl(sock, SIOCETHTOOL, &ifr)) {
-        VLOG_WARN("ioctl(SIOCETHTOOL) failed: %s", strerror(errno));
-        error = errno;
-        goto error_close_sock;
+        dp0_fd = open(fn, O_RDONLY | O_NONBLOCK);
+        if (dp0_fd < 0) {
+            VLOG_WARN_RL(&error_rl, "%s: open failed (%s)",
+                         fn, strerror(errno));
+            free(fn);
+            return errno;
+        }
+        free(fn);
     }
 
-    if (strcmp(drvinfo.driver, "openvswitch")) {
-        VLOG_WARN("%s is not an openvswitch device", name);
-        error = EOPNOTSUPP;
-        goto error_close_sock;
+    memset(&odp_port, 0, sizeof odp_port);
+    strncpy(odp_port.devname, name, sizeof odp_port.devname);
+    if (ioctl(dp0_fd, ODP_VPORT_QUERY, &odp_port)) {
+        if (errno != ENODEV) {
+            VLOG_WARN_RL(&error_rl, "%s: vport query failed (%s)",
+                         name, strerror(errno));
+        }
+        return errno;
+    } else if (!strcmp(odp_port.type, "internal")) {
+        *dp_idx = odp_port.dp_idx;
+        *port_no = odp_port.port;
+        return 0;
+    } else {
+        return EINVAL;
     }
+}
 
-    if (sscanf(drvinfo.bus_info, "%d.%d", &minor, &port_no) != 2) {
-        VLOG_WARN("%s ethtool bus_info has unexpected format", name);
-        error = EPROTOTYPE;
-        goto error_close_sock;
+static int
+lookup_minor(const char *name, int *minorp)
+{
+    int minor, port_no;
+    int error;
+
+    error = lookup_internal_device(name, &minor, &port_no);
+    if (error) {
+        return error;
     } else if (port_no != ODPP_LOCAL) {
         /* This is an Open vSwitch device but not the local port.  We
          * intentionally support only using the name of the local port as the
          * name of a datapath; otherwise, it would be too difficult to
          * enumerate all the names of a datapath. */
-        error = EOPNOTSUPP;
-        goto error_close_sock;
+        return EOPNOTSUPP;
+    } else {
+        *minorp = minor;
+        return 0;
     }
+}
 
-    *minorp = minor;
-    close(sock);
-    return 0;
+bool
+dpif_linux_is_internal_device(const char *name)
+{
+    int minor, port_no;
 
-error_close_sock:
-    close(sock);
-error:
-    return error;
+    return !lookup_internal_device(name, &minor, &port_no);
 }
 
 static int

diff --git a/lib/dpif-linux.h b/lib/dpif-linux.h
new file mode 100644 (file)
index 0000000..1dff4b7
--- /dev/null
+++ b/lib/dpif-linux.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2010 Nicira Networks.
+ *
+ * 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.
+ */
+
+#ifndef DPIF_LINUX_H
+#define DPIF_LINUX_H 1
+
+#include <stdbool.h>
+
+bool dpif_linux_is_internal_device(const char *name);
+
+#endif /* dpif-linux.h */

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index e9beebf1a408f1b11d1d75d83b2274371b2cbfb3..3a17a8f5d622274f57ca47331a4a78bdbb07f793 100644 (file)
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -47,6 +47,7 @@
 #include <unistd.h>
 
 #include "coverage.h"
+#include "dpif-linux.h"
 #include "dynamic-string.h"
 #include "fatal-signal.h"
 #include "hash.h"
@@ -1105,22 +1106,8 @@ netdev_linux_update_is_pseudo(struct netdev_dev_linux *netdev_dev)
         const char *type = netdev_dev_get_type(&netdev_dev->netdev_dev);
 
         netdev_dev->is_tap = !strcmp(type, "tap");
-        netdev_dev->is_internal = false;
-        if (!netdev_dev->is_tap) {
-            struct ethtool_drvinfo drvinfo;
-            int error;
-
-            memset(&drvinfo, 0, sizeof drvinfo);
-            error = netdev_linux_do_ethtool(name,
-                                            (struct ethtool_cmd *)&drvinfo,
-                                            ETHTOOL_GDRVINFO,
-                                            "ETHTOOL_GDRVINFO");
-
-            if (!error && !strcmp(drvinfo.driver, "openvswitch")) {
-                netdev_dev->is_internal = true;
-            }
-        }
-
+        netdev_dev->is_internal = (!netdev_dev->is_tap
+                                   && dpif_linux_is_internal_device(name));
         netdev_dev->cache_valid |= VALID_IS_PSEUDO;
     }
 }