[ovs-dev] [PATCH v2 3/6] netdev-linux: Cache error code from ether-addr ioctl.

Pravin B Shelar pshelar at nicira.com
Thu Mar 8 15:26:30 UTC 2012


Fixed according to comments from Ben.
v1-v2:
     - Changed check for VALID_ETHERADDR
     - Using notifier msg to update device ether addr.
     - Removed hwaddr_family param from set_etheraddr().

--8<--------------------------cut here-------------------------->8--

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
 lib/netdev-linux.c   |   86 ++++++++++++++++++++++++++++---------------------
 lib/rtnetlink-link.c |   14 ++++++--
 lib/rtnetlink-link.h |    3 ++
 3 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 3d05b3f..b338466 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -377,6 +377,7 @@ struct netdev_dev_linux {
     int vport_stats_error;      /* Cached error code from vport_get_stats().
                                    0 or an errno value. */
     int netdev_mtu_error;       /* Cached error code from SIOCGIFMTU or SIOCSIFMTU. */
+    int ether_addr_error;       /* Cached error code from set/get etheraddr. */
 
     struct ethtool_drvinfo drvinfo;  /* Cached from ETHTOOL_GDRVINFO. */
     struct tc *tc;
@@ -416,9 +417,8 @@ static int get_ifindex(const struct netdev *, int *ifindexp);
 static int do_set_addr(struct netdev *netdev,
                        int ioctl_nr, const char *ioctl_name,
                        struct in_addr addr);
-static int get_etheraddr(const char *netdev_name, uint8_t ea[ETH_ADDR_LEN]);
-static int set_etheraddr(const char *netdev_name, int hwaddr_family,
-                         const uint8_t[ETH_ADDR_LEN]);
+static void get_etheraddr(struct netdev_dev_linux *);
+static void set_etheraddr(struct netdev_dev_linux *, const uint8_t[ETH_ADDR_LEN]);
 static int get_stats_via_netlink(int ifindex, struct netdev_stats *stats);
 static int get_stats_via_proc(const char *netdev_name, struct netdev_stats *stats);
 static int af_packet_sock(void);
@@ -542,6 +542,10 @@ netdev_dev_linux_update(struct netdev_dev_linux *dev,
             dev->netdev_mtu_error = 0;
             dev->cache_valid |= VALID_MTU;
         }
+        if (!eth_addr_is_zero(change->addr)) {
+            memcpy(dev->etheraddr, change->addr, ETH_ADDR_LEN);
+            dev->cache_valid |= VALID_ETHERADDR;
+        }
     }
 }
 
@@ -1027,19 +1031,19 @@ netdev_linux_set_etheraddr(struct netdev *netdev_,
 {
     struct netdev_dev_linux *netdev_dev =
                                 netdev_dev_linux_cast(netdev_get_dev(netdev_));
-    int error;
 
-    if (!(netdev_dev->cache_valid & VALID_ETHERADDR)
-        || !eth_addr_equals(netdev_dev->etheraddr, mac)) {
-        error = set_etheraddr(netdev_get_name(netdev_), ARPHRD_ETHER, mac);
-        if (!error) {
-            netdev_dev->cache_valid |= VALID_ETHERADDR;
-            memcpy(netdev_dev->etheraddr, mac, ETH_ADDR_LEN);
+    if (netdev_dev->cache_valid & VALID_ETHERADDR) {
+        if (netdev_dev->ether_addr_error) {
+            return netdev_dev->ether_addr_error;
+        }
+        if (eth_addr_equals(netdev_dev->etheraddr, mac)) {
+            return 0;
         }
-    } else {
-        error = 0;
     }
-    return error;
+
+    set_etheraddr(netdev_dev, mac);
+
+    return netdev_dev->ether_addr_error;
 }
 
 /* Returns a pointer to 'netdev''s MAC address.  The caller must not modify or
@@ -1050,16 +1054,16 @@ netdev_linux_get_etheraddr(const struct netdev *netdev_,
 {
     struct netdev_dev_linux *netdev_dev =
                                 netdev_dev_linux_cast(netdev_get_dev(netdev_));
+
     if (!(netdev_dev->cache_valid & VALID_ETHERADDR)) {
-        int error = get_etheraddr(netdev_get_name(netdev_),
-                                  netdev_dev->etheraddr);
-        if (error) {
-            return error;
-        }
-        netdev_dev->cache_valid |= VALID_ETHERADDR;
+        get_etheraddr(netdev_dev);
     }
-    memcpy(mac, netdev_dev->etheraddr, ETH_ADDR_LEN);
-    return 0;
+
+    if (!netdev_dev->ether_addr_error) {
+        memcpy(mac, netdev_dev->etheraddr, ETH_ADDR_LEN);
+    }
+
+    return netdev_dev->ether_addr_error;
 }
 
 /* Returns the maximum size of transmitted (and received) packets on 'netdev',
@@ -4368,14 +4372,14 @@ get_ifindex(const struct netdev *netdev_, int *ifindexp)
     return 0;
 }
 
-static int
-get_etheraddr(const char *netdev_name, uint8_t ea[ETH_ADDR_LEN])
+static void
+get_etheraddr(struct netdev_dev_linux *netdev_dev)
 {
     struct ifreq ifr;
     int hwaddr_family;
 
     memset(&ifr, 0, sizeof ifr);
-    ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
+    ovs_strzcpy(ifr.ifr_name, netdev_dev->netdev_dev.name, sizeof ifr.ifr_name);
     COVERAGE_INC(netdev_get_hwaddr);
     if (ioctl(af_inet_sock, SIOCGIFHWADDR, &ifr) < 0) {
         /* ENODEV probably means that a vif disappeared asynchronously and
@@ -4383,35 +4387,43 @@ get_etheraddr(const char *netdev_name, uint8_t ea[ETH_ADDR_LEN])
          * to INFO for that case. */
         VLOG(errno == ENODEV ? VLL_INFO : VLL_ERR,
              "ioctl(SIOCGIFHWADDR) on %s device failed: %s",
-             netdev_name, strerror(errno));
-        return errno;
+             netdev_dev->netdev_dev.name, strerror(errno));
+        netdev_dev->ether_addr_error = errno;
+        goto out;
     }
     hwaddr_family = ifr.ifr_hwaddr.sa_family;
     if (hwaddr_family != AF_UNSPEC && hwaddr_family != ARPHRD_ETHER) {
         VLOG_WARN("%s device has unknown hardware address family %d",
-                  netdev_name, hwaddr_family);
+                  netdev_dev->netdev_dev.name, hwaddr_family);
     }
