diff mbox series

[ovs-dev,v1,09/12] ovn-br-controller: Program the openflows to the bridges.

Message ID 20250811101019.917977-1-numans@ovn.org
State Changes Requested
Headers show
Series OVN Bridge Controller service. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Numan Siddique Aug. 11, 2025, 10:10 a.m. UTC
From: Numan Siddique <numans@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 br-controller/automake.mk         |   2 +
 br-controller/br-ofctrl.c         | 731 ++++++++++++++++++++++++++++++
 br-controller/br-ofctrl.h         |  33 ++
 br-controller/en-bridge-data.c    |  40 ++
 br-controller/en-bridge-data.h    |   4 +
 br-controller/ovn-br-controller.c | 116 ++++-
 tests/automake.mk                 |   5 +-
 tests/ovn-br-controller.at        | 327 +++++++++++++
 tests/testsuite.at                |   1 +
 9 files changed, 1253 insertions(+), 6 deletions(-)
 create mode 100644 br-controller/br-ofctrl.c
 create mode 100644 br-controller/br-ofctrl.h
 create mode 100644 tests/ovn-br-controller.at

Comments

Mark Michelson Aug. 21, 2025, 9:28 p.m. UTC | #1
On 8/11/25 6:10 AM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>   br-controller/automake.mk         |   2 +
>   br-controller/br-ofctrl.c         | 731 ++++++++++++++++++++++++++++++
>   br-controller/br-ofctrl.h         |  33 ++
>   br-controller/en-bridge-data.c    |  40 ++
>   br-controller/en-bridge-data.h    |   4 +
>   br-controller/ovn-br-controller.c | 116 ++++-
>   tests/automake.mk                 |   5 +-
>   tests/ovn-br-controller.at        | 327 +++++++++++++
>   tests/testsuite.at                |   1 +
>   9 files changed, 1253 insertions(+), 6 deletions(-)
>   create mode 100644 br-controller/br-ofctrl.c
>   create mode 100644 br-controller/br-ofctrl.h
>   create mode 100644 tests/ovn-br-controller.at
> 
> diff --git a/br-controller/automake.mk b/br-controller/automake.mk
> index 4baea4f6fe..f8cae3a098 100644
> --- a/br-controller/automake.mk
> +++ b/br-controller/automake.mk
> @@ -2,6 +2,8 @@ bin_PROGRAMS += br-controller/ovn-br-controller
>   br_controller_ovn_br_controller_SOURCES = \
>   	br-controller/br-flow-mgr.c \
>   	br-controller/br-flow-mgr.h \
> +	br-controller/br-ofctrl.c \
> +	br-controller/br-ofctrl.h \
>   	br-controller/en-bridge-data.c \
>   	br-controller/en-bridge-data.h \
>   	br-controller/en-lflow.c \
> diff --git a/br-controller/br-ofctrl.c b/br-controller/br-ofctrl.c
> new file mode 100644
> index 0000000000..6c11c43530
> --- /dev/null
> +++ b/br-controller/br-ofctrl.c
> @@ -0,0 +1,731 @@
> +/*
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +/* OVS includes. */
> +#include "bitmap.h"
> +#include "byte-order.h"
> +#include "dirs.h"
> +#include "dp-packet.h"
> +#include "flow.h"
> +#include "hash.h"
> +#include "hindex.h"
> +#include "lib/socket-util.h"
> +#include "lib/util.h"
> +#include "lib/vswitch-idl.h"
> +#include "openflow/openflow.h"
> +#include "openvswitch/dynamic-string.h"
> +#include "openvswitch/hmap.h"
> +#include "openvswitch/list.h"
> +#include "openvswitch/match.h"
> +#include "openvswitch/ofp-actions.h"
> +#include "openvswitch/ofp-bundle.h"
> +#include "openvswitch/ofp-flow.h"
> +#include "openvswitch/ofp-group.h"
> +#include "openvswitch/ofp-match.h"
> +#include "openvswitch/ofp-msgs.h"
> +#include "openvswitch/ofp-meter.h"
> +#include "openvswitch/ofp-packet.h"
> +#include "openvswitch/ofp-print.h"
> +#include "openvswitch/ofp-util.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openvswitch/vlog.h"
> +#include "openvswitch/poll-loop.h"
> +#include "openvswitch/rconn.h"
> +
> +/* OVN includes. */
> +#include "br-flow-mgr.h"
> +#include "en-bridge-data.h"
> +#include "br-ofctrl.h"
> +#include "lib/ovn-util.h"
> +#include "lib/ovn-br-idl.h"
> +
> +VLOG_DEFINE_THIS_MODULE(brofctrl);
> +
> +/* Connection state machine. */
> +#define STATES                                  \
> +    STATE(S_NEW)                                \
> +    STATE(S_WAIT_BEFORE_CLEAR)                  \
> +    STATE(S_CLEAR_FLOWS)                        \
> +    STATE(S_UPDATE_FLOWS)
> +
> +enum br_ofctrl_state {
> +#define STATE(NAME) NAME,
> +    STATES
> +#undef STATE
> +};
> +
> +/* An in-flight update to the switch's flow table.
> + *
> + * When we receive a barrier reply from the switch with the given 'xid', we
> + * know that the switch is caught up to the requested sequence number
> + * 'req_cfg' (and make that available to the client via ofctrl_get_cur_cfg(),
> + * so that it can store it into external state, e.g., our Chassis record's
> + * nb_cfg column). */
> +struct br_ofctrl_flow_update {
> +    struct ovs_list list_node;  /* In 'flow_updates'. */
> +    ovs_be32 xid;               /* OpenFlow transaction ID for barrier. */
> +    uint64_t req_cfg;           /* Requested sequence number. */
> +};
> +
> +struct br_ofctrl {
> +    struct hmap_node hmap_node;
> +    char *bridge; /* key. */
> +
> +    /* OpenFlow connection to the switch. */
> +    struct rconn *swconn;
> +    int probe_interval;
> +    char *conn_target;
> +
> +    unsigned int wait_before_clear_time;
> +    /* The time when the state S_WAIT_BEFORE_CLEAR should complete.
> +     * If the timer is not started yet, it is set to 0. */
> +    long long int wait_before_clear_expire;
> +
> +    /* Currently in-flight updates. */
> +    struct ovs_list flow_updates;
> +
> +    /* req_cfg of latest committed flow update. */
> +    uint64_t cur_cfg;
> +    uint64_t old_req_cfg;
> +    bool skipped_last_time;
> +
> +    /* Indicates if we just went through the S_CLEAR_FLOWS state, which means
> +     * we need to perform a one time deletion for all the existing flows,
> +     * groups and meters. This can happen during initialization or OpenFlow
> +     * reconnection (e.g. after OVS restart). */
> +    bool ofctrl_initial_clear;
> +
> +    /* Last seen sequence number for 'swconn'.  When this differs from
> +     * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
> +    unsigned int seqno;
> +
> +    /* Counter for in-flight OpenFlow messages on 'swconn'.  We only send a new
> +     * round of flow table modifications to the switch when the counter falls
> +     * to zero, to avoid unbounded buffering. */
> +    struct rconn_packet_counter *tx_counter;
> +
> +    /* Current state. */
> +    enum br_ofctrl_state state;
> +};
> +
> +static struct hmap br_ofctrls = HMAP_INITIALIZER(&br_ofctrls);
> +
> +static struct br_ofctrl *br_ofctrl_get(const char *bridge);
> +static void br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
> +                          bool lflows_changed, bool pflows_changed);
> +static void br_ofctrl_destroy(struct br_ofctrl *);
> +static ovs_be32 queue_msg(struct br_ofctrl *, struct ofpbuf *);
> +static struct br_ofctrl_flow_update *br_ofctrl_flow_update_from_list_node(
> +    const struct ovs_list *);
> +static bool br_ofctrl_run__(struct br_ofctrl *);
> +static bool br_ofctrl_has_backlog(struct br_ofctrl *);
> +static bool br_ofctrl_can_put(struct br_ofctrl *);
> +
> +void
> +br_ofctrls_init(void)
> +{
> +
> +}
> +
> +void
> +br_ofctrls_destroy(void)
> +{
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH_POP (br_ofctrl, hmap_node, &br_ofctrls) {
> +        br_ofctrl_destroy(br_ofctrl);
> +    }
> +
> +    hmap_destroy(&br_ofctrls);
> +}
> +
> +void
> +br_ofctrls_add_or_update_bridge(struct ovn_bridge *br)
> +{
> +    ovs_assert(br->ovs_br);
> +
> +    struct br_ofctrl *br_ofctrl = br_ofctrl_get(br->db_br->name);
> +
> +    if (!br_ofctrl) {
> +        br_ofctrl = xzalloc(sizeof *br_ofctrl);
> +        br_ofctrl->bridge = xstrdup(br->db_br->name);
> +        br_ofctrl->swconn = rconn_create(0, 0, DSCP_DEFAULT,
> +                                         1 << OFP15_VERSION);
> +        br_ofctrl->tx_counter = rconn_packet_counter_create();
> +        ovs_list_init(&br_ofctrl->flow_updates);
> +
> +        hmap_insert(&br_ofctrls, &br_ofctrl->hmap_node,
> +                    hash_string(br_ofctrl->bridge, 0));
> +    } else {
> +        free(br_ofctrl->conn_target);
> +    }
> +
> +    br_ofctrl->probe_interval = br->probe_interval;
> +    br_ofctrl->conn_target = xstrdup(br->conn_target);
> +    br_ofctrl->wait_before_clear_time = br->wait_before_clear_time;
> +}
> +
> +void
> +br_ofctrls_remove_bridge(const char *bridge)
> +{
> +    struct br_ofctrl *br_ofctrl = br_ofctrl_get(bridge);
> +    if (br_ofctrl) {
> +        hmap_remove(&br_ofctrls, &br_ofctrl->hmap_node);
> +        br_ofctrl_destroy(br_ofctrl);
> +    }
> +}
> +
> +void
> +br_ofctrls_get_bridges(struct sset *managed_bridges)
> +{
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> +        sset_add(managed_bridges, br_ofctrl->bridge);
> +    }
> +}
> +
> +/* Runs the OpenFlow state machine against each bridge in the br_ofctrls hmap,
> + * which is local to the hypervisor on which we are running.
> + *
> + * Returns 'true' if an OpenFlow reconnect happened for any of the bridge;
> + * 'false' otherwise.
> + */
> +bool
> +br_ofctrls_run(void)
> +{
> +    bool reconnected = false;
> +
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> +        reconnected |= br_ofctrl_run__(br_ofctrl);
> +    }
> +
> +    return reconnected;
> +}
> +
> +/* Programs the flow table on the switch, if possible, by the flows
> + * added to the br-flow-mgr.
> + *
> + * This should be called after br_ofctrls_run() within the main loop. */
> +void
> +br_ofctrls_put(uint64_t req_cfg, bool lflows_changed, bool pflows_changed)
> +{
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> +        br_ofctrl_put(br_ofctrl, req_cfg, lflows_changed, pflows_changed);
> +    }
> +}
> +
> +void
> +br_ofctrls_wait(void)
> +{
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> +        rconn_run_wait(br_ofctrl->swconn);
> +        rconn_recv_wait(br_ofctrl->swconn);
> +    }
> +}
> +
> +uint64_t
> +br_ofctrl_get_cur_cfg(void)
> +{
> +    uint64_t of_cur_cfg = UINT64_MAX;
> +    struct br_ofctrl *br_ofctrl;
> +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> +        of_cur_cfg = MIN(of_cur_cfg, br_ofctrl->cur_cfg);
> +    }
> +
> +    return of_cur_cfg;
> +}
> +
> +/* Static functions. */
> +
> +static void
> +br_ofctrl_destroy(struct br_ofctrl *br_ofctrl)
> +{
> +    rconn_destroy(br_ofctrl->swconn);
> +    rconn_packet_counter_destroy(br_ofctrl->tx_counter);
> +    free(br_ofctrl->bridge);
> +    free(br_ofctrl);
> +}
> +
> +static struct br_ofctrl *
> +br_ofctrl_get(const char *bridge)
> +{
> +    struct br_ofctrl *br_ofctrl;
> +    uint32_t hash = hash_string(bridge, 0);
> +    HMAP_FOR_EACH_WITH_HASH (br_ofctrl, hmap_node, hash, &br_ofctrls) {
> +        if (!strcmp(br_ofctrl->bridge, bridge)) {
> +            return br_ofctrl;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static ovs_be32
> +queue_msg(struct br_ofctrl *br_ofctrl, struct ofpbuf *msg)
> +{
> +    const struct ofp_header *oh = msg->data;
> +    ovs_be32 xid_ = oh->xid;
> +    rconn_send(br_ofctrl->swconn, msg, br_ofctrl->tx_counter);
> +    return xid_;
> +}
> +
> +static void
> +log_openflow_rl(struct vlog_rate_limit *rl, enum vlog_level level,
> +                const struct ofp_header *oh, const char *title)
> +{
> +    if (!vlog_should_drop(&this_module, level, rl)) {
> +        char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 2);
> +        vlog(&this_module, level, "%s: %s", title, s);
> +        free(s);
> +    }
> +}
> +
> +static struct br_ofctrl_flow_update *
> +br_ofctrl_flow_update_from_list_node(const struct ovs_list *list_node)
> +{
> +    return CONTAINER_OF(list_node, struct br_ofctrl_flow_update, list_node);
> +}
> +
> +/* ofctrl state machine functions. */
> +
> +static void
> +ofctrl_recv(struct br_ofctrl *br_ofctrl, const struct ofp_header *oh,
> +            enum ofptype type)
> +{
> +    if (type == OFPTYPE_ECHO_REQUEST) {
> +        queue_msg(br_ofctrl, ofputil_encode_echo_reply(oh));
> +    } else if (type == OFPTYPE_ERROR) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
> +        log_openflow_rl(&rl, VLL_INFO, oh, "OpenFlow error");
> +        rconn_reconnect(br_ofctrl->swconn);
> +    } else {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
> +        log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored");
> +    }
> +}
> +
> +
> +/* S_NEW, for a new connection.
> + *
> + */
> +
> +static void
> +run_S_NEW(struct br_ofctrl *br_ofctrl)
> +{
> +    br_ofctrl->state = S_WAIT_BEFORE_CLEAR;
> +}
> +
> +static void
> +recv_S_NEW(struct br_ofctrl *br_ofctrl OVS_UNUSED,
> +           const struct ofp_header *oh OVS_UNUSED,
> +           enum ofptype type OVS_UNUSED)
> +{
> +    OVS_NOT_REACHED();
> +}
> +
> +/* S_WAIT_BEFORE_CLEAR, we are almost ready to set up flows, but just wait for
> + * a while until the initial flow compute to complete before we clear the
> + * existing flows in OVS, so that we won't end up with an empty flow table,
> + * which may cause data plane down time. */
> +static void
> +run_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl)
> +{
> +    if (!br_ofctrl->wait_before_clear_time ||
> +        (br_ofctrl->wait_before_clear_expire &&
> +         time_msec() >= br_ofctrl->wait_before_clear_expire)) {
> +        br_ofctrl->state = S_CLEAR_FLOWS;
> +        return;
> +    }
> +
> +    if (!br_ofctrl->wait_before_clear_expire) {
> +        /* Start the timer. */
> +        br_ofctrl->wait_before_clear_expire =
> +            time_msec() + br_ofctrl->wait_before_clear_time;
> +    }
> +    poll_timer_wait_until(br_ofctrl->wait_before_clear_expire);
> +}
> +
> +static void
> +recv_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl,
> +                         const struct ofp_header *oh, enum ofptype type)
> +{
> +    ofctrl_recv(br_ofctrl, oh, type);
> +}
> +
> +/* Sends an OFPT_TABLE_MOD to clear all flows, then transitions to
> + * S_UPDATE_FLOWS. */
> +
> +static void
> +run_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl)
> +{
> +    VLOG_DBG("clearing all flows for bridge %s", br_ofctrl->bridge);
> +
> +    /* Set the flag so that the ofctrl_run() can clear the existing flows,
> +     * groups and meters. We clear them in ofctrl_run() right before the new
> +     * ones are installed to avoid data plane downtime. */
> +    br_ofctrl->ofctrl_initial_clear = true;
> +
> +    /* Clear installed_flows, to match the state of the switch. */
> +    br_flow_flush_oflows(br_ofctrl->bridge);
> +
> +    /* All flow updates are irrelevant now. */
> +    struct br_ofctrl_flow_update *fup;
> +    LIST_FOR_EACH_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
> +        ovs_list_remove(&fup->list_node);
> +        free(fup);
> +    }
> +
> +    br_ofctrl->state = S_UPDATE_FLOWS;
> +
> +    /* Give a chance for the main loop to call ofctrl_put() in case there were

There are a couple of places in the file that reference ofctrl_put() 
instead of br_ofctrl_put().

> +     * pending flows waiting ofctrl state change to S_UPDATE_FLOWS. */
> +    poll_immediate_wake();
> +}
> +
> +static void
> +recv_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl,
> +                   const struct ofp_header *oh, enum ofptype type)
> +{
> +    ofctrl_recv(br_ofctrl, oh, type);
> +}
> +
> +/* S_UPDATE_FLOWS, for maintaining the flow table over time.
> + *
> + * Compare the installed flows to the ones we want.  Send OFPT_FLOW_MOD as
> + * necessary.
> + *
> + * This is a terminal state.  We only transition out of it if the connection
> + * drops. */
> +
> +static void
> +run_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl OVS_UNUSED)
> +{
> +    /* Nothing to do here.
> +     *
> +     * Being in this state enables ofctrl_put() to work, however. */
> +}
> +
> +static void
> +br_flow_updates_handle_barrier_reply(struct br_ofctrl *br_ofctrl,
> +                                     const struct ofp_header *oh)
> +{
> +    if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> +        return;
> +    }
> +
> +    struct br_ofctrl_flow_update *fup = br_ofctrl_flow_update_from_list_node(
> +        ovs_list_front(&br_ofctrl->flow_updates));
> +    if (fup->xid == oh->xid) {
> +        if (fup->req_cfg >= br_ofctrl->cur_cfg) {
> +            br_ofctrl->cur_cfg = fup->req_cfg;
> +        }
> +        ovs_list_remove(&fup->list_node);
> +        free(fup);
> +    }
> +}
> +
> +static void
> +recv_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl,
> +                    const struct ofp_header *oh, enum ofptype type)
> +{
> +    if (type == OFPTYPE_BARRIER_REPLY) {
> +        br_flow_updates_handle_barrier_reply(br_ofctrl, oh);
> +    } else {
> +        ofctrl_recv(br_ofctrl, oh, type);
> +    }
> +}
> +
> +static bool
> +br_ofctrl_run__(struct br_ofctrl *br_ofctrl)
> +{
> +    struct rconn *swconn = br_ofctrl->swconn;
> +
> +    ovn_update_swconn_at(swconn, br_ofctrl->conn_target,
> +                         br_ofctrl->probe_interval, "br_ofctrl");
> +    rconn_run(swconn);
> +
> +    if (!rconn_is_connected(swconn)) {
> +        return false;
> +    }
> +
> +    bool reconnected = false;
> +
> +    if (br_ofctrl->seqno != rconn_get_connection_seqno(swconn)) {
> +        br_ofctrl->seqno = rconn_get_connection_seqno(swconn);
> +        reconnected = true;
> +        br_ofctrl->state = S_NEW;
> +    }
> +
> +    bool progress = true;
> +    for (int i = 0; progress && i < 50; i++) {
> +        /* Allow the state machine to run. */
> +        enum br_ofctrl_state old_state = br_ofctrl->state;
> +        switch (br_ofctrl->state) {
> +#define STATE(NAME) case NAME: run_##NAME(br_ofctrl); break;
> +            STATES
> +#undef STATE
> +        default:
> +            OVS_NOT_REACHED();
> +        }
> +
> +        /* Try to process a received packet. */
> +        struct ofpbuf *msg = rconn_recv(swconn);
> +        if (msg) {
> +            const struct ofp_header *oh = msg->data;
> +            enum ofptype type;
> +            enum ofperr error;
> +
> +            error = ofptype_decode(&type, oh);
> +            if (!error) {
> +                switch (br_ofctrl->state) {
> +#define STATE(NAME) case NAME: recv_##NAME(br_ofctrl, oh, type); break;
> +                    STATES
> +#undef STATE
> +                default:
> +                    OVS_NOT_REACHED();
> +                }
> +            } else {
> +                char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 1);
> +                VLOG_WARN("could not decode OpenFlow message (%s): %s",
> +                          ofperr_to_string(error), s);
> +                free(s);
> +            }
> +
> +            ofpbuf_delete(msg);
> +        }
> +
> +        /* If we did some work, plan to go around again. */
> +        progress = old_state != br_ofctrl->state || msg;
> +    }
> +    if (progress) {
> +        /* We bailed out to limit the amount of work we do in one go, to allow
> +         * other code a chance to run.  We were still making progress at that
> +         * point, so ensure that we come back again without waiting. */
> +        poll_immediate_wake();
> +    }
> +
> +    return reconnected;
> +}
> +
> +static bool
> +br_ofctrl_has_backlog(struct br_ofctrl *br_ofctrl)
> +{
> +    if (rconn_packet_counter_n_packets(br_ofctrl->tx_counter)
> +        || rconn_get_version(br_ofctrl->swconn) < 0) {
> +        return true;
> +    }
> +    return false;
> +}
> +
> +/* The flow table can be updated if the connection to the switch is up and
> + * in the correct state and not backlogged with existing flow_mods.  (Our
> + * criteria for being backlogged appear very conservative, but the socket
> + * between ovn-controller and OVS provides some buffering.) */
> +static bool
> +br_ofctrl_can_put(struct br_ofctrl *br_ofctrl)
> +{
> +    if (br_ofctrl->state != S_UPDATE_FLOWS
> +        || br_ofctrl_has_backlog(br_ofctrl)) {
> +        return false;
> +    }
> +    return true;
> +}
> +
> +static struct ofpbuf *
> +encode_flow_mod(struct ofputil_flow_mod *fm)
> +{
> +    fm->buffer_id = UINT32_MAX;
> +    fm->out_port = OFPP_ANY;
> +    fm->out_group = OFPG_ANY;
> +    return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF15_OXM);
> +}
> +
> +static struct ofpbuf *
> +encode_bundle_add(struct ofpbuf *msg, struct ofputil_bundle_ctrl_msg *bc)
> +{
> +    struct ofputil_bundle_add_msg bam = {
> +        .bundle_id = bc->bundle_id,
> +        .flags     = bc->flags,
> +        .msg       = msg->data,
> +    };
> +    return ofputil_encode_bundle_add(OFP15_VERSION, &bam);
> +}
> +
> +static bool
> +add_flow_mod(struct ofputil_flow_mod *fm,
> +             struct ofputil_bundle_ctrl_msg *bc,
> +             struct ovs_list *msgs)
> +{
> +    struct ofpbuf *msg = encode_flow_mod(fm);
> +    struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
> +
> +    uint32_t flow_mod_len = msg->size;
> +    uint32_t bundle_len = bundle_msg->size;
> +
> +    ofpbuf_delete(msg);
> +
> +    if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
> +        ofpbuf_delete(bundle_msg);
> +
> +        return false;
> +    }
> +
> +    ovs_list_push_back(msgs, &bundle_msg->list_node);
> +    return true;
> +}
> +
> +static void
> +br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
> +              bool lflows_changed, bool pflows_changed)
> +{
> +    bool need_put = false;
> +
> +    if (lflows_changed || pflows_changed || br_ofctrl->skipped_last_time ||
> +        br_ofctrl->ofctrl_initial_clear) {
> +        need_put = true;
> +        br_ofctrl->old_req_cfg = req_cfg;
> +    } else if (req_cfg != br_ofctrl->old_req_cfg) {
> +        /* req_cfg changed since last ofctrl_put() call */
> +        if (br_ofctrl->cur_cfg == br_ofctrl->old_req_cfg) {
> +            /* If there are no updates pending, we were up-to-date already,
> +             * update with the new req_cfg.
> +             */
> +            if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> +                br_ofctrl->cur_cfg = req_cfg;
> +                br_ofctrl->old_req_cfg = req_cfg;
> +            }
> +        } else {
> +            need_put = true;
> +            br_ofctrl->old_req_cfg = req_cfg;
> +        }
> +    }
> +
> +    if (!need_put) {
> +        VLOG_DBG("ofctrl_put not needed for bridge %s", br_ofctrl->bridge);
> +        return;
> +    }
> +
> +    /* OpenFlow messages to send to the switch to bring it up-to-date. */
> +    struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
> +
> +    if (!br_ofctrl_can_put(br_ofctrl)) {
> +        VLOG_DBG("ofctrl_put can't be performed for bridge %s",
> +                 br_ofctrl->bridge);
> +
> +        br_ofctrl->skipped_last_time = true;
> +        return;
> +    }
> +
> +    /* Add all flow updates into a bundle. */
> +    static int bundle_id = 0;
> +    struct ofputil_bundle_ctrl_msg bc = {
> +        .bundle_id = bundle_id++,
> +        .flags     = OFPBF_ORDERED | OFPBF_ATOMIC,
> +    };
> +    struct ofpbuf *bundle_open, *bundle_commit;
> +
> +    /* Open a new bundle. */
> +    bc.type = OFPBCT_OPEN_REQUEST;
> +    bundle_open = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
> +    ovs_list_push_back(&msgs, &bundle_open->list_node);
> +
> +    if (br_ofctrl->ofctrl_initial_clear) {
> +        /* Send a flow_mod to delete all flows. */
> +        struct ofputil_flow_mod fm = {
> +            .table_id = OFPTT_ALL,
> +            .command = OFPFC_DELETE,
> +        };
> +        minimatch_init_catchall(&fm.match);
> +        add_flow_mod(&fm, &bc, &msgs);
> +        minimatch_destroy(&fm.match);
> +
> +        br_ofctrl->ofctrl_initial_clear = false;
> +    }
> +
> +    br_flow_populate_oflow_msgs(br_ofctrl->bridge, &msgs);
> +
> +    if (ovs_list_back(&msgs) == &bundle_open->list_node) {
> +        /* No flow updates.  Removing the bundle open request. */
> +        ovs_list_pop_back(&msgs);
> +        ofpbuf_delete(bundle_open);
> +    } else {
> +        /* Committing the bundle. */
> +        bc.type = OFPBCT_COMMIT_REQUEST;
> +        bundle_commit = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
> +        ovs_list_push_back(&msgs, &bundle_commit->list_node);
> +    }
> +
> +    if (!ovs_list_is_empty(&msgs)) {
> +        /* Add a barrier to the list of messages. */
> +        struct ofpbuf *barrier = ofputil_encode_barrier_request(OFP15_VERSION);
> +        const struct ofp_header *oh = barrier->data;
> +        ovs_be32 xid_ = oh->xid;
> +        ovs_list_push_back(&msgs, &barrier->list_node);
> +
> +        /* Queue the messages. */
> +        struct ofpbuf *msg;
> +        LIST_FOR_EACH_POP (msg, list_node, &msgs) {
> +            queue_msg(br_ofctrl, msg);
> +        }
> +
> +        /* Track the flow update. */
> +        struct br_ofctrl_flow_update *fup;
> +        LIST_FOR_EACH_REVERSE_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
> +            if (req_cfg < fup->req_cfg) {
> +                /* This ofctrl_flow_update is for a configuration later than
> +                 * 'req_cfg'.  This should not normally happen, because it
> +                 * means that the local seqno decreased and it should normally
> +                 * be monotonically increasing. */
> +                VLOG_WARN("req_cfg regressed from %"PRId64" to %"PRId64,
> +                          fup->req_cfg, req_cfg);
> +                ovs_list_remove(&fup->list_node);
> +                free(fup);
> +            } else if (req_cfg == fup->req_cfg) {
> +                /* This ofctrl_flow_update is for the same configuration as
> +                 * 'req_cfg'.  Probably, some change to the physical topology
> +                 * means that we had to revise the OpenFlow flow table even
> +                 * though the logical topology did not change.  Update fp->xid,
> +                 * so that we don't send a notification that we're up-to-date
> +                 * until we're really caught up. */
> +                VLOG_DBG("advanced xid target for req_cfg=%"PRId64, req_cfg);
> +                fup->xid = xid_;
> +                goto done;

This is the only place that the "done:" label is used. The "done:" label 
is in a place where the only thing that can happen is that the function 
immediately returns.

So instead of "goto done;" and the odd label placement, you can just 
"return;" here.

> +            } else {
> +                break;
> +            }
> +        }
> +
> +        /* Add a flow update. */
> +        fup = xmalloc(sizeof *fup);
> +        ovs_list_push_back(&br_ofctrl->flow_updates, &fup->list_node);
> +        fup->xid = xid_;
> +        fup->req_cfg = req_cfg;
> +    done:;
> +    } else if (!ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> +        /* Getting up-to-date with 'req_cfg' didn't require any extra flow
> +         * table changes, so whenever we get up-to-date with the most recent
> +         * flow table update, we're also up-to-date with 'req_cfg'. */
> +        struct br_ofctrl_flow_update *fup =
> +            br_ofctrl_flow_update_from_list_node(
> +                ovs_list_back(&br_ofctrl->flow_updates));
> +        fup->req_cfg = req_cfg;
> +    } else {
> +        /* We were completely up-to-date before and still are. */
> +        br_ofctrl->cur_cfg = req_cfg;
> +    }
> +}
> diff --git a/br-controller/br-ofctrl.h b/br-controller/br-ofctrl.h
> new file mode 100644
> index 0000000000..9b629e2123
> --- /dev/null
> +++ b/br-controller/br-ofctrl.h
> @@ -0,0 +1,33 @@
> +/*
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef BR_OFCTRL_H
> +#define BR_OFCTRL_H 1
> +
> +struct ovn_bridge;
> +struct shash;
> +
> +void br_ofctrls_init(void);
> +bool br_ofctrls_run(void);
> +void br_ofctrls_put(uint64_t req_cfg, bool lflows_changed,
> +                    bool pflows_changed);
> +void br_ofctrls_destroy(void);
> +void br_ofctrls_wait(void);
> +
> +void br_ofctrls_add_or_update_bridge(struct ovn_bridge *);
> +void br_ofctrls_remove_bridge(const char *);
> +uint64_t br_ofctrl_get_cur_cfg(void);
> +void br_ofctrls_get_bridges(struct sset *);
> +
> +#endif /* BR_OFCTRL_H */
> \ No newline at end of file
> diff --git a/br-controller/en-bridge-data.c b/br-controller/en-bridge-data.c
> index 483c784a37..5c10a1f1f1 100644
> --- a/br-controller/en-bridge-data.c
> +++ b/br-controller/en-bridge-data.c
> @@ -26,6 +26,7 @@
>   
>   /* OVN includes. */
>   #include "en-bridge-data.h"
> +#include "lib/dirs.h"
>   #include "lib/ovn-br-idl.h"
>   
>   VLOG_DEFINE_THIS_MODULE(en_bridge_data);
> @@ -40,6 +41,7 @@ static const struct ovsrec_bridge *ovsbridge_lookup_by_name(
>       struct ovsdb_idl_index *ovsrec_bridge_by_name,
>       const char *name);
>   static void build_ovn_bridge_iface_simap(struct ovn_bridge *);
> +static void update_ovn_br_remote(struct ovn_bridge *);
>   
>   void *
>   en_bridge_data_init(struct engine_node *node OVS_UNUSED,
> @@ -114,6 +116,7 @@ ovn_bridges_run(const struct ovnbrrec_bridge_table *br_table,
>   
>           br->ovs_br = ovs_br;
>           build_ovn_bridge_iface_simap(br);
> +        update_ovn_br_remote(br);
>       }
>   }
>   
> @@ -121,6 +124,7 @@ static void
>   ovn_bridge_destroy(struct ovn_bridge *br)
>   {
>       simap_destroy(&br->ovs_ifaces);
> +    free(br->conn_target);
>       free(br);
>   }
>   
> @@ -157,3 +161,39 @@ build_ovn_bridge_iface_simap(struct ovn_bridge *br)
>           }
>       }
>   }
> +
> +static void
> +update_ovn_br_remote(struct ovn_bridge *br)
> +{
> +    ovs_assert(br->ovs_br);
> +
> +    const char *ext_target = smap_get(&br->ovs_br->external_ids,
> +                                      "ovn-bridge-remote");
> +    char *target = ext_target
> +        ? xstrdup(ext_target)
> +        : xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br->ovs_br->name);
> +
> +    if (!br->conn_target || strcmp(br->conn_target, target)) {
> +        free(br->conn_target);
> +        br->conn_target = target;
> +    } else {
> +        free(target);
> +    }
> +
> +    unsigned long long probe_interval =
> +        smap_get_ullong(&br->ovs_br->external_ids,
> +                        "ovn-openflow-remote-probe-interval", 0);
> +    br->probe_interval = MIN(probe_interval / 1000, INT_MAX);
> +
> +    unsigned int _wait_before_clear_time =
> +        smap_get_uint(&br->ovs_br->external_ids,
> +                      "ovn-ofctrl-wait-before-clear", 0);
> +
> +    if (_wait_before_clear_time != br->wait_before_clear_time) {
> +        VLOG_INFO("ofctrl-wait-before-clear is now %u ms (was %u ms) "
> +                  "for bridge %s",
> +                  _wait_before_clear_time, br->wait_before_clear_time,
> +                  br->ovs_br->name);
> +        br->wait_before_clear_time = _wait_before_clear_time;
> +    }
> +}
> diff --git a/br-controller/en-bridge-data.h b/br-controller/en-bridge-data.h
> index b374798649..05ab556637 100644
> --- a/br-controller/en-bridge-data.h
> +++ b/br-controller/en-bridge-data.h
> @@ -26,6 +26,10 @@ struct ovn_bridge {
>   
>       /* simap of ovs interface names to ofport numbers. */
>       struct simap ovs_ifaces;
> +
> +    int probe_interval;
> +    char *conn_target;
> +    unsigned int wait_before_clear_time;
>   };
>   
>   struct ed_type_bridge_data {
> diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
> index ae0e192429..74f2b7a2d2 100644
> --- a/br-controller/ovn-br-controller.c
> +++ b/br-controller/ovn-br-controller.c
> @@ -35,11 +35,13 @@
>   
>   
>   /* OVN includes. */
> +#include "br-ofctrl.h"
>   #include "en-bridge-data.h"
>   #include "en-lflow.h"
>   #include "en-pflow.h"
>   #include "lib/ovn-br-idl.h"
>   #include "lib/inc-proc-eng.h"
> +#include "lib/ofctrl-seqno.h"
>   #include "lib/ovn-util.h"
>   
>   VLOG_DEFINE_THIS_MODULE(main);
> @@ -55,6 +57,9 @@ static const char *ssl_ca_cert_file;
>   /* --unixctl-path: Path to use for unixctl server socket. */
>   static char *unixctl_path;
>   
> +/* Registered ofctrl seqno type for br_cfg propagation. */
> +static size_t ofctrl_seq_type_br_cfg;
> +
>   #define BRCTL_NODES \
>       BRCTL_NODE(br_global) \
>       BRCTL_NODE(bridge) \
> @@ -110,7 +115,12 @@ en_br_controller_output_run(struct engine_node *node OVS_UNUSED,
>   /* Static function declarations. */
>   static void ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl);
>   static void update_br_db(struct ovsdb_idl *ovs_idl,
> -                         struct ovsdb_idl *ovn_br_idl);
> +                         struct ovsdb_idl *ovnbr_idl,
> +                         unsigned int *ovnbr_cond_seqno);
> +static unsigned int update_ovnbr_monitors(struct ovsdb_idl *);
> +static uint64_t get_ovnbr_cfg(const struct ovnbrrec_br_global_table *,
> +                                  unsigned int cond_seqno,
> +                                  unsigned int expected_cond_seqno);
>   
>   int
>   main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> @@ -138,6 +148,9 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>   
>       daemonize_complete();
>   
> +    /* Register ofctrl seqno types. */
> +    ofctrl_seq_type_br_cfg = ofctrl_seqno_add_type();
> +
>       /* Connect to OVS OVSDB instance. */
>       struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
>           ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
> @@ -206,8 +219,12 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>       engine_init(&en_br_controller_output, &engine_arg);
>       engine_ovsdb_node_add_index(&en_ovs_bridge, "name", ovsrec_bridge_by_name);
>   
> -    unsigned int ovs_cond_seqno = UINT_MAX;
> +    unsigned int ovnbr_expected_cond_seqno = UINT_MAX;
>       unsigned int ovnbr_cond_seqno = UINT_MAX;
> +    unsigned int ovs_cond_seqno = UINT_MAX;
> +
> +    struct ed_type_bridge_data *br_data =
> +        engine_get_internal_data(&en_bridge_data);
>   
>       /* Main loop. */
>       while (!exit_args.exiting) {
> @@ -224,7 +241,8 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>               ovs_cond_seqno = new_ovs_cond_seqno;
>           }
>   
> -        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl);
> +        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl,
> +                     &ovnbr_expected_cond_seqno);
>           struct ovsdb_idl_txn *ovnbr_idl_txn
>               = ovsdb_idl_loop_run(&ovnbr_idl_loop);
>           unsigned int new_ovnbr_cond_seqno
> @@ -251,10 +269,48 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>   
>           if (ovsdb_idl_has_ever_connected(ovnbr_idl_loop.idl) && cfg) {
>               engine_run(true);
> +
> +            br_data = engine_get_data(&en_bridge_data);
> +            if (br_data) {
> +                struct sset bridges_in_br_ofctrl =
> +                    SSET_INITIALIZER(&bridges_in_br_ofctrl);
> +                br_ofctrls_get_bridges(&bridges_in_br_ofctrl);
> +                struct shash_node *node;
> +                SHASH_FOR_EACH (node, &br_data->bridges) {
> +                    struct ovn_bridge *br = node->data;
> +
> +                    if (br->ovs_br) {
> +                        sset_find_and_delete(&bridges_in_br_ofctrl,
> +                                             br->db_br->name);
> +                        br_ofctrls_add_or_update_bridge(br);
> +                    }
> +                }
> +
> +                const char *bridge;
> +                SSET_FOR_EACH (bridge, &bridges_in_br_ofctrl) {
> +                    br_ofctrls_remove_bridge(bridge);
> +                }
> +
> +                sset_destroy(&bridges_in_br_ofctrl);
> +            }
> +
> +            br_ofctrls_run();
> +
> +            ofctrl_seqno_update_create(
> +                ofctrl_seq_type_br_cfg,
> +                get_ovnbr_cfg(ovnbrrec_br_global_table_get(ovnbr_idl_loop.idl),
> +                              ovnbr_cond_seqno, ovnbr_expected_cond_seqno));
> +
> +            br_ofctrls_put(ofctrl_seqno_get_req_cfg(),
> +                           engine_node_changed(&en_lflow_output),
> +                           engine_node_changed(&en_pflow_output));
> +
> +            ofctrl_seqno_run(br_ofctrl_get_cur_cfg());
>           }
>   
>           unixctl_server_run(unixctl);
>   
> +        br_ofctrls_wait();
>           unixctl_server_wait(unixctl);
>           if (exit_args.exiting) {
>               poll_immediate_wake();
> @@ -440,7 +496,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>   /* Retrieves the pointer to the OVN Bridge Controller database from 'ovs_idl'
>    * and updates 'brdb_idl' with that pointer. */
>   static void
> -update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
> +update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl,
> +             unsigned int *ovnbr_cond_seqno)
>   {
>       const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
>       if (!cfg) {
> @@ -449,4 +506,55 @@ update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
>   
>       const char *remote = smap_get(&cfg->external_ids, "ovn-br-remote");
>       ovsdb_idl_set_remote(ovnbr_idl, remote, true);
> +
> +    unsigned int next_cond_seqno = update_ovnbr_monitors(ovnbr_idl);
> +    if (ovnbr_cond_seqno) {
> +        *ovnbr_cond_seqno = next_cond_seqno;
> +    }
> +}
> +
> +/* Assume the table exists in the server schema and set its condition. */
> +#define ovnbr_table_set_req_mon_condition(idl, table, cond) \
> +    ovnbrrec_##table##_set_condition(idl, cond)
> +
> +static unsigned int
> +update_ovnbr_monitors(struct ovsdb_idl *ovnbr_idl)
> +{
> +    struct ovsdb_idl_condition br = OVSDB_IDL_CONDITION_INIT(&br);
> +    struct ovsdb_idl_condition lf = OVSDB_IDL_CONDITION_INIT(&lf);
> +
> +    ovsdb_idl_condition_add_clause_true(&br);
> +    ovsdb_idl_condition_add_clause_true(&lf);
> +
> +    unsigned int cond_seqnos[] = {
> +        ovnbr_table_set_req_mon_condition(ovnbr_idl, bridge, &br),
> +        ovnbr_table_set_req_mon_condition(ovnbr_idl, logical_flow, &lf),
> +    };
> +
> +    unsigned int expected_cond_seqno = 0;
> +    for (size_t i = 0; i < ARRAY_SIZE(cond_seqnos); i++) {
> +        expected_cond_seqno = MAX(expected_cond_seqno, cond_seqnos[i]);
> +    }
> +
> +    return expected_cond_seqno;
> +}
> +
> +static uint64_t
> +get_ovnbr_cfg(const struct ovnbrrec_br_global_table *br_global_table,
> +              unsigned int cond_seqno, unsigned int expected_cond_seqno)
> +{
> +    static uint64_t br_cfg = 0;
> +
> +    /* Delay getting br_cfg if there are monitor condition changes
> +     * in flight.  It might be that those changes would instruct the
> +     * server to send updates that happened before PR_Global.pr_cfg.
> +     */
> +    if (cond_seqno != expected_cond_seqno) {
> +        return br_cfg;
> +    }
> +
> +    const struct ovnbrrec_br_global *br_global
> +        = ovnbrrec_br_global_table_first(br_global_table);
> +    br_cfg = br_global ? br_global->br_cfg : 0;
> +    return br_cfg;
>   }
> diff --git a/tests/automake.mk b/tests/automake.mk
> index e6a31c6f69..84c0ea2219 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -46,7 +46,8 @@ TESTSUITE_AT = \
>   	tests/ovn-lflow-conj-ids.at \
>   	tests/ovn-ipsec.at \
>   	tests/ovn-vif-plug.at \
> -	tests/ovn-util.at
> +	tests/ovn-util.at \
> +	tests/ovn-br-controller.at
>   
>   SYSTEM_DPDK_TESTSUITE_AT = \
>   	tests/system-dpdk-testsuite.at \
> @@ -89,7 +90,7 @@ DISTCLEANFILES += tests/atconfig tests/atlocal
>   MULTINODE_TESTSUITE = $(srcdir)/tests/multinode-testsuite
>   MULTINODE_TESTSUITE_DIR = $(abs_top_builddir)/tests/multinode-testsuite.dir
>   MULTINODE_TESTSUITE_RESULTS = $(MULTINODE_TESTSUITE_DIR)/results
> -AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic
> +AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic:br-controller
>   
>   export ovs_srcdir
>   export ovs_builddir
> diff --git a/tests/ovn-br-controller.at b/tests/ovn-br-controller.at
> new file mode 100644
> index 0000000000..bb10e5bd10
> --- /dev/null
> +++ b/tests/ovn-br-controller.at
> @@ -0,0 +1,327 @@
> +AT_BANNER([ovn_br_controller])
> +
> +# OVN_BR_CONTROLLER_START(SIM_NAME)
> +#
> +# $1 - optional simulator name. If none is given, runs ovn-br-controller
> +#      in $ovs_dir.
> +# Starts the test with a setup with ovn bridge controller.  Each test case must first
> +# call this macro and ovn_start.
> +#
> +m4_define([OVN_BR_CONTROLLER_START], [
> +    AT_KEYWORDS([ovn-br-controller])
> +    mkdir -p "$ovs_dir" || return 1
> +    mkdir "$ovs_base"/ovn-br || return 1
> +
> +    dnl Create databases (vswitch).
> +    check ovsdb-tool create "$ovs_dir"/vswitchd.db $ovs_srcdir/vswitchd/vswitch.ovsschema
> +    check ovsdb-tool create "$ovs_base"/ovn-br/ovn-br.db "$abs_top_srcdir"/ovn-br.ovsschema
> +
> +    dnl Start ovsdb-server.
> +    start_daemon ovsdb-server --remote=punix:"$ovs_dir"/db.sock \
> +                             "$ovs_dir"/vswitchd.db
> +
> +    ovn_br_remote=unix:"$ovs_base"/ovn-br/ovnbr_db.sock
> +    dnl Start ovs-vswitchd.
> +    start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif
> +
> +    ovs-vsctl \
> +        -- set Open_vSwitch . external-ids:ovn-br-remote=$ovn_br_remote
> +    dnl Start ovsdb-server for ovn-br.
> +    as ovn-br start_daemon ovsdb-server --remote=punix:"$ovs_base"/ovn-br/ovnbr_db.sock \
> +        "$ovs_base"/ovn-br/ovn-br.db
> +
> +    which ovn-br-controller
> +    dnl Start ovn-br-controller.
> +    start_daemon ovn-br-controller
> +])
> +
> +m4_define([OVN_BR_CONTROLLER_STOP],[
> +   echo
> +   echo "Clean up ovn-br-controller related processes in $2"
> +   test -n "$2" && as "$2"
> +   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +   OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +   OVS_APP_EXIT_AND_WAIT([ovn-br-controller])
> +
> +   as ovn-br
> +   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +])
> +
> +AT_SETUP([ovn-br-controller - brctl test])
> +OVN_BR_CONTROLLER_START
> +
> +check as ovn-br ovn-brctl show
> +check as ovn-br ovn-brctl add-br br0
> +
> +AT_CHECK([as ovn-br ovn-brctl show | uuidfilt], [0],
> +  [bridge <0> (br0)
> +])
> +
> +AT_CHECK([as ovn-br ovn-brctl show br0 | uuidfilt], [0],
> +  [bridge <0> (br0)
> +])
> +
> +AT_CHECK([as ovn-br ovn-brctl show br1 | uuidfilt], [0], [],
> +  [ovn-brctl: no row "br1" in table Bridge
> +])
> +
> +check as ovn-br ovn-brctl del-br br0
> +check as ovn-br ovn-brctl show
> +
> +check as ovn-br ovn-brctl add-br br0
> +
> +check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && tcp" "drop;"
> +check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && udp" "next;"
> +check as ovn-br ovn-brctl add-flow br0 1 0 "ip4 && udp" "output;"
> +
> +check as ovn-br ovn-brctl add-br br1
> +
> +check as ovn-br ovn-brctl add-flow br1 0 1000 "ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10" "drop;"
> +check as ovn-br ovn-brctl add-flow br1 0 0 "1" "output;"
> +
> +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> +  [dnl
> +Bridge: br0 (<0>)
> +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> +  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
> +Bridge: br1 (<1>)
> +  table=0 , priority=1000 , match=(ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10), action=(drop;)
> +  table=0 , priority=0    , match=(1), action=(output;)
> +])
> +
> +as ovn-br ovn-brctl del-flows br1
> +
> +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> +  [dnl
> +Bridge: br0 (<0>)
> +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> +  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
> +])
> +
> +lflow_uuid=$(as ovn-br ovn-brctl --bare --columns _uuid find logical_flow table_id=1)
> +check as ovn-br ovn-brctl del-flow $lflow_uuid
> +
> +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> +  [dnl
> +Bridge: br0 (<0>)
> +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> +])
> +
> +OVN_BR_CONTROLLER_STOP
> +AT_CLEANUP
> +
> +AT_SETUP([ovn-br-controller - logical flows])
> +OVN_BR_CONTROLLER_START
> +
> +check as ovn-br ovn-brctl add-br br0
> +
> +check ovs-vsctl add-br br0
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 3])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> +NXST_FLOW reply:
> +])
> +
> +check ovs-vsctl add-port br0 p1 -- set interface p1 ofport-request=2
> +check ovs-vsctl add-port br0 p2 -- set interface p2 ofport-request=3
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 7])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=65, priority=100,reg15=0x3 actions=output:3
> +NXST_FLOW reply:
> +])
> +
> +check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p1"' "next;"
> +check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p2"' "drop;"
> +check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4 && tcp' "ip4.src <-> ip4.dst; tcp.dst = 8080; next;"
> +check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4' "next;"
> +check as ovn-br ovn-brctl add-flow br0 2 1000 '1' "output;"
> +
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 12])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
> + table=10, priority=1000 actions=resubmit(,64)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=65, priority=100,reg15=0x3 actions=output:3
> + table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
> + table=8, priority=1000,reg14=0x3 actions=drop
> + table=9, priority=1000,ip actions=resubmit(,10)
> + table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
> +NXST_FLOW reply:
> +])
> +
> +check ovs-vsctl del-port p2
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 9])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + table=10, priority=1000 actions=resubmit(,64)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
> + table=9, priority=1000,ip actions=resubmit(,10)
> + table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
> +NXST_FLOW reply:
> +])
> +
> +check ovs-vsctl add-br br1
> +check ovs-vsctl add-port br1 br1-p1 -- set interface br1-p1 ofport-request=1
> +check ovs-vsctl add-port br1 br1-p2 -- set interface br1-p2 ofport-request=2
> +
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 1])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> +NXST_FLOW reply:
> +])
> +
> +as ovn-br ovn-brctl --id=4830e8c3-9b6b-48db-ba52-e030d9db7256 create bridge name=br1

These UUIDs are repeated a few times in the test. It would make the test 
easier to read and update if the UUIDs were placed in a variable:

br_uuid=4830e8c3-9b6b-48db-ba52-e030d9db7256
as ovn-br ovn-brctl --id="$br_uuid" create bridge name=br1

> +as ovn-br ovn-brctl list bridge
> +
> +# check as ovn-br ovn-brctl add-br br1
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 7])
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x1 actions=output:1
> + table=65, priority=100,reg15=0x2 actions=output:2
> +NXST_FLOW reply:
> +])
> +
> +check as ovn-br ovn-brctl add-flow br1 1 1000 "ip4" "ct_snat;"
> +check as ovn-br ovn-brctl add-flow br1 2 1000 "ip4 && ct.new && ct.trk && ip4.src == 10.0.0.11" "ct_snat(100.64.0.11); next;"
> +check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p1\"" "outport = \"br1-p2\"; output;"
> +check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p2\"" "outport = \"br1-p1\"; output;"
> +
> +as ovn-br ovn-brctl --id=75bf46aa-4204-4e36-af23-6114f59e3fe8 create logical_flow \
> +match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"' \
> +actions="next;" bridge=4830e8c3-9b6b-48db-ba52-e030d9db7256 table_id=10 priority=1000
> +
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> + table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)

