netdev: Don't cache network device features.

The new implementation of secchan wants to get updates of network device
features by keeping a network device open for each port and checking its
features when notified of a port status change.  This wouldn't work, since
the features were cached once at startup.  This commit makes the netdev
code check the actual devices features on each call.

Also, generalizes do_ethtool() to be useful for other kinds of ethtool
operations.

diff --git a/lib/netdev.c b/lib/netdev.c
index 4a43ffdaea3c027c6456737fd3790980670b2c1d..3921c4a06d75ac78ef76d01f3dd6a07dceddb57f 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -77,6 +77,15 @@
 #define IFF_LOWER_UP 0x10000
 #endif
 
+/* These were introduced in Linux 2.6.14, so they might be missing if we have
+ * old headers. */
+#ifndef ADVERTISED_Pause
+#define ADVERTISED_Pause                (1 << 13)
+#endif
+#ifndef ADVERTISED_Asym_Pause
+#define ADVERTISED_Asym_Pause           (1 << 14)
+#endif
+
 #define THIS_MODULE VLM_netdev
 #include "vlog.h"
 
@@ -99,13 +108,6 @@ struct netdev {
     int txqlen;
     int hwaddr_family;
 
-    /* Bitmaps of OFPPF_* that describe features.  All bits disabled if
-     * unsupported or unavailable. */
-    uint32_t curr;              /* Current features. */
-    uint32_t advertised;        /* Features being advertised by the port. */
-    uint32_t supported;         /* Features supported by the port. */
-    uint32_t peer;              /* Features advertised by the peer. */
-
     int save_flags;             /* Initial device flags. */
     int changed_flags;          /* Flags that we changed. */
 };
@@ -182,130 +184,145 @@ get_ipv6_address(const char *name, struct in6_addr *in6)
     fclose(file);
 }
 
-static void
-do_ethtool(struct netdev *netdev) 
+static int
+do_ethtool(struct netdev *netdev, struct ethtool_cmd *ecmd,
+           int cmd, const char *cmd_name)
 {
     struct ifreq ifr;
-    struct ethtool_cmd ecmd;
-
-    netdev->curr = 0;
-    netdev->supported = 0;
-    netdev->advertised = 0;
-    netdev->peer = 0;
 
     memset(&ifr, 0, sizeof ifr);
     strncpy(ifr.ifr_name, netdev->name, sizeof ifr.ifr_name);
-    ifr.ifr_data = (caddr_t) &ecmd;
+    ifr.ifr_data = (caddr_t) ecmd;
 
-    memset(&ecmd, 0, sizeof ecmd);
-    ecmd.cmd = ETHTOOL_GSET;
+    ecmd->cmd = cmd;
     if (ioctl(netdev->netdev_fd, SIOCETHTOOL, &ifr) == 0) {
-        if (ecmd.supported & SUPPORTED_10baseT_Half) {
-            netdev->supported |= OFPPF_10MB_HD;
-        }
-        if (ecmd.supported & SUPPORTED_10baseT_Full) {
-            netdev->supported |= OFPPF_10MB_FD;
-        }
-        if (ecmd.supported & SUPPORTED_100baseT_Half)  {
-            netdev->supported |= OFPPF_100MB_HD;
-        }
-        if (ecmd.supported & SUPPORTED_100baseT_Full) {
-            netdev->supported |= OFPPF_100MB_FD;
-        }
-        if (ecmd.supported & SUPPORTED_1000baseT_Half) {
-            netdev->supported |= OFPPF_1GB_HD;
-        }
-        if (ecmd.supported & SUPPORTED_1000baseT_Full) {
-            netdev->supported |= OFPPF_1GB_FD;
-        }
-        if (ecmd.supported & SUPPORTED_10000baseT_Full) {
-            netdev->supported |= OFPPF_10GB_FD;
-        }
-        if (ecmd.supported & SUPPORTED_TP) {
-            netdev->supported |= OFPPF_COPPER;
-        }
-        if (ecmd.supported & SUPPORTED_FIBRE) {
-            netdev->supported |= OFPPF_FIBER;
-        }
-        if (ecmd.supported & SUPPORTED_Autoneg) {
-            netdev->supported |= OFPPF_AUTONEG;
-        }
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,14)
-        if (ecmd.supported & SUPPORTED_Pause) {
-            netdev->supported |= OFPPF_PAUSE;
-        }
-        if (ecmd.supported & SUPPORTED_Asym_Pause) {
-            netdev->supported |= OFPPF_PAUSE_ASYM;
-        }
-#endif /* kernel >= 2.6.14 */
+        return 0;
+    } else {
+        VLOG_WARN_RL(&rl, "ethtool command %s on network device %s failed: %s",
+                     cmd_name, netdev->name, strerror(errno));
+        return errno;
+    }
+}
 
