Message ID | 20241001151704.1627752-1-xsimonar@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v2] controller: Properly handle localnet flows in I+P. | 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 |
On Tue, Oct 1, 2024 at 5:17 PM Xavier Simonart <xsimonar@redhat.com> wrote: > Delete flows on localnet port deletion, and add localnet > related flows when peer ports are added. This was properly done when > recomputing, but not when doing IP. > > When peer ports are added, some flows such as chassis_mac flows > must be added. > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > --- > v2: rebase on origin/main for scapy fix. > --- > controller/physical.c | 9 ++++--- > tests/ovn.at | 58 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 63 insertions(+), 4 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index 16e312f9d..762a2e756 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -699,7 +699,7 @@ put_replace_chassis_mac_flows(const struct shash > *ct_zones, > put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p); > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, > rport_binding->header_.uuid.parts[0], > - &match, ofpacts_p, hc_uuid); > + &match, ofpacts_p, &localnet_port->header_.uuid); > > /* Provide second search criteria, i.e localnet port's > * vlan ID for conjunction flow */ > @@ -719,7 +719,7 @@ put_replace_chassis_mac_flows(const struct shash > *ct_zones, > conj->clause = 1; > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, > rport_binding->header_.uuid.parts[0], > - &match, ofpacts_p, hc_uuid); > + &match, ofpacts_p, &localnet_port->header_.uuid); > } > } > > @@ -2392,8 +2392,9 @@ physical_handle_flows_for_lport(const struct > sbrec_port_binding *pb, > struct local_datapath *ldp = > get_local_datapath(p_ctx->local_datapaths, > pb->datapath->tunnel_key); > - if (!strcmp(pb->type, "external")) { > - /* External lports have a dependency on the localnet port. > + if (!strcmp(pb->type, "external") || > + !strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) { > + /* Those lports have a dependency on the localnet port. > * We need to remove the flows of the localnet port as well > * and re-consider adding the flows for it. > */ > diff --git a/tests/ovn.at b/tests/ovn.at > index 826b52051..d2b14be53 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -39011,3 +39011,61 @@ OVN_CLEANUP([hv1],[hv2]) > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([localnet port flows after deletion]) > +ovn_start > +net_add n1 > + > +check ovn-nbctl ls-add sw0 > + > +for i in 1 2; do > + check ovn-nbctl lsp-add sw0 sw0-p${i} -- lsp-set-addresses sw0-p${i} > "00:00:10:01:02:0${i} 10.0.0.${i}" > + sim_add hv${i} > + as hv${i} > + ovs-vsctl add-br br-phys > + ovn_attach n1 br-phys 192.168.0.${i} > + ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > + ovs-vsctl add-port br-int vif${i} -- \ > + set Interface vif${i} external-ids:iface-id=sw0-p${i} \ > + options:tx_pcap=hv${i}/vif${i}-tx.pcap \ > + options:rxq_pcap=hv${i}/vif${i}-rx.pcap > +done > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 > +check ovn-nbctl lsp-add sw0 sw0-lr0 > +check ovn-nbctl lsp-set-type sw0-lr0 router > +check ovn-nbctl lsp-set-addresses sw0-lr0 router > +check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 > + > +check ovn-nbctl --wait=hv sync > +wait_for_ports_up > + > +# We should not have any flows in table OFTABLE_PHY_TO_LOG from in_port > different from vif1 and ovn-hv2-0 > +OVN_WAIT_REMOTE_INPUT_FLOWS(["hv1"],["hv2"]) > +of1=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=vif1) > +of2=$(as hv1 ovs-vsctl --bare --columns ofport find Interface > name=ovn-hv2-0) > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > "in_port=$of2" | wc -l], [0], [dnl > +0 > +]) > + > +# Add localnet port to sw0 > +check ovn-nbctl lsp-add sw0 ln-sw0 -- lsp-set-addresses ln-sw0 unknown -- > lsp-set-type ln-sw0 localnet > +check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1 -- > set logical_switch_port ln-sw0 tag_request=100 > + > +OVN_WAIT_PATCH_PORT_FLOWS(["ln-sw0"], ["hv1"]) > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > "in_port=$of2" | wc -l], [0], [dnl > +2 > +]) > + > +# Remove localnet port from sw0. Peer-ports flows should be deleted. > +check ovn-nbctl --wait=hv lsp-del ln-sw0 > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > "in_port=$of2" | wc -l], [0], [dnl > +0 > +]) > + > +OVN_CLEANUP([hv1],[hv2]) > + > +AT_CLEANUP > +]) > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
On Wed, Oct 2, 2024 at 5:32 AM Ales Musil <amusil@redhat.com> wrote: > > On Tue, Oct 1, 2024 at 5:17 PM Xavier Simonart <xsimonar@redhat.com> wrote: > > > Delete flows on localnet port deletion, and add localnet > > related flows when peer ports are added. This was properly done when > > recomputing, but not when doing IP. > > > > When peer ports are added, some flows such as chassis_mac flows > > must be added. > > > > Signed-off-by: Xavier Simonart <xsimonar@redhat.com> > > > > --- > > v2: rebase on origin/main for scapy fix. > > --- > > controller/physical.c | 9 ++++--- > > tests/ovn.at | 58 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/controller/physical.c b/controller/physical.c > > index 16e312f9d..762a2e756 100644 > > --- a/controller/physical.c > > +++ b/controller/physical.c > > @@ -699,7 +699,7 @@ put_replace_chassis_mac_flows(const struct shash > > *ct_zones, > > put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p); > > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, > > rport_binding->header_.uuid.parts[0], > > - &match, ofpacts_p, hc_uuid); > > + &match, ofpacts_p, &localnet_port->header_.uuid); > > > > /* Provide second search criteria, i.e localnet port's > > * vlan ID for conjunction flow */ > > @@ -719,7 +719,7 @@ put_replace_chassis_mac_flows(const struct shash > > *ct_zones, > > conj->clause = 1; > > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, > > rport_binding->header_.uuid.parts[0], > > - &match, ofpacts_p, hc_uuid); > > + &match, ofpacts_p, &localnet_port->header_.uuid); > > } > > } > > > > @@ -2392,8 +2392,9 @@ physical_handle_flows_for_lport(const struct > > sbrec_port_binding *pb, > > struct local_datapath *ldp = > > get_local_datapath(p_ctx->local_datapaths, > > pb->datapath->tunnel_key); > > - if (!strcmp(pb->type, "external")) { > > - /* External lports have a dependency on the localnet port. > > + if (!strcmp(pb->type, "external") || > > + !strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) { > > + /* Those lports have a dependency on the localnet port. > > * We need to remove the flows of the localnet port as well > > * and re-consider adding the flows for it. > > */ > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 826b52051..d2b14be53 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -39011,3 +39011,61 @@ OVN_CLEANUP([hv1],[hv2]) > > > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([localnet port flows after deletion]) > > +ovn_start > > +net_add n1 > > + > > +check ovn-nbctl ls-add sw0 > > + > > +for i in 1 2; do > > + check ovn-nbctl lsp-add sw0 sw0-p${i} -- lsp-set-addresses sw0-p${i} > > "00:00:10:01:02:0${i} 10.0.0.${i}" > > + sim_add hv${i} > > + as hv${i} > > + ovs-vsctl add-br br-phys > > + ovn_attach n1 br-phys 192.168.0.${i} > > + ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys > > + ovs-vsctl add-port br-int vif${i} -- \ > > + set Interface vif${i} external-ids:iface-id=sw0-p${i} \ > > + options:tx_pcap=hv${i}/vif${i}-tx.pcap \ > > + options:rxq_pcap=hv${i}/vif${i}-rx.pcap > > +done > > + > > +check ovn-nbctl lr-add lr0 > > +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 > > +check ovn-nbctl lsp-add sw0 sw0-lr0 > > +check ovn-nbctl lsp-set-type sw0-lr0 router > > +check ovn-nbctl lsp-set-addresses sw0-lr0 router > > +check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 > > + > > +check ovn-nbctl --wait=hv sync > > +wait_for_ports_up > > + > > +# We should not have any flows in table OFTABLE_PHY_TO_LOG from in_port > > different from vif1 and ovn-hv2-0 > > +OVN_WAIT_REMOTE_INPUT_FLOWS(["hv1"],["hv2"]) > > +of1=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=vif1) > > +of2=$(as hv1 ovs-vsctl --bare --columns ofport find Interface > > name=ovn-hv2-0) > > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > > "in_port=$of2" | wc -l], [0], [dnl > > +0 > > +]) > > + > > +# Add localnet port to sw0 > > +check ovn-nbctl lsp-add sw0 ln-sw0 -- lsp-set-addresses ln-sw0 unknown -- > > lsp-set-type ln-sw0 localnet > > +check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1 -- > > set logical_switch_port ln-sw0 tag_request=100 > > + > > +OVN_WAIT_PATCH_PORT_FLOWS(["ln-sw0"], ["hv1"]) > > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > > "in_port=$of2" | wc -l], [0], [dnl > > +2 > > +]) > > + > > +# Remove localnet port from sw0. Peer-ports flows should be deleted. > > +check ovn-nbctl --wait=hv lsp-del ln-sw0 > > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | > > grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v > > "in_port=$of2" | wc -l], [0], [dnl > > +0 > > +]) > > + > > +OVN_CLEANUP([hv1],[hv2]) > > + > > +AT_CLEANUP > > +]) > > -- > > 2.31.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> Thanks. Applied to main and backported till 23.09. Numan > > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com > <https://red.ht/sig> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/controller/physical.c b/controller/physical.c index 16e312f9d..762a2e756 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -699,7 +699,7 @@ put_replace_chassis_mac_flows(const struct shash *ct_zones, put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p); ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, rport_binding->header_.uuid.parts[0], - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &localnet_port->header_.uuid); /* Provide second search criteria, i.e localnet port's * vlan ID for conjunction flow */ @@ -719,7 +719,7 @@ put_replace_chassis_mac_flows(const struct shash *ct_zones, conj->clause = 1; ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 180, rport_binding->header_.uuid.parts[0], - &match, ofpacts_p, hc_uuid); + &match, ofpacts_p, &localnet_port->header_.uuid); } } @@ -2392,8 +2392,9 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, struct local_datapath *ldp = get_local_datapath(p_ctx->local_datapaths, pb->datapath->tunnel_key); - if (!strcmp(pb->type, "external")) { - /* External lports have a dependency on the localnet port. + if (!strcmp(pb->type, "external") || + !strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) { + /* Those lports have a dependency on the localnet port. * We need to remove the flows of the localnet port as well * and re-consider adding the flows for it. */ diff --git a/tests/ovn.at b/tests/ovn.at index 826b52051..d2b14be53 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -39011,3 +39011,61 @@ OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([localnet port flows after deletion]) +ovn_start +net_add n1 + +check ovn-nbctl ls-add sw0 + +for i in 1 2; do + check ovn-nbctl lsp-add sw0 sw0-p${i} -- lsp-set-addresses sw0-p${i} "00:00:10:01:02:0${i} 10.0.0.${i}" + sim_add hv${i} + as hv${i} + ovs-vsctl add-br br-phys + ovn_attach n1 br-phys 192.168.0.${i} + ovs-vsctl set open . external_ids:ovn-bridge-mappings=physnet1:br-phys + ovs-vsctl add-port br-int vif${i} -- \ + set Interface vif${i} external-ids:iface-id=sw0-p${i} \ + options:tx_pcap=hv${i}/vif${i}-tx.pcap \ + options:rxq_pcap=hv${i}/vif${i}-rx.pcap +done + +check ovn-nbctl lr-add lr0 +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24 +check ovn-nbctl lsp-add sw0 sw0-lr0 +check ovn-nbctl lsp-set-type sw0-lr0 router +check ovn-nbctl lsp-set-addresses sw0-lr0 router +check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0 + +check ovn-nbctl --wait=hv sync +wait_for_ports_up + +# We should not have any flows in table OFTABLE_PHY_TO_LOG from in_port different from vif1 and ovn-hv2-0 +OVN_WAIT_REMOTE_INPUT_FLOWS(["hv1"],["hv2"]) +of1=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=vif1) +of2=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-hv2-0) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl +0 +]) + +# Add localnet port to sw0 +check ovn-nbctl lsp-add sw0 ln-sw0 -- lsp-set-addresses ln-sw0 unknown -- lsp-set-type ln-sw0 localnet +check ovn-nbctl --wait=hv lsp-set-options ln-sw0 network_name=physnet1 -- set logical_switch_port ln-sw0 tag_request=100 + +OVN_WAIT_PATCH_PORT_FLOWS(["ln-sw0"], ["hv1"]) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl +2 +]) + +# Remove localnet port from sw0. Peer-ports flows should be deleted. +check ovn-nbctl --wait=hv lsp-del ln-sw0 +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=OFTABLE_PHY_TO_LOG | grep -v NXST_FLOW | grep "in_port=" | grep -v "in_port=$of1" | grep -v "in_port=$of2" | wc -l], [0], [dnl +0 +]) + +OVN_CLEANUP([hv1],[hv2]) + +AT_CLEANUP +])
Delete flows on localnet port deletion, and add localnet related flows when peer ports are added. This was properly done when recomputing, but not when doing IP. When peer ports are added, some flows such as chassis_mac flows must be added. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- v2: rebase on origin/main for scapy fix. --- controller/physical.c | 9 ++++--- tests/ovn.at | 58 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-)