How is the test predicting the conj_id value?

> + table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x1 actions=output:1
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> +NXST_FLOW reply:
> +])
> +
> +as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 match='"ip4 && sctp"'
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 12])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> + table=18, priority=1000,sctp actions=resubmit(,19)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x1 actions=output:1
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> +NXST_FLOW reply:
> +])
> +
> +# Make sure that the same conj_id is used when the lflow is updated with the conj match.
> +as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 \
> +match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"'
> +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
> +
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> + priority=0 actions=NORMAL
> + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> + table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)
> + table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
> + table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
> + table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
> + table=64, priority=0 actions=resubmit(,65)
> + table=65, priority=0 actions=NORMAL
> + table=65, priority=100,reg15=0x1 actions=output:1
> + table=65, priority=100,reg15=0x2 actions=output:2
> + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> +NXST_FLOW reply:
> +])
> +
> +OVN_BR_CONTROLLER_STOP
> +AT_CLEANUP
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 8e60bf82e1..5f5eabb42a 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -41,3 +41,4 @@ m4_include([tests/checkpatch.at])
>   m4_include([tests/ovn-ipsec.at])
>   m4_include([tests/ovn-vif-plug.at])
>   m4_include([tests/ovn-util.at])
> +m4_include([tests/ovn-br-controller.at])
Numan Siddique Oct. 8, 2025, 3:45 p.m. UTC | #2
On Thu, Aug 21, 2025 at 5:28 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> On 8/11/25 6:10 AM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >   br-controller/automake.mk         |   2 +
> >   br-controller/br-ofctrl.c         | 731 ++++++++++++++++++++++++++++++
> >   br-controller/br-ofctrl.h         |  33 ++
> >   br-controller/en-bridge-data.c    |  40 ++
> >   br-controller/en-bridge-data.h    |   4 +
> >   br-controller/ovn-br-controller.c | 116 ++++-
> >   tests/automake.mk                 |   5 +-
> >   tests/ovn-br-controller.at        | 327 +++++++++++++
> >   tests/testsuite.at                |   1 +
> >   9 files changed, 1253 insertions(+), 6 deletions(-)
> >   create mode 100644 br-controller/br-ofctrl.c
> >   create mode 100644 br-controller/br-ofctrl.h
> >   create mode 100644 tests/ovn-br-controller.at
> >
> > diff --git a/br-controller/automake.mk b/br-controller/automake.mk
> > index 4baea4f6fe..f8cae3a098 100644
> > --- a/br-controller/automake.mk
> > +++ b/br-controller/automake.mk
> > @@ -2,6 +2,8 @@ bin_PROGRAMS += br-controller/ovn-br-controller
> >   br_controller_ovn_br_controller_SOURCES = \
> >       br-controller/br-flow-mgr.c \
> >       br-controller/br-flow-mgr.h \
> > +     br-controller/br-ofctrl.c \
> > +     br-controller/br-ofctrl.h \
> >       br-controller/en-bridge-data.c \
> >       br-controller/en-bridge-data.h \
> >       br-controller/en-lflow.c \
> > diff --git a/br-controller/br-ofctrl.c b/br-controller/br-ofctrl.c
> > new file mode 100644
> > index 0000000000..6c11c43530
> > --- /dev/null
> > +++ b/br-controller/br-ofctrl.c
> > @@ -0,0 +1,731 @@
> > +/*
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#include <config.h>
> > +
> > +/* OVS includes. */
> > +#include "bitmap.h"
> > +#include "byte-order.h"
> > +#include "dirs.h"
> > +#include "dp-packet.h"
> > +#include "flow.h"
> > +#include "hash.h"
> > +#include "hindex.h"
> > +#include "lib/socket-util.h"
> > +#include "lib/util.h"
> > +#include "lib/vswitch-idl.h"
> > +#include "openflow/openflow.h"
> > +#include "openvswitch/dynamic-string.h"
> > +#include "openvswitch/hmap.h"
> > +#include "openvswitch/list.h"
> > +#include "openvswitch/match.h"
> > +#include "openvswitch/ofp-actions.h"
> > +#include "openvswitch/ofp-bundle.h"
> > +#include "openvswitch/ofp-flow.h"
> > +#include "openvswitch/ofp-group.h"
> > +#include "openvswitch/ofp-match.h"
> > +#include "openvswitch/ofp-msgs.h"
> > +#include "openvswitch/ofp-meter.h"
> > +#include "openvswitch/ofp-packet.h"
> > +#include "openvswitch/ofp-print.h"
> > +#include "openvswitch/ofp-util.h"
> > +#include "openvswitch/ofpbuf.h"
> > +#include "openvswitch/vlog.h"
> > +#include "openvswitch/poll-loop.h"
> > +#include "openvswitch/rconn.h"
> > +
> > +/* OVN includes. */
> > +#include "br-flow-mgr.h"
> > +#include "en-bridge-data.h"
> > +#include "br-ofctrl.h"
> > +#include "lib/ovn-util.h"
> > +#include "lib/ovn-br-idl.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(brofctrl);
> > +
> > +/* Connection state machine. */
> > +#define STATES                                  \
> > +    STATE(S_NEW)                                \
> > +    STATE(S_WAIT_BEFORE_CLEAR)                  \
> > +    STATE(S_CLEAR_FLOWS)                        \
> > +    STATE(S_UPDATE_FLOWS)
> > +
> > +enum br_ofctrl_state {
> > +#define STATE(NAME) NAME,
> > +    STATES
> > +#undef STATE
> > +};
> > +
> > +/* An in-flight update to the switch's flow table.
> > + *
> > + * When we receive a barrier reply from the switch with the given 'xid', we
> > + * know that the switch is caught up to the requested sequence number
> > + * 'req_cfg' (and make that available to the client via ofctrl_get_cur_cfg(),
> > + * so that it can store it into external state, e.g., our Chassis record's
> > + * nb_cfg column). */
> > +struct br_ofctrl_flow_update {
> > +    struct ovs_list list_node;  /* In 'flow_updates'. */
> > +    ovs_be32 xid;               /* OpenFlow transaction ID for barrier. */
> > +    uint64_t req_cfg;           /* Requested sequence number. */
> > +};
> > +
> > +struct br_ofctrl {
> > +    struct hmap_node hmap_node;
> > +    char *bridge; /* key. */
> > +
> > +    /* OpenFlow connection to the switch. */
> > +    struct rconn *swconn;
> > +    int probe_interval;
> > +    char *conn_target;
> > +
> > +    unsigned int wait_before_clear_time;
> > +    /* The time when the state S_WAIT_BEFORE_CLEAR should complete.
> > +     * If the timer is not started yet, it is set to 0. */
> > +    long long int wait_before_clear_expire;
> > +
> > +    /* Currently in-flight updates. */
> > +    struct ovs_list flow_updates;
> > +
> > +    /* req_cfg of latest committed flow update. */
> > +    uint64_t cur_cfg;
> > +    uint64_t old_req_cfg;
> > +    bool skipped_last_time;
> > +
> > +    /* Indicates if we just went through the S_CLEAR_FLOWS state, which means
> > +     * we need to perform a one time deletion for all the existing flows,
> > +     * groups and meters. This can happen during initialization or OpenFlow
> > +     * reconnection (e.g. after OVS restart). */
> > +    bool ofctrl_initial_clear;
> > +
> > +    /* Last seen sequence number for 'swconn'.  When this differs from
> > +     * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
> > +    unsigned int seqno;
> > +
> > +    /* Counter for in-flight OpenFlow messages on 'swconn'.  We only send a new
> > +     * round of flow table modifications to the switch when the counter falls
> > +     * to zero, to avoid unbounded buffering. */
> > +    struct rconn_packet_counter *tx_counter;
> > +
> > +    /* Current state. */
> > +    enum br_ofctrl_state state;
> > +};
> > +
> > +static struct hmap br_ofctrls = HMAP_INITIALIZER(&br_ofctrls);
> > +
> > +static struct br_ofctrl *br_ofctrl_get(const char *bridge);
> > +static void br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
> > +                          bool lflows_changed, bool pflows_changed);
> > +static void br_ofctrl_destroy(struct br_ofctrl *);
> > +static ovs_be32 queue_msg(struct br_ofctrl *, struct ofpbuf *);
> > +static struct br_ofctrl_flow_update *br_ofctrl_flow_update_from_list_node(
> > +    const struct ovs_list *);
> > +static bool br_ofctrl_run__(struct br_ofctrl *);
> > +static bool br_ofctrl_has_backlog(struct br_ofctrl *);
> > +static bool br_ofctrl_can_put(struct br_ofctrl *);
> > +
> > +void
> > +br_ofctrls_init(void)
> > +{
> > +
> > +}
> > +
> > +void
> > +br_ofctrls_destroy(void)
> > +{
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH_POP (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        br_ofctrl_destroy(br_ofctrl);
> > +    }
> > +
> > +    hmap_destroy(&br_ofctrls);
> > +}
> > +
> > +void
> > +br_ofctrls_add_or_update_bridge(struct ovn_bridge *br)
> > +{
> > +    ovs_assert(br->ovs_br);
> > +
> > +    struct br_ofctrl *br_ofctrl = br_ofctrl_get(br->db_br->name);
> > +
> > +    if (!br_ofctrl) {
> > +        br_ofctrl = xzalloc(sizeof *br_ofctrl);
> > +        br_ofctrl->bridge = xstrdup(br->db_br->name);
> > +        br_ofctrl->swconn = rconn_create(0, 0, DSCP_DEFAULT,
> > +                                         1 << OFP15_VERSION);
> > +        br_ofctrl->tx_counter = rconn_packet_counter_create();
> > +        ovs_list_init(&br_ofctrl->flow_updates);
> > +
> > +        hmap_insert(&br_ofctrls, &br_ofctrl->hmap_node,
> > +                    hash_string(br_ofctrl->bridge, 0));
> > +    } else {
> > +        free(br_ofctrl->conn_target);
> > +    }
> > +
> > +    br_ofctrl->probe_interval = br->probe_interval;
> > +    br_ofctrl->conn_target = xstrdup(br->conn_target);
> > +    br_ofctrl->wait_before_clear_time = br->wait_before_clear_time;
> > +}
> > +
> > +void
> > +br_ofctrls_remove_bridge(const char *bridge)
> > +{
> > +    struct br_ofctrl *br_ofctrl = br_ofctrl_get(bridge);
> > +    if (br_ofctrl) {
> > +        hmap_remove(&br_ofctrls, &br_ofctrl->hmap_node);
> > +        br_ofctrl_destroy(br_ofctrl);
> > +    }
> > +}
> > +
> > +void
> > +br_ofctrls_get_bridges(struct sset *managed_bridges)
> > +{
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        sset_add(managed_bridges, br_ofctrl->bridge);
> > +    }
> > +}
> > +
> > +/* Runs the OpenFlow state machine against each bridge in the br_ofctrls hmap,
> > + * which is local to the hypervisor on which we are running.
> > + *
> > + * Returns 'true' if an OpenFlow reconnect happened for any of the bridge;
> > + * 'false' otherwise.
> > + */
> > +bool
> > +br_ofctrls_run(void)
> > +{
> > +    bool reconnected = false;
> > +
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        reconnected |= br_ofctrl_run__(br_ofctrl);
> > +    }
> > +
> > +    return reconnected;
> > +}
> > +
> > +/* Programs the flow table on the switch, if possible, by the flows
> > + * added to the br-flow-mgr.
> > + *
> > + * This should be called after br_ofctrls_run() within the main loop. */
> > +void
> > +br_ofctrls_put(uint64_t req_cfg, bool lflows_changed, bool pflows_changed)
> > +{
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        br_ofctrl_put(br_ofctrl, req_cfg, lflows_changed, pflows_changed);
> > +    }
> > +}
> > +
> > +void
> > +br_ofctrls_wait(void)
> > +{
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        rconn_run_wait(br_ofctrl->swconn);
> > +        rconn_recv_wait(br_ofctrl->swconn);
> > +    }
> > +}
> > +
> > +uint64_t
> > +br_ofctrl_get_cur_cfg(void)
> > +{
> > +    uint64_t of_cur_cfg = UINT64_MAX;
> > +    struct br_ofctrl *br_ofctrl;
> > +    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
> > +        of_cur_cfg = MIN(of_cur_cfg, br_ofctrl->cur_cfg);
> > +    }
> > +
> > +    return of_cur_cfg;
> > +}
> > +
> > +/* Static functions. */
> > +
> > +static void
> > +br_ofctrl_destroy(struct br_ofctrl *br_ofctrl)
> > +{
> > +    rconn_destroy(br_ofctrl->swconn);
> > +    rconn_packet_counter_destroy(br_ofctrl->tx_counter);
> > +    free(br_ofctrl->bridge);
> > +    free(br_ofctrl);
> > +}
> > +
> > +static struct br_ofctrl *
> > +br_ofctrl_get(const char *bridge)
> > +{
> > +    struct br_ofctrl *br_ofctrl;
> > +    uint32_t hash = hash_string(bridge, 0);
> > +    HMAP_FOR_EACH_WITH_HASH (br_ofctrl, hmap_node, hash, &br_ofctrls) {
> > +        if (!strcmp(br_ofctrl->bridge, bridge)) {
> > +            return br_ofctrl;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +static ovs_be32
> > +queue_msg(struct br_ofctrl *br_ofctrl, struct ofpbuf *msg)
> > +{
> > +    const struct ofp_header *oh = msg->data;
> > +    ovs_be32 xid_ = oh->xid;
> > +    rconn_send(br_ofctrl->swconn, msg, br_ofctrl->tx_counter);
> > +    return xid_;
> > +}
> > +
> > +static void
> > +log_openflow_rl(struct vlog_rate_limit *rl, enum vlog_level level,
> > +                const struct ofp_header *oh, const char *title)
> > +{
> > +    if (!vlog_should_drop(&this_module, level, rl)) {
> > +        char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 2);
> > +        vlog(&this_module, level, "%s: %s", title, s);
> > +        free(s);
> > +    }
> > +}
> > +
> > +static struct br_ofctrl_flow_update *
> > +br_ofctrl_flow_update_from_list_node(const struct ovs_list *list_node)
> > +{
> > +    return CONTAINER_OF(list_node, struct br_ofctrl_flow_update, list_node);
> > +}
> > +
> > +/* ofctrl state machine functions. */
> > +
> > +static void
> > +ofctrl_recv(struct br_ofctrl *br_ofctrl, const struct ofp_header *oh,
> > +            enum ofptype type)
> > +{
> > +    if (type == OFPTYPE_ECHO_REQUEST) {
> > +        queue_msg(br_ofctrl, ofputil_encode_echo_reply(oh));
> > +    } else if (type == OFPTYPE_ERROR) {
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
> > +        log_openflow_rl(&rl, VLL_INFO, oh, "OpenFlow error");
> > +        rconn_reconnect(br_ofctrl->swconn);
> > +    } else {
> > +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
> > +        log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored");
> > +    }
> > +}
> > +
> > +
> > +/* S_NEW, for a new connection.
> > + *
> > + */
> > +
> > +static void
> > +run_S_NEW(struct br_ofctrl *br_ofctrl)
> > +{
> > +    br_ofctrl->state = S_WAIT_BEFORE_CLEAR;
> > +}
> > +
> > +static void
> > +recv_S_NEW(struct br_ofctrl *br_ofctrl OVS_UNUSED,
> > +           const struct ofp_header *oh OVS_UNUSED,
> > +           enum ofptype type OVS_UNUSED)
> > +{
> > +    OVS_NOT_REACHED();
> > +}
> > +
> > +/* S_WAIT_BEFORE_CLEAR, we are almost ready to set up flows, but just wait for
> > + * a while until the initial flow compute to complete before we clear the
> > + * existing flows in OVS, so that we won't end up with an empty flow table,
> > + * which may cause data plane down time. */
> > +static void
> > +run_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl)
> > +{
> > +    if (!br_ofctrl->wait_before_clear_time ||
> > +        (br_ofctrl->wait_before_clear_expire &&
> > +         time_msec() >= br_ofctrl->wait_before_clear_expire)) {
> > +        br_ofctrl->state = S_CLEAR_FLOWS;
> > +        return;
> > +    }
> > +
> > +    if (!br_ofctrl->wait_before_clear_expire) {
> > +        /* Start the timer. */
> > +        br_ofctrl->wait_before_clear_expire =
> > +            time_msec() + br_ofctrl->wait_before_clear_time;
> > +    }
> > +    poll_timer_wait_until(br_ofctrl->wait_before_clear_expire);
> > +}
> > +
> > +static void
> > +recv_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl,
> > +                         const struct ofp_header *oh, enum ofptype type)
> > +{
> > +    ofctrl_recv(br_ofctrl, oh, type);
> > +}
> > +
> > +/* Sends an OFPT_TABLE_MOD to clear all flows, then transitions to
> > + * S_UPDATE_FLOWS. */
> > +
> > +static void
> > +run_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl)
> > +{
> > +    VLOG_DBG("clearing all flows for bridge %s", br_ofctrl->bridge);
> > +
> > +    /* Set the flag so that the ofctrl_run() can clear the existing flows,
> > +     * groups and meters. We clear them in ofctrl_run() right before the new
> > +     * ones are installed to avoid data plane downtime. */
> > +    br_ofctrl->ofctrl_initial_clear = true;
> > +
> > +    /* Clear installed_flows, to match the state of the switch. */
> > +    br_flow_flush_oflows(br_ofctrl->bridge);
> > +
> > +    /* All flow updates are irrelevant now. */
> > +    struct br_ofctrl_flow_update *fup;
> > +    LIST_FOR_EACH_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
> > +        ovs_list_remove(&fup->list_node);
> > +        free(fup);
> > +    }
> > +
> > +    br_ofctrl->state = S_UPDATE_FLOWS;
> > +
> > +    /* Give a chance for the main loop to call ofctrl_put() in case there were
>
> There are a couple of places in the file that reference ofctrl_put()
> instead of br_ofctrl_put().

Ack.  I'll address it in v2.

>
> > +     * pending flows waiting ofctrl state change to S_UPDATE_FLOWS. */
> > +    poll_immediate_wake();
> > +}
> > +
> > +static void
> > +recv_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl,
> > +                   const struct ofp_header *oh, enum ofptype type)
> > +{
> > +    ofctrl_recv(br_ofctrl, oh, type);
> > +}
> > +
> > +/* S_UPDATE_FLOWS, for maintaining the flow table over time.
> > + *
> > + * Compare the installed flows to the ones we want.  Send OFPT_FLOW_MOD as
> > + * necessary.
> > + *
> > + * This is a terminal state.  We only transition out of it if the connection
> > + * drops. */
> > +
> > +static void
> > +run_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl OVS_UNUSED)
> > +{
> > +    /* Nothing to do here.
> > +     *
> > +     * Being in this state enables ofctrl_put() to work, however. */
> > +}
> > +
> > +static void
> > +br_flow_updates_handle_barrier_reply(struct br_ofctrl *br_ofctrl,
> > +                                     const struct ofp_header *oh)
> > +{
> > +    if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> > +        return;
> > +    }
> > +
> > +    struct br_ofctrl_flow_update *fup = br_ofctrl_flow_update_from_list_node(
> > +        ovs_list_front(&br_ofctrl->flow_updates));
> > +    if (fup->xid == oh->xid) {
> > +        if (fup->req_cfg >= br_ofctrl->cur_cfg) {
> > +            br_ofctrl->cur_cfg = fup->req_cfg;
> > +        }
> > +        ovs_list_remove(&fup->list_node);
> > +        free(fup);
> > +    }
> > +}
> > +
> > +static void
> > +recv_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl,
> > +                    const struct ofp_header *oh, enum ofptype type)
> > +{
> > +    if (type == OFPTYPE_BARRIER_REPLY) {
> > +        br_flow_updates_handle_barrier_reply(br_ofctrl, oh);
> > +    } else {
> > +        ofctrl_recv(br_ofctrl, oh, type);
> > +    }
> > +}
> > +
> > +static bool
> > +br_ofctrl_run__(struct br_ofctrl *br_ofctrl)
> > +{
> > +    struct rconn *swconn = br_ofctrl->swconn;
> > +
> > +    ovn_update_swconn_at(swconn, br_ofctrl->conn_target,
> > +                         br_ofctrl->probe_interval, "br_ofctrl");
> > +    rconn_run(swconn);
> > +
> > +    if (!rconn_is_connected(swconn)) {
> > +        return false;
> > +    }
> > +
> > +    bool reconnected = false;
> > +
> > +    if (br_ofctrl->seqno != rconn_get_connection_seqno(swconn)) {
> > +        br_ofctrl->seqno = rconn_get_connection_seqno(swconn);
> > +        reconnected = true;
> > +        br_ofctrl->state = S_NEW;
> > +    }
> > +
> > +    bool progress = true;
> > +    for (int i = 0; progress && i < 50; i++) {
> > +        /* Allow the state machine to run. */
> > +        enum br_ofctrl_state old_state = br_ofctrl->state;
> > +        switch (br_ofctrl->state) {
> > +#define STATE(NAME) case NAME: run_##NAME(br_ofctrl); break;
> > +            STATES
> > +#undef STATE
> > +        default:
> > +            OVS_NOT_REACHED();
> > +        }
> > +
> > +        /* Try to process a received packet. */
> > +        struct ofpbuf *msg = rconn_recv(swconn);
> > +        if (msg) {
> > +            const struct ofp_header *oh = msg->data;
> > +            enum ofptype type;
> > +            enum ofperr error;
> > +
> > +            error = ofptype_decode(&type, oh);
> > +            if (!error) {
> > +                switch (br_ofctrl->state) {
> > +#define STATE(NAME) case NAME: recv_##NAME(br_ofctrl, oh, type); break;
> > +                    STATES
> > +#undef STATE
> > +                default:
> > +                    OVS_NOT_REACHED();
> > +                }
> > +            } else {
> > +                char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 1);
> > +                VLOG_WARN("could not decode OpenFlow message (%s): %s",
> > +                          ofperr_to_string(error), s);
> > +                free(s);
> > +            }
> > +
> > +            ofpbuf_delete(msg);
> > +        }
> > +
> > +        /* If we did some work, plan to go around again. */
> > +        progress = old_state != br_ofctrl->state || msg;
> > +    }
> > +    if (progress) {
> > +        /* We bailed out to limit the amount of work we do in one go, to allow
> > +         * other code a chance to run.  We were still making progress at that
> > +         * point, so ensure that we come back again without waiting. */
> > +        poll_immediate_wake();
> > +    }
> > +
> > +    return reconnected;
> > +}
> > +
> > +static bool
> > +br_ofctrl_has_backlog(struct br_ofctrl *br_ofctrl)
> > +{
> > +    if (rconn_packet_counter_n_packets(br_ofctrl->tx_counter)
> > +        || rconn_get_version(br_ofctrl->swconn) < 0) {
> > +        return true;
> > +    }
> > +    return false;
> > +}
> > +
> > +/* The flow table can be updated if the connection to the switch is up and
> > + * in the correct state and not backlogged with existing flow_mods.  (Our
> > + * criteria for being backlogged appear very conservative, but the socket
> > + * between ovn-controller and OVS provides some buffering.) */
> > +static bool
> > +br_ofctrl_can_put(struct br_ofctrl *br_ofctrl)
> > +{
> > +    if (br_ofctrl->state != S_UPDATE_FLOWS
> > +        || br_ofctrl_has_backlog(br_ofctrl)) {
> > +        return false;
> > +    }
> > +    return true;
> > +}
> > +
> > +static struct ofpbuf *
> > +encode_flow_mod(struct ofputil_flow_mod *fm)
> > +{
> > +    fm->buffer_id = UINT32_MAX;
> > +    fm->out_port = OFPP_ANY;
> > +    fm->out_group = OFPG_ANY;
> > +    return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF15_OXM);
> > +}
> > +
> > +static struct ofpbuf *
> > +encode_bundle_add(struct ofpbuf *msg, struct ofputil_bundle_ctrl_msg *bc)
> > +{
> > +    struct ofputil_bundle_add_msg bam = {
> > +        .bundle_id = bc->bundle_id,
> > +        .flags     = bc->flags,
> > +        .msg       = msg->data,
> > +    };
> > +    return ofputil_encode_bundle_add(OFP15_VERSION, &bam);
> > +}
> > +
> > +static bool
> > +add_flow_mod(struct ofputil_flow_mod *fm,
> > +             struct ofputil_bundle_ctrl_msg *bc,
> > +             struct ovs_list *msgs)
> > +{
> > +    struct ofpbuf *msg = encode_flow_mod(fm);
> > +    struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
> > +
> > +    uint32_t flow_mod_len = msg->size;
> > +    uint32_t bundle_len = bundle_msg->size;
> > +
> > +    ofpbuf_delete(msg);
> > +
> > +    if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
> > +        ofpbuf_delete(bundle_msg);
> > +
> > +        return false;
> > +    }
> > +
> > +    ovs_list_push_back(msgs, &bundle_msg->list_node);
> > +    return true;
> > +}
> > +
> > +static void
> > +br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
> > +              bool lflows_changed, bool pflows_changed)
> > +{
> > +    bool need_put = false;
> > +
> > +    if (lflows_changed || pflows_changed || br_ofctrl->skipped_last_time ||
> > +        br_ofctrl->ofctrl_initial_clear) {
> > +        need_put = true;
> > +        br_ofctrl->old_req_cfg = req_cfg;
> > +    } else if (req_cfg != br_ofctrl->old_req_cfg) {
> > +        /* req_cfg changed since last ofctrl_put() call */
> > +        if (br_ofctrl->cur_cfg == br_ofctrl->old_req_cfg) {
> > +            /* If there are no updates pending, we were up-to-date already,
> > +             * update with the new req_cfg.
> > +             */
> > +            if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> > +                br_ofctrl->cur_cfg = req_cfg;
> > +                br_ofctrl->old_req_cfg = req_cfg;
> > +            }
> > +        } else {
> > +            need_put = true;
> > +            br_ofctrl->old_req_cfg = req_cfg;
> > +        }
> > +    }
> > +
> > +    if (!need_put) {
> > +        VLOG_DBG("ofctrl_put not needed for bridge %s", br_ofctrl->bridge);
> > +        return;
> > +    }
> > +
> > +    /* OpenFlow messages to send to the switch to bring it up-to-date. */
> > +    struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
> > +
> > +    if (!br_ofctrl_can_put(br_ofctrl)) {
> > +        VLOG_DBG("ofctrl_put can't be performed for bridge %s",
> > +                 br_ofctrl->bridge);
> > +
> > +        br_ofctrl->skipped_last_time = true;
> > +        return;
> > +    }
> > +
> > +    /* Add all flow updates into a bundle. */
> > +    static int bundle_id = 0;
> > +    struct ofputil_bundle_ctrl_msg bc = {
> > +        .bundle_id = bundle_id++,
> > +        .flags     = OFPBF_ORDERED | OFPBF_ATOMIC,
> > +    };
> > +    struct ofpbuf *bundle_open, *bundle_commit;
> > +
> > +    /* Open a new bundle. */
> > +    bc.type = OFPBCT_OPEN_REQUEST;
> > +    bundle_open = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
> > +    ovs_list_push_back(&msgs, &bundle_open->list_node);
> > +
> > +    if (br_ofctrl->ofctrl_initial_clear) {
> > +        /* Send a flow_mod to delete all flows. */
> > +        struct ofputil_flow_mod fm = {
> > +            .table_id = OFPTT_ALL,
> > +            .command = OFPFC_DELETE,
> > +        };
> > +        minimatch_init_catchall(&fm.match);
> > +        add_flow_mod(&fm, &bc, &msgs);
> > +        minimatch_destroy(&fm.match);
> > +
> > +        br_ofctrl->ofctrl_initial_clear = false;
> > +    }
> > +
> > +    br_flow_populate_oflow_msgs(br_ofctrl->bridge, &msgs);
> > +
> > +    if (ovs_list_back(&msgs) == &bundle_open->list_node) {
> > +        /* No flow updates.  Removing the bundle open request. */
> > +        ovs_list_pop_back(&msgs);
> > +        ofpbuf_delete(bundle_open);
> > +    } else {
> > +        /* Committing the bundle. */
> > +        bc.type = OFPBCT_COMMIT_REQUEST;
> > +        bundle_commit = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
> > +        ovs_list_push_back(&msgs, &bundle_commit->list_node);
> > +    }
> > +
> > +    if (!ovs_list_is_empty(&msgs)) {
> > +        /* Add a barrier to the list of messages. */
> > +        struct ofpbuf *barrier = ofputil_encode_barrier_request(OFP15_VERSION);
> > +        const struct ofp_header *oh = barrier->data;
> > +        ovs_be32 xid_ = oh->xid;
> > +        ovs_list_push_back(&msgs, &barrier->list_node);
> > +
> > +        /* Queue the messages. */
> > +        struct ofpbuf *msg;
> > +        LIST_FOR_EACH_POP (msg, list_node, &msgs) {
> > +            queue_msg(br_ofctrl, msg);
> > +        }
> > +
> > +        /* Track the flow update. */
> > +        struct br_ofctrl_flow_update *fup;
> > +        LIST_FOR_EACH_REVERSE_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
> > +            if (req_cfg < fup->req_cfg) {
> > +                /* This ofctrl_flow_update is for a configuration later than
> > +                 * 'req_cfg'.  This should not normally happen, because it
> > +                 * means that the local seqno decreased and it should normally
> > +                 * be monotonically increasing. */
> > +                VLOG_WARN("req_cfg regressed from %"PRId64" to %"PRId64,
> > +                          fup->req_cfg, req_cfg);
> > +                ovs_list_remove(&fup->list_node);
> > +                free(fup);
> > +            } else if (req_cfg == fup->req_cfg) {
> > +                /* This ofctrl_flow_update is for the same configuration as
> > +                 * 'req_cfg'.  Probably, some change to the physical topology
> > +                 * means that we had to revise the OpenFlow flow table even
> > +                 * though the logical topology did not change.  Update fp->xid,
> > +                 * so that we don't send a notification that we're up-to-date
> > +                 * until we're really caught up. */
> > +                VLOG_DBG("advanced xid target for req_cfg=%"PRId64, req_cfg);
> > +                fup->xid = xid_;
> > +                goto done;
>
> This is the only place that the "done:" label is used. The "done:" label
> is in a place where the only thing that can happen is that the function
> immediately returns.
>
> So instead of "goto done;" and the odd label placement, you can just
> "return;" here.

Ack.  I'll address it in v2.

>
> > +            } else {
> > +                break;
> > +            }
> > +        }
> > +
> > +        /* Add a flow update. */
> > +        fup = xmalloc(sizeof *fup);
> > +        ovs_list_push_back(&br_ofctrl->flow_updates, &fup->list_node);
> > +        fup->xid = xid_;
> > +        fup->req_cfg = req_cfg;
> > +    done:;
> > +    } else if (!ovs_list_is_empty(&br_ofctrl->flow_updates)) {
> > +        /* Getting up-to-date with 'req_cfg' didn't require any extra flow
> > +         * table changes, so whenever we get up-to-date with the most recent
> > +         * flow table update, we're also up-to-date with 'req_cfg'. */
> > +        struct br_ofctrl_flow_update *fup =
> > +            br_ofctrl_flow_update_from_list_node(
> > +                ovs_list_back(&br_ofctrl->flow_updates));
> > +        fup->req_cfg = req_cfg;
> > +    } else {
> > +        /* We were completely up-to-date before and still are. */
> > +        br_ofctrl->cur_cfg = req_cfg;
> > +    }
> > +}
> > diff --git a/br-controller/br-ofctrl.h b/br-controller/br-ofctrl.h
> > new file mode 100644
> > index 0000000000..9b629e2123
> > --- /dev/null
> > +++ b/br-controller/br-ofctrl.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + *     http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#ifndef BR_OFCTRL_H
> > +#define BR_OFCTRL_H 1
> > +
> > +struct ovn_bridge;
> > +struct shash;
> > +
> > +void br_ofctrls_init(void);
> > +bool br_ofctrls_run(void);
> > +void br_ofctrls_put(uint64_t req_cfg, bool lflows_changed,
> > +                    bool pflows_changed);
> > +void br_ofctrls_destroy(void);
> > +void br_ofctrls_wait(void);
> > +
> > +void br_ofctrls_add_or_update_bridge(struct ovn_bridge *);
> > +void br_ofctrls_remove_bridge(const char *);
> > +uint64_t br_ofctrl_get_cur_cfg(void);
> > +void br_ofctrls_get_bridges(struct sset *);
> > +
> > +#endif /* BR_OFCTRL_H */
> > \ No newline at end of file
> > diff --git a/br-controller/en-bridge-data.c b/br-controller/en-bridge-data.c
> > index 483c784a37..5c10a1f1f1 100644
> > --- a/br-controller/en-bridge-data.c
> > +++ b/br-controller/en-bridge-data.c
> > @@ -26,6 +26,7 @@
> >
> >   /* OVN includes. */
> >   #include "en-bridge-data.h"
> > +#include "lib/dirs.h"
> >   #include "lib/ovn-br-idl.h"
> >
> >   VLOG_DEFINE_THIS_MODULE(en_bridge_data);
> > @@ -40,6 +41,7 @@ static const struct ovsrec_bridge *ovsbridge_lookup_by_name(
> >       struct ovsdb_idl_index *ovsrec_bridge_by_name,
> >       const char *name);
> >   static void build_ovn_bridge_iface_simap(struct ovn_bridge *);
> > +static void update_ovn_br_remote(struct ovn_bridge *);
> >
> >   void *
> >   en_bridge_data_init(struct engine_node *node OVS_UNUSED,
> > @@ -114,6 +116,7 @@ ovn_bridges_run(const struct ovnbrrec_bridge_table *br_table,
> >
> >           br->ovs_br = ovs_br;
> >           build_ovn_bridge_iface_simap(br);
> > +        update_ovn_br_remote(br);
> >       }
> >   }
> >
> > @@ -121,6 +124,7 @@ static void
> >   ovn_bridge_destroy(struct ovn_bridge *br)
> >   {
> >       simap_destroy(&br->ovs_ifaces);
> > +    free(br->conn_target);
> >       free(br);
> >   }
> >
> > @@ -157,3 +161,39 @@ build_ovn_bridge_iface_simap(struct ovn_bridge *br)
> >           }
> >       }
> >   }
> > +
> > +static void
> > +update_ovn_br_remote(struct ovn_bridge *br)
> > +{
> > +    ovs_assert(br->ovs_br);
> > +
> > +    const char *ext_target = smap_get(&br->ovs_br->external_ids,
> > +                                      "ovn-bridge-remote");
> > +    char *target = ext_target
> > +        ? xstrdup(ext_target)
> > +        : xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br->ovs_br->name);
> > +
> > +    if (!br->conn_target || strcmp(br->conn_target, target)) {
> > +        free(br->conn_target);
> > +        br->conn_target = target;
> > +    } else {
> > +        free(target);
> > +    }
> > +
> > +    unsigned long long probe_interval =
> > +        smap_get_ullong(&br->ovs_br->external_ids,
> > +                        "ovn-openflow-remote-probe-interval", 0);
> > +    br->probe_interval = MIN(probe_interval / 1000, INT_MAX);
> > +
> > +    unsigned int _wait_before_clear_time =
> > +        smap_get_uint(&br->ovs_br->external_ids,
> > +                      "ovn-ofctrl-wait-before-clear", 0);
> > +
> > +    if (_wait_before_clear_time != br->wait_before_clear_time) {
> > +        VLOG_INFO("ofctrl-wait-before-clear is now %u ms (was %u ms) "
> > +                  "for bridge %s",
> > +                  _wait_before_clear_time, br->wait_before_clear_time,
> > +                  br->ovs_br->name);
> > +        br->wait_before_clear_time = _wait_before_clear_time;
> > +    }
> > +}
> > diff --git a/br-controller/en-bridge-data.h b/br-controller/en-bridge-data.h
> > index b374798649..05ab556637 100644
> > --- a/br-controller/en-bridge-data.h
> > +++ b/br-controller/en-bridge-data.h
> > @@ -26,6 +26,10 @@ struct ovn_bridge {
> >
> >       /* simap of ovs interface names to ofport numbers. */
> >       struct simap ovs_ifaces;
> > +
> > +    int probe_interval;
> > +    char *conn_target;
> > +    unsigned int wait_before_clear_time;
> >   };
> >
> >   struct ed_type_bridge_data {
> > diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
> > index ae0e192429..74f2b7a2d2 100644
> > --- a/br-controller/ovn-br-controller.c
> > +++ b/br-controller/ovn-br-controller.c
> > @@ -35,11 +35,13 @@
> >
> >
> >   /* OVN includes. */
> > +#include "br-ofctrl.h"
> >   #include "en-bridge-data.h"
> >   #include "en-lflow.h"
> >   #include "en-pflow.h"
> >   #include "lib/ovn-br-idl.h"
> >   #include "lib/inc-proc-eng.h"
> > +#include "lib/ofctrl-seqno.h"
> >   #include "lib/ovn-util.h"
> >
> >   VLOG_DEFINE_THIS_MODULE(main);
> > @@ -55,6 +57,9 @@ static const char *ssl_ca_cert_file;
> >   /* --unixctl-path: Path to use for unixctl server socket. */
> >   static char *unixctl_path;
> >
> > +/* Registered ofctrl seqno type for br_cfg propagation. */
> > +static size_t ofctrl_seq_type_br_cfg;
> > +
> >   #define BRCTL_NODES \
> >       BRCTL_NODE(br_global) \
> >       BRCTL_NODE(bridge) \
> > @@ -110,7 +115,12 @@ en_br_controller_output_run(struct engine_node *node OVS_UNUSED,
> >   /* Static function declarations. */
> >   static void ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl);
> >   static void update_br_db(struct ovsdb_idl *ovs_idl,
> > -                         struct ovsdb_idl *ovn_br_idl);
> > +                         struct ovsdb_idl *ovnbr_idl,
> > +                         unsigned int *ovnbr_cond_seqno);
> > +static unsigned int update_ovnbr_monitors(struct ovsdb_idl *);
> > +static uint64_t get_ovnbr_cfg(const struct ovnbrrec_br_global_table *,
> > +                                  unsigned int cond_seqno,
> > +                                  unsigned int expected_cond_seqno);
> >
> >   int
> >   main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> > @@ -138,6 +148,9 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >
> >       daemonize_complete();
> >
> > +    /* Register ofctrl seqno types. */
> > +    ofctrl_seq_type_br_cfg = ofctrl_seqno_add_type();
> > +
> >       /* Connect to OVS OVSDB instance. */
> >       struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> >           ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
> > @@ -206,8 +219,12 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >       engine_init(&en_br_controller_output, &engine_arg);
> >       engine_ovsdb_node_add_index(&en_ovs_bridge, "name", ovsrec_bridge_by_name);
> >
> > -    unsigned int ovs_cond_seqno = UINT_MAX;
> > +    unsigned int ovnbr_expected_cond_seqno = UINT_MAX;
> >       unsigned int ovnbr_cond_seqno = UINT_MAX;
> > +    unsigned int ovs_cond_seqno = UINT_MAX;
> > +
> > +    struct ed_type_bridge_data *br_data =
> > +        engine_get_internal_data(&en_bridge_data);
> >
> >       /* Main loop. */
> >       while (!exit_args.exiting) {
> > @@ -224,7 +241,8 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >               ovs_cond_seqno = new_ovs_cond_seqno;
> >           }
> >
> > -        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl);
> > +        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl,
> > +                     &ovnbr_expected_cond_seqno);
> >           struct ovsdb_idl_txn *ovnbr_idl_txn
> >               = ovsdb_idl_loop_run(&ovnbr_idl_loop);
> >           unsigned int new_ovnbr_cond_seqno
> > @@ -251,10 +269,48 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >
> >           if (ovsdb_idl_has_ever_connected(ovnbr_idl_loop.idl) && cfg) {
> >               engine_run(true);
> > +
> > +            br_data = engine_get_data(&en_bridge_data);
> > +            if (br_data) {
> > +                struct sset bridges_in_br_ofctrl =
> > +                    SSET_INITIALIZER(&bridges_in_br_ofctrl);
> > +                br_ofctrls_get_bridges(&bridges_in_br_ofctrl);
> > +                struct shash_node *node;
> > +                SHASH_FOR_EACH (node, &br_data->bridges) {
> > +                    struct ovn_bridge *br = node->data;
> > +
> > +                    if (br->ovs_br) {
> > +                        sset_find_and_delete(&bridges_in_br_ofctrl,
> > +                                             br->db_br->name);
> > +                        br_ofctrls_add_or_update_bridge(br);
> > +                    }
> > +                }
> > +
> > +                const char *bridge;
> > +                SSET_FOR_EACH (bridge, &bridges_in_br_ofctrl) {
> > +                    br_ofctrls_remove_bridge(bridge);
> > +                }
> > +
> > +                sset_destroy(&bridges_in_br_ofctrl);
> > +            }
> > +
> > +            br_ofctrls_run();
> > +
> > +            ofctrl_seqno_update_create(
> > +                ofctrl_seq_type_br_cfg,
> > +                get_ovnbr_cfg(ovnbrrec_br_global_table_get(ovnbr_idl_loop.idl),
> > +                              ovnbr_cond_seqno, ovnbr_expected_cond_seqno));
> > +
> > +            br_ofctrls_put(ofctrl_seqno_get_req_cfg(),
> > +                           engine_node_changed(&en_lflow_output),
> > +                           engine_node_changed(&en_pflow_output));
> > +
> > +            ofctrl_seqno_run(br_ofctrl_get_cur_cfg());
> >           }
> >
> >           unixctl_server_run(unixctl);
> >
> > +        br_ofctrls_wait();
> >           unixctl_server_wait(unixctl);
> >           if (exit_args.exiting) {
> >               poll_immediate_wake();
> > @@ -440,7 +496,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> >   /* Retrieves the pointer to the OVN Bridge Controller database from 'ovs_idl'
> >    * and updates 'brdb_idl' with that pointer. */
> >   static void
> > -update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
> > +update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl,
> > +             unsigned int *ovnbr_cond_seqno)
> >   {
> >       const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
> >       if (!cfg) {
> > @@ -449,4 +506,55 @@ update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
> >
> >       const char *remote = smap_get(&cfg->external_ids, "ovn-br-remote");
> >       ovsdb_idl_set_remote(ovnbr_idl, remote, true);
> > +
> > +    unsigned int next_cond_seqno = update_ovnbr_monitors(ovnbr_idl);
> > +    if (ovnbr_cond_seqno) {
> > +        *ovnbr_cond_seqno = next_cond_seqno;
> > +    }
> > +}
> > +
> > +/* Assume the table exists in the server schema and set its condition. */
> > +#define ovnbr_table_set_req_mon_condition(idl, table, cond) \
> > +    ovnbrrec_##table##_set_condition(idl, cond)
> > +
> > +static unsigned int
> > +update_ovnbr_monitors(struct ovsdb_idl *ovnbr_idl)
> > +{
> > +    struct ovsdb_idl_condition br = OVSDB_IDL_CONDITION_INIT(&br);
> > +    struct ovsdb_idl_condition lf = OVSDB_IDL_CONDITION_INIT(&lf);
> > +
> > +    ovsdb_idl_condition_add_clause_true(&br);
> > +    ovsdb_idl_condition_add_clause_true(&lf);
> > +
> > +    unsigned int cond_seqnos[] = {
> > +        ovnbr_table_set_req_mon_condition(ovnbr_idl, bridge, &br),
> > +        ovnbr_table_set_req_mon_condition(ovnbr_idl, logical_flow, &lf),
> > +    };
> > +
> > +    unsigned int expected_cond_seqno = 0;
> > +    for (size_t i = 0; i < ARRAY_SIZE(cond_seqnos); i++) {
> > +        expected_cond_seqno = MAX(expected_cond_seqno, cond_seqnos[i]);
> > +    }
> > +
> > +    return expected_cond_seqno;
> > +}
> > +
> > +static uint64_t
> > +get_ovnbr_cfg(const struct ovnbrrec_br_global_table *br_global_table,
> > +              unsigned int cond_seqno, unsigned int expected_cond_seqno)
> > +{
> > +    static uint64_t br_cfg = 0;
> > +
> > +    /* Delay getting br_cfg if there are monitor condition changes
> > +     * in flight.  It might be that those changes would instruct the
> > +     * server to send updates that happened before PR_Global.pr_cfg.
> > +     */
> > +    if (cond_seqno != expected_cond_seqno) {
> > +        return br_cfg;
> > +    }
> > +
> > +    const struct ovnbrrec_br_global *br_global
> > +        = ovnbrrec_br_global_table_first(br_global_table);
> > +    br_cfg = br_global ? br_global->br_cfg : 0;
> > +    return br_cfg;
> >   }
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index e6a31c6f69..84c0ea2219 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -46,7 +46,8 @@ TESTSUITE_AT = \
> >       tests/ovn-lflow-conj-ids.at \
> >       tests/ovn-ipsec.at \
> >       tests/ovn-vif-plug.at \
> > -     tests/ovn-util.at
> > +     tests/ovn-util.at \
> > +     tests/ovn-br-controller.at
> >
> >   SYSTEM_DPDK_TESTSUITE_AT = \
> >       tests/system-dpdk-testsuite.at \
> > @@ -89,7 +90,7 @@ DISTCLEANFILES += tests/atconfig tests/atlocal
> >   MULTINODE_TESTSUITE = $(srcdir)/tests/multinode-testsuite
> >   MULTINODE_TESTSUITE_DIR = $(abs_top_builddir)/tests/multinode-testsuite.dir
> >   MULTINODE_TESTSUITE_RESULTS = $(MULTINODE_TESTSUITE_DIR)/results
> > -AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic
> > +AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic:br-controller
> >
> >   export ovs_srcdir
> >   export ovs_builddir
> > diff --git a/tests/ovn-br-controller.at b/tests/ovn-br-controller.at
> > new file mode 100644
> > index 0000000000..bb10e5bd10
> > --- /dev/null
> > +++ b/tests/ovn-br-controller.at
> > @@ -0,0 +1,327 @@
> > +AT_BANNER([ovn_br_controller])
> > +
> > +# OVN_BR_CONTROLLER_START(SIM_NAME)
> > +#
> > +# $1 - optional simulator name. If none is given, runs ovn-br-controller
> > +#      in $ovs_dir.
> > +# Starts the test with a setup with ovn bridge controller.  Each test case must first
> > +# call this macro and ovn_start.
> > +#
> > +m4_define([OVN_BR_CONTROLLER_START], [
> > +    AT_KEYWORDS([ovn-br-controller])
> > +    mkdir -p "$ovs_dir" || return 1
> > +    mkdir "$ovs_base"/ovn-br || return 1
> > +
> > +    dnl Create databases (vswitch).
> > +    check ovsdb-tool create "$ovs_dir"/vswitchd.db $ovs_srcdir/vswitchd/vswitch.ovsschema
> > +    check ovsdb-tool create "$ovs_base"/ovn-br/ovn-br.db "$abs_top_srcdir"/ovn-br.ovsschema
> > +
> > +    dnl Start ovsdb-server.
> > +    start_daemon ovsdb-server --remote=punix:"$ovs_dir"/db.sock \
> > +                             "$ovs_dir"/vswitchd.db
> > +
> > +    ovn_br_remote=unix:"$ovs_base"/ovn-br/ovnbr_db.sock
> > +    dnl Start ovs-vswitchd.
> > +    start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif
> > +
> > +    ovs-vsctl \
> > +        -- set Open_vSwitch . external-ids:ovn-br-remote=$ovn_br_remote
> > +    dnl Start ovsdb-server for ovn-br.
> > +    as ovn-br start_daemon ovsdb-server --remote=punix:"$ovs_base"/ovn-br/ovnbr_db.sock \
> > +        "$ovs_base"/ovn-br/ovn-br.db
> > +
> > +    which ovn-br-controller
> > +    dnl Start ovn-br-controller.
> > +    start_daemon ovn-br-controller
> > +])
> > +
> > +m4_define([OVN_BR_CONTROLLER_STOP],[
> > +   echo
> > +   echo "Clean up ovn-br-controller related processes in $2"
> > +   test -n "$2" && as "$2"
> > +   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +   OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> > +   OVS_APP_EXIT_AND_WAIT([ovn-br-controller])
> > +
> > +   as ovn-br
> > +   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +])
> > +
> > +AT_SETUP([ovn-br-controller - brctl test])
> > +OVN_BR_CONTROLLER_START
> > +
> > +check as ovn-br ovn-brctl show
> > +check as ovn-br ovn-brctl add-br br0
> > +
> > +AT_CHECK([as ovn-br ovn-brctl show | uuidfilt], [0],
> > +  [bridge <0> (br0)
> > +])
> > +
> > +AT_CHECK([as ovn-br ovn-brctl show br0 | uuidfilt], [0],
> > +  [bridge <0> (br0)
> > +])
> > +
> > +AT_CHECK([as ovn-br ovn-brctl show br1 | uuidfilt], [0], [],
> > +  [ovn-brctl: no row "br1" in table Bridge
> > +])
> > +
> > +check as ovn-br ovn-brctl del-br br0
> > +check as ovn-br ovn-brctl show
> > +
> > +check as ovn-br ovn-brctl add-br br0
> > +
> > +check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && tcp" "drop;"
> > +check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && udp" "next;"
> > +check as ovn-br ovn-brctl add-flow br0 1 0 "ip4 && udp" "output;"
> > +
> > +check as ovn-br ovn-brctl add-br br1
> > +
> > +check as ovn-br ovn-brctl add-flow br1 0 1000 "ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10" "drop;"
> > +check as ovn-br ovn-brctl add-flow br1 0 0 "1" "output;"
> > +
> > +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> > +  [dnl
> > +Bridge: br0 (<0>)
> > +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> > +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> > +  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
> > +Bridge: br1 (<1>)
> > +  table=0 , priority=1000 , match=(ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10), action=(drop;)
> > +  table=0 , priority=0    , match=(1), action=(output;)
> > +])
> > +
> > +as ovn-br ovn-brctl del-flows br1
> > +
> > +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> > +  [dnl
> > +Bridge: br0 (<0>)
> > +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> > +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> > +  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
> > +])
> > +
> > +lflow_uuid=$(as ovn-br ovn-brctl --bare --columns _uuid find logical_flow table_id=1)
> > +check as ovn-br ovn-brctl del-flow $lflow_uuid
> > +
> > +AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
> > +  [dnl
> > +Bridge: br0 (<0>)
> > +  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
> > +  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
> > +])
> > +
> > +OVN_BR_CONTROLLER_STOP
> > +AT_CLEANUP
> > +
> > +AT_SETUP([ovn-br-controller - logical flows])
> > +OVN_BR_CONTROLLER_START
> > +
> > +check as ovn-br ovn-brctl add-br br0
> > +
> > +check ovs-vsctl add-br br0
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 3])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > +NXST_FLOW reply:
> > +])
> > +
> > +check ovs-vsctl add-port br0 p1 -- set interface p1 ofport-request=2
> > +check ovs-vsctl add-port br0 p2 -- set interface p2 ofport-request=3
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 7])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=65, priority=100,reg15=0x3 actions=output:3
> > +NXST_FLOW reply:
> > +])
> > +
> > +check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p1"' "next;"
> > +check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p2"' "drop;"
> > +check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4 && tcp' "ip4.src <-> ip4.dst; tcp.dst = 8080; next;"
> > +check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4' "next;"
> > +check as ovn-br ovn-brctl add-flow br0 2 1000 '1' "output;"
> > +
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 12])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
> > + table=10, priority=1000 actions=resubmit(,64)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=65, priority=100,reg15=0x3 actions=output:3
> > + table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
> > + table=8, priority=1000,reg14=0x3 actions=drop
> > + table=9, priority=1000,ip actions=resubmit(,10)
> > + table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
> > +NXST_FLOW reply:
> > +])
> > +
> > +check ovs-vsctl del-port p2
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 9])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + table=10, priority=1000 actions=resubmit(,64)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
> > + table=9, priority=1000,ip actions=resubmit(,10)
> > + table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
> > +NXST_FLOW reply:
> > +])
> > +
> > +check ovs-vsctl add-br br1
> > +check ovs-vsctl add-port br1 br1-p1 -- set interface br1-p1 ofport-request=1
> > +check ovs-vsctl add-port br1 br1-p2 -- set interface br1-p2 ofport-request=2
> > +
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 1])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > +NXST_FLOW reply:
> > +])
> > +
> > +as ovn-br ovn-brctl --id=4830e8c3-9b6b-48db-ba52-e030d9db7256 create bridge name=br1
>
> These UUIDs are repeated a few times in the test. It would make the test
> easier to read and update if the UUIDs were placed in a variable:

