| Message ID | 20210821045846.2740435-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] controller: Don't allocate zone ids for non-VIF port bindings. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Tested-by: Vladislav Odintsov <odivlad@gmail.com> Regards, Vladislav Odintsov > On 21 Aug 2021, at 07:58, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > The commit 6fb87aad8c3("controller: Improve ct zone handling.") > caused a regression. After this commit ovn-controller is allocating > zone id for non VIF port bindings and this is causing unexpected > behavior. This patch fixes this issue. Also added a test case > to cover this scenario. > > Fixes: 6fb87aad8c3("controller: Improve ct zone handling.") > Reported-by: Vladislav Odintsov <odivlad@gmail.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > controller/ovn-controller.c | 5 +++ > tests/ovn.at | 69 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 678419ab3..739048cf8 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1919,6 +1919,11 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) > struct shash_node *shash_node; > SHASH_FOR_EACH (shash_node, &tdp->lports) { > struct tracked_lport *t_lport = shash_node->data; > + if (strcmp(t_lport->pb->type, "")) { > + /* We allocate zone-id's only to VIF lports. */ > + continue; > + } > + > if (t_lport->tracked_type == TRACKED_RESOURCE_NEW) { > if (!simap_contains(&ct_zones_data->current, > t_lport->pb->logical_port)) { > diff --git a/tests/ovn.at b/tests/ovn.at > index f1ebb088b..9329dd828 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -28041,3 +28041,72 @@ primary lport : [[sw0-port2]] > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn-controller ct zone I-P handling]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check as hv1 ovs-vsctl add-port br-int vif11 \ > + -- set interface vif11 external_ids:iface-id=sw0-port1 > + > +ovn-nbctl ls-add sw0 > +ovn-nbctl lsp-add sw0 sw0-port1 > +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3" > + > +ovn-nbctl lr-add lr0 > +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > +ovn-nbctl lsp-add sw0 sw0-lr0 > +ovn-nbctl lsp-set-type sw0-lr0 router > +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 > +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 > + > +wait_for_ports_up sw0-port1 > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep sw0-port1 -c], [0], [1 > +]) > + > +# There should be no ct-zone id allocated for sw0-lr0 and lr0-sw0 > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep sw0-lr0], [1], []) > + > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep lr0-sw0], [1], []) > + > +check as hv1 ovs-vsctl add-port br-int vif12 \ > + -- set interface vif12 external_ids:iface-id=sw1-port1 > + > +ovn-nbctl ls-add sw1 > +ovn-nbctl lsp-add sw1 sw1-port1 > + > +wait_for_ports_up sw1-port1 > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep sw1-port1 -c], [0], [1 > +]) > + > +# Attach sw1 to lr0 > +ovn-nbctl lsp-add sw1 sw1-lr0 > +ovn-nbctl lsp-set-type sw1-lr0 router > +ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02 > +ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 > + > +# There should be no ct-zone id allocated for sw1-lr0 > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep sw1-lr0], [1], []) > + > +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 > + > +# There should be no ct-zone id allocated for lr0-sw1 > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > +grep lr0-sw1], [1], []) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Numan, thanks for the quick fix. This worked for me. Regards, Vladislav Odintsov > On 23 Aug 2021, at 18:24, Vladislav Odintsov <odivlad@gmail.com> wrote: > > Tested-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> > > Regards, > Vladislav Odintsov > >> On 21 Aug 2021, at 07:58, numans@ovn.org <mailto:numans@ovn.org> wrote: >> >> From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> >> >> The commit 6fb87aad8c3("controller: Improve ct zone handling.") >> caused a regression. After this commit ovn-controller is allocating >> zone id for non VIF port bindings and this is causing unexpected >> behavior. This patch fixes this issue. Also added a test case >> to cover this scenario. >> >> Fixes: 6fb87aad8c3("controller: Improve ct zone handling.") >> Reported-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> >> Signed-off-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> >> --- >> controller/ovn-controller.c | 5 +++ >> tests/ovn.at <http://ovn.at/> | 69 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 74 insertions(+) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 678419ab3..739048cf8 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -1919,6 +1919,11 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) >> struct shash_node *shash_node; >> SHASH_FOR_EACH (shash_node, &tdp->lports) { >> struct tracked_lport *t_lport = shash_node->data; >> + if (strcmp(t_lport->pb->type, "")) { >> + /* We allocate zone-id's only to VIF lports. */ >> + continue; >> + } >> + >> if (t_lport->tracked_type == TRACKED_RESOURCE_NEW) { >> if (!simap_contains(&ct_zones_data->current, >> t_lport->pb->logical_port)) { >> diff --git a/tests/ovn.at <http://ovn.at/> b/tests/ovn.at <http://ovn.at/> >> index f1ebb088b..9329dd828 100644 >> --- a/tests/ovn.at <http://ovn.at/> >> +++ b/tests/ovn.at <http://ovn.at/> >> @@ -28041,3 +28041,72 @@ primary lport : [[sw0-port2]] >> OVN_CLEANUP([hv1]) >> AT_CLEANUP >> ]) >> + >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([ovn-controller ct zone I-P handling]) >> +ovn_start >> + >> +net_add n1 >> + >> +sim_add hv1 >> +as hv1 >> +ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.1 >> + >> +check as hv1 ovs-vsctl add-port br-int vif11 \ >> + -- set interface vif11 external_ids:iface-id=sw0-port1 >> + >> +ovn-nbctl ls-add sw0 >> +ovn-nbctl lsp-add sw0 sw0-port1 >> +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3" >> + >> +ovn-nbctl lr-add lr0 >> +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >> +ovn-nbctl lsp-add sw0 sw0-lr0 >> +ovn-nbctl lsp-set-type sw0-lr0 router >> +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 >> +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 >> + >> +wait_for_ports_up sw0-port1 >> + >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep sw0-port1 -c], [0], [1 >> +]) >> + >> +# There should be no ct-zone id allocated for sw0-lr0 and lr0-sw0 >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep sw0-lr0], [1], []) >> + >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep lr0-sw0], [1], []) >> + >> +check as hv1 ovs-vsctl add-port br-int vif12 \ >> + -- set interface vif12 external_ids:iface-id=sw1-port1 >> + >> +ovn-nbctl ls-add sw1 >> +ovn-nbctl lsp-add sw1 sw1-port1 >> + >> +wait_for_ports_up sw1-port1 >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep sw1-port1 -c], [0], [1 >> +]) >> + >> +# Attach sw1 to lr0 >> +ovn-nbctl lsp-add sw1 sw1-lr0 >> +ovn-nbctl lsp-set-type sw1-lr0 router >> +ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02 >> +ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 >> + >> +# There should be no ct-zone id allocated for sw1-lr0 >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep sw1-lr0], [1], []) >> + >> +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 >> + >> +# There should be no ct-zone id allocated for lr0-sw1 >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ >> +grep lr0-sw1], [1], []) >> + >> +OVN_CLEANUP([hv1]) >> +AT_CLEANUP >> +]) >> -- >> 2.31.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Aug 23, 2021 at 11:26 AM Vladislav Odintsov <odivlad@gmail.com> wrote: > > Hi Numan, > thanks for the quick fix. This worked for me. > > Regards, > Vladislav Odintsov > > > On 23 Aug 2021, at 18:24, Vladislav Odintsov <odivlad@gmail.com> wrote: > > > > Tested-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> Thanks for testing it out. I applied this patch to the main branch. Numan > > > > Regards, > > Vladislav Odintsov > > > >> On 21 Aug 2021, at 07:58, numans@ovn.org <mailto:numans@ovn.org> wrote: > >> > >> From: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > >> > >> The commit 6fb87aad8c3("controller: Improve ct zone handling.") > >> caused a regression. After this commit ovn-controller is allocating > >> zone id for non VIF port bindings and this is causing unexpected > >> behavior. This patch fixes this issue. Also added a test case > >> to cover this scenario. > >> > >> Fixes: 6fb87aad8c3("controller: Improve ct zone handling.") > >> Reported-by: Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> > >> Signed-off-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > >> --- > >> controller/ovn-controller.c | 5 +++ > >> tests/ovn.at <http://ovn.at/> | 69 +++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 74 insertions(+) > >> > >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > >> index 678419ab3..739048cf8 100644 > >> --- a/controller/ovn-controller.c > >> +++ b/controller/ovn-controller.c > >> @@ -1919,6 +1919,11 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) > >> struct shash_node *shash_node; > >> SHASH_FOR_EACH (shash_node, &tdp->lports) { > >> struct tracked_lport *t_lport = shash_node->data; > >> + if (strcmp(t_lport->pb->type, "")) { > >> + /* We allocate zone-id's only to VIF lports. */ > >> + continue; > >> + } > >> + > >> if (t_lport->tracked_type == TRACKED_RESOURCE_NEW) { > >> if (!simap_contains(&ct_zones_data->current, > >> t_lport->pb->logical_port)) { > >> diff --git a/tests/ovn.at <http://ovn.at/> b/tests/ovn.at <http://ovn.at/> > >> index f1ebb088b..9329dd828 100644 > >> --- a/tests/ovn.at <http://ovn.at/> > >> +++ b/tests/ovn.at <http://ovn.at/> > >> @@ -28041,3 +28041,72 @@ primary lport : [[sw0-port2]] > >> OVN_CLEANUP([hv1]) > >> AT_CLEANUP > >> ]) > >> + > >> +OVN_FOR_EACH_NORTHD([ > >> +AT_SETUP([ovn-controller ct zone I-P handling]) > >> +ovn_start > >> + > >> +net_add n1 > >> + > >> +sim_add hv1 > >> +as hv1 > >> +ovs-vsctl add-br br-phys > >> +ovn_attach n1 br-phys 192.168.0.1 > >> + > >> +check as hv1 ovs-vsctl add-port br-int vif11 \ > >> + -- set interface vif11 external_ids:iface-id=sw0-port1 > >> + > >> +ovn-nbctl ls-add sw0 > >> +ovn-nbctl lsp-add sw0 sw0-port1 > >> +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3" > >> + > >> +ovn-nbctl lr-add lr0 > >> +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 > >> +ovn-nbctl lsp-add sw0 sw0-lr0 > >> +ovn-nbctl lsp-set-type sw0-lr0 router > >> +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 > >> +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 > >> + > >> +wait_for_ports_up sw0-port1 > >> + > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep sw0-port1 -c], [0], [1 > >> +]) > >> + > >> +# There should be no ct-zone id allocated for sw0-lr0 and lr0-sw0 > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep sw0-lr0], [1], []) > >> + > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep lr0-sw0], [1], []) > >> + > >> +check as hv1 ovs-vsctl add-port br-int vif12 \ > >> + -- set interface vif12 external_ids:iface-id=sw1-port1 > >> + > >> +ovn-nbctl ls-add sw1 > >> +ovn-nbctl lsp-add sw1 sw1-port1 > >> + > >> +wait_for_ports_up sw1-port1 > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep sw1-port1 -c], [0], [1 > >> +]) > >> + > >> +# Attach sw1 to lr0 > >> +ovn-nbctl lsp-add sw1 sw1-lr0 > >> +ovn-nbctl lsp-set-type sw1-lr0 router > >> +ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02 > >> +ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 > >> + > >> +# There should be no ct-zone id allocated for sw1-lr0 > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep sw1-lr0], [1], []) > >> + > >> +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 > >> + > >> +# There should be no ct-zone id allocated for lr0-sw1 > >> +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ > >> +grep lr0-sw1], [1], []) > >> + > >> +OVN_CLEANUP([hv1]) > >> +AT_CLEANUP > >> +]) > >> -- > >> 2.31.1 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org <mailto:dev@openvswitch.org> > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 678419ab3..739048cf8 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1919,6 +1919,11 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) struct shash_node *shash_node; SHASH_FOR_EACH (shash_node, &tdp->lports) { struct tracked_lport *t_lport = shash_node->data; + if (strcmp(t_lport->pb->type, "")) { + /* We allocate zone-id's only to VIF lports. */ + continue; + } + if (t_lport->tracked_type == TRACKED_RESOURCE_NEW) { if (!simap_contains(&ct_zones_data->current, t_lport->pb->logical_port)) { diff --git a/tests/ovn.at b/tests/ovn.at index f1ebb088b..9329dd828 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -28041,3 +28041,72 @@ primary lport : [[sw0-port2]] OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller ct zone I-P handling]) +ovn_start + +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +check as hv1 ovs-vsctl add-port br-int vif11 \ + -- set interface vif11 external_ids:iface-id=sw0-port1 + +ovn-nbctl ls-add sw0 +ovn-nbctl lsp-add sw0 sw0-port1 +ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3" + +ovn-nbctl lr-add lr0 +ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 +ovn-nbctl lsp-add sw0 sw0-lr0 +ovn-nbctl lsp-set-type sw0-lr0 router +ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01 +ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 + +wait_for_ports_up sw0-port1 + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep sw0-port1 -c], [0], [1 +]) + +# There should be no ct-zone id allocated for sw0-lr0 and lr0-sw0 +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep sw0-lr0], [1], []) + +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep lr0-sw0], [1], []) + +check as hv1 ovs-vsctl add-port br-int vif12 \ + -- set interface vif12 external_ids:iface-id=sw1-port1 + +ovn-nbctl ls-add sw1 +ovn-nbctl lsp-add sw1 sw1-port1 + +wait_for_ports_up sw1-port1 +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep sw1-port1 -c], [0], [1 +]) + +# Attach sw1 to lr0 +ovn-nbctl lsp-add sw1 sw1-lr0 +ovn-nbctl lsp-set-type sw1-lr0 router +ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02 +ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 + +# There should be no ct-zone id allocated for sw1-lr0 +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep sw1-lr0], [1], []) + +ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.1/24 + +# There should be no ct-zone id allocated for lr0-sw1 +AT_CHECK([as hv1 ovn-appctl -t ovn-controller ct-zone-list | \ +grep lr0-sw1], [1], []) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +])