| Message ID | 20200701153422.267380-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn,v2] ovn-controller: Configure hwaddr for the integration bridge | expand |
Excellent! Thanks, Numan! Acked-by: Mark Michelson <mmichels@redhat.com> On 7/1/20 11:34 AM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > When a first non-local port is added to the integration bridge, it results > in the recalculation of datapath-id by ovs-vswitchd forcing all > active connections to the controllers to reconnect. > > This patch avoids these reconnections between ovs-vswitchd and ovn-controller > by setting the hwaddr for the integration bridge when ovn-controller creates the > integration bridge. ovs-vswitchd uses the bridge:hwaddr if set to generate the > datapath-id. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > v1 -> v2 > ----- > * Added the comments in the code as suggested by Mark. > > controller/ovn-controller.c | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index b0ee60a1a..e355007b8 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -277,10 +277,33 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn, > bridge = ovsrec_bridge_insert(ovs_idl_txn); > ovsrec_bridge_set_name(bridge, bridge_name); > ovsrec_bridge_set_fail_mode(bridge, "secure"); > - const struct smap oc = SMAP_CONST1(&oc, "disable-in-band", "true"); > - ovsrec_bridge_set_other_config(bridge, &oc); > ovsrec_bridge_set_ports(bridge, &port, 1); > > + struct smap oc = SMAP_INITIALIZER(&oc); > + smap_add(&oc, "disable-in-band", "true"); > + > + /* When a first non-local port is added to the integration bridge, it > + * results in the recalculation of datapath-id by ovs-vswitchd forcing all > + * active connections to the controllers to reconnect. > + * > + * We can avoid the disconnection by setting the 'other_config:hwaddr' for > + * the integration bridge. ovs-vswitchd uses this hwaddr to calculate the > + * datapath-id and it doesn't recalculate the datapath-id later when the > + * first non-local port is added. > + * > + * So generate a random mac and set the 'hwaddr' option in the > + * other_config. > + * */ > + struct eth_addr br_hwaddr; > + eth_addr_random(&br_hwaddr); > + char ea_s[ETH_ADDR_STRLEN + 1]; > + snprintf(ea_s, sizeof ea_s, ETH_ADDR_FMT, > + ETH_ADDR_ARGS(br_hwaddr)); > + smap_add(&oc, "hwaddr", ea_s); > + > + ovsrec_bridge_set_other_config(bridge, &oc); > + smap_destroy(&oc); > + > struct ovsrec_bridge **bridges; > size_t bytes = sizeof *bridges * cfg->n_bridges; > bridges = xmalloc(bytes + sizeof *bridges); >
On Fri, Jul 3, 2020 at 12:11 AM Mark Michelson <mmichels@redhat.com> wrote: > Excellent! Thanks, Numan! > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Thanks for the review. I applied this patch to master. Numan > On 7/1/20 11:34 AM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > When a first non-local port is added to the integration bridge, it > results > > in the recalculation of datapath-id by ovs-vswitchd forcing all > > active connections to the controllers to reconnect. > > > > This patch avoids these reconnections between ovs-vswitchd and > ovn-controller > > by setting the hwaddr for the integration bridge when ovn-controller > creates the > > integration bridge. ovs-vswitchd uses the bridge:hwaddr if set to > generate the > > datapath-id. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > v1 -> v2 > > ----- > > * Added the comments in the code as suggested by Mark. > > > > controller/ovn-controller.c | 27 +++++++++++++++++++++++++-- > > 1 file changed, 25 insertions(+), 2 deletions(-) > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index b0ee60a1a..e355007b8 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -277,10 +277,33 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn, > > bridge = ovsrec_bridge_insert(ovs_idl_txn); > > ovsrec_bridge_set_name(bridge, bridge_name); > > ovsrec_bridge_set_fail_mode(bridge, "secure"); > > - const struct smap oc = SMAP_CONST1(&oc, "disable-in-band", "true"); > > - ovsrec_bridge_set_other_config(bridge, &oc); > > ovsrec_bridge_set_ports(bridge, &port, 1); > > > > + struct smap oc = SMAP_INITIALIZER(&oc); > > + smap_add(&oc, "disable-in-band", "true"); > > + > > + /* When a first non-local port is added to the integration bridge, > it > > + * results in the recalculation of datapath-id by ovs-vswitchd > forcing all > > + * active connections to the controllers to reconnect. > > + * > > + * We can avoid the disconnection by setting the > 'other_config:hwaddr' for > > + * the integration bridge. ovs-vswitchd uses this hwaddr to > calculate the > > + * datapath-id and it doesn't recalculate the datapath-id later > when the > > + * first non-local port is added. > > + * > > + * So generate a random mac and set the 'hwaddr' option in the > > + * other_config. > > + * */ > > + struct eth_addr br_hwaddr; > > + eth_addr_random(&br_hwaddr); > > + char ea_s[ETH_ADDR_STRLEN + 1]; > > + snprintf(ea_s, sizeof ea_s, ETH_ADDR_FMT, > > + ETH_ADDR_ARGS(br_hwaddr)); > > + smap_add(&oc, "hwaddr", ea_s); > > + > > + ovsrec_bridge_set_other_config(bridge, &oc); > > + smap_destroy(&oc); > > + > > struct ovsrec_bridge **bridges; > > size_t bytes = sizeof *bridges * cfg->n_bridges; > > bridges = xmalloc(bytes + sizeof *bridges); > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b0ee60a1a..e355007b8 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -277,10 +277,33 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn, bridge = ovsrec_bridge_insert(ovs_idl_txn); ovsrec_bridge_set_name(bridge, bridge_name); ovsrec_bridge_set_fail_mode(bridge, "secure"); - const struct smap oc = SMAP_CONST1(&oc, "disable-in-band", "true"); - ovsrec_bridge_set_other_config(bridge, &oc); ovsrec_bridge_set_ports(bridge, &port, 1); + struct smap oc = SMAP_INITIALIZER(&oc); + smap_add(&oc, "disable-in-band", "true"); + + /* When a first non-local port is added to the integration bridge, it + * results in the recalculation of datapath-id by ovs-vswitchd forcing all + * active connections to the controllers to reconnect. + * + * We can avoid the disconnection by setting the 'other_config:hwaddr' for + * the integration bridge. ovs-vswitchd uses this hwaddr to calculate the + * datapath-id and it doesn't recalculate the datapath-id later when the + * first non-local port is added. + * + * So generate a random mac and set the 'hwaddr' option in the + * other_config. + * */ + struct eth_addr br_hwaddr; + eth_addr_random(&br_hwaddr); + char ea_s[ETH_ADDR_STRLEN + 1]; + snprintf(ea_s, sizeof ea_s, ETH_ADDR_FMT, + ETH_ADDR_ARGS(br_hwaddr)); + smap_add(&oc, "hwaddr", ea_s); + + ovsrec_bridge_set_other_config(bridge, &oc); + smap_destroy(&oc); + struct ovsrec_bridge **bridges; size_t bytes = sizeof *bridges * cfg->n_bridges; bridges = xmalloc(bytes + sizeof *bridges);