Ack.

>
> br_uuid=4830e8c3-9b6b-48db-ba52-e030d9db7256
> as ovn-br ovn-brctl --id="$br_uuid" create bridge name=br1
>
> > +as ovn-br ovn-brctl list bridge
> > +
> > +# check as ovn-br ovn-brctl add-br br1
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 7])
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x1 actions=output:1
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > +NXST_FLOW reply:
> > +])
> > +
> > +check as ovn-br ovn-brctl add-flow br1 1 1000 "ip4" "ct_snat;"
> > +check as ovn-br ovn-brctl add-flow br1 2 1000 "ip4 && ct.new && ct.trk && ip4.src == 10.0.0.11" "ct_snat(100.64.0.11); next;"
> > +check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p1\"" "outport = \"br1-p2\"; output;"
> > +check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p2\"" "outport = \"br1-p1\"; output;"
> > +
> > +as ovn-br ovn-brctl --id=75bf46aa-4204-4e36-af23-6114f59e3fe8 create logical_flow \
> > +match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"' \
> > +actions="next;" bridge=4830e8c3-9b6b-48db-ba52-e030d9db7256 table_id=10 priority=1000
> > +
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> > + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> > + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> > + table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)
>
> How is the test predicting the conj_id value?

The function lflow_conj_ids_alloc() [1] uses the hash of lflow uuid
and the datapath uuid to generate the conj id.

