On Sun, May 22, 2016 at 04:36:19PM -0500, Ryan Moats wrote:
> Ensure that the entire port binding table is processed
> when chassis are added/removed or when get_local_iface_ids
> finds new ports on the local vswitch.
>
> Side effects:
> - Persist local_datapaths and patch_datapaths across runs so
> that changes to either can be used as a trigger to reset
> incremental flow processing.
> - Persist all_lports structure
> - Revert commit 9baaabfff3c7df014e9acbd4c68189b568552ca9
In a commit message, please give the title of the commit in addition to
the commit number. (Then, if for some reason we move to a rebased repo
later, or switch away from Git (!), it is still possible to find
commits.)
> as these changes are not desirable once local_datatpath
> is persisted.
>
> Signed-off-by: Ryan Moats <rmo...@us.ibm.com>
Thanks for the revision!
"sparse" pointed out that local_datapaths_by_uuid should be static:
../ovn/controller/binding.c:140:13: warning: symbol
'local_datapaths_by_uuid' was not declared. Should it be static?
s/datpath/datapath/ in this comment in ovn-controller.c:
/* Contains "struct local_datpath" nodes whose hash values are the
* tunnel_key of datapaths with at least one local port binding. */
main() calls hmap_clear() on a couple of hmaps. Doesn't it need to
iterate through and free their elements first? Could it alternatively
just call reset_binding_processing()?
The OVS name prefix convention would be better served with the name
binding_reset_processing().
Same question as in patch 1 about potential memory leak here in
binding.c:
+ HMAP_FOR_EACH_POP(old_ld, hmap_node, &keep_local_datapath_by_uuid) {
+ ;
+ }
+ hmap_destroy(&keep_local_datapath_by_uuid);
consider_local_datapath() is indented 4 spaces too far.
The following changes weren't immediately obvious to me, can you hint
about why they are needed?
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index 4808146..e07e829 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -185,7 +185,8 @@ add_bridge_mappings(struct controller_ctx *ctx,
> * to create patch ports for it. */
> continue;
> }
> - if (ld->localnet_port) {
> + if (ld->localnet_port && strcmp(ld->localnet_port->logical_port,
> + binding->logical_port)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> VLOG_WARN_RL(&rl, "localnet port '%s' already set for
> datapath "
> "'%"PRId64"', skipping the new port '%s'.",
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 576c695..d1e8479 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -310,7 +310,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
> mff_ovn_geneve,
> if (!binding->chassis) {
> continue;
> }
> - if (localnet_port) {
> + if (localnet_port && localnet_port->logical_port) {
> ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
> localnet_port->logical_port));
> if (!ofport) {
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