-        /* Set the advertised features */
-        if (ecmd.advertising & ADVERTISED_10baseT_Half) {
-            netdev->advertised |= OFPPF_10MB_HD;
-        }
-        if (ecmd.advertising & ADVERTISED_10baseT_Full) {
-            netdev->advertised |= OFPPF_10MB_FD;
-        }
-        if (ecmd.advertising & ADVERTISED_100baseT_Half) {
-            netdev->advertised |= OFPPF_100MB_HD;
-        }
-        if (ecmd.advertising & ADVERTISED_100baseT_Full) {
-            netdev->advertised |= OFPPF_100MB_FD;
-        }
-        if (ecmd.advertising & ADVERTISED_1000baseT_Half) {
-            netdev->advertised |= OFPPF_1GB_HD;
-        }
-        if (ecmd.advertising & ADVERTISED_1000baseT_Full) {
-            netdev->advertised |= OFPPF_1GB_FD;
-        }
-        if (ecmd.advertising & ADVERTISED_10000baseT_Full) {
-            netdev->advertised |= OFPPF_10GB_FD;
-        }
-        if (ecmd.advertising & ADVERTISED_TP) {
-            netdev->advertised |= OFPPF_COPPER;
-        }
-        if (ecmd.advertising & ADVERTISED_FIBRE) {
-            netdev->advertised |= OFPPF_FIBER;
-        }
-        if (ecmd.advertising & ADVERTISED_Autoneg) {
-            netdev->advertised |= OFPPF_AUTONEG;
-        }
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,14)
-        if (ecmd.advertising & ADVERTISED_Pause) {
-            netdev->advertised |= OFPPF_PAUSE;
-        }
-        if (ecmd.advertising & ADVERTISED_Asym_Pause) {
-            netdev->advertised |= OFPPF_PAUSE_ASYM;
-        }
-#endif /* kernel >= 2.6.14 */
+static int
+do_get_features(struct netdev *netdev,
+                uint32_t *current, uint32_t *advertised,
+                uint32_t *supported, uint32_t *peer)
+{
+    struct ethtool_cmd ecmd;
+    int error;
 
-        /* Set the current features */
-        if (ecmd.speed == SPEED_10) {
-            netdev->curr = (ecmd.duplex) ? OFPPF_10MB_FD : OFPPF_10MB_HD;
-        }
-        else if (ecmd.speed == SPEED_100) {
-            netdev->curr = (ecmd.duplex) ? OFPPF_100MB_FD : OFPPF_100MB_HD;
-        }
-        else if (ecmd.speed == SPEED_1000) {
-            netdev->curr = (ecmd.duplex) ? OFPPF_1GB_FD : OFPPF_1GB_HD;
-        }
-        else if (ecmd.speed == SPEED_10000) {
-            netdev->curr = OFPPF_10GB_FD;
-        }
+    *current = 0;
+    *supported = 0;
+    *advertised = 0;
+    *peer = 0;
 
-        if (ecmd.port == PORT_TP) {
-            netdev->curr |= OFPPF_COPPER;
-        }
-        else if (ecmd.port == PORT_FIBRE) {
-            netdev->curr |= OFPPF_FIBER;
-        }
+    memset(&ecmd, 0, sizeof ecmd);
+    error = do_ethtool(netdev, &ecmd, ETHTOOL_GSET, "ETHTOOL_GSET");
+    if (error) {
+        return error;
+    }
 
-        if (ecmd.autoneg) {
-            netdev->curr |= OFPPF_AUTONEG;
-        }
-    } else {
-        VLOG_DBG("ioctl(SIOCETHTOOL) failed: %s", strerror(errno));
+    if (ecmd.supported & SUPPORTED_10baseT_Half) {
+        *supported |= OFPPF_10MB_HD;
+    }
+    if (ecmd.supported & SUPPORTED_10baseT_Full) {
+        *supported |= OFPPF_10MB_FD;
+    }
+    if (ecmd.supported & SUPPORTED_100baseT_Half)  {
+        *supported |= OFPPF_100MB_HD;
+    }
+    if (ecmd.supported & SUPPORTED_100baseT_Full) {
+        *supported |= OFPPF_100MB_FD;
+    }
+    if (ecmd.supported & SUPPORTED_1000baseT_Half) {
+        *supported |= OFPPF_1GB_HD;
+    }
+    if (ecmd.supported & SUPPORTED_1000baseT_Full) {
+        *supported |= OFPPF_1GB_FD;
+    }
+    if (ecmd.supported & SUPPORTED_10000baseT_Full) {
+        *supported |= OFPPF_10GB_FD;
+    }
+    if (ecmd.supported & SUPPORTED_TP) {
+        *supported |= OFPPF_COPPER;
+    }
+    if (ecmd.supported & SUPPORTED_FIBRE) {
+        *supported |= OFPPF_FIBER;
+    }
+    if (ecmd.supported & SUPPORTED_Autoneg) {
+        *supported |= OFPPF_AUTONEG;
+    }
+    if (ecmd.supported & SUPPORTED_Pause) {
+        *supported |= OFPPF_PAUSE;
+    }
+    if (ecmd.supported & SUPPORTED_Asym_Pause) {
+        *supported |= OFPPF_PAUSE_ASYM;
+    }
+
+    /* Set the advertised features */
+    if (ecmd.advertising & ADVERTISED_10baseT_Half) {
+        *advertised |= OFPPF_10MB_HD;
+    }
+    if (ecmd.advertising & ADVERTISED_10baseT_Full) {
+        *advertised |= OFPPF_10MB_FD;
+    }
+    if (ecmd.advertising & ADVERTISED_100baseT_Half) {
+        *advertised |= OFPPF_100MB_HD;
+    }
+    if (ecmd.advertising & ADVERTISED_100baseT_Full) {
+        *advertised |= OFPPF_100MB_FD;
     }
+    if (ecmd.advertising & ADVERTISED_1000baseT_Half) {
+        *advertised |= OFPPF_1GB_HD;
+    }
+    if (ecmd.advertising & ADVERTISED_1000baseT_Full) {
+        *advertised |= OFPPF_1GB_FD;
+    }
+    if (ecmd.advertising & ADVERTISED_10000baseT_Full) {
+        *advertised |= OFPPF_10GB_FD;
+    }
+    if (ecmd.advertising & ADVERTISED_TP) {
+        *advertised |= OFPPF_COPPER;
+    }
+    if (ecmd.advertising & ADVERTISED_FIBRE) {
+        *advertised |= OFPPF_FIBER;
+    }
+    if (ecmd.advertising & ADVERTISED_Autoneg) {
+        *advertised |= OFPPF_AUTONEG;
+    }
+    if (ecmd.advertising & ADVERTISED_Pause) {
+        *advertised |= OFPPF_PAUSE;
+    }
+    if (ecmd.advertising & ADVERTISED_Asym_Pause) {
+        *advertised |= OFPPF_PAUSE_ASYM;
+    }
+
+    /* Set the current features */
+    if (ecmd.speed == SPEED_10) {
+        *current = (ecmd.duplex) ? OFPPF_10MB_FD : OFPPF_10MB_HD;
+    }
+    else if (ecmd.speed == SPEED_100) {
+        *current = (ecmd.duplex) ? OFPPF_100MB_FD : OFPPF_100MB_HD;
+    }
+    else if (ecmd.speed == SPEED_1000) {
+        *current = (ecmd.duplex) ? OFPPF_1GB_FD : OFPPF_1GB_HD;
+    }
+    else if (ecmd.speed == SPEED_10000) {
+        *current = OFPPF_10GB_FD;
+    }
+
+    if (ecmd.port == PORT_TP) {
+        *current |= OFPPF_COPPER;
+    }
+    else if (ecmd.port == PORT_FIBRE) {
+        *current |= OFPPF_FIBER;
+    }
+
+    if (ecmd.autoneg) {
+        *current |= OFPPF_AUTONEG;
+    }
+    return 0;
 }
 
 /* Opens the network device named 'name' (e.g. "eth0") and returns zero if
@@ -470,9 +487,6 @@ do_open_netdev(const char *name, int ethertype, int tap_fd,
     netdev->mtu = mtu;
     netdev->in6 = in6;
 
-    /* Get speed, features. */
-    do_ethtool(netdev);
-
     /* Save flags to restore at close or exit. */
     error = get_flags(netdev->name, &netdev->save_flags);
     if (error) {
@@ -698,25 +712,22 @@ netdev_get_mtu(const struct netdev *netdev)
     return netdev->mtu;
 }
 
