From 9fe3b9a2eeace9689def807e78e35c79e4b1814b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 29 Dec 2010 14:20:16 -0800 Subject: [PATCH] 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 Acked-by: Jesse Gross --- 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 74a7dfdb2..b9e54ab1f 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 fee55b2f3..ba02d4111 100644 --- 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 530784ce4..f7314a0b2 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -15,7 +15,8 @@ */ #include -#include "dpif.h" + +#include "dpif-linux.h" #include #include @@ -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 index 000000000..1dff4b76c --- /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 + +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 e9beebf1a..3a17a8f5d 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -47,6 +47,7 @@ #include #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; } } -- 2.43.0