On Mon, Oct 14, 2024 at 12:10 PM Xavier Simonart <[email protected]> wrote:
> Before this patch, peer ports might be missing if, at the time of > adding the datapath to local the port is not yet fully claimed > (i.e. pb->chassis still empty) > > Signed-off-by: Xavier Simonart <[email protected]> > > --- > v2: - Rebased on main > - Addressed Ales' comment: > - Updated tests > - Use OVN_WAIT_PATCH_PORT_FLOWS macro). > - Use check for ovn-nbctl. > - Removed unused switches & leftovers. > > Signed-off-by: Xavier Simonart <[email protected]> > --- > controller/binding.c | 48 ++++++++++++---- > controller/local_data.c | 6 +- > tests/system-ovn.at | 119 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 162 insertions(+), 11 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 492aef530..5b33468ab 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1871,14 +1871,35 @@ consider_nonvif_lport_(const struct > sbrec_port_binding *pb, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > + struct local_datapath *ld = > + get_local_datapath(b_ctx_out->local_datapaths, > + pb->datapath->tunnel_key); > + > if (our_chassis) { > update_local_lports(pb->logical_port, b_ctx_out); > - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, > - b_ctx_in->sbrec_port_binding_by_datapath, > - b_ctx_in->sbrec_port_binding_by_name, > - pb->datapath, b_ctx_in->chassis_rec, > - b_ctx_out->local_datapaths, > - b_ctx_out->tracked_dp_bindings); > + if (!ld) { > + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, > + b_ctx_in->sbrec_port_binding_by_datapath, > + b_ctx_in->sbrec_port_binding_by_name, > + pb->datapath, b_ctx_in->chassis_rec, > + b_ctx_out->local_datapaths, > + b_ctx_out->tracked_dp_bindings); > + } else { > + /* Add the peer datapath to the local datapaths if it's > + * not present yet. > + */ > + if (need_add_peer_to_local( > + b_ctx_in->sbrec_port_binding_by_name, pb, > + b_ctx_in->chassis_rec)) { > > nit: It could be combined into "else if" to avoid the second nesting. > + add_local_datapath_peer_port( > + pb, b_ctx_in->chassis_rec, > + b_ctx_in->sbrec_datapath_binding_by_key, > + b_ctx_in->sbrec_port_binding_by_datapath, > + b_ctx_in->sbrec_port_binding_by_name, > + ld, b_ctx_out->local_datapaths, > + b_ctx_out->tracked_dp_bindings); > + } > + } > > update_related_lport(pb, b_ctx_out); > return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL, > @@ -1895,10 +1916,17 @@ consider_nonvif_lport_(const struct > sbrec_port_binding *pb, > || is_additional_chassis(pb, b_ctx_in->chassis_rec) > || if_status_is_port_claimed(b_ctx_out->if_mgr, > pb->logical_port)) { > - return release_lport(pb, b_ctx_in->chassis_rec, > - !b_ctx_in->ovnsb_idl_txn, > - b_ctx_out->tracked_dp_bindings, > - b_ctx_out->if_mgr); > + if (!release_lport(pb, b_ctx_in->chassis_rec, > + !b_ctx_in->ovnsb_idl_txn, > + b_ctx_out->tracked_dp_bindings, > + b_ctx_out->if_mgr)) { > + return false; > + } > + > + if (ld) { > + remove_local_datapath_peer_port(pb, ld, > + b_ctx_out->local_datapaths); > + } > } > > return true; > diff --git a/controller/local_data.c b/controller/local_data.c > index f1a7ce00e..e12523d61 100644 > --- a/controller/local_data.c > +++ b/controller/local_data.c > @@ -208,7 +208,11 @@ add_local_datapath_peer_port( > struct hmap *tracked_datapaths) > { > const struct sbrec_port_binding *peer; > - peer = lport_get_peer(pb, sbrec_port_binding_by_name); > + if (!strcmp(pb->type, "l3gateway")) { > nit: Extra space > + peer = lport_get_l3gw_peer(pb, sbrec_port_binding_by_name); > + } else { > + peer = lport_get_peer(pb, sbrec_port_binding_by_name); > + } > > if (!peer) { > return; > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 861b1cb99..a23471bf8 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -13929,3 +13929,122 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/.*error > receiving.*/d > /.*terminating with signal 15.*/d"]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([IPv6 prefix delegation - l3gateway ports creation]) > +AT_SKIP_IF([test $HAVE_DHCPD = no]) > +AT_SKIP_IF([test $HAVE_TCPDUMP = no]) > +AT_KEYWORDS([ovn-ipv6-prefix_d]) > +ovn_start > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_BR([br-int]) > +ADD_BR([br-ext]) > + > +ovs-vsctl set-fail-mode br-ext standalone > +# Set external-ids in br-int needed for ovn-controller > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . > external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure > other-config:disable-in-band=true > + > +# Start ovn-controller > +start_daemon ovn-controller > + > +ADD_NAMESPACES(server) > +ADD_VETH(s1, server, br-ext, "2001:1db8:3333::2/64", "f0:00:00:01:02:05", > \ > + "2001:1db8:3333::1", "nodad") > + > +ovn-nbctl create Logical_Router name=R1 options:chassis=hv1 > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl ls-add public > + > +# Add localnet port on public and router ports between R1 and public. > +# This should create the patch ports. > +# This is done early in the test as this used to cause some recomputes > (due > +# to runtime data handler for ovs_interface). > +# Hence, we avoid such recompute later in the test, so we only rely on > I+P. > +AT_CHECK([ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=phynet:br-ext]) > +check ovn-nbctl lsp-add public ln1 \ > + -- lsp-set-addresses ln1 unknown \ > + -- lsp-set-type ln1 localnet \ > + -- lsp-set-options ln1 network_name=phynet > + > +check ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 > +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port > public-rp \ > + type=router options:router-port=rp-public \ > + -- lsp-set-addresses public-rp router > + > +# Wait for a flow outputing to patch port, so we know ovn-controller > handled the patch port creation in OVS. > +OVN_WAIT_PATCH_PORT_FLOWS(["ln1"], ["./"]) > + > +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24 > + > +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \ > + type=router options:router-port=rp-sw0 \ > + -- lsp-set-addresses sw0-rp router > +check ovn-nbctl lsp-add sw0 sw01 \ > + -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2" > + > +check ovn-nbctl set logical_router_port rp-public > options:prefix_delegation=true > +check ovn-nbctl set logical_router_port rp-public options:prefix=true > +check ovn-nbctl set logical_router_port rp-sw0 options:prefix=true > + > +OVN_POPULATE_ARP > + > +check ovn-nbctl --wait=hv sync > + > +cat > /etc/dhcp/dhcpd.conf <<EOF > +option dhcp-rebinding-time 10; > +option dhcp-renewal-time 5; > +subnet6 2001:1db8:3333::/64 { > + prefix6 2001:1db8:3333:100:: 2001:1db8:3333:111:: /96; > +} > +EOF > +rm -f /var/lib/dhcp/dhcpd6.leases > +touch /var/lib/dhcp/dhcpd6.leases > +chown root:dhcpd /var/lib/dhcp /var/lib/dhcp/dhcpd6.leases > +chmod 775 /var/lib/dhcp > +chmod 664 /var/lib/dhcp/dhcpd6.leases > + > +NETNS_START_TCPDUMP([server], [-nni s1], [server]) > +NETNS_DAEMONIZE([server], [dhcpd -6 -f s1 > dhcpd.log 2>&1], [dhcpd.pid]) > +ovn-nbctl --wait=hv sync > + > +AT_CHECK([ovn-appctl debug/dump-peer-ports | sort], [0], [dnl > +dp R1 : local = rp-public, remote = public-rp > +dp R1 : local = rp-sw0, remote = sw0-rp > +dp public : local = public-rp, remote = rp-public > +dp sw0 : local = sw0-rp, remote = rp-sw0 > +]) > + > +OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-public > ipv6_prefix | cut -c4-15)" = ""]) > +OVS_WAIT_WHILE([test "$(ovn-nbctl get logical_router_port rp-sw0 > ipv6_prefix | cut -c4-15)" = ""]) > + > +AT_CHECK([ovn-nbctl get logical_router_port rp-public ipv6_prefix | cut > -c3-16], [0], [dnl > +[2001:1db8:3333] > +]) > +AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut > -c3-16], [0], [dnl > +[2001:1db8:3333] > +]) > + > +OVS_APP_EXIT_AND_WAIT([ovn-controller]) > + > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as ovn-nb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as northd > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > + > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > +/failed to query port patch-.*/d > +/.*terminating with signal 15.*/d"]) > +AT_CLEANUP > +]) > -- > 2.31.1 > > Other than that it looks good. Acked-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
