While this patch passes the ovn test suite, it is not working correctly, so
consider this is "Self-NACK"
and I'll work on getting it fixed tomorrow...
Ryan
Ryan Moats/Omaha/IBM@IBMUS wrote on 02/16/2016 02:30:20 PM:
> From: Ryan Moats/Omaha/IBM@IBMUS
> To: dev@openvswitch.org
> Cc: russ...@ovn.org, Ryan Moats/Omaha/IBM@IBMUS
> Date: 02/16/2016 02:30 PM
> Subject: [PATCH v4 2/3] [ovn-controller] Add logical flow
> incremental processing
>
> From: RYAN D. MOATS <rmo...@us.ibm.com>
>
> Make flow table persistent in ovn controller
>
> This is a prerequisite for incremental processing.
>
> Signed-off-by: RYAN D. MOATS <rmo...@us.ibm.com>
> ---
> ovn/controller/ofctrl.c | 99 +++++++++++++++++++++++++++
> +-----------
> ovn/controller/ofctrl.h | 2 +
> ovn/controller/ovn-controller.c | 4 +-
> 3 files changed, 75 insertions(+), 30 deletions(-)
>
> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
> index 3297fb3..e5f4ebf 100644
> --- a/ovn/controller/ofctrl.c
> +++ b/ovn/controller/ofctrl.c
> @@ -50,6 +50,8 @@ struct ovn_flow {
> static uint32_t ovn_flow_hash(const struct ovn_flow *);
> static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table,
> const struct ovn_flow *target);
> +static struct ovn_flow *ovn_flow_lookup2(struct hmap *flow_table,
> + const struct ovn_flow *target);
> 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 *);
> @@ -484,21 +486,41 @@ ofctrl_add_flow(struct hmap *desired_flows,
> 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);
> - if (!VLOG_DROP_INFO(&rl)) {
> - char *s = ovn_flow_to_string(f);
> - VLOG_INFO("dropping duplicate flow: %s", s);
> - free(s);
> - }
> + struct ovn_flow *d = ovn_flow_lookup2(desired_flows, f);
> + if (d) {
> + if (!match_equal(&d->match, &f->match)) {
>
> - ovn_flow_destroy(f);
> - return;
> + hmap_remove(desired_flows, &d->hmap_node);
> + ovn_flow_destroy(d);
> + } else {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
5);
> + if (!VLOG_DROP_INFO(&rl)) {
> + char *s = ovn_flow_to_string(f);
> + VLOG_INFO("dropping duplicate flow: %s", s);
> + free(s);
> + }
> +
> + ovn_flow_destroy(f);
> + return;
> + }
> }
>
> hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash);
> }
>
> +/* duplicate an ovn_flow structure */
> +struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source)
> +{
> + struct ovn_flow *answer = xmalloc(sizeof *answer);
> + answer->table_id = source->table_id;
> + answer->priority = source->priority;
> + answer->match = source->match;
> + answer->ofpacts = xmemdup(source->ofpacts, source->ofpacts_len);
> + answer->ofpacts_len = source->ofpacts_len;
> + answer->hmap_node.hash = ovn_flow_hash(answer);
> + return answer;
> +}
> +
> /* ovn_flow. */
>
> /* Returns a hash of the key in 'f'. */
> @@ -528,6 +550,25 @@ ovn_flow_lookup(struct hmap *flow_table, const
> struct ovn_flow *target)
> return NULL;
> }
>
> +/* Finds and returns an ovn_flow in 'flow_table" whose table_id,
priority
> + * and actions are the same as the target, or NULL if there is none */
> +static struct ovn_flow *
> +ovn_flow_lookup2(struct hmap *flow_table, const struct ovn_flow *target)
> +{
> + struct ovn_flow *f;
> +
> + HMAP_FOR_EACH_WITH_HASH (f, hmap_node, target->hmap_node.hash,
> + flow_table) {
> + if (f->table_id == target->table_id
> + && f->priority == target->priority
> + && ofpacts_equal(f->ofpacts, f->ofpacts_len,
> + target->ofpacts, target->ofpacts_len)) {
> + return f;
> + }
> + }
> + return NULL;
> +}
> +
> static char *
> ovn_flow_to_string(const struct ovn_flow *f)
> {
> @@ -607,7 +648,6 @@ ofctrl_put(struct hmap *flow_table)
> * to wake up and retry. */
> if (state != S_UPDATE_FLOWS
> || rconn_packet_counter_n_packets(tx_counter)) {
> - ovn_flow_table_clear(flow_table);
> return;
> }
>
> @@ -659,25 +699,28 @@ ofctrl_put(struct hmap *flow_table)
> }
> }
>
> - /* 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. */
> + /* Since flow table has all flows, now we need to add the ones that
> + * aren't in the installed flow table. */
> struct ovn_flow *d;
> HMAP_FOR_EACH_SAFE (d, next, hmap_node, flow_table) {
> - /* Send flow_mod to add flow. */
> - struct ofputil_flow_mod fm = {
> - .match = d->match,
> - .priority = d->priority,
> - .table_id = d->table_id,
> - .ofpacts = d->ofpacts,
> - .ofpacts_len = d->ofpacts_len,
> - .command = OFPFC_ADD,
> - };
> - queue_flow_mod(&fm);
> - ovn_flow_log(d, "adding");
> -
> - /* 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);
> + struct ovn_flow *i = ovn_flow_lookup(&installed_flows, d);
> + if (!i) {
> + /* Send flow_mod to add flow. */
> + struct ofputil_flow_mod fm = {
> + .match = d->match,
> + .priority = d->priority,
> + .table_id = d->table_id,
> + .ofpacts = d->ofpacts,
> + .ofpacts_len = d->ofpacts_len,
> + .command = OFPFC_ADD,
> + };
> + queue_flow_mod(&fm);
> + ovn_flow_log(d, "adding");
> +
> + /* Copy 'd' from 'flow_table' to installed_flows. */
> + struct ovn_flow *new_node = ofctrl_dup_flow(d);
> + hmap_insert(&installed_flows, &new_node->hmap_node,
> + new_node->hmap_node.hash);
> + }
> }
> }
> diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
> index 93ef8ea..d3fabc0 100644
> --- a/ovn/controller/ofctrl.h
> +++ b/ovn/controller/ofctrl.h
> @@ -34,6 +34,8 @@ void ofctrl_put(struct hmap *flows);
> void ofctrl_wait(void);
> void ofctrl_destroy(void);
>
> +struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source);
> +
> /* Flow table interface to the rest of ovn-controller. */
> void ofctrl_add_flow(struct hmap *flows, uint8_t table_id,
uint16_tpriority,
> const struct match *, const struct ofpbuf
*ofpacts);
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index 3638342..5a4174e 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -205,6 +205,8 @@ main(int argc, char *argv[])
> bool exiting;
> int retval;
>
> + struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> +
> ovs_cmdl_proctitle_init(argc, argv);
> set_program_name(argv[0]);
> service_start(&argc, &argv);
> @@ -299,14 +301,12 @@ main(int argc, char *argv[])
>
> pinctrl_run(&ctx, br_int);
>
> - struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> lflow_run(&ctx, &flow_table, &ct_zones, &local_datapaths);
> if (chassis_id) {
> physical_run(&ctx, mff_ovn_geneve,
> br_int, chassis_id, &ct_zones,
&flow_table);
> }
> ofctrl_put(&flow_table);
> - hmap_destroy(&flow_table);
> }
>
> /* local_datapaths contains bare hmap_node instances.
> --
> 1.7.1
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
