[ovs-dev] [PATCH 21/22] ofctrl: Negotiate OVN Geneve option.

Alex Wang alexw at nicira.com
Sat Aug 1 05:04:35 UTC 2015


Is there any documentation for the ovs side geneve negotiation?

Also one question below:

Thanks,
Alex Wang,

On Sun, Jul 19, 2015 at 3:45 PM, Ben Pfaff <blp at nicira.com> wrote:

> This won't really get used until the next commit.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ovn/controller/ofctrl.c         | 470
> ++++++++++++++++++++++++++++------------
>  ovn/controller/ofctrl.h         |   5 +-
>  ovn/controller/ovn-controller.c |   6 +-
>  ovn/controller/physical.h       |   7 +
>  4 files changed, 348 insertions(+), 140 deletions(-)
>
> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
> index b1c421c..d8a0573 100644
> --- a/ovn/controller/ofctrl.c
> +++ b/ovn/controller/ofctrl.c
> @@ -27,9 +27,10 @@
>  #include "openflow/openflow.h"
>  #include "openvswitch/vlog.h"
>  #include "ovn-controller.h"
> -#include "vswitch-idl.h"
> +#include "physical.h"
>  #include "rconn.h"
>  #include "socket-util.h"
> +#include "vswitch-idl.h"
>
>  VLOG_DEFINE_THIS_MODULE(ofctrl);
>
> @@ -53,6 +54,9 @@ static char *ovn_flow_to_string(const struct ovn_flow *);
>  static void ovn_flow_log(const struct ovn_flow *, const char *action);
>  static void ovn_flow_destroy(struct ovn_flow *);
>
> +static ovs_be32 queue_msg(struct ofpbuf *);
> +static void queue_flow_mod(struct ofputil_flow_mod *);
> +
>  /* OpenFlow connection to the switch. */
>  static struct rconn *swconn;
>
> @@ -60,6 +64,25 @@ static struct rconn *swconn;
>   * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
>  static unsigned int seqno;
>
> +/* Connection state machine. */
> +#define STATES                                  \
> +    STATE(S_NEW)                                \
> +    STATE(S_GENEVE_TABLE_REQUESTED)             \
> +    STATE(S_GENEVE_TABLE_MOD_SENT)              \
> +    STATE(S_CLEAR_FLOWS)                        \
> +    STATE(S_UPDATE_FLOWS)
> +enum ofctrl_state {
> +#define STATE(NAME) NAME,
> +    STATES
> +#undef STATE
> +};
> +
> +/* Current state. */
> +static enum ofctrl_state state;
> +
> +/* Transaction IDs for messages in flight to the switch. */
> +static ovs_be32 xid, xid2;
> +
>  /* 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. */
> @@ -69,11 +92,15 @@ static struct rconn_packet_counter *tx_counter;
>   * installed in the switch. */
>  static struct hmap installed_flows;
>
> +/* MFF_* field ID for our Geneve option.  In S_GENEVE_TABLE_MOD_SENT,
> this is
> + * the option we requested (we don't know whether we obtained it yet).  In
> + * S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
> +static enum mf_field_id mff_ovn_geneve;
> +
>  static void ovn_flow_table_clear(struct hmap *flow_table);
>  static void ovn_flow_table_destroy(struct hmap *flow_table);
>
> -static void ofctrl_update_flows(struct hmap *desired_flows);
> -static void ofctrl_recv(const struct ofpbuf *msg);
> +static void ofctrl_recv(const struct ofp_header *, enum ofptype);
>
>  void
>  ofctrl_init(void)
> @@ -82,15 +109,244 @@ ofctrl_init(void)
>      tx_counter = rconn_packet_counter_create();
>      hmap_init(&installed_flows);
>  }
> +
> +/* S_NEW, for a new connection.
> + *
> + * Sends NXT_GENEVE_TABLE_REQUEST and transitions to
> + * S_GENEVE_TABLE_REQUESTED. */
>
> -/* Attempts to update the OpenFlow flows in bridge 'br_int' to those in
> - * 'flow_table'.  Removes all of the flows from 'flow_table' and frees
> them.
> +static void
> +run_S_NEW(void)
> +{
> +    struct ofpbuf *buf = ofpraw_alloc(OFPRAW_NXT_GENEVE_TABLE_REQUEST,
> +                                      rconn_get_version(swconn), 0);
> +    xid = queue_msg(buf);
> +    state = S_GENEVE_TABLE_REQUESTED;
> +}
> +
> +static void
> +recv_S_NEW(const struct ofp_header *oh OVS_UNUSED,
> +           enum ofptype type OVS_UNUSED)
> +{
> +    OVS_NOT_REACHED();
> +}
> +
> +/* S_GENEVE_TABLE_REQUESTED, when NXT_GENEVE_TABLE_REQUEST has been sent
> + * and we're waiting for a reply.
>   *
> - * The flow table will only be updated if we've got an OpenFlow
> connection to
> - * 'br_int' and it's not backlogged.  Otherwise, it'll have to wait until
> the
> - * next iteration. */
> -void
> -ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
> + * If we receive an NXT_GENEVE_TABLE_REPLY:
> + *
> + *     - If it contains our tunnel metadata option, assign its field ID to
> + *       mff_ovn_geneve and transition to S_CLEAR_FLOWS.
> + *
> + *     - Otherwise, if there is an unused tunnel metadata field ID, send
> + *       NXT_GENEVE_TABLE_MOD and OFPT_BARRIER_REQUEST, and transition to
> + *       S_GENEVE_TABLE_MOD_SENT.
> + *
> + *     - Otherwise, log an error, disable Geneve, and transition to
> + *       S_CLEAR_FLOWS.
> + *
> + * If we receive an OFPT_ERROR:
> + *
> + *     - Log an error, disable Geneve, and transition to S_CLEAR_FLOWS. */
> +
> +static void
> +run_S_GENEVE_TABLE_REQUESTED(void)
> +{
> +}
> +
> +static void
> +recv_S_GENEVE_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype
> type)
> +{
> +    if (oh->xid != xid) {
> +        ofctrl_recv(oh, type);
> +    } else if (type == OFPTYPE_NXT_GENEVE_TABLE_REPLY) {
> +        struct ofputil_geneve_table_reply reply;
> +        enum ofperr error = ofputil_decode_geneve_table_reply(oh, &reply);
> +        if (error) {
> +            VLOG_ERR("failed to decode Geneve table request (%s)",
> +                     ofperr_to_string(error));
> +            goto error;
> +        }
> +
> +        const struct ofputil_geneve_map *map;
> +        uint64_t md_free = UINT64_MAX;
> +        BUILD_ASSERT(TUN_METADATA_NUM_OPTS == 64);
> +
> +        LIST_FOR_EACH (map, list_node, &reply.mappings) {
> +            if (map->option_class == OVN_GENEVE_CLASS
> +                && map->option_type == OVN_GENEVE_TYPE
> +                && map->option_len == OVN_GENEVE_LEN) {
> +                if (map->index >= TUN_METADATA_NUM_OPTS) {
> +                    VLOG_ERR("desired Geneve tunnel option 0x%"PRIx16","
> +                             "%"PRIu8",%"PRIu8" already in use with "
> +                             "unsupported index %"PRIu16,
> +                             map->option_class, map->option_type,
> +                             map->option_len, map->index);
> +                    goto error;
> +                } else {
> +                    mff_ovn_geneve = MFF_TUN_METADATA0 + map->index;
> +                    state = S_CLEAR_FLOWS;
> +                    return;
> +                }
> +            }
> +
> +            if (map->index < TUN_METADATA_NUM_OPTS) {
> +                md_free &= ~(UINT64_C(1) << map->index);
> +            }
> +        }
> +
> +        VLOG_DBG("OVN Geneve option not found");
> +        if (!md_free) {
> +            VLOG_ERR("no Geneve options free for use by OVN");
> +            goto error;
> +        }
> +
> +        unsigned int index = rightmost_1bit_idx(md_free);
> +        mff_ovn_geneve = MFF_TUN_METADATA0 + index;
> +        struct ofputil_geneve_map gm;
> +        gm.option_class = OVN_GENEVE_CLASS;
> +        gm.option_type = OVN_GENEVE_TYPE;
> +        gm.option_len = OVN_GENEVE_LEN;
> +        gm.index = index;
> +
> +        struct ofputil_geneve_table_mod gtm;
> +        gtm.command = NXGTMC_ADD;
> +        list_init(&gtm.mappings);
> +        list_push_back(&gtm.mappings, &gm.list_node);
> +
> +        xid = queue_msg(ofputil_encode_geneve_table_mod(OFP13_VERSION,
> &gtm));
> +        xid2 = queue_msg(ofputil_encode_barrier_request(OFP13_VERSION));
> +        state = S_GENEVE_TABLE_MOD_SENT;
> +    } else if (type == OFPTYPE_ERROR) {
> +        VLOG_ERR("switch refused to allocate Geneve option (%s)",
> +                 ofperr_to_string(ofperr_decode_msg(oh, NULL)));
> +        goto error;
> +    } else {
> +        char *s = ofp_to_string(oh, ntohs(oh->length), 1);
> +        VLOG_ERR("unexpected reply to Geneve table request (%s)",
> +                 s);
> +        free(s);
> +        goto error;
> +    }
> +    return;
> +
> +error:
> +    mff_ovn_geneve = 0;
> +    state = S_CLEAR_FLOWS;
> +}
> +
> +/* S_GENEVE_TABLE_MOD_SENT, when NXT_GENEVE_TABLE_MOD and
> OFPT_BARRIER_REQUEST
> + * have been sent and we're waiting for a reply to one or the other.
> + *
> + * If we receive an OFPT_ERROR:
> + *
> + *     - If the error is NXGTMFC_ALREADY_MAPPED or NXGTMFC_DUP_ENTRY, we
> + *       raced with some other controller.  Transition to S_NEW.
> + *
> + *     - Otherwise, log an error, disable Geneve, and transition to
> + *       S_CLEAR_FLOWS.
> + *
> + * If we receive OFPT_BARRIER_REPLY:
> + *
> + *     - Set the tunnel metadata field ID to the one that we requested.
> + *       Transition to S_CLEAR_FLOWS.
> + */
> +
> +static void
> +run_S_GENEVE_TABLE_MOD_SENT(void)
> +{
> +}
> +
> +static void
> +recv_S_GENEVE_TABLE_MOD_SENT(const struct ofp_header *oh, enum ofptype
> type)
> +{
> +    if (oh->xid != xid && oh->xid != xid2) {
> +        ofctrl_recv(oh, type);
> +    } else if (oh->xid == xid2 && type == OFPTYPE_BARRIER_REPLY) {
> +        state = S_CLEAR_FLOWS;
> +    } else if (oh->xid == xid && type == OFPTYPE_ERROR) {
> +        enum ofperr error = ofperr_decode_msg(oh, NULL);
> +        if (error == OFPERR_NXGTMFC_ALREADY_MAPPED ||
> +            error == OFPERR_NXGTMFC_DUP_ENTRY) {
> +            VLOG_INFO("raced with another controller adding "
> +                      "Geneve option (%s); trying again",
> +                      ofperr_to_string(error));
> +            state = S_NEW;
> +        } else {
> +            VLOG_ERR("error adding Geneve option (%s)",
> +                     ofperr_to_string(error));
> +            goto error;
> +        }
> +    } else {
> +        char *s = ofp_to_string(oh, ntohs(oh->length), 1);
> +        VLOG_ERR("unexpected reply to Geneve option allocation request
> (%s)",
> +                 s);
> +        free(s);
> +        goto error;
> +    }
> +    return;
> +
> +error:
> +    state = S_CLEAR_FLOWS;
> +}
> +
> +/* S_CLEAR_FLOWS, after we've established a Geneve metadata field ID and
> it's
> + * time to set up some flows.
> + *
> + * Sends an OFPT_TABLE_MOD to clear all flows, then transitions to
> + * S_UPDATE_FLOWS. */
> +
> +static void
> +run_S_CLEAR_FLOWS(void)
> +{
> +    /* Send a flow_mod to delete all flows. */
> +    struct ofputil_flow_mod fm = {
> +        .match = MATCH_CATCHALL_INITIALIZER,
> +        .table_id = OFPTT_ALL,
> +        .command = OFPFC_DELETE,
> +    };
> +    queue_flow_mod(&fm);
> +    VLOG_DBG("clearing all flows");
> +
> +    /* Clear installed_flows, to match the state of the switch. */
> +    ovn_flow_table_clear(&installed_flows);
> +
> +    state = S_UPDATE_FLOWS;
> +}
> +
> +static void
> +recv_S_CLEAR_FLOWS(const struct ofp_header *oh, enum ofptype type)
> +{
> +    ofctrl_recv(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(void)
> +{
> +}
> +
> +
> +static void
> +recv_S_UPDATE_FLOWS(const struct ofp_header *oh, enum ofptype type)
> +{
> +    ofctrl_recv(oh, type);
> +}
> +
> +/* Runs the OpenFlow state machine against 'br_int', which is local to the
> + * hypervisor on which we are running.  Attempts to negotiate a Geneve
> option
> + * field for class OVN_GENEVE_CLASS, type OVN_GENEVE_TYPE.  If successful,
> + * returns the MFF_* field ID for the option, otherwise returns 0. */
> +enum mf_field_id
> +ofctrl_run(const struct ovsrec_bridge *br_int)
>  {
>      if (br_int) {
>          char *target;
> @@ -107,24 +363,56 @@ ofctrl_run(const struct ovsrec_bridge *br_int,
> struct hmap *flow_table)
>      rconn_run(swconn);
>
>      if (!rconn_is_connected(swconn)) {
> -        goto exit;
> +        return 0;
>      }
> -    if (!rconn_packet_counter_n_packets(tx_counter)) {
> -        ofctrl_update_flows(flow_table);
> +    if (seqno != rconn_get_connection_seqno(swconn)) {
> +        seqno = rconn_get_connection_seqno(swconn);
> +        state = S_NEW;
>      }
>
> -    for (int i = 0; i < 50; i++) {
> +    enum ofctrl_state old_state;
> +    do {
> +        old_state = state;
> +        switch (state) {
> +#define STATE(NAME) case NAME: run_##NAME(); break;
> +            STATES
> +#undef STATE
> +        default:
> +            OVS_NOT_REACHED();
> +        }
> +    } while (state != old_state);
> +
> +    for (int i = 0; state == old_state && i < 50; i++) {
>          struct ofpbuf *msg = rconn_recv(swconn);
>          if (!msg) {
>              break;
>          }
>
> -        ofctrl_recv(msg);
> +        const struct ofp_header *oh = msg->data;
> +        enum ofptype type;
> +        enum ofperr error;
> +
> +        error = ofptype_decode(&type, oh);
> +        if (!error) {
> +            switch (state) {
> +#define STATE(NAME) case NAME: recv_##NAME(oh, type); break;
> +                STATES
> +#undef STATE
> +            default:
> +                OVS_NOT_REACHED();
> +            }
> +        } else {
> +            char *s = ofp_to_string(oh, ntohs(oh->length), 1);
> +            VLOG_WARN("could not decode OpenFlow message (%s): %s",
> +                      ofperr_to_string(error), s);
> +            free(s);
> +        }
> +
>          ofpbuf_delete(msg);
>      }
>
> -exit:
> -    ovn_flow_table_clear(flow_table);
> +    return (state == S_CLEAR_FLOWS || state == S_UPDATE_FLOWS
> +            ? mff_ovn_geneve : 0);
>  }
>
>  void
> @@ -142,109 +430,28 @@ ofctrl_destroy(void)
>      rconn_packet_counter_destroy(tx_counter);
>  }
>
> -static void
> +static ovs_be32
>  queue_msg(struct ofpbuf *msg)
>  {
> +    const struct ofp_header *oh = msg->data;
> +    ovs_be32 xid = oh->xid;
>      rconn_send(swconn, msg, tx_counter);
> +    return xid;
>  }
>
>  static void
> -ofctrl_recv(const struct ofpbuf *msg)
> +ofctrl_recv(const struct ofp_header *oh, enum ofptype type)
>  {
> -    enum ofptype type;
> -    struct ofpbuf b;
> -
> -    b = *msg;
> -    if (ofptype_pull(&type, &b)) {
> -        return;
> -    }
> -
> -    switch (type) {
> -    case OFPTYPE_ECHO_REQUEST:
> -        queue_msg(make_echo_reply(msg->data));
> -        break;
> -
> -    case OFPTYPE_ECHO_REPLY:
> -    case OFPTYPE_PACKET_IN:
> -    case OFPTYPE_PORT_STATUS:
> -    case OFPTYPE_FLOW_REMOVED:
> -        /* Nothing to do. */
> -        break;
> -
> -    case OFPTYPE_HELLO:
> -    case OFPTYPE_ERROR:
> -    case OFPTYPE_FEATURES_REQUEST:
> -    case OFPTYPE_FEATURES_REPLY:
> -    case OFPTYPE_GET_CONFIG_REQUEST:
> -    case OFPTYPE_GET_CONFIG_REPLY:
> -    case OFPTYPE_SET_CONFIG:
> -    case OFPTYPE_PACKET_OUT:
> -    case OFPTYPE_FLOW_MOD:
> -    case OFPTYPE_GROUP_MOD:
> -    case OFPTYPE_PORT_MOD:
> -    case OFPTYPE_TABLE_MOD:
> -    case OFPTYPE_BARRIER_REQUEST:
> -    case OFPTYPE_BARRIER_REPLY:
> -    case OFPTYPE_QUEUE_GET_CONFIG_REQUEST:
> -    case OFPTYPE_QUEUE_GET_CONFIG_REPLY:
> -    case OFPTYPE_DESC_STATS_REQUEST:
> -    case OFPTYPE_DESC_STATS_REPLY:
> -    case OFPTYPE_FLOW_STATS_REQUEST:
> -    case OFPTYPE_FLOW_STATS_REPLY:
> -    case OFPTYPE_AGGREGATE_STATS_REQUEST:
> -    case OFPTYPE_AGGREGATE_STATS_REPLY:
> -    case OFPTYPE_TABLE_STATS_REQUEST:
> -    case OFPTYPE_TABLE_STATS_REPLY:
> -    case OFPTYPE_PORT_STATS_REQUEST:
> -    case OFPTYPE_PORT_STATS_REPLY:
> -    case OFPTYPE_QUEUE_STATS_REQUEST:
> -    case OFPTYPE_QUEUE_STATS_REPLY:
> -    case OFPTYPE_PORT_DESC_STATS_REQUEST:
> -    case OFPTYPE_PORT_DESC_STATS_REPLY:
> -    case OFPTYPE_ROLE_REQUEST:
> -    case OFPTYPE_ROLE_REPLY:
> -    case OFPTYPE_ROLE_STATUS:
> -    case OFPTYPE_SET_FLOW_FORMAT:
> -    case OFPTYPE_FLOW_MOD_TABLE_ID:
> -    case OFPTYPE_SET_PACKET_IN_FORMAT:
> -    case OFPTYPE_FLOW_AGE:
> -    case OFPTYPE_SET_CONTROLLER_ID:
> -    case OFPTYPE_FLOW_MONITOR_STATS_REQUEST:
> -    case OFPTYPE_FLOW_MONITOR_STATS_REPLY:
> -    case OFPTYPE_FLOW_MONITOR_CANCEL:
> -    case OFPTYPE_FLOW_MONITOR_PAUSED:
> -    case OFPTYPE_FLOW_MONITOR_RESUMED:
> -    case OFPTYPE_GET_ASYNC_REQUEST:
> -    case OFPTYPE_GET_ASYNC_REPLY:
> -    case OFPTYPE_SET_ASYNC_CONFIG:
> -    case OFPTYPE_METER_MOD:
> -    case OFPTYPE_GROUP_STATS_REQUEST:
> -    case OFPTYPE_GROUP_STATS_REPLY:
> -    case OFPTYPE_GROUP_DESC_STATS_REQUEST:
> -    case OFPTYPE_GROUP_DESC_STATS_REPLY:
> -    case OFPTYPE_GROUP_FEATURES_STATS_REQUEST:
> -    case OFPTYPE_GROUP_FEATURES_STATS_REPLY:
> -    case OFPTYPE_METER_STATS_REQUEST:
> -    case OFPTYPE_METER_STATS_REPLY:
> -    case OFPTYPE_METER_CONFIG_STATS_REQUEST:
> -    case OFPTYPE_METER_CONFIG_STATS_REPLY:
> -    case OFPTYPE_METER_FEATURES_STATS_REQUEST:
> -    case OFPTYPE_METER_FEATURES_STATS_REPLY:
> -    case OFPTYPE_TABLE_FEATURES_STATS_REQUEST:
> -    case OFPTYPE_TABLE_FEATURES_STATS_REPLY:
> -    case OFPTYPE_TABLE_DESC_REQUEST:
> -    case OFPTYPE_TABLE_DESC_REPLY:
> -    case OFPTYPE_BUNDLE_CONTROL:
> -    case OFPTYPE_BUNDLE_ADD_MESSAGE:
> -    case OFPTYPE_NXT_GENEVE_TABLE_MOD:
> -    case OFPTYPE_NXT_GENEVE_TABLE_REQUEST:
> -    case OFPTYPE_NXT_GENEVE_TABLE_REPLY:
> -    default:
> -        /* Messages that are generally unexpected. */
> +    if (type == OFPTYPE_ECHO_REQUEST) {
> +        queue_msg(make_echo_reply(oh));
> +    } else if (type != OFPTYPE_ECHO_REPLY &&
> +               type != OFPTYPE_PACKET_IN &&
> +               type != OFPTYPE_PORT_STATUS &&
> +               type != OFPTYPE_FLOW_REMOVED) {
>          if (VLOG_IS_DBG_ENABLED()) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30,
> 300);
>
> -            char *s = ofp_to_string(msg->data, msg->size, 2);
> +            char *s = ofp_to_string(oh, ntohs(oh->length), 2);
>              VLOG_DBG_RL(&rl, "OpenFlow packet ignored: %s", s);
>              free(s);
>          }
> @@ -272,6 +479,7 @@ ofctrl_add_flow(struct hmap *desired_flows,
>      f->match = *match;
>      f->ofpacts = xmemdup(actions->data, actions->size);
>      f->ofpacts_len = actions->size;
> +    f->hmap_node.hash = ovn_flow_hash(f);
>
>      if (ovn_flow_lookup(desired_flows, f)) {
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -285,7 +493,7 @@ ofctrl_add_flow(struct hmap *desired_flows,
>          return;
>      }
>
> -    hmap_insert(desired_flows, &f->hmap_node, ovn_flow_hash(f));
> +    hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash);
>  }
>
>  /* ovn_flow. */
> @@ -359,6 +567,7 @@ ovn_flow_table_clear(struct hmap *flow_table)
>          ovn_flow_destroy(f);
>      }
>  }
> +
>  static void
>  ovn_flow_table_destroy(struct hmap *flow_table)
>  {
> @@ -377,26 +586,13 @@ queue_flow_mod(struct ofputil_flow_mod *fm)
>      queue_msg(ofputil_encode_flow_mod(fm, OFPUTIL_P_OF13_OXM));
>  }
>
> -static void
> -ofctrl_update_flows(struct hmap *desired_flows)
> +void
> +ofctrl_put(struct hmap *flow_table)
>  {
> -    /* If we've (re)connected, don't make any assumptions about the flows
> in
> -     * the switch: delete all of them.  (We'll immediately repopulate it
> -     * below.) */
> -    if (seqno != rconn_get_connection_seqno(swconn)) {
> -        seqno = rconn_get_connection_seqno(swconn);
> -
> -        /* Send a flow_mod to delete all flows. */
> -        struct ofputil_flow_mod fm = {
> -            .match = MATCH_CATCHALL_INITIALIZER,
> -            .table_id = OFPTT_ALL,
> -            .command = OFPFC_DELETE,
> -        };
> -        queue_flow_mod(&fm);
> -        VLOG_DBG("clearing all flows");
> -
> -        /* Clear installed_flows, to match the state of the switch. */
> -        ovn_flow_table_clear(&installed_flows);
> +    if (state != S_UPDATE_FLOWS
> +        || rconn_packet_counter_n_packets(tx_counter)) {
> +        ovn_flow_table_clear(flow_table);
> +        return;
>      }
>
>

Should we use maybe a boolean to mark this case, so that ofctrl_wait
could register a wake up event for re-updating the flows?  Wonder if there
could be a delayed flow update issue~





>      /* Iterate through all of the installed flows.  If any of them are no
> @@ -404,7 +600,7 @@ ofctrl_update_flows(struct hmap *desired_flows)
>       * actions, update them. */
>      struct ovn_flow *i, *next;
>      HMAP_FOR_EACH_SAFE (i, next, hmap_node, &installed_flows) {
> -        struct ovn_flow *d = ovn_flow_lookup(desired_flows, i);
> +        struct ovn_flow *d = ovn_flow_lookup(flow_table, i);
>          if (!d) {
>              /* Installed flow is no longer desirable.  Delete it from the
>               * switch and from installed_flows. */
> @@ -442,16 +638,16 @@ ofctrl_update_flows(struct hmap *desired_flows)
>                  d->ofpacts_len = 0;
>              }
>
> -            hmap_remove(desired_flows, &d->hmap_node);
> +            hmap_remove(flow_table, &d->hmap_node);
>              ovn_flow_destroy(d);
>          }
>      }
>
> -    /* The previous loop removed from 'desired_flows' all of the flows
> that are
> -     * already installed.  Thus, any flows remaining in 'desired_flows'
> need to
> +    /* The previous loop removed from 'flow_table' all of the flows that
> are
> +     * already installed.  Thus, any flows remaining in 'flow_table' need
> to
>       * be added to the flow table. */
>      struct ovn_flow *d;
> -    HMAP_FOR_EACH_SAFE (d, next, hmap_node, desired_flows) {
> +    HMAP_FOR_EACH_SAFE (d, next, hmap_node, flow_table) {
>          /* Send flow_mod to add flow. */
>          struct ofputil_flow_mod fm = {
>              .match = d->match,
> @@ -464,8 +660,8 @@ ofctrl_update_flows(struct hmap *desired_flows)
>          queue_flow_mod(&fm);
>          ovn_flow_log(d, "adding");
>
> -        /* Move 'd' from 'desired_flows' to installed_flows. */
> -        hmap_remove(desired_flows, &d->hmap_node);
> +        /* Move 'd' from 'flow_table' to installed_flows. */
> +        hmap_remove(flow_table, &d->hmap_node);
>          hmap_insert(&installed_flows, &d->hmap_node, d->hmap_node.hash);
>      }
>  }
> diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
> index fc07d51..b43527a 100644
> --- a/ovn/controller/ofctrl.h
> +++ b/ovn/controller/ofctrl.h
> @@ -19,6 +19,8 @@
>
>  #include <stdint.h>
>
> +#include "meta-flow.h"
> +
>  struct controller_ctx;
>  struct hmap;
>  struct match;
> @@ -27,7 +29,8 @@ struct ovsrec_bridge;
>
>  /* Interface for OVN main loop. */
>  void ofctrl_init(void);
> -void ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap
> *flow_table);
> +enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int);
> +void ofctrl_put(struct hmap *flows);
>  void ofctrl_wait(void);
>  void ofctrl_destroy(void);
>
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index 9d20245..488dce7 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -265,12 +265,14 @@ main(int argc, char *argv[])
>          }
>
>          if (br_int) {
> +            enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
> +
>              struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
>              rule_run(&ctx, &flow_table);
> -            if (chassis_id) {
>                  physical_run(&ctx, br_int, chassis_id, &flow_table);
> +            if (chassis_id && mff_ovn_geneve) {
>              }
> -            ofctrl_run(br_int, &flow_table);
> +            ofctrl_put(&flow_table);
>              hmap_destroy(&flow_table);
>          }
>
> diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
> index 9de76de..82baa2f 100644
> --- a/ovn/controller/physical.h
> +++ b/ovn/controller/physical.h
> @@ -29,6 +29,13 @@ struct hmap;
>  struct ovsdb_idl;
>  struct ovsrec_bridge;
>
> +/* OVN Geneve option information.
> + *
> + * Keep these in sync with the documentation in ovn-architecture(7). */
> +#define OVN_GENEVE_CLASS 0xffff
> +#define OVN_GENEVE_TYPE 0
> +#define OVN_GENEVE_LEN 4
> +
>  void physical_register_ovs_idl(struct ovsdb_idl *);
>  void physical_run(struct controller_ctx *, const struct ovsrec_bridge
> *br_int,
>                    const char *chassis_id,
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list