So this test creates a bridge and a logical flow with conjunction with
the hard coded uuids
to predict the conj_id.

[1] - https://github.com/ovn-org/ovn/blob/main/controller/lflow-conj-ids.c#L90

Thanks
Numan

>
> > + table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x1 actions=output:1
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> > +NXST_FLOW reply:
> > +])
> > +
> > +as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 match='"ip4 && sctp"'
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 12])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> > + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> > + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> > + table=18, priority=1000,sctp actions=resubmit(,19)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x1 actions=output:1
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> > +NXST_FLOW reply:
> > +])
> > +
> > +# Make sure that the same conj_id is used when the lflow is updated with the conj match.
> > +as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 \
> > +match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"'
> > +OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
> > +
> > +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
> > + priority=0 actions=NORMAL
> > + priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
> > + priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
> > + table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
> > + table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
> > + table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
> > + table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)
> > + table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
> > + table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
> > + table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
> > + table=64, priority=0 actions=resubmit(,65)
> > + table=65, priority=0 actions=NORMAL
> > + table=65, priority=100,reg15=0x1 actions=output:1
> > + table=65, priority=100,reg15=0x2 actions=output:2
> > + table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
> > +NXST_FLOW reply:
> > +])
> > +
> > +OVN_BR_CONTROLLER_STOP
> > +AT_CLEANUP
> > diff --git a/tests/testsuite.at b/tests/testsuite.at
> > index 8e60bf82e1..5f5eabb42a 100644
> > --- a/tests/testsuite.at
> > +++ b/tests/testsuite.at
> > @@ -41,3 +41,4 @@ m4_include([tests/checkpatch.at])
> >   m4_include([tests/ovn-ipsec.at])
> >   m4_include([tests/ovn-vif-plug.at])
> >   m4_include([tests/ovn-util.at])
> > +m4_include([tests/ovn-br-controller.at])
>
diff mbox series

