Skip to content

Commit

Permalink
northd: Autodiscover centralize_routing.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
  • Loading branch information
@felixhuettner @dceara
felixhuettner authored and dceara committed Dec 11, 2024
1 parent 88e3561 commit cefe6ac
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 91 deletions.
2 changes: 2 additions & 0 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Post v24.09.0
- ovn-ic: Add support for route tag to prevent route learning.
- Support for STT tunnels in ovn-encap-type is deprecated and will be
removed in the next release.
- 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
--------------------------
Expand Down
6 changes: 2 additions & 4 deletions northd/northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2332,7 +2332,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.
Expand All @@ -2343,7 +2342,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;
Expand Down Expand Up @@ -2503,8 +2502,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
Expand Down
34 changes: 0 additions & 34 deletions ovn-nb.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3645,40 +3645,6 @@ or
</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>

<column name="options" key="ic-route-tag" type='{"type": "string"}'>
<p>
This option expects a name of a route-tag that's present in the
Expand Down
7 changes: 0 additions & 7 deletions tests/multinode.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
48 changes: 2 additions & 46 deletions tests/ovn-northd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13736,52 +13736,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
Expand Down

0 comments on commit cefe6ac

Please sign in to comment.