On Mon, Nov 21, 2022 at 3:00 PM Mark Michelson <[email protected]> wrote: > > Acked-by: Mark Michelson <[email protected]> > > Do you have any idea why we were iterating through all OVS bridges to > remove tunnels? The only reason I could think of is if the "ovn-bridge" > setting changed between runs of ovn-controller. Then we'd be sure to > clean up the old bridge's tunnels.
Sadly no idea. I suspect it's just always been assumed that there's one controller running, and that all tunnel ports that have ovn-chassis-id set belong to this single controller instance (a reasonable assumption). > > On 10/18/22 14:33, Ihar Hrachyshka wrote: > > When multiple controllers are running using the same vswitchd, > > controllers should delete only those tunnel ports that belong to the > > integration bridge that is managed by the controller instance. > > > > This makes sure multiple controllers don't step on each other when > > running using the same vswitchd instance. > > > > Signed-off-by: Ihar Hrachyshka <[email protected]> > > --- > > controller/encaps.c | 42 +++++++++----------- > > controller/encaps.h | 1 - > > controller/ovn-controller.c | 3 +- > > tests/ovn.at | 79 +++++++++++++++++++++++++++++++++++++ > > 4 files changed, 99 insertions(+), 26 deletions(-) > > > > diff --git a/controller/encaps.c b/controller/encaps.c > > index 0bc887983..ead8925ae 100644 > > --- a/controller/encaps.c > > +++ b/controller/encaps.c > > @@ -397,7 +397,6 @@ chassis_tzones_overlap(const struct sset > > *transport_zones, > > > > void > > encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > > - const struct ovsrec_bridge_table *bridge_table, > > const struct ovsrec_bridge *br_int, > > const struct sbrec_chassis_table *chassis_table, > > const struct sbrec_chassis *this_chassis, > > @@ -410,7 +409,6 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > > } > > > > const struct sbrec_chassis *chassis_rec; > > - const struct ovsrec_bridge *br; > > > > struct tunnel_ctx tc = { > > .chassis = SHASH_INITIALIZER(&tc.chassis), > > @@ -428,27 +426,25 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > > /* Collect all port names into tc.port_names. > > * > > * Collect all the OVN-created tunnels into tc.tunnel_hmap. */ > > - OVSREC_BRIDGE_TABLE_FOR_EACH (br, bridge_table) { > > - for (size_t i = 0; i < br->n_ports; i++) { > > - const struct ovsrec_port *port = br->ports[i]; > > - sset_add(&tc.port_names, port->name); > > - > > - /* > > - * note that the id here is not just the chassis name, but the > > - * combination of <chassis_name><delim><encap_ip> > > - */ > > - const char *id = smap_get(&port->external_ids, > > "ovn-chassis-id"); > > - if (id) { > > - if (!shash_find(&tc.chassis, id)) { > > - struct chassis_node *chassis = xzalloc(sizeof > > *chassis); > > - chassis->bridge = br; > > - chassis->port = port; > > - shash_add_assert(&tc.chassis, id, chassis); > > - } else { > > - /* Duplicate port for ovn-chassis-id. Arbitrarily > > choose > > - * to delete this one. */ > > - ovsrec_bridge_update_ports_delvalue(br, port); > > - } > > + for (size_t i = 0; i < br_int->n_ports; i++) { > > + const struct ovsrec_port *port = br_int->ports[i]; > > + sset_add(&tc.port_names, port->name); > > + > > + /* > > + * note that the id here is not just the chassis name, but the > > + * combination of <chassis_name><delim><encap_ip> > > + */ > > + const char *id = smap_get(&port->external_ids, "ovn-chassis-id"); > > + if (id) { > > + if (!shash_find(&tc.chassis, id)) { > > + struct chassis_node *chassis = xzalloc(sizeof *chassis); > > + chassis->bridge = br_int; > > + chassis->port = port; > > + shash_add_assert(&tc.chassis, id, chassis); > > + } else { > > + /* Duplicate port for ovn-chassis-id. Arbitrarily choose > > + * to delete this one. */ > > + ovsrec_bridge_update_ports_delvalue(br_int, port); > > } > > } > > } > > diff --git a/controller/encaps.h b/controller/encaps.h > > index 16032f15b..7197050b6 100644 > > --- a/controller/encaps.h > > +++ b/controller/encaps.h > > @@ -32,7 +32,6 @@ struct sset; > > > > void encaps_register_ovs_idl(struct ovsdb_idl *); > > void encaps_run(struct ovsdb_idl_txn *ovs_idl_txn, > > - const struct ovsrec_bridge_table *, > > const struct ovsrec_bridge *br_int, > > const struct sbrec_chassis_table *, > > const struct sbrec_chassis *, > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index e26d46535..6a3aec317 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -4230,8 +4230,7 @@ main(int argc, char *argv[]) > > } > > > > if (chassis) { > > - encaps_run(ovs_idl_txn, > > - bridge_table, br_int, > > + encaps_run(ovs_idl_txn, br_int, > > > > sbrec_chassis_table_get(ovnsb_idl_loop.idl), > > chassis, > > sbrec_sb_global_first(ovnsb_idl_loop.idl), > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 2508a224e..c6f06e3e6 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -33100,3 +33100,82 @@ AT_CHECK(test x$encap_hv1_ip == x) > > OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([controllers don't touch tunnels that are not on br-int]) > > +ovn_start > > +net_add n1 > > + > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys1 > > +ovn_attach n1 br-phys1 192.168.0.1 > > + > > +# use a port as a canary in the mine to wait until the controller is up > > +# (meaning, ssl configuration was read from the database) > > +ovn-nbctl ls-add ls > > +ovn-nbctl lsp-add ls lp > > +ovs-vsctl -- add-port br-int vif -- \ > > + set interface vif external-ids:iface-id=lp > > +wait_for_ports_up > > +ovn-nbctl --wait=hv sync > > + > > +# the file is read once at startup so it's safe to write it > > +# here after the first ovn-controller has started > > +echo hv2 > ${OVN_SYSCONFDIR}/system-id-override > > +ovs-vsctl add-br br-phys2 > > + > > +# This function is similar to ovn_attach but makes sure it doesn't > > +# mess with another controller settings > > +start_virtual_controller() { > > + local net=$1 bridge=$2 ip=$3 masklen=${4-24} encap=${5-geneve,vxlan} > > systemid=${6-$sandbox} cli_args=${@:7} > > + net_attach $net $bridge || return 1 > > + > > + mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g` > > + arp_table="$arp_table $sandbox,$bridge,$ip,$mac" > > + ovs-appctl netdev-dummy/ip4addr $bridge $ip/$masklen >/dev/null || > > return 1 > > + ovs-appctl ovs/route/add $ip/$masklen $bridge >/dev/null || return 1 > > + > > + local ovn_remote > > + if test X$HAVE_OPENSSL = Xyes; then > > + ovn_remote=$SSL_OVN_SB_DB > > + else > > + ovn_remote=unix:$ovs_base/ovn-sb/ovn-sb.sock > > + fi > > + ovs-vsctl \ > > + -- set Open_vSwitch . > > external-ids:ovn-remote-$systemid=$ovn_remote \ > > + -- set Open_vSwitch . external-ids:ovn-encap-type-$systemid=$encap > > \ > > + -- set Open_vSwitch . external-ids:ovn-encap-ip-$systemid=$ip \ > > + -- set Open_vSwitch . external-ids:ovn-bridge-$systemid=br-int-2 \ > > + -- --may-exist add-br br-int-2 \ > > + -- set bridge br-int-2 fail-mode=secure > > other-config:disable-in-band=true \ > > + || return 1 > > + > > + ovn-controller --enable-dummy-vif-plug ${cli_args} -vconsole:off > > --detach --no-chdir > > +} > > + > > +# for some reason SSL ovsdb configuration overrides CLI, so > > +# delete ssl config from ovsdb to give CLI arguments priority > > +ovs-vsctl del-ssl > > + > > +start_virtual_controller n1 br-phys2 192.168.0.2 24 geneve,vxlan hv2 \ > > + --pidfile=${OVS_RUNDIR}/ovn-controller-2.pid \ > > + --log-file=${OVS_RUNDIR}/ovn-controller-2.log \ > > + -p $PKIDIR/testpki-hv2-privkey.pem \ > > + -c $PKIDIR/testpki-hv2-cert.pem \ > > + -C $PKIDIR/testpki-cacert.pem > > +pidfile="$OVS_RUNDIR"/ovn-controller-2.pid > > +on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`" > > + > > +# check that both tunnel ports are present, meaning controllers > > +# don't step on each other > > +OVS_WAIT_UNTIL([ovs-vsctl --columns _uuid --bare find Port \ > > + name=ovn-hv1-hv2-0 | wc -l], [0],[[1 > > +]]) > > +OVS_WAIT_UNTIL([ovs-vsctl --columns _uuid --bare find Port \ > > + name=ovn-hv2-hv1-0 | wc -l], [0],[[1 > > +]]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > +]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