Patch

diff --git a/br-controller/automake.mk b/br-controller/automake.mk
index 4baea4f6fe..f8cae3a098 100644
--- a/br-controller/automake.mk
+++ b/br-controller/automake.mk
@@ -2,6 +2,8 @@  bin_PROGRAMS += br-controller/ovn-br-controller
 br_controller_ovn_br_controller_SOURCES = \
 	br-controller/br-flow-mgr.c \
 	br-controller/br-flow-mgr.h \
+	br-controller/br-ofctrl.c \
+	br-controller/br-ofctrl.h \
 	br-controller/en-bridge-data.c \
 	br-controller/en-bridge-data.h \
 	br-controller/en-lflow.c \
diff --git a/br-controller/br-ofctrl.c b/br-controller/br-ofctrl.c
new file mode 100644
index 0000000000..6c11c43530
--- /dev/null
+++ b/br-controller/br-ofctrl.c
@@ -0,0 +1,731 @@ 
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+
+/* OVS includes. */
+#include "bitmap.h"
+#include "byte-order.h"
+#include "dirs.h"
+#include "dp-packet.h"
+#include "flow.h"
+#include "hash.h"
+#include "hindex.h"
+#include "lib/socket-util.h"
+#include "lib/util.h"
+#include "lib/vswitch-idl.h"
+#include "openflow/openflow.h"
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/hmap.h"
+#include "openvswitch/list.h"
+#include "openvswitch/match.h"
+#include "openvswitch/ofp-actions.h"
+#include "openvswitch/ofp-bundle.h"
+#include "openvswitch/ofp-flow.h"
+#include "openvswitch/ofp-group.h"
+#include "openvswitch/ofp-match.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofp-meter.h"
+#include "openvswitch/ofp-packet.h"
+#include "openvswitch/ofp-print.h"
+#include "openvswitch/ofp-util.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/vlog.h"
+#include "openvswitch/poll-loop.h"
+#include "openvswitch/rconn.h"
+
+/* OVN includes. */
+#include "br-flow-mgr.h"
+#include "en-bridge-data.h"
+#include "br-ofctrl.h"
+#include "lib/ovn-util.h"
+#include "lib/ovn-br-idl.h"
+
+VLOG_DEFINE_THIS_MODULE(brofctrl);
+
+/* Connection state machine. */
+#define STATES                                  \
+    STATE(S_NEW)                                \
+    STATE(S_WAIT_BEFORE_CLEAR)                  \
+    STATE(S_CLEAR_FLOWS)                        \
+    STATE(S_UPDATE_FLOWS)
+
+enum br_ofctrl_state {
+#define STATE(NAME) NAME,
+    STATES
+#undef STATE
+};
+
+/* An in-flight update to the switch's flow table.
+ *
+ * When we receive a barrier reply from the switch with the given 'xid', we
+ * know that the switch is caught up to the requested sequence number
+ * 'req_cfg' (and make that available to the client via ofctrl_get_cur_cfg(),
+ * so that it can store it into external state, e.g., our Chassis record's
+ * nb_cfg column). */
+struct br_ofctrl_flow_update {
+    struct ovs_list list_node;  /* In 'flow_updates'. */
+    ovs_be32 xid;               /* OpenFlow transaction ID for barrier. */
+    uint64_t req_cfg;           /* Requested sequence number. */
+};
+
+struct br_ofctrl {
+    struct hmap_node hmap_node;
+    char *bridge; /* key. */
+
+    /* OpenFlow connection to the switch. */
+    struct rconn *swconn;
+    int probe_interval;
+    char *conn_target;
+
+    unsigned int wait_before_clear_time;
+    /* The time when the state S_WAIT_BEFORE_CLEAR should complete.
+     * If the timer is not started yet, it is set to 0. */
+    long long int wait_before_clear_expire;
+
+    /* Currently in-flight updates. */
+    struct ovs_list flow_updates;
+
+    /* req_cfg of latest committed flow update. */
+    uint64_t cur_cfg;
+    uint64_t old_req_cfg;
+    bool skipped_last_time;
+
+    /* Indicates if we just went through the S_CLEAR_FLOWS state, which means
+     * we need to perform a one time deletion for all the existing flows,
+     * groups and meters. This can happen during initialization or OpenFlow
+     * reconnection (e.g. after OVS restart). */
+    bool ofctrl_initial_clear;
+
+    /* Last seen sequence number for 'swconn'.  When this differs from
+     * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
+    unsigned int seqno;
+
+    /* Counter for in-flight OpenFlow messages on 'swconn'.  We only send a new
+     * round of flow table modifications to the switch when the counter falls
+     * to zero, to avoid unbounded buffering. */
+    struct rconn_packet_counter *tx_counter;
+
+    /* Current state. */
+    enum br_ofctrl_state state;
+};
+
+static struct hmap br_ofctrls = HMAP_INITIALIZER(&br_ofctrls);
+
+static struct br_ofctrl *br_ofctrl_get(const char *bridge);
+static void br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
+                          bool lflows_changed, bool pflows_changed);
+static void br_ofctrl_destroy(struct br_ofctrl *);
+static ovs_be32 queue_msg(struct br_ofctrl *, struct ofpbuf *);
+static struct br_ofctrl_flow_update *br_ofctrl_flow_update_from_list_node(
+    const struct ovs_list *);
+static bool br_ofctrl_run__(struct br_ofctrl *);
+static bool br_ofctrl_has_backlog(struct br_ofctrl *);
+static bool br_ofctrl_can_put(struct br_ofctrl *);
+
+void
+br_ofctrls_init(void)
+{
+
+}
+
+void
+br_ofctrls_destroy(void)
+{
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH_POP (br_ofctrl, hmap_node, &br_ofctrls) {
+        br_ofctrl_destroy(br_ofctrl);
+    }
+
+    hmap_destroy(&br_ofctrls);
+}
+
+void
+br_ofctrls_add_or_update_bridge(struct ovn_bridge *br)
+{
+    ovs_assert(br->ovs_br);
+
+    struct br_ofctrl *br_ofctrl = br_ofctrl_get(br->db_br->name);
+
+    if (!br_ofctrl) {
+        br_ofctrl = xzalloc(sizeof *br_ofctrl);
+        br_ofctrl->bridge = xstrdup(br->db_br->name);
+        br_ofctrl->swconn = rconn_create(0, 0, DSCP_DEFAULT,
+                                         1 << OFP15_VERSION);
+        br_ofctrl->tx_counter = rconn_packet_counter_create();
+        ovs_list_init(&br_ofctrl->flow_updates);
+
+        hmap_insert(&br_ofctrls, &br_ofctrl->hmap_node,
+                    hash_string(br_ofctrl->bridge, 0));
+    } else {
+        free(br_ofctrl->conn_target);
+    }
+
+    br_ofctrl->probe_interval = br->probe_interval;
+    br_ofctrl->conn_target = xstrdup(br->conn_target);
+    br_ofctrl->wait_before_clear_time = br->wait_before_clear_time;
+}
+
+void
+br_ofctrls_remove_bridge(const char *bridge)
+{
+    struct br_ofctrl *br_ofctrl = br_ofctrl_get(bridge);
+    if (br_ofctrl) {
+        hmap_remove(&br_ofctrls, &br_ofctrl->hmap_node);
+        br_ofctrl_destroy(br_ofctrl);
+    }
+}
+
+void
+br_ofctrls_get_bridges(struct sset *managed_bridges)
+{
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
+        sset_add(managed_bridges, br_ofctrl->bridge);
+    }
+}
+
+/* Runs the OpenFlow state machine against each bridge in the br_ofctrls hmap,
+ * which is local to the hypervisor on which we are running.
+ *
+ * Returns 'true' if an OpenFlow reconnect happened for any of the bridge;
+ * 'false' otherwise.
+ */
+bool
+br_ofctrls_run(void)
+{
+    bool reconnected = false;
+
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
+        reconnected |= br_ofctrl_run__(br_ofctrl);
+    }
+
+    return reconnected;
+}
+
+/* Programs the flow table on the switch, if possible, by the flows
+ * added to the br-flow-mgr.
+ *
+ * This should be called after br_ofctrls_run() within the main loop. */
+void
+br_ofctrls_put(uint64_t req_cfg, bool lflows_changed, bool pflows_changed)
+{
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
+        br_ofctrl_put(br_ofctrl, req_cfg, lflows_changed, pflows_changed);
+    }
+}
+
+void
+br_ofctrls_wait(void)
+{
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
+        rconn_run_wait(br_ofctrl->swconn);
+        rconn_recv_wait(br_ofctrl->swconn);
+    }
+}
+
+uint64_t
+br_ofctrl_get_cur_cfg(void)
+{
+    uint64_t of_cur_cfg = UINT64_MAX;
+    struct br_ofctrl *br_ofctrl;
+    HMAP_FOR_EACH (br_ofctrl, hmap_node, &br_ofctrls) {
+        of_cur_cfg = MIN(of_cur_cfg, br_ofctrl->cur_cfg);
+    }
+
+    return of_cur_cfg;
+}
+
+/* Static functions. */
+
+static void
+br_ofctrl_destroy(struct br_ofctrl *br_ofctrl)
+{
+    rconn_destroy(br_ofctrl->swconn);
+    rconn_packet_counter_destroy(br_ofctrl->tx_counter);
+    free(br_ofctrl->bridge);
+    free(br_ofctrl);
+}
+
+static struct br_ofctrl *
+br_ofctrl_get(const char *bridge)
+{
+    struct br_ofctrl *br_ofctrl;
+    uint32_t hash = hash_string(bridge, 0);
+    HMAP_FOR_EACH_WITH_HASH (br_ofctrl, hmap_node, hash, &br_ofctrls) {
+        if (!strcmp(br_ofctrl->bridge, bridge)) {
+            return br_ofctrl;
+        }
+    }
+
+    return NULL;
+}
+
+static ovs_be32
+queue_msg(struct br_ofctrl *br_ofctrl, struct ofpbuf *msg)
+{
+    const struct ofp_header *oh = msg->data;
+    ovs_be32 xid_ = oh->xid;
+    rconn_send(br_ofctrl->swconn, msg, br_ofctrl->tx_counter);
+    return xid_;
+}
+
+static void
+log_openflow_rl(struct vlog_rate_limit *rl, enum vlog_level level,
+                const struct ofp_header *oh, const char *title)
+{
+    if (!vlog_should_drop(&this_module, level, rl)) {
+        char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 2);
+        vlog(&this_module, level, "%s: %s", title, s);
+        free(s);
+    }
+}
+
+static struct br_ofctrl_flow_update *
+br_ofctrl_flow_update_from_list_node(const struct ovs_list *list_node)
+{
+    return CONTAINER_OF(list_node, struct br_ofctrl_flow_update, list_node);
+}
+
+/* ofctrl state machine functions. */
+
+static void
+ofctrl_recv(struct br_ofctrl *br_ofctrl, const struct ofp_header *oh,
+            enum ofptype type)
+{
+    if (type == OFPTYPE_ECHO_REQUEST) {
+        queue_msg(br_ofctrl, ofputil_encode_echo_reply(oh));
+    } else if (type == OFPTYPE_ERROR) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
+        log_openflow_rl(&rl, VLL_INFO, oh, "OpenFlow error");
+        rconn_reconnect(br_ofctrl->swconn);
+    } else {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
+        log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored");
+    }
+}
+
+
+/* S_NEW, for a new connection.
+ *
+ */
+
+static void
+run_S_NEW(struct br_ofctrl *br_ofctrl)
+{
+    br_ofctrl->state = S_WAIT_BEFORE_CLEAR;
+}
+
+static void
+recv_S_NEW(struct br_ofctrl *br_ofctrl OVS_UNUSED,
+           const struct ofp_header *oh OVS_UNUSED,
+           enum ofptype type OVS_UNUSED)
+{
+    OVS_NOT_REACHED();
+}
+
+/* S_WAIT_BEFORE_CLEAR, we are almost ready to set up flows, but just wait for
+ * a while until the initial flow compute to complete before we clear the
+ * existing flows in OVS, so that we won't end up with an empty flow table,
+ * which may cause data plane down time. */
+static void
+run_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl)
+{
+    if (!br_ofctrl->wait_before_clear_time ||
+        (br_ofctrl->wait_before_clear_expire &&
+         time_msec() >= br_ofctrl->wait_before_clear_expire)) {
+        br_ofctrl->state = S_CLEAR_FLOWS;
+        return;
+    }
+
+    if (!br_ofctrl->wait_before_clear_expire) {
+        /* Start the timer. */
+        br_ofctrl->wait_before_clear_expire =
+            time_msec() + br_ofctrl->wait_before_clear_time;
+    }
+    poll_timer_wait_until(br_ofctrl->wait_before_clear_expire);
+}
+
+static void
+recv_S_WAIT_BEFORE_CLEAR(struct br_ofctrl *br_ofctrl,
+                         const struct ofp_header *oh, enum ofptype type)
+{
+    ofctrl_recv(br_ofctrl, oh, type);
+}
+
+/* Sends an OFPT_TABLE_MOD to clear all flows, then transitions to
+ * S_UPDATE_FLOWS. */
+
+static void
+run_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl)
+{
+    VLOG_DBG("clearing all flows for bridge %s", br_ofctrl->bridge);
+
+    /* Set the flag so that the ofctrl_run() can clear the existing flows,
+     * groups and meters. We clear them in ofctrl_run() right before the new
+     * ones are installed to avoid data plane downtime. */
+    br_ofctrl->ofctrl_initial_clear = true;
+
+    /* Clear installed_flows, to match the state of the switch. */
+    br_flow_flush_oflows(br_ofctrl->bridge);
+
+    /* All flow updates are irrelevant now. */
+    struct br_ofctrl_flow_update *fup;
+    LIST_FOR_EACH_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
+        ovs_list_remove(&fup->list_node);
+        free(fup);
+    }
+
+    br_ofctrl->state = S_UPDATE_FLOWS;
+
+    /* Give a chance for the main loop to call ofctrl_put() in case there were
+     * pending flows waiting ofctrl state change to S_UPDATE_FLOWS. */
+    poll_immediate_wake();
+}
+
+static void
+recv_S_CLEAR_FLOWS(struct br_ofctrl *br_ofctrl,
+                   const struct ofp_header *oh, enum ofptype type)
+{
+    ofctrl_recv(br_ofctrl, oh, type);
+}
+
+/* S_UPDATE_FLOWS, for maintaining the flow table over time.
+ *
+ * Compare the installed flows to the ones we want.  Send OFPT_FLOW_MOD as
+ * necessary.
+ *
+ * This is a terminal state.  We only transition out of it if the connection
+ * drops. */
+
+static void
+run_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl OVS_UNUSED)
+{
+    /* Nothing to do here.
+     *
+     * Being in this state enables ofctrl_put() to work, however. */
+}
+
+static void
+br_flow_updates_handle_barrier_reply(struct br_ofctrl *br_ofctrl,
+                                     const struct ofp_header *oh)
+{
+    if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
+        return;
+    }
+
+    struct br_ofctrl_flow_update *fup = br_ofctrl_flow_update_from_list_node(
+        ovs_list_front(&br_ofctrl->flow_updates));
+    if (fup->xid == oh->xid) {
+        if (fup->req_cfg >= br_ofctrl->cur_cfg) {
+            br_ofctrl->cur_cfg = fup->req_cfg;
+        }
+        ovs_list_remove(&fup->list_node);
+        free(fup);
+    }
+}
+
+static void
+recv_S_UPDATE_FLOWS(struct br_ofctrl *br_ofctrl,
+                    const struct ofp_header *oh, enum ofptype type)
+{
+    if (type == OFPTYPE_BARRIER_REPLY) {
+        br_flow_updates_handle_barrier_reply(br_ofctrl, oh);
+    } else {
+        ofctrl_recv(br_ofctrl, oh, type);
+    }
+}
+
+static bool
+br_ofctrl_run__(struct br_ofctrl *br_ofctrl)
+{
+    struct rconn *swconn = br_ofctrl->swconn;
+
+    ovn_update_swconn_at(swconn, br_ofctrl->conn_target,
+                         br_ofctrl->probe_interval, "br_ofctrl");
+    rconn_run(swconn);
+
+    if (!rconn_is_connected(swconn)) {
+        return false;
+    }
+
+    bool reconnected = false;
+
+    if (br_ofctrl->seqno != rconn_get_connection_seqno(swconn)) {
+        br_ofctrl->seqno = rconn_get_connection_seqno(swconn);
+        reconnected = true;
+        br_ofctrl->state = S_NEW;
+    }
+
+    bool progress = true;
+    for (int i = 0; progress && i < 50; i++) {
+        /* Allow the state machine to run. */
+        enum br_ofctrl_state old_state = br_ofctrl->state;
+        switch (br_ofctrl->state) {
+#define STATE(NAME) case NAME: run_##NAME(br_ofctrl); break;
+            STATES
+#undef STATE
+        default:
+            OVS_NOT_REACHED();
+        }
+
+        /* Try to process a received packet. */
+        struct ofpbuf *msg = rconn_recv(swconn);
+        if (msg) {
+            const struct ofp_header *oh = msg->data;
+            enum ofptype type;
+            enum ofperr error;
+
+            error = ofptype_decode(&type, oh);
+            if (!error) {
+                switch (br_ofctrl->state) {
+#define STATE(NAME) case NAME: recv_##NAME(br_ofctrl, oh, type); break;
+                    STATES
+#undef STATE
+                default:
+                    OVS_NOT_REACHED();
+                }
+            } else {
+                char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 1);
+                VLOG_WARN("could not decode OpenFlow message (%s): %s",
+                          ofperr_to_string(error), s);
+                free(s);
+            }
+
+            ofpbuf_delete(msg);
+        }
+
+        /* If we did some work, plan to go around again. */
+        progress = old_state != br_ofctrl->state || msg;
+    }
+    if (progress) {
+        /* We bailed out to limit the amount of work we do in one go, to allow
+         * other code a chance to run.  We were still making progress at that
+         * point, so ensure that we come back again without waiting. */
+        poll_immediate_wake();
+    }
+
+    return reconnected;
+}
+
+static bool
+br_ofctrl_has_backlog(struct br_ofctrl *br_ofctrl)
+{
+    if (rconn_packet_counter_n_packets(br_ofctrl->tx_counter)
+        || rconn_get_version(br_ofctrl->swconn) < 0) {
+        return true;
+    }
+    return false;
+}
+
+/* The flow table can be updated if the connection to the switch is up and
+ * in the correct state and not backlogged with existing flow_mods.  (Our
+ * criteria for being backlogged appear very conservative, but the socket
+ * between ovn-controller and OVS provides some buffering.) */
+static bool
+br_ofctrl_can_put(struct br_ofctrl *br_ofctrl)
+{
+    if (br_ofctrl->state != S_UPDATE_FLOWS
+        || br_ofctrl_has_backlog(br_ofctrl)) {
+        return false;
+    }
+    return true;
+}
+
+static struct ofpbuf *
+encode_flow_mod(struct ofputil_flow_mod *fm)
+{
+    fm->buffer_id = UINT32_MAX;
+    fm->out_port = OFPP_ANY;
+    fm->out_group = OFPG_ANY;
+    return ofputil_encode_flow_mod(fm, OFPUTIL_P_OF15_OXM);
+}
+
+static struct ofpbuf *
+encode_bundle_add(struct ofpbuf *msg, struct ofputil_bundle_ctrl_msg *bc)
+{
+    struct ofputil_bundle_add_msg bam = {
+        .bundle_id = bc->bundle_id,
+        .flags     = bc->flags,
+        .msg       = msg->data,
+    };
+    return ofputil_encode_bundle_add(OFP15_VERSION, &bam);
+}
+
+static bool
+add_flow_mod(struct ofputil_flow_mod *fm,
+             struct ofputil_bundle_ctrl_msg *bc,
+             struct ovs_list *msgs)
+{
+    struct ofpbuf *msg = encode_flow_mod(fm);
+    struct ofpbuf *bundle_msg = encode_bundle_add(msg, bc);
+
+    uint32_t flow_mod_len = msg->size;
+    uint32_t bundle_len = bundle_msg->size;
+
+    ofpbuf_delete(msg);
+
+    if (flow_mod_len > UINT16_MAX || bundle_len > UINT16_MAX) {
+        ofpbuf_delete(bundle_msg);
+
+        return false;
+    }
+
+    ovs_list_push_back(msgs, &bundle_msg->list_node);
+    return true;
+}
+
+static void
+br_ofctrl_put(struct br_ofctrl *br_ofctrl, uint64_t req_cfg,
+              bool lflows_changed, bool pflows_changed)
+{
+    bool need_put = false;
+
+    if (lflows_changed || pflows_changed || br_ofctrl->skipped_last_time ||
+        br_ofctrl->ofctrl_initial_clear) {
+        need_put = true;
+        br_ofctrl->old_req_cfg = req_cfg;
+    } else if (req_cfg != br_ofctrl->old_req_cfg) {
+        /* req_cfg changed since last ofctrl_put() call */
+        if (br_ofctrl->cur_cfg == br_ofctrl->old_req_cfg) {
+            /* If there are no updates pending, we were up-to-date already,
+             * update with the new req_cfg.
+             */
+            if (ovs_list_is_empty(&br_ofctrl->flow_updates)) {
+                br_ofctrl->cur_cfg = req_cfg;
+                br_ofctrl->old_req_cfg = req_cfg;
+            }
+        } else {
+            need_put = true;
+            br_ofctrl->old_req_cfg = req_cfg;
+        }
+    }
+
+    if (!need_put) {
+        VLOG_DBG("ofctrl_put not needed for bridge %s", br_ofctrl->bridge);
+        return;
+    }
+
+    /* OpenFlow messages to send to the switch to bring it up-to-date. */
+    struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
+
+    if (!br_ofctrl_can_put(br_ofctrl)) {
+        VLOG_DBG("ofctrl_put can't be performed for bridge %s",
+                 br_ofctrl->bridge);
+
+        br_ofctrl->skipped_last_time = true;
+        return;
+    }
+
+    /* Add all flow updates into a bundle. */
+    static int bundle_id = 0;
+    struct ofputil_bundle_ctrl_msg bc = {
+        .bundle_id = bundle_id++,
+        .flags     = OFPBF_ORDERED | OFPBF_ATOMIC,
+    };
+    struct ofpbuf *bundle_open, *bundle_commit;
+
+    /* Open a new bundle. */
+    bc.type = OFPBCT_OPEN_REQUEST;
+    bundle_open = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
+    ovs_list_push_back(&msgs, &bundle_open->list_node);
+
+    if (br_ofctrl->ofctrl_initial_clear) {
+        /* Send a flow_mod to delete all flows. */
+        struct ofputil_flow_mod fm = {
+            .table_id = OFPTT_ALL,
+            .command = OFPFC_DELETE,
+        };
+        minimatch_init_catchall(&fm.match);
+        add_flow_mod(&fm, &bc, &msgs);
+        minimatch_destroy(&fm.match);
+
+        br_ofctrl->ofctrl_initial_clear = false;
+    }
+
+    br_flow_populate_oflow_msgs(br_ofctrl->bridge, &msgs);
+
+    if (ovs_list_back(&msgs) == &bundle_open->list_node) {
+        /* No flow updates.  Removing the bundle open request. */
+        ovs_list_pop_back(&msgs);
+        ofpbuf_delete(bundle_open);
+    } else {
+        /* Committing the bundle. */
+        bc.type = OFPBCT_COMMIT_REQUEST;
+        bundle_commit = ofputil_encode_bundle_ctrl_request(OFP15_VERSION, &bc);
+        ovs_list_push_back(&msgs, &bundle_commit->list_node);
+    }
+
+    if (!ovs_list_is_empty(&msgs)) {
+        /* Add a barrier to the list of messages. */
+        struct ofpbuf *barrier = ofputil_encode_barrier_request(OFP15_VERSION);
+        const struct ofp_header *oh = barrier->data;
+        ovs_be32 xid_ = oh->xid;
+        ovs_list_push_back(&msgs, &barrier->list_node);
+
+        /* Queue the messages. */
+        struct ofpbuf *msg;
+        LIST_FOR_EACH_POP (msg, list_node, &msgs) {
+            queue_msg(br_ofctrl, msg);
+        }
+
+        /* Track the flow update. */
+        struct br_ofctrl_flow_update *fup;
+        LIST_FOR_EACH_REVERSE_SAFE (fup, list_node, &br_ofctrl->flow_updates) {
+            if (req_cfg < fup->req_cfg) {
+                /* This ofctrl_flow_update is for a configuration later than
+                 * 'req_cfg'.  This should not normally happen, because it
+                 * means that the local seqno decreased and it should normally
+                 * be monotonically increasing. */
+                VLOG_WARN("req_cfg regressed from %"PRId64" to %"PRId64,
+                          fup->req_cfg, req_cfg);
+                ovs_list_remove(&fup->list_node);
+                free(fup);
+            } else if (req_cfg == fup->req_cfg) {
+                /* This ofctrl_flow_update is for the same configuration as
+                 * 'req_cfg'.  Probably, some change to the physical topology
+                 * means that we had to revise the OpenFlow flow table even
+                 * though the logical topology did not change.  Update fp->xid,
+                 * so that we don't send a notification that we're up-to-date
+                 * until we're really caught up. */
+                VLOG_DBG("advanced xid target for req_cfg=%"PRId64, req_cfg);
+                fup->xid = xid_;
+                goto done;
+            } else {
+                break;
+            }
+        }
+
+        /* Add a flow update. */
+        fup = xmalloc(sizeof *fup);
+        ovs_list_push_back(&br_ofctrl->flow_updates, &fup->list_node);
+        fup->xid = xid_;
+        fup->req_cfg = req_cfg;
+    done:;
+    } else if (!ovs_list_is_empty(&br_ofctrl->flow_updates)) {
+        /* Getting up-to-date with 'req_cfg' didn't require any extra flow
+         * table changes, so whenever we get up-to-date with the most recent
+         * flow table update, we're also up-to-date with 'req_cfg'. */
+        struct br_ofctrl_flow_update *fup =
+            br_ofctrl_flow_update_from_list_node(
+                ovs_list_back(&br_ofctrl->flow_updates));
+        fup->req_cfg = req_cfg;
+    } else {
+        /* We were completely up-to-date before and still are. */
+        br_ofctrl->cur_cfg = req_cfg;
+    }
+}
diff --git a/br-controller/br-ofctrl.h b/br-controller/br-ofctrl.h
new file mode 100644
index 0000000000..9b629e2123
--- /dev/null
+++ b/br-controller/br-ofctrl.h
@@ -0,0 +1,33 @@ 
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef BR_OFCTRL_H
+#define BR_OFCTRL_H 1
+
+struct ovn_bridge;
+struct shash;
+
+void br_ofctrls_init(void);
+bool br_ofctrls_run(void);
+void br_ofctrls_put(uint64_t req_cfg, bool lflows_changed,
+                    bool pflows_changed);
+void br_ofctrls_destroy(void);
+void br_ofctrls_wait(void);
+
+void br_ofctrls_add_or_update_bridge(struct ovn_bridge *);
+void br_ofctrls_remove_bridge(const char *);
+uint64_t br_ofctrl_get_cur_cfg(void);
+void br_ofctrls_get_bridges(struct sset *);
+
+#endif /* BR_OFCTRL_H */
\ No newline at end of file
diff --git a/br-controller/en-bridge-data.c b/br-controller/en-bridge-data.c
index 483c784a37..5c10a1f1f1 100644
--- a/br-controller/en-bridge-data.c
+++ b/br-controller/en-bridge-data.c
@@ -26,6 +26,7 @@ 
 
 /* OVN includes. */
 #include "en-bridge-data.h"