-    memcpy(ea, ifr.ifr_hwaddr.sa_data, ETH_ADDR_LEN);
-    return 0;
+    memcpy(netdev_dev->etheraddr, ifr.ifr_hwaddr.sa_data, ETH_ADDR_LEN);
+    netdev_dev->ether_addr_error = 0;
+
+out:
+    netdev_dev->cache_valid |= VALID_ETHERADDR;
 }
 
-static int
-set_etheraddr(const char *netdev_name, int hwaddr_family,
+static void
+set_etheraddr(struct netdev_dev_linux *netdev_dev,
               const uint8_t mac[ETH_ADDR_LEN])
 {
     struct ifreq ifr;
 
     memset(&ifr, 0, sizeof ifr);
-    ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
-    ifr.ifr_hwaddr.sa_family = hwaddr_family;
+    ovs_strzcpy(ifr.ifr_name, netdev_dev->netdev_dev.name, sizeof ifr.ifr_name);
+    ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER;
     memcpy(ifr.ifr_hwaddr.sa_data, mac, ETH_ADDR_LEN);
     COVERAGE_INC(netdev_set_hwaddr);
     if (ioctl(af_inet_sock, SIOCSIFHWADDR, &ifr) < 0) {
         VLOG_ERR("ioctl(SIOCSIFHWADDR) on %s device failed: %s",
-                 netdev_name, strerror(errno));
-        return errno;
-    }
-    return 0;
+                 netdev_dev->netdev_dev.name, strerror(errno));
+        netdev_dev->ether_addr_error = errno;
+        goto out;
+    }
+    memcpy(netdev_dev->etheraddr, mac, ETH_ADDR_LEN);
+    netdev_dev->ether_addr_error = 0;
+out:
+    netdev_dev->cache_valid |= VALID_ETHERADDR;
 }
 
 static int
diff --git a/lib/rtnetlink-link.c b/lib/rtnetlink-link.c
index d14f0e3..c5fa4e7 100644
--- a/lib/rtnetlink-link.c
+++ b/lib/rtnetlink-link.c
@@ -43,9 +43,11 @@ rtnetlink_link_parse(struct ofpbuf *buf,
      * There are *many* more fields in these messages, but currently we
      * only care about these fields. */
     static const struct nl_policy policy[] = {
-        [IFLA_IFNAME] = { .type = NL_A_STRING, .optional = false },
-        [IFLA_MASTER] = { .type = NL_A_U32,    .optional = true },
-        [IFLA_MTU]    = { .type = NL_A_U32,    .optional = true },
+        [IFLA_IFNAME]  = { .type = NL_A_STRING, .optional = false, },
+        [IFLA_MASTER]  = { .type = NL_A_U32,    .optional = true,  },
+        [IFLA_MTU]     = { .type = NL_A_U32,    .optional = true,  },
+        [IFLA_ADDRESS] = { .type = NL_A_UNSPEC, .optional = true,
+                           .max_len = ETH_ALEN, }
     };
 
     static struct nlattr *attrs[ARRAY_SIZE(policy)];
@@ -72,6 +74,12 @@ rtnetlink_link_parse(struct ofpbuf *buf,
                                   ? nl_attr_get_u32(attrs[IFLA_MTU])
                                   : 0);
 
+        if (attrs[IFLA_ADDRESS] &&
+           (nl_attr_get_size(attrs[IFLA_ADDRESS]) == ETH_ALEN)) {
+            memcpy(change->addr, nl_attr_get(attrs[IFLA_ADDRESS]), ETH_ALEN);
+        } else {
+            memset(change->addr, 0, ETH_ALEN);
+        }
     }
 
     return parsed;
diff --git a/lib/rtnetlink-link.h b/lib/rtnetlink-link.h
index cfdb24b..7c64c94 100644
--- a/lib/rtnetlink-link.h
+++ b/lib/rtnetlink-link.h
@@ -18,6 +18,8 @@
 #define RTNETLINK_LINK_H 1
 
 #include <stdbool.h>
+#include <stdint.h>
+#include <linux/if_ether.h>
 
 struct ofpbuf;
 struct nln_notifier;
@@ -38,6 +40,7 @@ struct rtnetlink_link_change {
     const char *ifname;         /* Name of network device. */
     int master_ifindex;         /* Ifindex of datapath master (0 if none). */
     int mtu;                    /* Current MTU. */
+    uint8_t addr[ETH_ALEN];
     unsigned int ifi_flags;     /* Flags of network device. */
 };
 
-- 
1.7.1




More information about the dev mailing list