Message ID | 20240423115333.2796843-3-xsimonar@redhat.com |
---|---|
State | Accepted |
Delegated to: | Numan Siddique |
Headers | show |
Series | Fix I+P versus recompute differences. | 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 | success | github build: passed |
On Tue, Apr 23, 2024 at 7:54 AM Xavier Simonart <xsimonar@redhat.com> wrote: > > This patches fixes flows not properly deleted in scenarios similar to: > foo1 (hv1) - foo - R1 - join - R2 (chassis = hv2) - alice - alice1 (hv2). > > When R2 is deleted, alice_R2 changed from l3gateway to patch, and, on hv1, > alice_R2 was added to related ports. > When R2 was added back (together with R2-alice and R2-join), alice_R2 changed > back from patch to l3gateway and alice_R2 remained in related_lports on hv1. > > This is now fixed: a l3_gateway port is not a related_port if not for our chassis. > > A test case has been modified to highlight the error, but also to > make the test easier to read (e.g avoid using same name for a port and > for a switch). > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > controller/binding.c | 12 ++++++++---- > tests/ovn.at | 12 ++++++++---- > 2 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index c9658cb2a..3b36ed881 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1824,7 +1824,7 @@ consider_localport(const struct sbrec_port_binding *pb, > */ > static bool > consider_nonvif_lport_(const struct sbrec_port_binding *pb, > - bool our_chassis, > + bool our_chassis, bool is_ha_chassis, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > @@ -1844,6 +1844,9 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, > b_ctx_out->if_mgr, > b_ctx_out->postponed_ports); > } > + if (!is_ha_chassis) { > + remove_related_lport(pb, b_ctx_out); > + } > > if (pb->chassis == b_ctx_in->chassis_rec > || is_additional_chassis(pb, b_ctx_in->chassis_rec) > @@ -1867,7 +1870,7 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, > bool our_chassis = chassis_id && !strcmp(chassis_id, > b_ctx_in->chassis_rec->name); > > - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > } > > static bool > @@ -1879,7 +1882,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, > bool our_chassis = chassis_id && !strcmp(chassis_id, > b_ctx_in->chassis_rec->name); > > - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); > } > > static void > @@ -1942,7 +1945,8 @@ consider_ha_lport(const struct sbrec_port_binding *pb, > update_related_lport(pb, b_ctx_out); > } > > - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); > + return consider_nonvif_lport_(pb, our_chassis, is_ha_chassis, b_ctx_in, > + b_ctx_out); > } > > static bool > diff --git a/tests/ovn.at b/tests/ovn.at > index f974cbb15..3c888aaf5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -7781,9 +7781,9 @@ ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \ > type=router options:router-port=foo addresses=\"00:00:01:01:02:03\" > > # Connect alice to R2 > -ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24 > -ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \ > - type=router options:router-port=alice addresses=\"00:00:02:01:02:03\" > +ovn-nbctl lrp-add R2 R2-alice 00:00:02:01:02:03 172.16.1.1/24 > +ovn-nbctl lsp-add alice alice-R2 -- set Logical_Switch_Port alice-R2 \ > + type=router options:router-port=R2-alice addresses=\"00:00:02:01:02:03\" > > # Connect R1 to join > ovn-nbctl lrp-add R1 R1_join 00:00:04:01:02:03 20.0.0.1/24 > @@ -7871,11 +7871,13 @@ echo "----------------------------" > echo $expected > expected > OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) > > +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) > + > # Delete the router and re-create it. Things should work as before. > ovn-nbctl lr-del R2 > ovn-nbctl create Logical_Router name=R2 options:chassis="hv2" > # Connect alice to R2 > -ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24 > +ovn-nbctl lrp-add R2 R2-alice 00:00:02:01:02:03 172.16.1.1/24 > # Connect R2 to join > ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04 20.0.0.2/24 > > @@ -7887,6 +7889,8 @@ R2 static_routes @lrt > wait_for_ports_up > check ovn-nbctl --wait=hv sync > > +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) > + > # Send the packet again. > as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/binding.c b/controller/binding.c index c9658cb2a..3b36ed881 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1824,7 +1824,7 @@ consider_localport(const struct sbrec_port_binding *pb, */ static bool consider_nonvif_lport_(const struct sbrec_port_binding *pb, - bool our_chassis, + bool our_chassis, bool is_ha_chassis, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { @@ -1844,6 +1844,9 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, b_ctx_out->if_mgr, b_ctx_out->postponed_ports); } + if (!is_ha_chassis) { + remove_related_lport(pb, b_ctx_out); + } if (pb->chassis == b_ctx_in->chassis_rec || is_additional_chassis(pb, b_ctx_in->chassis_rec) @@ -1867,7 +1870,7 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } static bool @@ -1879,7 +1882,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } static void @@ -1942,7 +1945,8 @@ consider_ha_lport(const struct sbrec_port_binding *pb, update_related_lport(pb, b_ctx_out); } - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, is_ha_chassis, b_ctx_in, + b_ctx_out); } static bool diff --git a/tests/ovn.at b/tests/ovn.at index f974cbb15..3c888aaf5 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -7781,9 +7781,9 @@ ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \ type=router options:router-port=foo addresses=\"00:00:01:01:02:03\" # Connect alice to R2 -ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24 -ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \ - type=router options:router-port=alice addresses=\"00:00:02:01:02:03\" +ovn-nbctl lrp-add R2 R2-alice 00:00:02:01:02:03 172.16.1.1/24 +ovn-nbctl lsp-add alice alice-R2 -- set Logical_Switch_Port alice-R2 \ + type=router options:router-port=R2-alice addresses=\"00:00:02:01:02:03\" # Connect R1 to join ovn-nbctl lrp-add R1 R1_join 00:00:04:01:02:03 20.0.0.1/24 @@ -7871,11 +7871,13 @@ echo "----------------------------" echo $expected > expected OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected]) +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) + # Delete the router and re-create it. Things should work as before. ovn-nbctl lr-del R2 ovn-nbctl create Logical_Router name=R2 options:chassis="hv2" # Connect alice to R2 -ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24 +ovn-nbctl lrp-add R2 R2-alice 00:00:02:01:02:03 172.16.1.1/24 # Connect R2 to join ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04 20.0.0.2/24 @@ -7887,6 +7889,8 @@ R2 static_routes @lrt wait_for_ports_up check ovn-nbctl --wait=hv sync +CHECK_FLOWS_AFTER_RECOMPUTE([hv1], [hv1]) + # Send the packet again. as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
This patches fixes flows not properly deleted in scenarios similar to: foo1 (hv1) - foo - R1 - join - R2 (chassis = hv2) - alice - alice1 (hv2). When R2 is deleted, alice_R2 changed from l3gateway to patch, and, on hv1, alice_R2 was added to related ports. When R2 was added back (together with R2-alice and R2-join), alice_R2 changed back from patch to l3gateway and alice_R2 remained in related_lports on hv1. This is now fixed: a l3_gateway port is not a related_port if not for our chassis. A test case has been modified to highlight the error, but also to make the test easier to read (e.g avoid using same name for a port and for a switch). Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- controller/binding.c | 12 ++++++++---- tests/ovn.at | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-)