+#include "lib/dirs.h"
 #include "lib/ovn-br-idl.h"
 
 VLOG_DEFINE_THIS_MODULE(en_bridge_data);
@@ -40,6 +41,7 @@  static const struct ovsrec_bridge *ovsbridge_lookup_by_name(
     struct ovsdb_idl_index *ovsrec_bridge_by_name,
     const char *name);
 static void build_ovn_bridge_iface_simap(struct ovn_bridge *);
+static void update_ovn_br_remote(struct ovn_bridge *);
 
 void *
 en_bridge_data_init(struct engine_node *node OVS_UNUSED,
@@ -114,6 +116,7 @@  ovn_bridges_run(const struct ovnbrrec_bridge_table *br_table,
 
         br->ovs_br = ovs_br;
         build_ovn_bridge_iface_simap(br);
+        update_ovn_br_remote(br);
     }
 }
 
@@ -121,6 +124,7 @@  static void
 ovn_bridge_destroy(struct ovn_bridge *br)
 {
     simap_destroy(&br->ovs_ifaces);
+    free(br->conn_target);
     free(br);
 }
 
@@ -157,3 +161,39 @@  build_ovn_bridge_iface_simap(struct ovn_bridge *br)
         }
     }
 }
+
+static void
+update_ovn_br_remote(struct ovn_bridge *br)
+{
+    ovs_assert(br->ovs_br);
+
+    const char *ext_target = smap_get(&br->ovs_br->external_ids,
+                                      "ovn-bridge-remote");
+    char *target = ext_target
+        ? xstrdup(ext_target)
+        : xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br->ovs_br->name);
+
+    if (!br->conn_target || strcmp(br->conn_target, target)) {
+        free(br->conn_target);
+        br->conn_target = target;
+    } else {
+        free(target);
+    }
+
+    unsigned long long probe_interval =
+        smap_get_ullong(&br->ovs_br->external_ids,
+                        "ovn-openflow-remote-probe-interval", 0);
+    br->probe_interval = MIN(probe_interval / 1000, INT_MAX);
+
+    unsigned int _wait_before_clear_time =
+        smap_get_uint(&br->ovs_br->external_ids,
+                      "ovn-ofctrl-wait-before-clear", 0);
+
+    if (_wait_before_clear_time != br->wait_before_clear_time) {
+        VLOG_INFO("ofctrl-wait-before-clear is now %u ms (was %u ms) "
+                  "for bridge %s",
+                  _wait_before_clear_time, br->wait_before_clear_time,
+                  br->ovs_br->name);
+        br->wait_before_clear_time = _wait_before_clear_time;
+    }
+}
diff --git a/br-controller/en-bridge-data.h b/br-controller/en-bridge-data.h
index b374798649..05ab556637 100644
--- a/br-controller/en-bridge-data.h
+++ b/br-controller/en-bridge-data.h
@@ -26,6 +26,10 @@  struct ovn_bridge {
 
     /* simap of ovs interface names to ofport numbers. */
     struct simap ovs_ifaces;
+
+    int probe_interval;
+    char *conn_target;
+    unsigned int wait_before_clear_time;
 };
 
 struct ed_type_bridge_data {
diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
index ae0e192429..74f2b7a2d2 100644
--- a/br-controller/ovn-br-controller.c
+++ b/br-controller/ovn-br-controller.c
@@ -35,11 +35,13 @@ 
 
 
 /* OVN includes. */
+#include "br-ofctrl.h"
 #include "en-bridge-data.h"
 #include "en-lflow.h"
 #include "en-pflow.h"
 #include "lib/ovn-br-idl.h"
 #include "lib/inc-proc-eng.h"
+#include "lib/ofctrl-seqno.h"
 #include "lib/ovn-util.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
@@ -55,6 +57,9 @@  static const char *ssl_ca_cert_file;
 /* --unixctl-path: Path to use for unixctl server socket. */
 static char *unixctl_path;
 
+/* Registered ofctrl seqno type for br_cfg propagation. */
+static size_t ofctrl_seq_type_br_cfg;
+
 #define BRCTL_NODES \
     BRCTL_NODE(br_global) \
     BRCTL_NODE(bridge) \
@@ -110,7 +115,12 @@  en_br_controller_output_run(struct engine_node *node OVS_UNUSED,
 /* Static function declarations. */
 static void ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl);
 static void update_br_db(struct ovsdb_idl *ovs_idl,
-                         struct ovsdb_idl *ovn_br_idl);
+                         struct ovsdb_idl *ovnbr_idl,
+                         unsigned int *ovnbr_cond_seqno);
+static unsigned int update_ovnbr_monitors(struct ovsdb_idl *);
+static uint64_t get_ovnbr_cfg(const struct ovnbrrec_br_global_table *,
+                                  unsigned int cond_seqno,
+                                  unsigned int expected_cond_seqno);
 
 int
 main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
