On 4/14/25 4:05 PM, Dumitru Ceara wrote: > On 4/4/25 6:20 PM, Lorenzo Bianconi via dev wrote: >>> This tests OVN BGP capabilities in a multinode environment by setting up >>> two sets of a ToR switch, connected to an OVN node with BGP set up. The >>> pair then form a connection and then the ToR switch can connect to a >>> guest-vm which is accessible via a distributed gateway logical router port. >>> >>> Signed-off-by: MJ Ponsonby <mj.ponso...@canonical.com> >>
Hi MJ, I was experimenting with FRR and BGP+EVPN trying to figure out missing bits in OVN so I used your multinode test as a starting point. Thanks again for spending the time on this! I did find a couple more small issues (please see below). Aside from that, while trying to understand better how things work, I took the liberty of making some changes which (at least for me) made it more readable. In case that helps and if you wish to integrate it in your next revision here's the incremental change: https://github.com/dceara/ovn/commit/e2c4b8f Another thing we might want to add is a few more comments; at least for the setup_frr()/setup_ovn_bgp()/add_guest_vm_and_connections() functions. > > Hi MJ, Lorenzo, > > Thanks for the patch and review! Sorry for the delay on my side. > Next to Lorenzo's comments I also have a few small ones below. > >> Hi MJ and Frode, >> >> thx for the patch, LGTM, just some (minor) comments inline. >> >> Regards, >> Lorenzo >> >>> --- >>> tests/multinode.at | 241 +++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 241 insertions(+) >>> >>> diff --git a/tests/multinode.at b/tests/multinode.at >>> index 68c9eba22..eb8047e7a 100644 >>> --- a/tests/multinode.at >>> +++ b/tests/multinode.at >>> @@ -3030,4 +3030,245 @@ m_as ovn-chassis-3 killall tcpdump >>> >>> AT_CLEANUP >>> >>> +AT_SETUP([ovn multinode bgp unnumbered]) >>> >>> +check_fake_multinode_setup >>> +cleanup_multinode_resources >>> + >> >> unnecessary blank line >> >>> + >>> +setup_frr() { >>> + container_prefix=$1 >>> + container_number=$2 >>> + br_name=br-$container_prefix-$container_number >>> + >>> + check m_as $container_prefix-$container_number ovs-vsctl add-br >>> $br_name >>> + on_exit "m_as $container_prefix-$container_number ovs-vsctl del-br >>> br-$container_prefix-$container_number" >>> + check m_as $container_prefix-$container_number ip netns add frr-ns >>> + on_exit "m_as $container_prefix-$container_number ip netns del frr-ns" >>> + check m_as $container_prefix-$container_number ip netns exec frr-ns ip >>> link set lo up >>> + m_as $container_prefix-$container_number ovs-vsctl add-port $br_name >>> ext0 -- set interface ext0 type=internal >>> + m_as $container_prefix-$container_number ovs-vsctl add-port $br_name >>> ext1 -- set interface ext1 type=internal >>> + >>> + m_as $container_prefix-$container_number ip link set ext1 netns frr-ns >>> + m_as $container_prefix-$container_number ip netns exec frr-ns ip link >>> set ext1 up >>> + m_as $container_prefix-$container_number ip netns exec frr-ns ip addr >>> add >>> 4$container_number.4$container_number.4$container_number.4$container_number >>> dev ext1 >>> + m_as $container_prefix-$container_number ip link set ext0 up >>> + >>> + check m_as $container_prefix-$container_number sed -i >>> 's/bgpd=no/bgpd=yes/g' /etc/frr/daemons >>> + check m_as $container_prefix-$container_number sed -i >>> 's/StartLimitBurst=.*/StartLimitBurst=100/g' >>> /usr/lib/systemd/system/frr.service >>> + check m_as $container_prefix-$container_number systemctl daemon-reload >>> + >>> + check m_as $container_prefix-$container_number mkdir -p /etc/frr/frr-ns >>> + if $(m_as ovn-gw-1 grep -qi fedora /etc/os-release); then > > What if we add a helper to multinode-macros.at? E.g.: > > +# m_is_fedora > +# > +# Checks if the ovn-fake-multinode deployment is running fedora-based images. > +m_is_fedora() { > + m_central_as grep -qi fedora /etc/os-release > +} > + > > And use it as: > > if m_is_fedora; then > ... > else > ... > fi > > where needed? > >>> + check m_as $container_prefix-$container_number chown -R frr:frr >>> /etc/frr/frr-ns >>> + check m_as $container_prefix-$container_number mkdir -p >>> /run/frr/frr-ns >>> + check m_as $container_prefix-$container_number chown -R frr:frr >>> /run/frr/frr-ns >>> + check m_as $container_prefix-$container_number cp -r >>> /etc/frr/daemons /etc/frr/frr.conf /etc/frr/frr-ns/ >>> + else >>> + check m_as $container_prefix-$container_number cp -r >>> /etc/frr/daemons /etc/frr/frr.conf /etc/frr/support_bundle_commands.conf >>> /etc/frr/frr-ns/ >>> + fi >>> + on_exit "m_as $container_prefix-$container_number rm -rf >>> /etc/frr/frr-ns" >>> + check m_as $container_prefix-$container_number rm -rf >>> /etc/frr/frr-ns/vtysh.conf >>> + check m_as $container_prefix-$container_number touch >>> /etc/frr/frr-ns/vtysh.conf >>> + >>> + check m_as $container_prefix-$container_number systemctl stop frr >>> + if $(m_as ovn-gw-1 grep -qi fedora /etc/os-release); then >>> + m_as $container_prefix-$container_number ip netns exec frr-ns >>> /usr/libexec/frr/frrinit.sh start frr-ns >>> + on_exit "m_as $container_prefix-$container_number ip netns exec >>> frr-ns /usr/libexec/frr/frrinit.sh stop frr-ns" >>> + else >>> + m_as $container_prefix-$container_number ip netns exec frr-ns >>> /usr/lib/frr/frrinit.sh start frr-ns >>> + on_exit "m_as $container_prefix-$container_number ip netns exec >>> frr-ns /usr/lib/frr/frrinit.sh stop frr-ns" >>> + fi >>> + on_exit "m_as $container_prefix-$container_number systemctl stop frr" >>> + check m_as $container_prefix-$container_number systemctl start frr >>> + flag="-N frr-ns" >>> + if $(m_as ovn-gw-1 grep -qi fedora /etc/os-release); then >>> + flag=--vty_socket /run/frr/frr-ns > > I guess this should be between quotes: > > flag="--vty_socket /run/frr/frr-ns" > >>> + fi >>> + echo "configure >>> + ! >>> + ip prefix-list accept-all seq 5 permit any >>> + ! >>> + router bgp 4200000${container_number}00 >>> + bgp router-id >>> ${container_number}0.${container_number}0.${container_number}0.${container_number}0 >>> + neighbor ext1 interface remote-as external >>> + ! >>> + address-family ipv4 unicast >>> + neighbor ext1 soft-reconfiguration inbound >>> + neighbor ext1 prefix-list accept-all in >>> + exit-address-family >>> + ! >>> + address-family ipv6 unicast >>> + neighbor ext1 soft-reconfiguration inbound >>> + neighbor ext1 activate >>> + exit-address-family >>> + !" | podman exec -i $container_prefix-$container_number vtysh $flag >>> +} >>> + >>> +setup_ovn_bgp() { >>> + >> >> unnecessary blank line >> >>> + container_prefix=$1 >>> + container_number=$2 >>> + br_name=br-$container_prefix-$container_number >>> + >>> + check m_as $container_prefix-$container_number ovs-vsctl set >>> Open_vSwitch . >>> external-ids:ovn-bridge-mappings="physnet_$container_prefix-${container_number}_ext0:$br_name" >>> + >>> + check multinode_nbctl --wait=hv lr-add >>> lr-$container_prefix-${container_number}-ext0 >>> + check multinode_nbctl --wait=hv set Logical_Router >>> lr-$container_prefix-$container_number-ext0 >>> options:chassis=$container_prefix-$container_number The --wait=hv seems unnecessary here. >>> + check multinode_nbctl set Logical_Router >>> lr-$container_prefix-$container_number-ext0 options:dynamic-routing=true >>> options:requested-tnl-key=${container_number}0 >>> + >>> + check multinode_nbctl lrp-add >>> lr-$container_prefix-$container_number-ext0 >>> lrp-$container_prefix-$container_number-ext0 >>> ${container_number}2:fb:d6:66:99:${container_number}c >>> + check multinode_nbctl lrp-set-options >>> lrp-$container_prefix-$container_number-ext0 >>> dynamic-routing-maintain-vrf=true dynamic-routing-redistribute=nat >>> + >>> + check multinode_nbctl ls-add >>> ls-$container_prefix-$container_number-ext0 >>> + >>> + check multinode_nbctl lsp-add >>> ls-$container_prefix-$container_number-ext0 >>> lsp-$container_prefix-$container_number-ext0 >>> + check multinode_nbctl lsp-set-type >>> lsp-$container_prefix-$container_number-ext0 router >>> + check multinode_nbctl lsp-set-options >>> lsp-$container_prefix-$container_number-ext0 >>> router-port=lrp-$container_prefix-$container_number-ext0 >>> + check multinode_nbctl lsp-set-addresses >>> lsp-$container_prefix-$container_number-ext0 router >>> + >>> + check multinode_nbctl lsp-add >>> ls-$container_prefix-$container_number-ext0 >>> patch-$container_prefix-$container_number-ext0 >>> + check multinode_nbctl lsp-set-addresses >>> patch-$container_prefix-$container_number-ext0 unknown >>> + check multinode_nbctl lsp-set-type >>> patch-$container_prefix-$container_number-ext0 localnet >>> + check multinode_nbctl --wait=hv lsp-set-options >>> patch-$container_prefix-$container_number-ext0 >>> network_name=physnet_$container_prefix-${container_number}_ext0 >>> + >>> + OVS_WAIT_UNTIL([m_as $container_prefix-$container_number ip link | >>> grep -q ovnvrf${container_number}0:.*UP]) >>> + >>> + check multinode_nbctl lsp-add >>> ls-$container_prefix-$container_number-ext0 >>> lsp-$container_prefix-$container_number-ext0-bgp >>> + check multinode_nbctl lsp-set-addresses >>> lsp-$container_prefix-$container_number-ext0-bgp unknown >>> + >>> + check multinode_nbctl add Logical_Router_Port >>> lrp-$container_prefix-$container_number-ext0 options >>> routing-protocols=\"BGP,BFD\" >>> routing-protocol-redirect=lsp-$container_prefix-$container_number-ext0-bgp >>> + check multinode_nbctl set Logical_Router_Port >>> lrp-$container_prefix-$container_number-ext0 >>> ipv6_ra_configs:send_periodic=true >>> + check multinode_nbctl set Logical_Router_Port >>> lrp-$container_prefix-$container_number-ext0 >>> ipv6_ra_configs:address_mode=slaac >>> + check multinode_nbctl set Logical_Router_Port >>> lrp-$container_prefix-$container_number-ext0 ipv6_ra_configs:max_interval=1 >>> + check multinode_nbctl set Logical_Router_Port >>> lrp-$container_prefix-$container_number-ext0 ipv6_ra_configs:min_interval=1 >>> + >>> + check m_as $container_prefix-$container_number ovs-vsctl add-port >>> br-int ext0-bgp -- set Interface ext0-bgp type=internal >>> mac=\"${container_number}2:fb:d6:66:99:${container_number}c\" >>> external-ids:iface-id=lsp-$container_prefix-$container_number-ext0-bgp >>> + on_exit "m_as $container_prefix-$container_number ovs-vsctl del-port >>> br-int ext0-bgp" >>> + check m_as $container_prefix-$container_number ip link set dev >>> ext0-bgp master ovnvrf${container_number}0 >>> + check m_as $container_prefix-$container_number ip link set dev >>> ext0-bgp up Instead we should probably add "--wait=hv sync" here (and wait for ports to be up). >>> + >>> + echo "configure >>> + ip prefix-list no-default seq 5 deny 0.0.0.0/0 >>> + ip prefix-list no-default seq 10 permit 0.0.0.0/0 le 32 >>> + ipv6 prefix-list no-default seq 5 deny ::/0 >>> + ipv6 prefix-list no-default seq 10 permit ::/0 le 128 >>> + vrf ovnvrf${container_number}0 >>> + exit-vrf >>> + router bgp 42${container_number}0000000 vrf ovnvrf${container_number}0 >>> + bgp router-id >>> ${container_number}4.${container_number}4.${container_number}4.${container_number}4 >>> + neighbor ext0-bgp interface remote-as external >>> + address-family ipv4 unicast >>> + redistribute kernel >>> + neighbor ext0-bgp prefix-list no-default out >>> + exit-address-family >>> + address-family ipv6 unicast >>> + neighbor ext0-bgp soft-reconfiguration inbound >>> + neighbor ext0-bgp prefix-list no-default out >>> + redistribute kernel >>> + neighbor ext0-bgp activate >>> + exit-address-family >>> + do copy running-config startup-config" | podman exec -i >>> $container_prefix-$container_number vtysh >>> +} >>> + >>> +add_guest_vm_and_connections() { >>> + container_prefix=$1 >>> + container_number=$2 >>> + >>> + gw_lr="lr-${container_prefix}-${container_number}-ext0" >>> + lrp_to_join="lrp${container_number}-to-join" >>> + lsp_join_to_lrp="join-to-lrp${container_number}" >>> + lrp_guest="lrp-guest${container_number}" >>> + >>> + ls_g="ls-guest${container_number}" >>> + lsp_g_lrg="lsp-guest${container_number}-lr-guest" >>> + lsp_g_iface="lsp-guest${container_number}-guest-vm" >>> + lrp_g_lsg="lrp-guest-ls-guest${container_number}" >>> + >>> + guest_gw_ip="192.168.10.1" >>> + guest_gw_cidr="$guest_gw_ip/24" >>> + guest_vm_ip="192.168.10.10" >>> + guest_vm_cidr="$guest_vm_ip/24" This creates a duplicate network (same network on two router ports) on lr-guest. >> >> in order to make the code more readable I guess we can reduce the variable >> usage. What do you think? >> >>> + >>> + check multinode_nbctl lrp-add $gw_lr $lrp_to_join >>> 00:00:ff:00:00:0${container_number} >>> + check multinode_nbctl lrp-set-options $lrp_to_join >>> dynamic-routing-redistribute=nat >>> + check multinode_nbctl lsp-add $join_ls $lsp_join_to_lrp >>> + check multinode_nbctl lsp-set-type $lsp_join_to_lrp router >>> + check multinode_nbctl lsp-set-options $lsp_join_to_lrp >>> router-port=$lrp_to_join >>> + check multinode_nbctl lsp-set-addresses $lsp_join_to_lrp router >>> + >>> + check multinode_nbctl ls-add $ls_g >>> + check multinode_nbctl lrp-add $lr_guest $lrp_g_lsg >>> 00:16:03:0${container_number}:03:03 $guest_gw_cidr >>> + check multinode_nbctl lsp-add $ls_g $lsp_g_lrg >>> + check multinode_nbctl lsp-set-type $lsp_g_lrg router >>> + check multinode_nbctl lsp-set-options $lsp_g_lrg router-port=$lrp_g_lsg >>> + check multinode_nbctl lsp-set-addresses $lsp_g_lrg router >>> + check multinode_nbctl lsp-add $ls_g $lsp_g_iface >>> + check multinode_nbctl lsp-set-addresses $lsp_g_iface >>> '00:16:0'${container_number}':00:02:02 '$guest_vm_cidr'' >>> + >>> + m_as ${container_prefix}-${container_number} /data/create_fake_vm.sh >>> $lsp_g_iface $guest_vm_ns 00:16:0${container_number}:00:02:02 1342 >>> $guest_vm_ip 24 $guest_gw_ip 1000::${container_number}3/64 1000::a >>> + neighbor_lla=$(m_as ${container_prefix}-${container_number} vtysh -c >>> "show bgp vrf ovnvrf${container_number}0 neighbor ext0-bgp" | grep >>> "^Foreign host:" | awk '{print $3}' | tr -d ',') >>> + check multinode_nbctl lr-route-add >>> lr-${container_prefix}-${container_number}-ext0 "0.0.0.0/0" $neighbor_lla >>> lrp-${container_prefix}-${container_number}-ext0 >>> + >>> + check multinode_nbctl lr-route-add $lr_guest >>> "4${container_number}.0.0.0/8" fe80::200:ffff:fe00:${container_number} >>> $lrp_guest_join >>> +} >>> + >>> +setup_frr "ovn-gw" 1 >>> +setup_ovn_bgp "ovn-gw" 1 >>> + >>> +setup_frr "ovn-gw" 2 >>> +setup_ovn_bgp "ovn-gw" 2 >>> + >>> +OVS_WAIT_UNTIL([m_as ovn-gw-2 vtysh -c 'show bgp vrf ovnvrf20 neighbors' | >>> grep -qE 'Connections established 1']) >>> +OVS_WAIT_UNTIL([m_as ovn-gw-1 vtysh -c 'show bgp vrf ovnvrf10 neighbors' | >>> grep -qE 'Connections established 1']) >>> + >>> +# Tor <-> ovn-gw via bgp >>> +# lr-guest with distributed gateway port >>> +# bgp on lr-ovn-gw-2-ext0 >>> +# >>> +# guest-1 guest-2 >>> +# \ / >>> +# lr-guest >>> +# DGP >>> +# | >>> +# ls-join >>> +# / \ >>> +# tor <-> lr-ovn-gw-2-ext0* lr-ovn-gw-1-ext0* <-> tor >>> +# | | >>> +# ls-ovn-gw-2-ext0 ls-ovn-gw-1-ext0 >>> +# >>> +# >>> +# >>> + >>> +join_ls="ls-join" >>> +lsp_join_guest="lsp-join-guest" >>> + >>> +lr_guest="lr-guest" >>> +lrp_guest_join="lrp-guest-join-dgp" >>> + >>> +guest_vm_iface="guest-vm" >>> +guest_vm_ns="ns-guest" >>> + >>> +check multinode_nbctl ls-add $join_ls >>> + >>> +check multinode_nbctl lr-add $lr_guest >>> +check multinode_nbctl lrp-add $lr_guest $lrp_guest_join 00:16:06:12:f0:0d >>> +check multinode_nbctl lsp-add $join_ls $lsp_join_guest >>> +check multinode_nbctl lsp-set-type $lsp_join_guest router >>> +check multinode_nbctl lsp-set-options $lsp_join_guest >>> router-port=$lrp_guest_join >>> +check multinode_nbctl lsp-set-addresses $lsp_join_guest router >>> +check multinode_nbctl lrp-set-gateway-chassis $lrp_guest_join ovn-gw-1 20 >>> +check multinode_nbctl lrp-set-gateway-chassis $lrp_guest_join ovn-gw-2 20 >>> + >>> +add_guest_vm_and_connections "ovn-gw" 1 >>> +add_guest_vm_and_connections "ovn-gw" 2 >>> + >>> +check multinode_nbctl --gateway-port $lrp_guest_join --add-route >>> lr-nat-add $lr_guest dnat_and_snat 172.16.10.2 192.168.10.10 >>> + >>> +OVS_WAIT_UNTIL([m_central_as ovn-sbctl list Advertised_Route | grep -q >>> 172.16.10.2]) >>> +OVS_WAIT_UNTIL([m_as ovn-gw-1 ip netns exec frr-ns ip route | grep -q >>> 'ext1']) >>> +OVS_WAIT_UNTIL([m_as ovn-gw-1 ip netns exec frr-ns ping -W 1 -c 1 >>> 172.16.10.2]) >>> +OVS_WAIT_UNTIL([m_as ovn-gw-2 ip netns exec frr-ns ip route | grep -q >>> 'ext1']) >>> +OVS_WAIT_UNTIL([m_as ovn-gw-2 ip netns exec frr-ns ping -W 1 -c 1 >>> 172.16.10.2]) >>> + >>> +AT_CLEANUP >>> -- > > Otherwise, I tested this patch and it seems to work fine both with Fedora > and with Ubuntu images in ovn-fake-multinode. > > Thanks, > Dumitru > Best regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
