On Thu, Dec 11, 2025 at 01:52:03PM +0100, Dumitru Ceara wrote:
> On 12/10/25 3:48 PM, Ales Musil via dev wrote:
> > On Wed, Dec 10, 2025 at 12:06 PM Mairtin O'Loingsigh via dev <
> > [email protected]> wrote:
> > 
> >> This enables the testing of transit routers created using ovn-ic-nbctl with
> >> ovn-fake-multinode.
> >>
> >> Signed-off-by: Mairtin O'Loingsigh <[email protected]>
> >> ---
> 
> Hi Mairtin, Ales,
> 
> Thanks for the patch and review!
> 
> >>
> >  tests/multinode-macros.at | 34 +++++++++++++++++++++++++++++++
> >>  tests/multinode.at        | 43 ++++++++++++++++-----------------------
> >>  2 files changed, 51 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/tests/multinode-macros.at b/tests/multinode-macros.at
> >> index 487696a62..a522a10cc 100644
> >> --- a/tests/multinode-macros.at
> >> +++ b/tests/multinode-macros.at
> >> @@ -175,6 +175,15 @@ check_fake_multinode_setup() {
> >>  cleanup_multinode_resources_by_nodes() {
> >>      m_as ovn-central-az1 rm -f /etc/ovn/ovnnb_db.db
> >>      m_as ovn-central-az1 /usr/share/ovn/scripts/ovn-ctl restart_northd
> >> +
> >> +    m_as ovn-central-az1 rm -f /etc/ovn/ovn_ic_nb_db.db
> >> +    m_as ovn-central-az1 rm -f /etc/ovn/ovn_ic_sb_db.db
> >> +    m_as ovn-central-az1 /usr/share/ovn/scripts/ovn-ctl \
> >> +        --db-ic-nb-create-insecure-remote=yes \
> >> +        --db-ic-sb-create-insecure-remote=yes \
> >> +        restart_ic_ovsdb
> >> +
> >> +    m_as ovn-central-az1 ovn-nbctl set NB_Global . name=ovn-central-az1
> >>      check m_as ovn-central-az1 ovn-nbctl --wait=sb sync
> >>      for c; do
> >>          m_as $c ovs-vsctl del-br br-int
> >> @@ -250,6 +259,31 @@ multinode_setup_controller() {
> >>      m_as $c ip link set eth2 up
> >>  }
> >>
> >> +# multinode_cleanup_ic NODE
> >> +#
> >> +# Removes previously set nothd on specified node
> 
> Nit: this reads a bit weird.  I'd change it to:
> 
> Cleans up ovn-ic on specified node.
> 
> >> +multinode_cleanup_ic() {
> >> +    c=$1
> >> +    # Cleanup existing one
> >> +    m_as $c /usr/share/ovn/scripts/ovn-ctl stop_ic
> >> +}
> >> +
> >> +# multinode_setup_ic NODE IC_DB_IP
> >> +#
> >> +# Sets up ovn_ic on specified node.
> >> +multinode_setup_ic() {
> >> +    c=$1
> >> +    remote=$2
> >> +
> >> +    m_as $c /usr/share/ovn/scripts/ovn-ctl stop_ic
> >> +
> >> +    check m_as $c ovn-nbctl set NB_Global . name=$c
> >> +    m_as $c ovs-vsctl set open_vswitch .
> >> external_ids:ovn-is-interconn=true
> >> +    m_as $c /usr/share/ovn/scripts/ovn-ctl start_ic \
> >> +        --ovn-ic-nb-db=tcp:$remote:6645 \
> >> +        --ovn-ic-sb-db=tcp:$remote:6646
> >> +}
> >> +
> >>  # m_count_rows TABLE [CONDITION...]
> >>  #
> >>  # Prints the number of rows in TABLE (that satisfy CONDITION).
> >> diff --git a/tests/multinode.at b/tests/multinode.at
> >> index 2f74487c8..79b0448cc 100644
> >> --- a/tests/multinode.at
> >> +++ b/tests/multinode.at
> >> @@ -2387,12 +2387,24 @@ cleanup_multinode_resources
> 
> This is part of the following test:
> 
> AT_SETUP([ovn multinode - Transit Router basic functionality])
> 
> But to me the test seems to cover the whole "Transit Router"
> functionality now so I'd just rename it to:
> 
> AT_SETUP([ovn multinode - Transit Router])
> 
> >>  for i in 1 2; do
> >>      chassis="ovn-chassis-$i"
> >>      ip=$(m_as $chassis ip -4 addr show eth1 | grep inet | awk '{print
> >> $2}' | cut -d'/' -f1)
> >> +    central_ip=$(m_central_as ip -4 addr show eth1 | grep inet | awk
> >> '{print $2}' | cut -d'/' -f1)
> >>
> >>      multinode_setup_northd $chassis
> >>      multinode_setup_controller $chassis $chassis $ip $ip
> >> +    multinode_setup_ic $chassis $central_ip
> >>
> >>      check m_as $chassis ovs-vsctl set open .
> >> external_ids:ovn-monitor-all=true
> >>      check m_as $chassis ovs-vsctl set open .
> >> external_ids:ovn-is-interconn=true
> >> +done
> >> +
> >> +# Do the ovn-ic setup
> 
> Nit: comments should be sentences, missing '.'.
> 
> >> +check m_central_as ovn-ic-nbctl tr-add tr
> >> +check m_central_as ovn-ic-nbctl trp-add tr tr-gw1 00:00:00:00:30:02
> >> 100.65.0.2/30 100:65::2/126 chassis=ovn-chassis-1
> >> +check m_central_as ovn-ic-nbctl trp-add tr tr-gw2 00:00:00:00:30:06
> >> 100.65.0.6/30 100:65::6/126 chassis=ovn-chassis-2
> 
> Nit: I'd split these lines.
> 
> >> +check m_central_as ovn-ic-nbctl --wait=sb sync
> >> +
> >> +for i in 1 2; do
> >> +    chassis="ovn-chassis-$i"
> >>
> >>      check m_as $chassis ovn-nbctl ls-add public
> >>
> >> @@ -2409,7 +2421,6 @@ for i in 1 2; do
> >>
> >>      check m_as $chassis ovn-nbctl lsp-add-router-port ts ts-tr tr-ts
> >>
> >> -    check m_as $chassis ovn-nbctl lr-add tr
> >>      check m_as $chassis ovn-nbctl lrp-add tr tr-ts 00:00:00:00:10:00
> >> 10.100.200.1/24 10:200::1/64
> >>      check m_as $chassis ovn-nbctl set logical_router tr
> >> options:requested-tnl-key=20
> >>      check m_as $chassis ovn-nbctl lrp-set-gateway-chassis tr-ts $chassis
> >> @@ -2436,11 +2447,11 @@ check m_as ovn-chassis-2 ovn-nbctl lr-nat-add gw
> >> snat 100.65.0.5 192.168.100.0/2
> >>  check m_as ovn-chassis-2 ovn-nbctl lr-nat-add gw snat 100:65::5 1000::/64
> >>
> >>  # Add peer ports between GW and TR
> >> -check m_as ovn-chassis-1 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:01
> >> 100.65.0.1/30 100:65::1/126 peer=tr-gw
> >> -check m_as ovn-chassis-1 ovn-nbctl lrp-add tr tr-gw 00:00:00:00:30:02
> >> 100.65.0.2/30 100:65::2/126 peer=gw-tr
> >> +check m_as ovn-chassis-1 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:01
> >> 100.65.0.1/30 100:65::1/126 peer=tr-gw1
> >> +check m_as ovn-chassis-1 ovn-nbctl set logical_router_port tr-gw1
> >> peer="gw-tr"
> >>
> >> -check m_as ovn-chassis-2 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:05
> >> 100.65.0.5/30 100:65::5/126 peer=tr-gw
> >> -check m_as ovn-chassis-2 ovn-nbctl lrp-add tr tr-gw 00:00:00:00:30:06
> >> 100.65.0.6/30 100:65::6/126 peer=gw-tr
> >> +check m_as ovn-chassis-2 ovn-nbctl lrp-add gw gw-tr 00:00:00:00:30:05
> >> 100.65.0.5/30 100:65::5/126 peer=tr-gw2
> >> +check m_as ovn-chassis-2 ovn-nbctl set logical_router_port tr-gw2
> >> peer="gw-tr"
> >>
> >>  # Add routes for the TS subnet
> >>  check m_as ovn-chassis-1 ovn-nbctl lr-route-add gw 10.100.200.0/24
> >> 100.65.0.2
> >> @@ -2454,30 +2465,9 @@ check m_as ovn-chassis-1 ovn-nbctl ls-lb-add ts lb0
> >>  check m_as ovn-chassis-1 ovn-nbctl --match="eth.dst == 00:00:00:00:10:11"
> >> lr-nat-add tr snat 172.16.0.2 10.100.200.0/24
> >>  check m_as ovn-chassis-1 ovn-nbctl set logical_router tr
> >> options:ct-commit-all="true"
> >>
> >> -# Add mutual remote ports
> >> -check m_as ovn-chassis-1 ovn-nbctl lrp-add tr tr-az2 00:00:00:00:30:06
> >> 100.65.0.6/30 100:65::6/126
> >> -check m_as ovn-chassis-1 ovn-nbctl set logical_router_port tr-az2
> >> options:requested-chassis=ovn-chassis-2
> >> -
> >> -check m_as ovn-chassis-2 ovn-nbctl lrp-add tr tr-az1 00:00:00:00:30:02
> >> 100.65.0.2/30 100:65::2/126
> >> -check m_as ovn-chassis-2 ovn-nbctl set logical_router_port tr-az1
> >> options:requested-chassis=ovn-chassis-1
> >> -
> >> -# Important set the proper tunnel keys
> >> -check m_as ovn-chassis-1 ovn-nbctl set logical_router_port tr-gw
> >> options:requested-tnl-key=10
> >> -check m_as ovn-chassis-1 ovn-nbctl set logical_router_port tr-az2
> >> options:requested-tnl-key=20
> >> -
> >> -check m_as ovn-chassis-2 ovn-nbctl set logical_router_port tr-gw
> >> options:requested-tnl-key=20
> >> -check m_as ovn-chassis-2 ovn-nbctl set logical_router_port tr-az1
> >> options:requested-tnl-key=10
> >> -
> >>  check m_as ovn-chassis-1 ovn-nbctl lsp-add public external
> >>  check m_as ovn-chassis-1 ovn-nbctl lsp-set-addresses external
> >> "00:00:00:00:20:10 192.168.100.10 1000::10"
> >>
> >> -# Add mutual chassis
> >> -check m_as ovn-chassis-1 ovn-sbctl chassis-add ovn-chassis-2 geneve
> >> $(m_as ovn-chassis-2 ip -4 addr show eth1 | grep inet | awk '{print $2}' |
> >> cut -d'/' -f1)
> >> -check m_as ovn-chassis-1 ovn-sbctl set chassis ovn-chassis-2
> >> other_config:is-remote=true
> >> -
> >> -check m_as ovn-chassis-2 ovn-sbctl chassis-add ovn-chassis-1 geneve
> >> $(m_as ovn-chassis-1 ip -4 addr show eth1 | grep inet | awk '{print $2}' |
> >> cut -d'/' -f1)
> >> -check m_as ovn-chassis-2 ovn-sbctl set chassis ovn-chassis-1
> >> other_config:is-remote=true
> >> -
> >>  # Configure ports on the transit switch as remotes
> >>  check m_as ovn-chassis-1 ovn-nbctl lsp-set-type pod20 remote
> >>  check m_as ovn-chassis-1 ovn-nbctl lsp-set-options pod10
> >> requested-chassis=ovn-chassis-1
> >> @@ -2544,6 +2534,7 @@ for i in 1 2; do
> >>      chassis="ovn-chassis-$i"
> >>      ip=$(m_as $chassis ip -4 addr show eth1 | grep inet | awk '{print
> >> $2}' | cut -d'/' -f1)
> >>
> >> +    multinode_cleanup_ic $chassis
> >>      multinode_setup_controller $chassis $chassis $ip "170.168.0.2"
> >>      multinode_cleanup_northd $chassis
> >>  done
> >> --
> >> 2.51.1
> >>
> >> _______________________________________________
> >> dev mailing list
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >>
> > 
> > Thank you for the v3 Mairtin,
> > 
> > I think this is better for now. There is a Reported-at tag missing,
> > other than that it looks good.
> 
> I added:
> 
> Reported-at: https://issues.redhat.com/browse/FDP-1734
> 
> And fixed up the small things I mentioned above and then I pushed the
> patch to main.
> 
> > 
> > Acked-by: Ales Musil <[email protected]>
> > 
> > Regards,
> > Ales
> 
> Regards,
> Dumitru
> 
> 
> 

Thanks for the fixes and merging.

Regards,
Mairtin

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to