-/* Returns the features supported by 'netdev' of type 'type', as a bitmap 
- * of bits from enum ofp_phy_features, in host byte order. */
-uint32_t
-netdev_get_features(struct netdev *netdev, int type) 
+/* 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
+ * successful, otherwise a positive errno value.  On failure, all of the
+ * passed-in values are set to 0. */
+int
+netdev_get_features(struct netdev *netdev,
+                    uint32_t *current, uint32_t *advertised,
+                    uint32_t *supported, uint32_t *peer)
 {
-    do_ethtool(netdev);
-    switch (type) {
-    case NETDEV_FEAT_CURRENT:
-        return netdev->curr;
-    case NETDEV_FEAT_ADVERTISED:
-        return netdev->advertised;
-    case NETDEV_FEAT_SUPPORTED:
-        return netdev->supported;
-    case NETDEV_FEAT_PEER:
-        return netdev->peer;
-    default:
-        VLOG_WARN("Unknown feature type: %d\n", type);
-        return 0;
-    }
+    uint32_t dummy[4];
+    return do_get_features(netdev,
+                           current ? current : &dummy[0],
+                           advertised ? advertised : &dummy[1],
+                           supported ? supported : &dummy[2],
+                           peer ? peer : &dummy[3]);
 }
 
 /* If 'netdev' has an assigned IPv4 address, sets '*in4' to that address (if

diff --git a/lib/netdev.h b/lib/netdev.h
index 7dff6eff58e0163f0c31e1b87dbdc22bf59884a9..d60fe3560c7dc21b82153cb4af6abd8f84ee126c 100644 (file)
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -49,13 +49,6 @@ struct in_addr;
 struct in6_addr;
 struct svec;
 
-enum netdev_feature_type {
-    NETDEV_FEAT_CURRENT,
-    NETDEV_FEAT_ADVERTISED,
-    NETDEV_FEAT_SUPPORTED,
-    NETDEV_FEAT_PEER
-};
-
 enum netdev_flags {
     NETDEV_UP = 0x0001,         /* Device enabled? */
     NETDEV_PROMISC = 0x0002,    /* Promiscuous mode? */
