"dev" <dev-boun...@openvswitch.org> wrote on 06/23/2016 07:00:51 PM:
> From: Ben Pfaff <b...@ovn.org>
> To: dev@openvswitch.org
> Cc: Ben Pfaff <b...@ovn.org>
> Date: 06/23/2016 07:01 PM
> Subject: [ovs-dev] [PATCH] ovn-controller: Use new ovsdb-idl helpers
> to make logic more readable.
> Sent by: "dev" <dev-boun...@openvswitch.org>
>
> Also there were lots of 'continue's sprinkled around that didn't seem to
> be needed given some simple code rearrangement.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> ovn/controller/binding.c | 11 ++++-------
> ovn/controller/encaps.c | 19 +++++--------------
> ovn/controller/lport.c | 18 ++++++------------
> ovsdb/ovsdb-idlc.in | 12 ++++++++++++
> 4 files changed, 27 insertions(+), 33 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index e36c8f4..9921a49 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -266,15 +266,12 @@ binding_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> process_full_binding = false;
> } else {
> SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl)
{
> - bool is_deleted = sbrec_port_binding_row_get_seqno
(binding_rec,
> - OVSDB_IDL_CHANGE_DELETE) > 0;
> -
> - if (is_deleted) {
> + if (sbrec_port_binding_is_deleted(binding_rec)) {
> remove_local_datapath_by_binding(local_datapaths,
> binding_rec);
> - continue;
> + } else {
> + consider_local_datapath(ctx, &lports, chassis_rec,
> binding_rec,
> + local_datapaths);
> }
> - consider_local_datapath(ctx, &lports, chassis_rec,
binding_rec,
> - local_datapaths);
> }
> }
>
> diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> index 495f8f6..18268a6 100644
> --- a/ovn/controller/encaps.c
> +++ b/ovn/controller/encaps.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2015 Nicira, Inc.
> +/* Copyright (c) 2015, 2016 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -404,12 +404,7 @@ encaps_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> process_full_encaps = false;
> } else {
> SBREC_CHASSIS_FOR_EACH_TRACKED (chassis_rec, ctx->ovnsb_idl) {
> - bool is_deleted = sbrec_chassis_row_get_seqno(chassis_rec,
> -
> OVSDB_IDL_CHANGE_DELETE) > 0;
> - bool is_new = sbrec_chassis_row_get_seqno(chassis_rec,
> -
> OVSDB_IDL_CHANGE_MODIFY) == 0;
> -
> - if (is_deleted) {
> + if (sbrec_chassis_is_deleted(chassis_rec)) {
> /* Lookup the tunnel by row uuid and remove it. */
> struct port_hash_node *port_hash =
> port_lookup_by_uuid(&tc.tunnel_hmap_by_uuid,
> @@ -424,14 +419,10 @@ encaps_run(struct controller_ctx *ctx, const
> struct ovsrec_bridge *br_int,
> free(port_hash);
> binding_reset_processing();
> }
> - continue;
> - }
> - if (!is_new) {
> - check_and_update_tunnel(chassis_rec);
> - continue;
> - } else {
> + } else if (sbrec_chassis_is_new(chassis_rec)) {
> check_and_add_tunnel(chassis_rec, chassis_id);
> - continue;
> + } else {
> + check_and_update_tunnel(chassis_rec);
> }
> }
> }
> diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
> index 2ce6387..a5e9ad3 100644
> --- a/ovn/controller/lport.c
> +++ b/ovn/controller/lport.c
> @@ -107,14 +107,11 @@ lport_index_fill(struct lport_index *lports,
> struct ovsdb_idl *ovnsb_idl)
> full_lport_rebuild = false;
> } else {
> SBREC_PORT_BINDING_FOR_EACH_TRACKED (pb, ovnsb_idl) {
> - bool is_delete = sbrec_port_binding_row_get_seqno(pb,
> - OVSDB_IDL_CHANGE_DELETE) > 0;
> -
> - if (is_delete) {
> + if (sbrec_port_binding_is_deleted(pb)) {
> lport_index_remove(lports, &pb->header_.uuid);
> - continue;
> + } else {
> + consider_lport_index(lports, pb);
> }
> - consider_lport_index(lports, pb);
> }
> }
> }
> @@ -248,14 +245,11 @@ mcgroup_index_fill(struct mcgroup_index
> *mcgroups, struct ovsdb_idl *ovnsb_idl)
> full_mc_rebuild = false;
> } else {
> SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mg, ovnsb_idl) {
> - bool is_delete = sbrec_multicast_group_row_get_seqno(mg,
> - OVSDB_IDL_CHANGE_DELETE) > 0;
> -
> - if (is_delete) {
> + if (sbrec_multicast_group_is_deleted(mg)) {
> mcgroup_index_remove(mcgroups, &mg->header_.uuid);
> - continue;
> + } else {
> + consider_mcgroup_index(mcgroups, mg);
> }
> - consider_mcgroup_index(mcgroups, mg);
> }
> }
> }
> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index 19a86dc..0d836c0 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -186,6 +186,18 @@ const struct %(s)s *%(s)s_track_get_next(const
> struct %(s)s *);
> (ROW); \\
> (ROW) = %(s)s_track_get_next(ROW))
>
> +/* Returns true if 'row' was inserted since the last change
> tracking reset. */
> +static inline bool %(s)s_is_new(const struct %(s)s *row)
> +{
> + return %(s)s_row_get_seqno(row, OVSDB_IDL_CHANGE_MODIFY) == 0;
> +}
> +
> +/* Returns true if 'row' was deleted since the last change
trackingreset. */
> +static inline bool %(s)s_is_deleted(const struct %(s)s *row)
> +{
> + return %(s)s_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > 0;
> +}
> +
> void %(s)s_init(struct %(s)s *);
> void %(s)s_delete(const struct %(s)s *);
> struct %(s)s *%(s)s_insert(struct ovsdb_idl_txn *);
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
I'll not only ack this, I'll pull it in and base my residual incremental
processing patches on it because it makes it *MUCH* cleaner:
Acked-by: Ryan Moats <rmo...@us.ibm.com>
In fact .... Happily-acked-by :)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
