[ovs-dev] [netlink v3 10/16] datapath: Drop datapath index and port number from Ethtool output.

Ben Pfaff blp at nicira.com
Thu Dec 30 00:56:31 UTC 2010


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 at nicira.com>
---
 datapath/vport-internal_dev.c |    3 -
 lib/automake.mk               |    1 +
 lib/dpif-linux.c              |   94 ++++++++++++++++++++++++-----------------
 lib/dpif-linux.h              |   24 ++++++++++
 lib/netdev-linux.c            |   19 +-------
 5 files changed, 83 insertions(+), 58 deletions(-)
 create mode 100644 lib/dpif-linux.h

diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 74a7dfd..b9e54ab 100644
--- 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 f7d1499..4b5b757 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -182,6 +182,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 5cfe336..0101ea1 100644
--- 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>
@@ -68,6 +69,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);
@@ -635,60 +637,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
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 1efbfd8..d2071cf 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -46,6 +46,7 @@
 #include <unistd.h>
 
 #include "coverage.h"
+#include "dpif-linux.h"
 #include "dynamic-string.h"
 #include "fatal-signal.h"
 #include "hash.h"
@@ -1043,22 +1044,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;
     }
 }
-- 
1.7.1





More information about the dev mailing list