From 07c318f40481d0fd10516193fdf472c6356f0cd3 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 23 Jul 2009 14:50:55 -0700 Subject: [PATCH] vswitchd: Avoid netdev_nodev_*() functions. The netdev_nodev_*() functions have always been a bit of a kluge. It's better to keep a network device open than to open it every time that it is needed. This commit converts all of the users of these functions to use the corresponding functions that take a "struct netdev *" instead. --- vswitchd/bridge.c | 125 +++++++++++++++++++++------------------ vswitchd/ovs-brcompatd.c | 10 +++- 2 files changed, 75 insertions(+), 60 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index cd683c0e..8a63ef8b 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -201,10 +201,11 @@ static void bridge_fetch_dp_ifaces(struct bridge *); static void bridge_flush(struct bridge *); static void bridge_pick_local_hw_addr(struct bridge *, uint8_t ea[ETH_ADDR_LEN], - const char **devname); + struct iface **hw_addr_iface); static uint64_t bridge_pick_datapath_id(struct bridge *, const uint8_t bridge_ea[ETH_ADDR_LEN], - const char *devname); + struct iface *hw_addr_iface); +static struct iface *bridge_get_local_iface(struct bridge *); static uint64_t dpid_from_hash(const void *, size_t nbytes); static void bridge_unixctl_fdb_show(struct unixctl_conn *, const char *args); @@ -379,15 +380,9 @@ init_iface_netdev(struct bridge *br UNUSED, struct iface *iface, } static bool -check_iface_dp_ifidx(struct bridge *br, struct iface *iface, - void *local_ifacep_) +check_iface_dp_ifidx(struct bridge *br, struct iface *iface, void *aux UNUSED) { - struct iface **local_ifacep = local_ifacep_; - if (iface->dp_ifidx >= 0) { - if (iface->dp_ifidx == ODPP_LOCAL) { - *local_ifacep = iface; - } VLOG_DBG("%s has interface %s on port %d", dpif_name(br->dpif), iface->name, iface->dp_ifidx); @@ -549,8 +544,8 @@ bridge_reconfigure(void) LIST_FOR_EACH (br, struct bridge, node, &all_bridges) { uint8_t ea[8]; uint64_t dpid; - struct iface *local_iface = NULL; - const char *devname; + struct iface *local_iface; + struct iface *hw_addr_iface; uint8_t engine_type, engine_id; bool add_id_to_iface = false; struct svec nf_hosts; @@ -558,13 +553,13 @@ bridge_reconfigure(void) bridge_fetch_dp_ifaces(br); iterate_and_prune_ifaces(br, init_iface_netdev, NULL); - local_iface = NULL; - iterate_and_prune_ifaces(br, check_iface_dp_ifidx, &local_iface); + iterate_and_prune_ifaces(br, check_iface_dp_ifidx, NULL); /* Pick local port hardware address, datapath ID. */ - bridge_pick_local_hw_addr(br, ea, &devname); + bridge_pick_local_hw_addr(br, ea, &hw_addr_iface); + local_iface = bridge_get_local_iface(br); if (local_iface) { - int error = netdev_nodev_set_etheraddr(local_iface->name, ea); + int error = netdev_set_etheraddr(local_iface->netdev, ea); if (error) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_ERR_RL(&rl, "bridge %s: failed to set bridge " @@ -573,7 +568,7 @@ bridge_reconfigure(void) } } - dpid = bridge_pick_datapath_id(br, ea, devname); + dpid = bridge_pick_datapath_id(br, ea, hw_addr_iface); ofproto_set_datapath_id(br->ofproto, dpid); /* Set NetFlow configuration on this bridge. */ @@ -633,13 +628,13 @@ bridge_reconfigure(void) static void bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN], - const char **devname) + struct iface **hw_addr_iface) { uint64_t requested_ea; size_t i, j; int error; - *devname = NULL; + *hw_addr_iface = NULL; /* Did the user request a particular MAC? */ requested_ea = cfg_get_mac(0, "bridge.%s.mac", br->name); @@ -671,14 +666,14 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN], || cfg_get_bool(0, "iface.%s.internal", iface->name)) { continue; } - error = netdev_nodev_get_etheraddr(iface->name, iface_ea); + error = netdev_get_etheraddr(iface->netdev, iface_ea); if (!error) { if (!eth_addr_is_multicast(iface_ea) && !eth_addr_is_reserved(iface_ea) && !eth_addr_is_zero(iface_ea) && memcmp(iface_ea, ea, ETH_ADDR_LEN) < 0) { memcpy(ea, iface_ea, ETH_ADDR_LEN); - *devname = iface->name; + *hw_addr_iface = iface; } } else { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); @@ -689,7 +684,7 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN], } if (eth_addr_is_multicast(ea) || eth_addr_is_vif(ea)) { memcpy(ea, br->default_ea, ETH_ADDR_LEN); - *devname = NULL; + *hw_addr_iface = NULL; VLOG_WARN("bridge %s: using default bridge Ethernet " "address "ETH_ADDR_FMT, br->name, ETH_ADDR_ARGS(ea)); } else { @@ -700,13 +695,13 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN], /* Choose and returns the datapath ID for bridge 'br' given that the bridge * Ethernet address is 'bridge_ea'. If 'bridge_ea' is the Ethernet address of - * a network device, then that network device's name must be passed in as - * 'devname'; if 'bridge_ea' was derived some other way, then 'devname' must be - * passed in as a null pointer. */ + * an interface on 'br', then that interface must be passed in as + * 'hw_addr_iface'; if 'bridge_ea' was derived some other way, then + * 'hw_addr_iface' must be passed in as a null pointer. */ static uint64_t bridge_pick_datapath_id(struct bridge *br, const uint8_t bridge_ea[ETH_ADDR_LEN], - const char *devname) + struct iface *hw_addr_iface) { /* * The procedure for choosing a bridge MAC address will, in the most @@ -727,9 +722,10 @@ bridge_pick_datapath_id(struct bridge *br, return dpid; } - if (devname) { + if (hw_addr_iface) { int vlan; - if (!netdev_get_vlan_vid(devname, &vlan)) { + if (!netdev_get_vlan_vid(netdev_get_name(hw_addr_iface->netdev), + &vlan)) { /* * A bridge whose MAC address is taken from a VLAN network device * (that is, a network device created with vconfig(8) or similar @@ -839,6 +835,26 @@ bridge_flush(struct bridge *br) mac_learning_flush(br->ml); } } + +/* Returns the 'br' interface for the ODPP_LOCAL port, or null if 'br' has no + * such interface. */ +static struct iface * +bridge_get_local_iface(struct bridge *br) +{ + size_t i, j; + + for (i = 0; i < br->n_ports; i++) { + struct port *port = br->ports[i]; + for (j = 0; j < port->n_ifaces; j++) { + struct iface *iface = port->ifaces[j]; + if (iface->dp_ifidx == ODPP_LOCAL) { + return iface; + } + } + } + + return NULL; +} /* Bridge unixctl user interface functions. */ static void @@ -1169,10 +1185,8 @@ bridge_reconfigure_controller(struct bridge *br) cfg_get_string(0, "%s.accept-regex", pfx), update_resolv_conf); } else { - char local_name[IF_NAMESIZE]; - struct netdev *netdev; + struct iface *local_iface; bool in_band; - int error; in_band = (!cfg_is_valid(CFG_BOOL | CFG_REQUIRED, "%s.in-band", pfx) @@ -1180,37 +1194,32 @@ bridge_reconfigure_controller(struct bridge *br) ofproto_set_discovery(br->ofproto, false, NULL, NULL); ofproto_set_in_band(br->ofproto, in_band); - error = dpif_port_get_name(br->dpif, ODPP_LOCAL, - local_name, sizeof local_name); - if (!error) { - error = netdev_open(local_name, NETDEV_ETH_TYPE_NONE, &netdev); - } - if (!error) { - if (cfg_is_valid(CFG_IP | CFG_REQUIRED, "%s.ip", pfx)) { - struct in_addr ip, mask, gateway; - ip.s_addr = cfg_get_ip(0, "%s.ip", pfx); - mask.s_addr = cfg_get_ip(0, "%s.netmask", pfx); - gateway.s_addr = cfg_get_ip(0, "%s.gateway", pfx); - - netdev_turn_flags_on(netdev, NETDEV_UP, true); - if (!mask.s_addr) { - mask.s_addr = guess_netmask(ip.s_addr); - } - if (!netdev_set_in4(netdev, ip, mask)) { - VLOG_INFO("bridge %s: configured IP address "IP_FMT", " - "netmask "IP_FMT, - br->name, IP_ARGS(&ip.s_addr), - IP_ARGS(&mask.s_addr)); - } + local_iface = bridge_get_local_iface(br); + if (local_iface + && cfg_is_valid(CFG_IP | CFG_REQUIRED, "%s.ip", pfx)) { + struct netdev *netdev = local_iface->netdev; + struct in_addr ip, mask, gateway; + ip.s_addr = cfg_get_ip(0, "%s.ip", pfx); + mask.s_addr = cfg_get_ip(0, "%s.netmask", pfx); + gateway.s_addr = cfg_get_ip(0, "%s.gateway", pfx); + + netdev_turn_flags_on(netdev, NETDEV_UP, true); + if (!mask.s_addr) { + mask.s_addr = guess_netmask(ip.s_addr); + } + if (!netdev_set_in4(netdev, ip, mask)) { + VLOG_INFO("bridge %s: configured IP address "IP_FMT", " + "netmask "IP_FMT, + br->name, IP_ARGS(&ip.s_addr), + IP_ARGS(&mask.s_addr)); + } - if (gateway.s_addr) { - if (!netdev_add_router(netdev, gateway)) { - VLOG_INFO("bridge %s: configured gateway "IP_FMT, - br->name, IP_ARGS(&gateway.s_addr)); - } + if (gateway.s_addr) { + if (!netdev_add_router(netdev, gateway)) { + VLOG_INFO("bridge %s: configured gateway "IP_FMT, + br->name, IP_ARGS(&gateway.s_addr)); } } - netdev_close(netdev); } } diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c index ff829846..306de136 100644 --- a/vswitchd/ovs-brcompatd.c +++ b/vswitchd/ovs-brcompatd.c @@ -611,8 +611,14 @@ handle_fdb_query_cmd(struct ofpbuf *buffer) for (i = 0; i < ifaces.n; i++) { const char *iface_name = ifaces.names[i]; struct mac *mac = &local_macs[n_local_macs]; - if (!netdev_nodev_get_etheraddr(iface_name, mac->addr)) { - n_local_macs++; + struct netdev *netdev; + + error = netdev_open(iface_name, NETDEV_ETH_TYPE_NONE, &netdev); + if (netdev) { + if (!netdev_get_etheraddr(netdev, mac->addr)) { + n_local_macs++; + } + netdev_close(netdev); } } svec_destroy(&ifaces); -- 2.30.2