From 7174e5aed2a6d2877c1760d2ae921ae24d7accb5 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 13 Apr 2011 11:10:44 -0700 Subject: [PATCH] bridge: Tolerate missing Port and Interface records for local port. Until now, ovs-vswitchd has been unable to configure IP addresses and routes for bridges whose Bridge records lack a Port and an Interface record for the bridge's local port (e.g. OFPP_LOCAL, the port with the same name as the bridge itself). When such a bridge was reconfigured, ovs-vswitchd would output a log message that worried people. This commit fixes the internal limitation that led to the message being printed. Bug #5385. --- lib/ovsdb-idl.c | 9 ++++++++ lib/ovsdb-idl.h | 4 +++- vswitchd/bridge.c | 59 +++++++++++++++++++++++++++++++++++------------ 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index e1b53f4eb..fd15ea96f 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -1108,6 +1108,15 @@ ovsdb_idl_get(const struct ovsdb_idl_row *row, return ovsdb_idl_read(row, column); } + +/* Returns false if 'row' was obtained from the IDL, true if it was initialized + * to all-zero-bits by some other entity. If 'row' was set up some other way + * then the return value is indeterminate. */ +bool +ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *row) +{ + return row->table == NULL; +} /* Transactions. */ diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index 302abd6a6..d11fb0e02 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010 Nicira Networks. +/* Copyright (c) 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -105,6 +105,8 @@ const struct ovsdb_datum *ovsdb_idl_get(const struct ovsdb_idl_row *, const struct ovsdb_idl_column *, enum ovsdb_atomic_type key_type, enum ovsdb_atomic_type value_type); + +bool ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *); /* Transactions. */ diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 4be0d1e6f..f304ac014 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -226,6 +226,11 @@ struct bridge { /* Port mirroring. */ struct mirror *mirrors[MAX_MIRRORS]; + + /* Synthetic local port if necessary. */ + struct ovsrec_port synth_local_port; + struct ovsrec_interface synth_local_iface; + struct ovsrec_interface *synth_local_ifacep; }; /* List of all bridges. */ @@ -312,6 +317,7 @@ static void iface_update_cfm(struct iface *); static bool iface_refresh_cfm_stats(struct iface *iface); static void iface_update_carrier(struct iface *); static bool iface_get_carrier(const struct iface *); +static bool iface_is_synthetic(const struct iface *); static void shash_from_ovs_idl_map(char **keys, char **values, size_t n, struct shash *); @@ -1193,6 +1199,10 @@ iface_refresh_status(struct iface *iface) int64_t mtu_64; int error; + if (iface_is_synthetic(iface)) { + return; + } + shash_init(&sh); if (!netdev_get_status(iface->netdev, &sh)) { @@ -1314,6 +1324,10 @@ iface_refresh_stats(struct iface *iface) struct netdev_stats stats; + if (iface_is_synthetic(iface)) { + return; + } + /* Intentionally ignore return value, since errors will set 'stats' to * all-1s, and we will deal with that correctly below. */ netdev_get_stats(iface->netdev, &stats); @@ -1748,6 +1762,7 @@ bridge_destroy(struct bridge *br) hmap_destroy(&br->ifaces); hmap_destroy(&br->ports); shash_destroy(&br->iface_by_name); + free(br->synth_local_iface.type); free(br->name); free(br); } @@ -1873,22 +1888,28 @@ bridge_reconfigure_one(struct bridge *br) br->name, name); } } + if (!shash_find(&new_ports, br->name)) { + struct dpif_port dpif_port; + char *type; - /* If we have a controller, then we need a local port. Complain if the - * user didn't specify one. - * - * XXX perhaps we should synthesize a port ourselves in this case. */ - if (bridge_get_controllers(br, NULL)) { - char local_name[IF_NAMESIZE]; - int error; + VLOG_WARN("bridge %s: no port named %s, synthesizing one", + br->name, br->name); - error = dpif_port_get_name(br->dpif, ODPP_LOCAL, - local_name, sizeof local_name); - if (!error && !shash_find(&new_ports, local_name)) { - VLOG_WARN("bridge %s: controller specified but no local port " - "(port named %s) defined", - br->name, local_name); - } + dpif_port_query_by_number(br->dpif, ODPP_LOCAL, &dpif_port); + type = xstrdup(dpif_port.type ? dpif_port.type : "internal"); + dpif_port_destroy(&dpif_port); + + br->synth_local_port.interfaces = &br->synth_local_ifacep; + br->synth_local_port.n_interfaces = 1; + br->synth_local_port.name = br->name; + + br->synth_local_iface.name = br->name; + free(br->synth_local_iface.type); + br->synth_local_iface.type = type; + + br->synth_local_ifacep = &br->synth_local_iface; + + shash_add(&new_ports, br->name, &br->synth_local_port); } /* Get rid of deleted ports. @@ -4516,7 +4537,7 @@ iface_set_mac(struct iface *iface) static void iface_set_ofport(const struct ovsrec_interface *if_cfg, int64_t ofport) { - if (if_cfg) { + if (if_cfg && !ovsdb_idl_row_is_synthetic(&if_cfg->header_)) { ovsrec_interface_set_ofport(if_cfg, &ofport, 1); } } @@ -4691,6 +4712,14 @@ iface_get_carrier(const struct iface *iface) ? netdev_get_carrier(iface->netdev) : netdev_get_miimon(iface->netdev)); } + +/* Returns true if 'iface' is synthetic, that is, if we constructed it locally + * instead of obtaining it from the database. */ +static bool +iface_is_synthetic(const struct iface *iface) +{ + return ovsdb_idl_row_is_synthetic(&iface->cfg->header_); +} /* Port mirroring. */ -- 2.43.0