@@ -111,7 +104,9 @@ int netdev_set_etheraddr(struct netdev *, const uint8_t mac[6]);
 const uint8_t *netdev_get_etheraddr(const struct netdev *);
 const char *netdev_get_name(const struct netdev *);
 int netdev_get_mtu(const struct netdev *);
-uint32_t netdev_get_features(struct netdev *, int);
+int netdev_get_features(struct netdev *,
+                        uint32_t *current, uint32_t *advertised,
+                        uint32_t *supported, uint32_t *peer);
 bool netdev_get_in4(const struct netdev *, struct in_addr *);
 int netdev_set_in4(struct netdev *, struct in_addr addr, struct in_addr mask);
 int netdev_add_router(struct in_addr router);

diff --git a/udatapath/datapath.c b/udatapath/datapath.c
index 838f7b552f8deb4a81169bf4575410b446d0152e..d46fd128e46bd9baf27d598845f16e13bfe290c6 100644 (file)
--- a/udatapath/datapath.c
+++ b/udatapath/datapath.c
@@ -657,6 +657,8 @@ dp_output_control(struct datapath *dp, struct ofpbuf *buffer, int in_port,
 static void
 fill_port_desc(struct sw_port *p, struct ofp_phy_port *desc)
 {
+    uint32_t curr, advertised, supported, peer;
+
     desc->port_no = htons(p->port_no);
     strncpy((char *) desc->name, netdev_get_name(p->netdev),
             sizeof desc->name);
@@ -664,12 +666,11 @@ fill_port_desc(struct sw_port *p, struct ofp_phy_port *desc)
     memcpy(desc->hw_addr, netdev_get_etheraddr(p->netdev), ETH_ADDR_LEN);
     desc->config = htonl(p->config);
     desc->state = htonl(p->state);
-    desc->curr = htonl(netdev_get_features(p->netdev, NETDEV_FEAT_CURRENT));
-    desc->supported = htonl(netdev_get_features(p->netdev, 
-                NETDEV_FEAT_SUPPORTED));
-    desc->advertised = htonl(netdev_get_features(p->netdev, 
-                NETDEV_FEAT_ADVERTISED));
-    desc->peer = htonl(netdev_get_features(p->netdev, NETDEV_FEAT_PEER));
+    netdev_get_features(p->netdev, &curr, &advertised, &supported, &peer);
+    desc->curr = htonl(curr);
+    desc->supported = htonl(supported);
+    desc->advertised = htonl(advertised);
+    desc->peer = htonl(peer);
 }
 
 static void