[ovs-dev] [PATCH ovn v2] ovn-controller: Configure hwaddr for the integration bridge

[Mark Michelson]

Excellent! Thanks, Numan!

Acked-by: Mark Michelson <mmichels at redhat.com>

On 7/1/20 11:34 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>
> ---
> 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);
>