[ovs-dev] [PATCH ovn] ovn-controller: Configure hwaddr for the integration bridge
Mark Michelson
mmichels at redhat.com
Wed Jul 1 14:44:21 UTC 2020
Hi Numan,
I approve of this change, but you should add a comment to the code about
why we're setting a random hwaddr to the bridge. It is completely
non-obvious why we are doing it just from reading the code.
On 4/30/20 4:10 AM, numans at ovn.org wrote:
> From: Numan Siddique <numans at 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 at ovn.org>
> ---
> controller/ovn-controller.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6ff897325..299e1e544 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -277,10 +277,21 @@ 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");
> +
> + 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);
>
More information about the dev
mailing list