| Message ID | 1609930394-13680-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] binding: Do not clear container lbinding->pb when parent is deleted. | expand |
On Wed, Jan 6, 2021 at 4:23 PM Dumitru Ceara <dceara@redhat.com> wrote: > > When a parent Port_Binding is deleted we shouldn't clear the children's > 'pb' field. Container port bindings have their own Port_Binding SB > record so the child_lbinding->pb field should be cleared only when > their corresponding SB record is deleted. > > Whithout this fix when a parent Port_Binding "remove" followed by "add" > operations are received in the same iteration in ovn-controller, > consider_container_lport() can be called with "pb == NULL" causing a > crash. > > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Thanks Dumitru for finding this issue. I applied this patch to master and backported upto branch 20.06. Numan > --- > controller/binding.c | 3 +-- > tests/ovn.at | 7 +++++++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index cb60c5d..e632203 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -958,8 +958,7 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec, > } > } > > - /* Clear the local bindings' 'pb' and 'iface'. */ > - l->pb = NULL; > + /* Clear the local bindings' 'iface'. */ > l->iface = NULL; > } > > diff --git a/tests/ovn.at b/tests/ovn.at > index 66b1d73..55dcea4 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -9126,6 +9126,13 @@ OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) > OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) > OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) > > +# Move VM1 to a new logical switch. > +ovn-nbctl ls-add mgmt2 > +ovn-nbctl lsp-del vm1 -- lsp-add mgmt2 vm1 > +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) > +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) > +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) > + > as hv1 ovs-vsctl del-port vm1 > OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)]) > OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)]) > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 1/6/21 3:09 PM, Numan Siddique wrote: > On Wed, Jan 6, 2021 at 4:23 PM Dumitru Ceara <dceara@redhat.com> wrote: >> >> When a parent Port_Binding is deleted we shouldn't clear the children's >> 'pb' field. Container port bindings have their own Port_Binding SB >> record so the child_lbinding->pb field should be cleared only when >> their corresponding SB record is deleted. >> >> Whithout this fix when a parent Port_Binding "remove" followed by "add" >> operations are received in the same iteration in ovn-controller, >> consider_container_lport() can be called with "pb == NULL" causing a >> crash. >> >> Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > Thanks Dumitru for finding this issue. I applied this patch to master > and backported upto branch 20.06. > > Numan > Thanks!
diff --git a/controller/binding.c b/controller/binding.c index cb60c5d..e632203 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -958,8 +958,7 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec, } } - /* Clear the local bindings' 'pb' and 'iface'. */ - l->pb = NULL; + /* Clear the local bindings' 'iface'. */ l->iface = NULL; } diff --git a/tests/ovn.at b/tests/ovn.at index 66b1d73..55dcea4 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9126,6 +9126,13 @@ OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) +# Move VM1 to a new logical switch. +ovn-nbctl ls-add mgmt2 +ovn-nbctl lsp-del vm1 -- lsp-add mgmt2 vm1 +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) + as hv1 ovs-vsctl del-port vm1 OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)]) OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)])
When a parent Port_Binding is deleted we shouldn't clear the children's 'pb' field. Container port bindings have their own Port_Binding SB record so the child_lbinding->pb field should be cleared only when their corresponding SB record is deleted. Whithout this fix when a parent Port_Binding "remove" followed by "add" operations are received in the same iteration in ovn-controller, consider_container_lport() can be called with "pb == NULL" causing a crash. Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- controller/binding.c | 3 +-- tests/ovn.at | 7 +++++++ 2 files changed, 8 insertions(+), 2 deletions(-)