The ARP request for given dnat_and_snat coming from external network
could be processed on multiple chassis because the flows would check
only the residency of the logical_port. This could lead to ToR
confusion as the reply would come from multiple chassis.
This also has another consequence, for gARP that would be sent as
request. OVN would attempt to learn the gARP on multiple chassis
which could lead to increase in SB race condition during MAC Binding
insert.
To prevent both condition restrict the check with logical_ip.
Now the reply to request will be generated only by the chassis
where the IP actually resides. Same goes for gARP, only the DGP
will attempt to learn the MAC Binding.
Fixes: 289b370dc621 ("northd: Fix for distributed dnat_and_snat ARP
resolution.")
Reported-at: https://redhat.atlassian.net/browse/FDP-3468
Signed-off-by: Ales Musil <[email protected]>
---
northd/northd.c | 41 ++++++++++++++++++-----------------------
tests/ovn-northd.at | 19 ++++++++++++++-----
2 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 685f6900c..e19eda4ab 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10112,8 +10112,6 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
struct lflow_table *lflows,
const struct ls_arp_record *ar)
{
- struct sset distributed_nat_ports =
- SSET_INITIALIZER(&distributed_nat_ports);
struct hmapx resident_ports = HMAPX_INITIALIZER(&resident_ports);
struct ds match = DS_EMPTY_INITIALIZER;
@@ -10128,21 +10126,8 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
}
}
- struct hmapx_node *hmapx_node;
- HMAPX_FOR_EACH (hmapx_node, &ar->nat_records) {
- struct lr_nat_record *nr = hmapx_node->data;
-
- for (size_t i = 0; i < nr->n_nat_entries; i++) {
- struct ovn_nat *ent = &nr->nat_entries[i];
- if (ent->is_valid && ent->is_distributed) {
- sset_add(&distributed_nat_ports, ent->nb->logical_port);
- }
- }
- }
-
if (!hmapx_is_empty(&od->phys_ports) && !hmapx_is_empty(&resident_ports)) {
struct hmapx_node *node;
- const char *port_name;
HMAPX_FOR_EACH (node, &od->phys_ports) {
op = node->data;
@@ -10168,20 +10153,30 @@ build_lswitch_arp_chassis_resident(const struct
ovn_datapath *od,
ds_cstr(&match), "next;", ar->lflow_ref);
}
- SSET_FOR_EACH (port_name, &distributed_nat_ports) {
- ds_clear(&match);
- ds_put_format(&match, REGBIT_EXT_ARP " == 1 "
- "&& is_chassis_resident(\"%s\")",
- port_name);
- ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 75,
- ds_cstr(&match), "next;", ar->lflow_ref);
+ struct hmapx_node *hmapx_node;
+ HMAPX_FOR_EACH (hmapx_node, &ar->nat_records) {
+ struct lr_nat_record *nr = hmapx_node->data;
+ for (size_t i = 0; i < nr->n_nat_entries; i++) {
+ struct ovn_nat *ent = &nr->nat_entries[i];
+ if (!ent->is_valid || !ent->is_distributed ||
+ nat_entry_is_v6(ent)) {
+ continue;
+ }
+
+ ds_clear(&match);
+ ds_put_format(&match, REGBIT_EXT_ARP " == 1 && arp.tpa == %s "
+ "&& is_chassis_resident(\"%s\")",
+ ent->ext_addrs.ipv4_addrs[0].addr_s,
+ ent->nb->logical_port);
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 85,
+ ds_cstr(&match), "next;", ar->lflow_ref);
+ }
}
ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 70,
REGBIT_EXT_ARP" == 1", "drop;", ar->lflow_ref);
}
- sset_destroy(&distributed_nat_ports);
hmapx_destroy(&resident_ports);
ds_destroy(&match);
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 60258a6f2..30ef96262 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -19417,7 +19417,7 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
ls_in_apply_port_sec | ovn_strip_lflow
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-down_link")), action=(next;)
- table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("down_vif1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 192.168.0.3 && is_chassis_resident("down_vif1")), action=(next;)
])
check ovn-nbctl --wait=sb lr-nat-del lr1 dnat_and_snat 192.168.0.3
@@ -19472,9 +19472,9 @@ check ovn-sbctl chassis-add ch1 geneve 127.0.0.1
check ovn-nbctl ls-add sw0
check ovn-nbctl lsp-add sw0 sw0-port1
-check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:03 10.0.0.3"
+check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:03 10.0.0.3
10.0.0.5"
check ovn-nbctl lsp-add sw0 sw0-port2
-check ovn-nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:04 10.0.0.4"
+check ovn-nbctl lsp-set-addresses sw0-port2 "50:54:00:00:00:04 10.0.0.4
10.0.0.6"
# Create a logical router and attach both logical switches
check ovn-nbctl lr-add lr0
@@ -19494,6 +19494,9 @@ check ovn-nbctl lrp-set-gateway-chassis lr0-public hv1
check ovn-nbctl lsp-add-localnet-port public ln-public public
check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.50 10.0.0.3 sw0-port1
f0:00:00:00:00:03
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.55 10.0.0.5 sw0-port1
f0:00:00:00:00:03
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.60 10.0.0.4 sw0-port2
f0:00:00:00:00:04
+check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.168.0.66 10.0.0.6 sw0-port2
f0:00:00:00:00:04
check ovn-nbctl --wait=sb sync
@@ -19502,7 +19505,10 @@ AT_CHECK([ovn-sbctl lflow-list public | grep
ls_in_apply_port_sec | ovn_strip_lf
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-lr0-public")), action=(next;)
- table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.50 && is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.55 && is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.60 && is_chassis_resident("sw0-port2")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.66 && is_chassis_resident("sw0-port2")), action=(next;)
])
ovn-nbctl show
@@ -19527,7 +19533,10 @@ AT_CHECK([ovn-sbctl lflow-list public | grep
ls_in_apply_port_sec | ovn_strip_lf
table=??(ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=70 , match=(reg0[[22]] == 1),
action=(drop;)
table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("cr-lr0-public")), action=(next;)
- table=??(ls_in_apply_port_sec), priority=75 , match=(reg0[[22]] == 1 &&
is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.50 && is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.55 && is_chassis_resident("sw0-port1")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.60 && is_chassis_resident("sw0-port2")), action=(next;)
+ table=??(ls_in_apply_port_sec), priority=85 , match=(reg0[[22]] == 1 &&
arp.tpa == 172.168.0.66 && is_chassis_resident("sw0-port2")), action=(next;)
])
OVN_CLEANUP_NORTHD
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
