| Message ID | 1467908665-6537-1-git-send-email-rmoats@us.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jul 07, 2016 at 11:24:25AM -0500, Ryan Moats wrote: > Currently, when address set value changes, ovn controller > doesn't remove the old entry from the tracking hash, it > just adds the new one, leading to multiple entries for the > same symbol. > > Fix this behavior. > > ToDo: figure out a test to avoid this in the future. Should "ovn-controller: " be added to the summary title? Cascardo. > > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> > --- > ovn/controller/lflow.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > index 05e1eaf..00d1d6e 100644 > --- a/ovn/controller/lflow.c > +++ b/ovn/controller/lflow.c > @@ -258,6 +258,7 @@ update_address_sets(struct controller_ctx *ctx) > * if the symtab entry needs to be updated due to a change. */ > sset_find_and_delete(&cur_addr_set_names, addr_set_rec->name); > if (!address_sets_match(addr_set, addr_set_rec)) { > + shash_find_and_delete(&local_address_sets, addr_set_rec->name); > expr_macros_remove(&expr_address_sets, addr_set_rec->name); > address_set_destroy(addr_set); > addr_set = NULL; > -- > 2.7.4 (Apple Git-66) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On Thu, Jul 7, 2016 at 12:18 PM, Thadeu Lima de Souza Cascardo < cascardo@redhat.com> wrote: > On Thu, Jul 07, 2016 at 11:24:25AM -0500, Ryan Moats wrote: > > Currently, when address set value changes, ovn controller > > doesn't remove the old entry from the tracking hash, it > > just adds the new one, leading to multiple entries for the > > same symbol. > > > > Fix this behavior. > > > > ToDo: figure out a test to avoid this in the future. > > Should "ovn-controller: " be added to the summary title? > Yes, that's a good suggestion. > Cascardo. > > > > > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> > > --- > > ovn/controller/lflow.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c > > index 05e1eaf..00d1d6e 100644 > > --- a/ovn/controller/lflow.c > > +++ b/ovn/controller/lflow.c > > @@ -258,6 +258,7 @@ update_address_sets(struct controller_ctx *ctx) > > * if the symtab entry needs to be updated due to a change. > */ > > sset_find_and_delete(&cur_addr_set_names, > addr_set_rec->name); > > if (!address_sets_match(addr_set, addr_set_rec)) { > > + shash_find_and_delete(&local_address_sets, > addr_set_rec->name); > > expr_macros_remove(&expr_address_sets, > addr_set_rec->name); > > address_set_destroy(addr_set); > > addr_set = NULL; > > -- > > 2.7.4 (Apple Git-66) > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
On Thu, Jul 7, 2016 at 1:04 PM, Russell Bryant <russell@ovn.org> wrote: > > > On Thu, Jul 7, 2016 at 12:18 PM, Thadeu Lima de Souza Cascardo < > cascardo@redhat.com> wrote: > >> On Thu, Jul 07, 2016 at 11:24:25AM -0500, Ryan Moats wrote: >> > Currently, when address set value changes, ovn controller >> > doesn't remove the old entry from the tracking hash, it >> > just adds the new one, leading to multiple entries for the >> > same symbol. >> > >> > Fix this behavior. >> > >> > ToDo: figure out a test to avoid this in the future. >> >> Should "ovn-controller: " be added to the summary title? >> > > Yes, that's a good suggestion. > The other feedback I had that I gave on IRC was that I'd like to work out a test case that would have exposed the crash this fixes. > > >> Cascardo. >> >> > >> > Signed-off-by: Ryan Moats <rmoats@us.ibm.com> >> > --- >> > ovn/controller/lflow.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c >> > index 05e1eaf..00d1d6e 100644 >> > --- a/ovn/controller/lflow.c >> > +++ b/ovn/controller/lflow.c >> > @@ -258,6 +258,7 @@ update_address_sets(struct controller_ctx *ctx) >> > * if the symtab entry needs to be updated due to a >> change. */ >> > sset_find_and_delete(&cur_addr_set_names, >> addr_set_rec->name); >> > if (!address_sets_match(addr_set, addr_set_rec)) { >> > + shash_find_and_delete(&local_address_sets, >> addr_set_rec->name); >> > expr_macros_remove(&expr_address_sets, >> addr_set_rec->name); >> > address_set_destroy(addr_set); >> > addr_set = NULL; >> > -- >> > 2.7.4 (Apple Git-66) >> > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > http://openvswitch.org/mailman/listinfo/dev >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev >> > > > > -- > Russell Bryant >
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index 05e1eaf..00d1d6e 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -258,6 +258,7 @@ update_address_sets(struct controller_ctx *ctx) * if the symtab entry needs to be updated due to a change. */ sset_find_and_delete(&cur_addr_set_names, addr_set_rec->name); if (!address_sets_match(addr_set, addr_set_rec)) { + shash_find_and_delete(&local_address_sets, addr_set_rec->name); expr_macros_remove(&expr_address_sets, addr_set_rec->name); address_set_destroy(addr_set); addr_set = NULL;
Currently, when address set value changes, ovn controller doesn't remove the old entry from the tracking hash, it just adds the new one, leading to multiple entries for the same symbol. Fix this behavior. ToDo: figure out a test to avoid this in the future. Signed-off-by: Ryan Moats <rmoats@us.ibm.com> --- ovn/controller/lflow.c | 1 + 1 file changed, 1 insertion(+)