On Thu, 2017-05-11 at 16:16 -0700, Andy Zhou wrote: > When multiple bridges connects to the same controller, the controller > status should be maintained separately for each bridge. Current > logic pushes status updates only based on the connection string, > which is the same across multiple bridges when connecting to the > same controller. > > Report-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2017-May/044412.html > Reported-by: Tulio Ribeiro <[email protected]> > Signed-off-by: Andy Zhou <[email protected]> > --- > AUTHORS.rst | 1 + > tests/bridge.at | 33 +++++++++++++++++++++++++++++++++ > vswitchd/bridge.c | 33 +++++++++++++++------------------ > 3 files changed, 49 insertions(+), 18 deletions(-) > > diff --git a/AUTHORS.rst b/AUTHORS.rst > index 59639756b09f..147ce48d15ca 100644 > --- a/AUTHORS.rst > +++ b/AUTHORS.rst > @@ -530,6 +530,7 @@ Teemu Koponen [email protected] > Thomas Morin [email protected] > Timothy Chen [email protected] > Torbjorn Tornkvist [email protected] > +Tulio Ribeiro [email protected] > Tytus Kurek [email protected] > Valentin Bud [email protected] > Vasiliy Tolstov [email protected] > diff --git a/tests/bridge.at b/tests/bridge.at > index 3dbabe511780..58b27d445062 100644 > --- a/tests/bridge.at > +++ b/tests/bridge.at > @@ -38,3 +38,36 @@ dummy@ovs-dummy: hit:0 missed:0 > OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > AT_CLEANUP > + > +dnl When multiple bridges are connected to the same controller, make > +dnl sure their status are tracked independently. > +AT_SETUP([bridge - multiple bridges share a controller]) > +OVS_VSWITCHD_START( > + [add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ]) > + > +dnl Start ovs-testcontroller > +AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore]) > + > +dnl Add the controller to both bridges, 5 seconds apart. > +AT_CHECK([ovs-vsctl set-controller br0 unix:controller]) > +AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore]) > +AT_CHECK([ovs-vsctl set-controller br1 unix:controller]) > + > +dnl Wait for the controller connection to come up > +for i in `seq 0 7` > +do > + AT_CHECK([ovs-appctl time/warp 10], [0], [ignore]) > +done > + > +dnl Make sure the connection status are different > +AT_CHECK([ovs-vsctl --columns=status list controller | sort], [0], [dnl > + > +status : {sec_since_connect="0", state=ACTIVE} > +status : {sec_since_connect="5", state=ACTIVE} > +]) > + > +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > +AT_CLEANUP > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 31203d1ec232..972146e8da6d 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -2704,34 +2704,31 @@ static void > refresh_controller_status(void) > { > struct bridge *br; > - struct shash info; > - const struct ovsrec_controller *cfg; > - > - shash_init(&info); > > /* Accumulate status for controllers on all bridges. */ > HMAP_FOR_EACH (br, node, &all_bridges) { > + struct shash info = SHASH_INITIALIZER(&info); > ofproto_get_ofproto_controller_info(br->ofproto, &info); > - } > > - /* Update each controller in the database with current status. */ > - OVSREC_CONTROLLER_FOR_EACH(cfg, idl) { > - struct ofproto_controller_info *cinfo = > - shash_find_data(&info, cfg->target); > + /* Update each controller of the bridge in the database with > + * current status. */ > + struct ovsrec_controller **controllers; > + size_t n_controllers = bridge_get_controllers(br, &controllers); > + size_t i; > + for (i = 0; i < n_controllers; i++) { > + struct ovsrec_controller *cfg = controllers[i]; > + struct ofproto_controller_info *cinfo = > + shash_find_data(&info, cfg->target); > > - if (cinfo) { > + ovs_assert(cinfo); > ovsrec_controller_set_is_connected(cfg, cinfo->is_connected); > - ovsrec_controller_set_role(cfg, ofp12_controller_role_to_str( > - cinfo->role)); > + const char *role = ofp12_controller_role_to_str(cinfo->role); > + ovsrec_controller_set_role(cfg, role); > ovsrec_controller_set_status(cfg, &cinfo->pairs); > - } else { > - ovsrec_controller_set_is_connected(cfg, false); > - ovsrec_controller_set_role(cfg, NULL); > - ovsrec_controller_set_status(cfg, NULL); > } > - } > > - ofproto_free_ofproto_controller_info(&info); > + ofproto_free_ofproto_controller_info(&info); > + } > } > > /* Update interface and mirror statistics if necessary. */
I can't that I'm super familiar with this code but this LGTM. Reviewed-by: Greg Rose <gvrose@[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