@@ -138,6 +148,9 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 
     daemonize_complete();
 
+    /* Register ofctrl seqno types. */
+    ofctrl_seq_type_br_cfg = ofctrl_seqno_add_type();
+
     /* Connect to OVS OVSDB instance. */
     struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
         ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
@@ -206,8 +219,12 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
     engine_init(&en_br_controller_output, &engine_arg);
     engine_ovsdb_node_add_index(&en_ovs_bridge, "name", ovsrec_bridge_by_name);
 
-    unsigned int ovs_cond_seqno = UINT_MAX;
+    unsigned int ovnbr_expected_cond_seqno = UINT_MAX;
     unsigned int ovnbr_cond_seqno = UINT_MAX;
+    unsigned int ovs_cond_seqno = UINT_MAX;
+
+    struct ed_type_bridge_data *br_data =
+        engine_get_internal_data(&en_bridge_data);
 
     /* Main loop. */
     while (!exit_args.exiting) {
@@ -224,7 +241,8 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
             ovs_cond_seqno = new_ovs_cond_seqno;
         }
 
-        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl);
+        update_br_db(ovs_idl_loop.idl, ovnbr_idl_loop.idl,
+                     &ovnbr_expected_cond_seqno);
         struct ovsdb_idl_txn *ovnbr_idl_txn
             = ovsdb_idl_loop_run(&ovnbr_idl_loop);
         unsigned int new_ovnbr_cond_seqno
@@ -251,10 +269,48 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 
         if (ovsdb_idl_has_ever_connected(ovnbr_idl_loop.idl) && cfg) {
             engine_run(true);
+
+            br_data = engine_get_data(&en_bridge_data);
+            if (br_data) {
+                struct sset bridges_in_br_ofctrl =
+                    SSET_INITIALIZER(&bridges_in_br_ofctrl);
+                br_ofctrls_get_bridges(&bridges_in_br_ofctrl);
+                struct shash_node *node;
+                SHASH_FOR_EACH (node, &br_data->bridges) {
+                    struct ovn_bridge *br = node->data;
+
+                    if (br->ovs_br) {
+                        sset_find_and_delete(&bridges_in_br_ofctrl,
+                                             br->db_br->name);
+                        br_ofctrls_add_or_update_bridge(br);
+                    }
+                }
+
+                const char *bridge;
+                SSET_FOR_EACH (bridge, &bridges_in_br_ofctrl) {
+                    br_ofctrls_remove_bridge(bridge);
+                }
+
+                sset_destroy(&bridges_in_br_ofctrl);
+            }
+
+            br_ofctrls_run();
+
+            ofctrl_seqno_update_create(
+                ofctrl_seq_type_br_cfg,
+                get_ovnbr_cfg(ovnbrrec_br_global_table_get(ovnbr_idl_loop.idl),
+                              ovnbr_cond_seqno, ovnbr_expected_cond_seqno));
+
+            br_ofctrls_put(ofctrl_seqno_get_req_cfg(),
+                           engine_node_changed(&en_lflow_output),
+                           engine_node_changed(&en_pflow_output));
+
+            ofctrl_seqno_run(br_ofctrl_get_cur_cfg());
         }
 
         unixctl_server_run(unixctl);
 
+        br_ofctrls_wait();
         unixctl_server_wait(unixctl);
         if (exit_args.exiting) {
             poll_immediate_wake();
@@ -440,7 +496,8 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 /* Retrieves the pointer to the OVN Bridge Controller database from 'ovs_idl'
  * and updates 'brdb_idl' with that pointer. */
 static void
-update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
+update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl,
+             unsigned int *ovnbr_cond_seqno)
 {
     const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
     if (!cfg) {
@@ -449,4 +506,55 @@  update_br_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnbr_idl)
 
     const char *remote = smap_get(&cfg->external_ids, "ovn-br-remote");
     ovsdb_idl_set_remote(ovnbr_idl, remote, true);
+
+    unsigned int next_cond_seqno = update_ovnbr_monitors(ovnbr_idl);
+    if (ovnbr_cond_seqno) {
+        *ovnbr_cond_seqno = next_cond_seqno;
+    }
+}
+
+/* Assume the table exists in the server schema and set its condition. */
+#define ovnbr_table_set_req_mon_condition(idl, table, cond) \
+    ovnbrrec_##table##_set_condition(idl, cond)
+
+static unsigned int
+update_ovnbr_monitors(struct ovsdb_idl *ovnbr_idl)
+{
+    struct ovsdb_idl_condition br = OVSDB_IDL_CONDITION_INIT(&br);
+    struct ovsdb_idl_condition lf = OVSDB_IDL_CONDITION_INIT(&lf);
+
+    ovsdb_idl_condition_add_clause_true(&br);
+    ovsdb_idl_condition_add_clause_true(&lf);
+
+    unsigned int cond_seqnos[] = {
+        ovnbr_table_set_req_mon_condition(ovnbr_idl, bridge, &br),
+        ovnbr_table_set_req_mon_condition(ovnbr_idl, logical_flow, &lf),
+    };
+
+    unsigned int expected_cond_seqno = 0;
+    for (size_t i = 0; i < ARRAY_SIZE(cond_seqnos); i++) {
+        expected_cond_seqno = MAX(expected_cond_seqno, cond_seqnos[i]);
+    }
+
+    return expected_cond_seqno;
+}
+
+static uint64_t
+get_ovnbr_cfg(const struct ovnbrrec_br_global_table *br_global_table,
+              unsigned int cond_seqno, unsigned int expected_cond_seqno)
+{
+    static uint64_t br_cfg = 0;
+
+    /* Delay getting br_cfg if there are monitor condition changes
+     * in flight.  It might be that those changes would instruct the
+     * server to send updates that happened before PR_Global.pr_cfg.
+     */
+    if (cond_seqno != expected_cond_seqno) {
+        return br_cfg;
+    }
+
+    const struct ovnbrrec_br_global *br_global
+        = ovnbrrec_br_global_table_first(br_global_table);
+    br_cfg = br_global ? br_global->br_cfg : 0;
+    return br_cfg;
 }
diff --git a/tests/automake.mk b/tests/automake.mk
index e6a31c6f69..84c0ea2219 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -46,7 +46,8 @@  TESTSUITE_AT = \
 	tests/ovn-lflow-conj-ids.at \
 	tests/ovn-ipsec.at \
 	tests/ovn-vif-plug.at \
-	tests/ovn-util.at
+	tests/ovn-util.at \
+	tests/ovn-br-controller.at
 
 SYSTEM_DPDK_TESTSUITE_AT = \
 	tests/system-dpdk-testsuite.at \
@@ -89,7 +90,7 @@  DISTCLEANFILES += tests/atconfig tests/atlocal
 MULTINODE_TESTSUITE = $(srcdir)/tests/multinode-testsuite
 MULTINODE_TESTSUITE_DIR = $(abs_top_builddir)/tests/multinode-testsuite.dir
 MULTINODE_TESTSUITE_RESULTS = $(MULTINODE_TESTSUITE_DIR)/results
-AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic
+AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic:br-controller
 
 export ovs_srcdir
 export ovs_builddir
diff --git a/tests/ovn-br-controller.at b/tests/ovn-br-controller.at
new file mode 100644
index 0000000000..bb10e5bd10
--- /dev/null
+++ b/tests/ovn-br-controller.at
@@ -0,0 +1,327 @@ 
+AT_BANNER([ovn_br_controller])
+
+# OVN_BR_CONTROLLER_START(SIM_NAME)
+#
+# $1 - optional simulator name. If none is given, runs ovn-br-controller
+#      in $ovs_dir.
+# Starts the test with a setup with ovn bridge controller.  Each test case must first
+# call this macro and ovn_start.
+#
+m4_define([OVN_BR_CONTROLLER_START], [
+    AT_KEYWORDS([ovn-br-controller])
+    mkdir -p "$ovs_dir" || return 1
+    mkdir "$ovs_base"/ovn-br || return 1
+
+    dnl Create databases (vswitch).
+    check ovsdb-tool create "$ovs_dir"/vswitchd.db $ovs_srcdir/vswitchd/vswitch.ovsschema
+    check ovsdb-tool create "$ovs_base"/ovn-br/ovn-br.db "$abs_top_srcdir"/ovn-br.ovsschema
+
+    dnl Start ovsdb-server.
+    start_daemon ovsdb-server --remote=punix:"$ovs_dir"/db.sock \
+                             "$ovs_dir"/vswitchd.db
+
+    ovn_br_remote=unix:"$ovs_base"/ovn-br/ovnbr_db.sock
+    dnl Start ovs-vswitchd.
+    start_daemon ovs-vswitchd --enable-dummy=system -vvconn -vofproto_dpif
+
+    ovs-vsctl \
+        -- set Open_vSwitch . external-ids:ovn-br-remote=$ovn_br_remote
+    dnl Start ovsdb-server for ovn-br.
+    as ovn-br start_daemon ovsdb-server --remote=punix:"$ovs_base"/ovn-br/ovnbr_db.sock \
+        "$ovs_base"/ovn-br/ovn-br.db
+
+    which ovn-br-controller
+    dnl Start ovn-br-controller.
+    start_daemon ovn-br-controller
+])
+
+m4_define([OVN_BR_CONTROLLER_STOP],[
+   echo
+   echo "Clean up ovn-br-controller related processes in $2"
+   test -n "$2" && as "$2"
+   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+   OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+   OVS_APP_EXIT_AND_WAIT([ovn-br-controller])
+
+   as ovn-br
+   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+])
+
+AT_SETUP([ovn-br-controller - brctl test])
+OVN_BR_CONTROLLER_START
+
+check as ovn-br ovn-brctl show
+check as ovn-br ovn-brctl add-br br0
+
+AT_CHECK([as ovn-br ovn-brctl show | uuidfilt], [0],
+  [bridge <0> (br0)
+])
+
+AT_CHECK([as ovn-br ovn-brctl show br0 | uuidfilt], [0],
+  [bridge <0> (br0)
+])
+
+AT_CHECK([as ovn-br ovn-brctl show br1 | uuidfilt], [0], [],
+  [ovn-brctl: no row "br1" in table Bridge
+])
+
+check as ovn-br ovn-brctl del-br br0
+check as ovn-br ovn-brctl show
+
+check as ovn-br ovn-brctl add-br br0
+
+check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && tcp" "drop;"
+check as ovn-br ovn-brctl add-flow br0 0 1000 "ip4 && udp" "next;"
+check as ovn-br ovn-brctl add-flow br0 1 0 "ip4 && udp" "output;"
+
+check as ovn-br ovn-brctl add-br br1
+
+check as ovn-br ovn-brctl add-flow br1 0 1000 "ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10" "drop;"
+check as ovn-br ovn-brctl add-flow br1 0 0 "1" "output;"
+
+AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
+  [dnl
+Bridge: br0 (<0>)
+  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
+  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
+  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
+Bridge: br1 (<1>)
+  table=0 , priority=1000 , match=(ip4 && tcp.dst == 1000 && ip4.dst == 10.0.0.10), action=(drop;)
+  table=0 , priority=0    , match=(1), action=(output;)
+])
+
+as ovn-br ovn-brctl del-flows br1
+
+AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
+  [dnl
+Bridge: br0 (<0>)
+  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
+  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
+  table=1 , priority=0    , match=(ip4 && udp), action=(output;)
+])
+
+lflow_uuid=$(as ovn-br ovn-brctl --bare --columns _uuid find logical_flow table_id=1)
+check as ovn-br ovn-brctl del-flow $lflow_uuid
+
+AT_CHECK([as ovn-br ovn-brctl dump-flows | uuidfilt], [0],
+  [dnl
+Bridge: br0 (<0>)
+  table=0 , priority=1000 , match=(ip4 && tcp), action=(drop;)
+  table=0 , priority=1000 , match=(ip4 && udp), action=(next;)
+])
+
+OVN_BR_CONTROLLER_STOP
+AT_CLEANUP
+
+AT_SETUP([ovn-br-controller - logical flows])
+OVN_BR_CONTROLLER_START
+
+check as ovn-br ovn-brctl add-br br0
+
+check ovs-vsctl add-br br0
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 3])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+NXST_FLOW reply:
+])
+
+check ovs-vsctl add-port br0 p1 -- set interface p1 ofport-request=2
+check ovs-vsctl add-port br0 p2 -- set interface p2 ofport-request=3
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 7])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=65, priority=100,reg15=0x3 actions=output:3
+NXST_FLOW reply:
+])
+
+check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p1"' "next;"
+check as ovn-br ovn-brctl add-flow br0 0 1000 'inport == "p2"' "drop;"
+check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4 && tcp' "ip4.src <-> ip4.dst; tcp.dst = 8080; next;"
+check as ovn-br ovn-brctl add-flow br0 1 1000 'ip4' "next;"
+check as ovn-br ovn-brctl add-flow br0 2 1000 '1' "output;"
+
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 12])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=3 actions=load:0x3->NXM_NX_REG14[[]],resubmit(,8)
+ table=10, priority=1000 actions=resubmit(,64)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=65, priority=100,reg15=0x3 actions=output:3
+ table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
+ table=8, priority=1000,reg14=0x3 actions=drop
+ table=9, priority=1000,ip actions=resubmit(,10)
+ table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
+NXST_FLOW reply:
+])
+
+check ovs-vsctl del-port p2
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br0 | grep -v NXST_FLOW | wc -l` -eq 9])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br0 | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ table=10, priority=1000 actions=resubmit(,64)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=8, priority=1000,reg14=0x2 actions=resubmit(,9)
+ table=9, priority=1000,ip actions=resubmit(,10)
+ table=9, priority=1000,tcp actions=push:NXM_OF_IP_DST[[]],push:NXM_OF_IP_SRC[[]],pop:NXM_OF_IP_DST[[]],pop:NXM_OF_IP_SRC[[]],mod_tp_dst:8080,resubmit(,10)
+NXST_FLOW reply:
+])
+
+check ovs-vsctl add-br br1
+check ovs-vsctl add-port br1 br1-p1 -- set interface br1-p1 ofport-request=1
+check ovs-vsctl add-port br1 br1-p2 -- set interface br1-p2 ofport-request=2
+
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 1])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+NXST_FLOW reply:
+])
+
+as ovn-br ovn-brctl --id=4830e8c3-9b6b-48db-ba52-e030d9db7256 create bridge name=br1
+as ovn-br ovn-brctl list bridge
+
+# check as ovn-br ovn-brctl add-br br1
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 7])
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x1 actions=output:1
+ table=65, priority=100,reg15=0x2 actions=output:2
+NXST_FLOW reply:
+])
+
+check as ovn-br ovn-brctl add-flow br1 1 1000 "ip4" "ct_snat;"
+check as ovn-br ovn-brctl add-flow br1 2 1000 "ip4 && ct.new && ct.trk && ip4.src == 10.0.0.11" "ct_snat(100.64.0.11); next;"
+check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p1\"" "outport = \"br1-p2\"; output;"
+check as ovn-br ovn-brctl add-flow br1 3 1000 "inport == \"br1-p2\"" "outport = \"br1-p1\"; output;"
+
+as ovn-br ovn-brctl --id=75bf46aa-4204-4e36-af23-6114f59e3fe8 create logical_flow \
+match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"' \
+actions="next;" bridge=4830e8c3-9b6b-48db-ba52-e030d9db7256 table_id=10 priority=1000
+
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
+ table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
+ table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
+ table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)
+ table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x1 actions=output:1
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
+NXST_FLOW reply:
+])
+
+as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 match='"ip4 && sctp"'
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 12])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
+ table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
+ table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
+ table=18, priority=1000,sctp actions=resubmit(,19)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x1 actions=output:1
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
+NXST_FLOW reply:
+])
+
+# Make sure that the same conj_id is used when the lflow is updated with the conj match.
+as ovn-br ovn-brctl set logical_flow 75bf46aa-4204-4e36-af23-6114f59e3fe8 \
+match='"ip4 && tcp.src > 0 && tcp.src < 1000 && tcp.dst > 1000 && tcp.dst < 2000"'
+OVS_WAIT_UNTIL([test `ovs-ofctl dump-flows br1 | grep -v NXST_FLOW | wc -l` -eq 35])
+
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br1 | sort | ofctl_strip_all], [0], [dnl
+ priority=0 actions=NORMAL
+ priority=100,in_port=1 actions=load:0x1->NXM_NX_REG14[[]],resubmit(,8)
+ priority=100,in_port=2 actions=load:0x2->NXM_NX_REG14[[]],resubmit(,8)
+ table=10, priority=1000,ct_state=+new+trk,ip,nw_src=10.0.0.11 actions=ct(commit,table=11,zone=NXM_NX_REG12[[0..15]],nat(src=100.64.0.11)),resubmit(,11)
+ table=11, priority=1000,reg14=0x1 actions=load:0x2->NXM_NX_REG15[[]],resubmit(,64)
+ table=11, priority=1000,reg14=0x2 actions=load:0x1->NXM_NX_REG15[[]],resubmit(,64)
+ table=18, priority=1000,conj_id=1644032429,tcp actions=resubmit(,19)
+ table=18, priority=1000,tcp,tp_dst=0x3ea/0xfffe actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x3ec/0xfffc actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x3f0/0xfff0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x400/0xfe00 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x600/0xff00 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x700/0xff80 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x780/0xffc0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=0x7c0/0xfff0 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_dst=1001 actions=conjunction(1644032429,1/2)
+ table=18, priority=1000,tcp,tp_src=0x1/0xfe01 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x10/0xfe10 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x100/0xff00 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x2/0xfe02 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x20/0xfe20 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x200/0xff00 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x300/0xff80 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x380/0xffc0 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x3c0/0xffe0 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x3e0/0xfff8 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x4/0xfe04 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x40/0xfe40 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x8/0xfe08 actions=conjunction(1644032429,2/2)
+ table=18, priority=1000,tcp,tp_src=0x80/0xfe80 actions=conjunction(1644032429,2/2)
+ table=64, priority=0 actions=resubmit(,65)
+ table=65, priority=0 actions=NORMAL
+ table=65, priority=100,reg15=0x1 actions=output:1
+ table=65, priority=100,reg15=0x2 actions=output:2
+ table=9, priority=1000,ip actions=ct(table=10,zone=NXM_NX_REG12[[0..15]],nat)
+NXST_FLOW reply:
+])
+
+OVN_BR_CONTROLLER_STOP
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 8e60bf82e1..5f5eabb42a 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -41,3 +41,4 @@  m4_include([tests/checkpatch.at])
 m4_include([tests/ovn-ipsec.at])
 m4_include([tests/ovn-vif-plug.at])
 m4_include([tests/ovn-util.at])
+m4_include([tests/ovn-br-controller.at])