diff mbox series

[ovs-dev,v3,09/33] northd: Autodiscover centralize_routing.

Message ID 8ac6eda5ed9ee2cfe5b07baf7a4cc8e856313926.1732630355.git.felix.huettner@stackit.cloud
State Changes Requested
Headers show
Series OVN Fabric integration | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success

Commit Message

Felix Huettner Nov. 26, 2024, 2:38 p.m. UTC
There is no need to set this manually. In all cases where a user would
set option:centralize_routing they would not work without this setting.
Therefor we enable it automatically.

Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
 NEWS                |  2 ++
 northd/northd.c     |  6 ++----
 ovn-nb.xml          | 34 --------------------------------
 tests/multinode.at  |  7 -------
 tests/ovn-northd.at | 48 ++-------------------------------------------
 5 files changed, 6 insertions(+), 91 deletions(-)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index aaa3964a9..6deecf037 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@ 
 Post v24.09.0
 -------------
+  - The LRP option 'centralize_routing' has been removed. The behavior is now
+    enabled in all cases where it is needed.
 
 OVN v24.09.0 - 13 Sep 2024
 --------------------------
diff --git a/northd/northd.c b/northd/northd.c
index 0050919b1..617ecc8dd 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2329,7 +2329,6 @@  create_cr_port(struct ovn_port *op, struct hmap *ports,
  * Chassis resident port needs to be created if the following
  * conditionsd are met:
  *   - op is a distributed gateway port
- *   - op has the option 'centralize_routing' set to true
  *   - op is the only distributed gateway port attached to its
  *     router
  *   - op's peer logical switch has no localnet ports.
@@ -2340,7 +2339,7 @@  peer_needs_cr_port_creation(struct ovn_port *op)
     if ((op->nbrp->n_gateway_chassis || op->nbrp->ha_chassis_group)
         && op->od->n_l3dgw_ports == 1 && op->peer && op->peer->nbsp
         && !op->peer->od->n_localnet_ports) {
-        return smap_get_bool(&op->nbrp->options, "centralize_routing", false);
+        return true;
     }
 
     return false;
@@ -2500,8 +2499,7 @@  join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table,
      * peer if
      *  - DGP's router has only one DGP and
      *  - Its peer is a logical switch port and
-     *  - It's peer's logical switch has no localnet ports and
-     *  - option 'centralize_routing' is set to true for the DGP.
+     *  - It's peer's logical switch has no localnet ports
      *
      * This is required to support
      *   - NAT via geneve (for the overlay provider networks) and
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 2836f58f5..a7d9d4444 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -3628,40 +3628,6 @@  or
           <ref column="options" key="gateway_mtu"/> option.
         </p>
       </column>
-
-      <column name="options" key="centralize_routing"
-              type='{"type": "boolean"}'>
-        <p>
-          This option is applicable only if the router port is a
-          distributed gateway port i.e if the <ref table="Logical_Router_Port"
-          column="ha_chassis_group"/> column or
-          <ref table="Logical_Router_Port" column="gateway_chassis"/>
-          is set.
-        </p>
-
-        <p>
-          If set to <code>true</code>, routing for the router port's
-          networks (set in the column <ref table="Logical_Router_Port"
-          column="networks"/>) is centralized on the gateway chassis
-          which claims this distributed gateway port.
-        </p>
-
-        <p>
-          Additionally for this option to take effect, below conditions
-          must be met:
-        </p>
-
-        <ul>
-          <li>
-            The Logical router has only one distributed gateway port.
-          </li>
-
-          <li>
-            The router port's peer logical switch has no localnet ports.
-          </li>
-
-        </ul>
-      </column>
     </group>
 
     <group title="Attachment">
diff --git a/tests/multinode.at b/tests/multinode.at
index 408e1118d..9e1c6d29a 100644
--- a/tests/multinode.at
+++ b/tests/multinode.at
@@ -1197,13 +1197,6 @@  run_ns_traffic
 # Delete the localnet port by changing the type of ln-public to VIF port.
 check multinode_nbctl --wait=hv lsp-set-type ln-public ""
 
-# cr-port should not be created for public-lr0 since the option
-# centralize_routing=true is not yet set for lr0-public.
-m_check_row_count Port_Binding 0 logical_port=cr-public-lr0
-
-# Set the option - centralize_routing now.
-check multinode_nbctl --wait=hv set logical_router_port lr0-public options:centralize_routing=true
-
 m_check_row_count Port_Binding 1 logical_port=cr-public-lr0
 m_check_column chassisredirect Port_Binding type logical_port=cr-public-lr0
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 63fcf3f71..4588a65c6 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -13675,52 +13675,8 @@  check_flows_no_cr_port_for_public_lr0
 # Remove the localnet port from public logical switch.
 check ovn-nbctl --wait=sb lsp-set-type ln-public ""
 
-# Check that the lflows are as expected and there is no cr port
-# created for "public-lr0"  when public has no localnet port
-# since public doesn't have the option "overlay_provider_network=true"
-# set.
-check_row_count Port_Binding 0 logical_port=cr-public-lr0
-
-ovn-sbctl dump-flows lr0 > lr0flows
-ovn-sbctl dump-flows public > publicflows
-
-AT_CHECK([grep -e "172.168.0.110" -e "172.168.0.120" -e "10.0.0.3" -e "20.0.0.3" -e "30:54:00:00:00:03"  -e "sw0-port1" lr0flows | ovn_strip_lflows], [0], [dnl
-  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 30:54:00:00:00:03 && inport == "lr0-public" && is_chassis_resident("sw0-port1")), action=(xreg0[[0..47]] = 00:00:00:00:ff:02; next;)
-  table=??(lr_in_arp_resolve  ), priority=100  , match=(outport == "lr0-public" && reg0 == 172.168.0.110), action=(eth.dst = 30:54:00:00:00:03; next;)
-  table=??(lr_in_arp_resolve  ), priority=100  , match=(outport == "lr0-public" && reg0 == 172.168.0.120), action=(eth.dst = 00:00:00:00:ff:02; next;)
-  table=??(lr_in_arp_resolve  ), priority=150  , match=(inport == "lr0-public" && outport == "lr0-public" && ip4.dst == 172.168.0.110), action=(drop;)
-  table=??(lr_in_arp_resolve  ), priority=150  , match=(inport == "lr0-public" && outport == "lr0-public" && ip4.dst == 172.168.0.120), action=(drop;)
-  table=??(lr_in_dnat         ), priority=100  , match=(ip && ip4.dst == 172.168.0.110 && inport == "lr0-public"), action=(ct_dnat(10.0.0.3);)
-  table=??(lr_in_dnat         ), priority=100  , match=(ip && ip4.dst == 172.168.0.120 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_dnat(20.0.0.3);)
-  table=??(lr_in_gw_redirect  ), priority=100  , match=(ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("sw0-port1")), action=(eth.src = 30:54:00:00:00:03; reg1 = 172.168.0.110; next;)
-  table=??(lr_in_ip_input     ), priority=90   , match=(arp.op == 1 && arp.tpa == 172.168.0.110), action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
-  table=??(lr_in_ip_input     ), priority=90   , match=(arp.op == 1 && arp.tpa == 172.168.0.120), action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
-  table=??(lr_in_ip_input     ), priority=91   , match=(inport == "lr0-public" && arp.op == 1 && arp.tpa == 172.168.0.110), action=(drop;)
-  table=??(lr_in_ip_input     ), priority=91   , match=(inport == "lr0-public" && arp.op == 1 && arp.tpa == 172.168.0.120), action=(drop;)
-  table=??(lr_in_ip_input     ), priority=92   , match=(inport == "lr0-public" && arp.op == 1 && arp.tpa == 172.168.0.110 && is_chassis_resident("sw0-port1")), action=(eth.dst = eth.src; eth.src = 30:54:00:00:00:03; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 30:54:00:00:00:03; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
-  table=??(lr_in_ip_input     ), priority=92   , match=(inport == "lr0-public" && arp.op == 1 && arp.tpa == 172.168.0.120 && is_chassis_resident("cr-lr0-public")), action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
-  table=??(lr_in_unsnat       ), priority=100  , match=(ip && ip4.dst == 172.168.0.110 && inport == "lr0-public"), action=(ct_snat;)
-  table=??(lr_in_unsnat       ), priority=100  , match=(ip && ip4.dst == 172.168.0.120 && inport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_snat;)
-  table=??(lr_out_egr_loop    ), priority=100  , match=(ip4.dst == 172.168.0.110 && outport == "lr0-public" && is_chassis_resident("sw0-port1")), action=(clone { ct_clear; inport = outport; outport = ""; eth.dst <-> eth.src; flags = 0; flags.loopback = 1; reg0 = 0; reg1 = 0; reg2 = 0; reg3 = 0; reg4 = 0; reg5 = 0; reg6 = 0; reg7 = 0; reg8 = 0; reg9 = 0; reg9[[0]] = 1; next(pipeline=ingress, table=??); };)
-  table=??(lr_out_egr_loop    ), priority=100  , match=(ip4.dst == 172.168.0.120 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(clone { ct_clear; inport = outport; outport = ""; eth.dst <-> eth.src; flags = 0; flags.loopback = 1; reg0 = 0; reg1 = 0; reg2 = 0; reg3 = 0; reg4 = 0; reg5 = 0; reg6 = 0; reg7 = 0; reg8 = 0; reg9 = 0; reg9[[0]] = 1; next(pipeline=ingress, table=??); };)
-  table=??(lr_out_snat        ), priority=161  , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public" && is_chassis_resident("sw0-port1") && (!ct.trk || !ct.rpl)), action=(eth.src = 30:54:00:00:00:03; ct_snat(172.168.0.110);)
-  table=??(lr_out_snat        ), priority=161  , match=(ip && ip4.src == 20.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.120);)
-  table=??(lr_out_undnat      ), priority=100  , match=(ip && ip4.src == 10.0.0.3 && outport == "lr0-public"), action=(eth.src = 30:54:00:00:00:03; ct_dnat;)
-  table=??(lr_out_undnat      ), priority=100  , match=(ip && ip4.src == 20.0.0.3 && outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), action=(ct_dnat;)
-])
-
-AT_CHECK([grep -e "172.168.0.110" -e "172.168.0.120" -e "10.0.0.3" -e "20.0.0.3" -e "30:54:00:00:00:03"  -e "sw0-port1" publicflows | ovn_strip_lflows], [0], [dnl
-  table=??(ls_in_l2_lkup      ), priority=50   , match=(eth.dst == 30:54:00:00:00:03 && is_chassis_resident("sw0-port1")), action=(outport = "public-lr0"; output;)
-  table=??(ls_in_l2_lkup      ), priority=75   , match=(eth.src == {00:00:00:00:ff:02, 30:54:00:00:00:03} && (arp.op == 1 || rarp.op == 3 || nd_ns)), action=(outport = "_MC_flood_l2"; output;)
-  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && arp.op == 1 && arp.tpa == 172.168.0.110), action=(clone {outport = "public-lr0"; output; }; outport = "_MC_flood_l2"; output;)
-  table=??(ls_in_l2_lkup      ), priority=80   , match=(flags[[1]] == 0 && arp.op == 1 && arp.tpa == 172.168.0.120), action=(clone {outport = "public-lr0"; output; }; outport = "_MC_flood_l2"; output;)
-])
-
-
-# Set the option "centralize_routing=true" for lr0-public.
-check ovn-nbctl --wait=sb set logical_router_port lr0-public options:centralize_routing=true
-
-# Check that the lflows are as expected and there is cr port created for public-lr0.
+# we know we still need the cr port so we check that the lflows are as
+# expected and there is cr port created for public-lr0.
 check_flows_cr_port_for_public_lr0
 
 # Set the type of ln-public back to localnet