Hi Lorenzo,
I have a couple of comments below. In addition, please add a test for
this scenario to the testsuite.
On 9/19/22 09:22, Lorenzo Bianconi wrote:
Keep FIP traffic distributed and do not centralize it even if the
CMS sets redirect-type option to bridged for distributed gateway port.
Tested-by: Jianlin Shi <[email protected]>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2007120
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
northd/northd.c | 29 +++++++++++++++++++++++++++++
northd/northd.h | 2 ++
northd/ovn-northd.8.xml | 27 +++++++++++++++++++++++++++
3 files changed, 58 insertions(+)
diff --git a/northd/northd.c b/northd/northd.c
index 84440a47f..5c1ddf5c2 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2616,6 +2616,13 @@ join_logical_ports(struct northd_input *input_data,
op->od = od;
ovs_list_push_back(&od->port_list, &op->dp_node);
+ const char *redirect_type = smap_get(&nbrp->options,
+ "redirect-type");
+ if (!od->redirect_bridged && redirect_type &&
+ !strcasecmp(redirect_type, "bridged")) {
+ od->redirect_bridged = true;
+ }
Nit-picky optimization: Only call smap_get() if od->redirect_bridge is
false.
+
if (op->nbrp->ha_chassis_group ||
op->nbrp->n_gateway_chassis) {
/* Additional "derived" ovn_port crp represents the
@@ -13731,6 +13738,28 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath
*od, struct hmap *lflows,
100, ds_cstr(match),
ds_cstr(actions),
&nat->header_);
+ if (od->redirect_bridged && distributed) {
+ ds_clear(match);
+ ds_put_format(
+ match,
+ "outport == %s && ip%s.src == %s "
+ "&& is_chassis_resident(\"%s\")",
+ od->l3dgw_ports[0]->json_key,
+ is_v6 ? "6" : "4", nat->logical_ip,
+ nat->logical_port);
+ ds_clear(actions);
+ if (is_v6) {
+ ds_put_cstr(actions,
+ "get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;");
+ } else {
+ ds_put_cstr(actions,
+ "get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;");
+ }
It looks like the logic of the if-else statement above is reversed.
Shouldn't the is_v6 case use get_nd() instead of get_arp()?
+ ovn_lflow_add_with_hint(lflows, od,
+ S_ROUTER_IN_ARP_RESOLVE, 90,
+ ds_cstr(match), ds_cstr(actions),
+ &nat->header_);
+ }
sset_add(&nat_entries, nat->external_ip);
}
}
diff --git a/northd/northd.h b/northd/northd.h
index aa9a3ae6e..60601803f 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -229,6 +229,8 @@ struct ovn_datapath {
size_t n_nat_entries;
bool has_distributed_nat;
+ /* router datapath has a logical port with redirect-type set to bridged. */
+ bool redirect_bridged;
/* Set of nat external ips on the router. */
struct sset external_ips;
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index dae961c87..1d5a46a0d 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -4053,6 +4053,33 @@ outport = <var>P</var>
</p>
</li>
+ <li>
+ <p>
+ If the router datapath runs a port with <code>redirect-type</code>
+ set to <code>bridged</code>, for each distributed NAT rule with IP
+ <var>A</var> in the
+ <ref column="logical_ip" table="NAT" db="OVN_Northbound"/> column
+ and logical port <var>P</var> in the
+ <ref column="logical_port" table="NAT" db="OVN_Northbound"/> column
+ of <ref table="NAT" db="OVN_Northbound"/> table, a priority-90 flow
+ with the match <code>outport === <var>Q</var> && ip.src ===
Shouldn't "===" be "=="?
+ <var>A</var> && is_chassis_resident(<var>P</var>)</code>,
+ where <code>Q</code> is the distributed logical router port and
+ action <code>get_arp(outport, reg0); next;</code> for IPv4 and
+ <code>get_nd(outport, xxreg0); next;</code> for IPv6.
+ </p>
+
+ <p>
+ A priority-0 logical flow with match <code>ip4</code> has actions
+ <code>get_arp(outport, reg0); next;</code>.
+ </p>
+
+ <p>
+ A priority-0 logical flow with match <code>ip6</code> has actions
+ <code>get_nd(outport, xxreg0); next;</code>.
+ </p>
I think you can remove the mention of the priority-0 flows. The patch
isn't adding new priority-0 flows relating to the bridged redirect-type,
and the pre-existing priority-0 flows are already documented in the
"Dynamic MAC bindings" section.
+ </li>
+
<li>
<p>
Traffic with IP destination an address owned by the router should be
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
