Hi Lorenzo,
Please add the following to the commit message:
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2066990
I have a few more things below.
On 3/29/22 09:39, Lorenzo Bianconi wrote:
When a nat rule is configured in stateless mode there is a 1:1 mapping
between external_ip and logical_ip. Do not modify dst/src ips in
S_ROUTER_IN_UNSNAT/S_ROUTER_OUT_UNDNAT stages for stateless nat entries
since they will be properly modified in S_ROUTER_IN_DNAT/S_ROUTER_OUT_SNAT
stages.
This patch will allow respecting allowed_ext_ips for stateless nat
rules.
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
northd/northd.c | 41 +++++++++++++++++++++++++++++++++++------
tests/ovn-northd.at | 4 ++--
tests/ovn.at | 4 ++--
3 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index a2cf8d6fc..ba0e1f9d7 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11890,6 +11890,38 @@ build_gateway_redirect_flows_for_lrouter(
ds_cstr(match), ds_cstr(actions),
stage_hint);
}
+
+ for (int i = 0; i < od->nbr->n_nat; i++) {
+ const struct nbrec_nat *nat = nat = od->nbr->nat[i];
There appears to be an extra assignment above.
+ if (!lrouter_nat_is_stateless(nat) ||
+ strcmp(nat->type, "dnat_and_snat")) {
+ continue;
+ }
+
+ bool is_v6 = false;
+ ovs_be32 ip, mask;
+ char *error = ip_parse_masked(nat->external_ip, &ip, &mask);
+ if (error || mask != OVS_BE32_MAX) {
+ free(error);
+ struct in6_addr ipv6, mask_v6, v6_exact = IN6ADDR_EXACT_INIT;
+ error = ipv6_parse_masked(nat->external_ip, &ipv6, &mask_v6);
+ if (error || memcmp(&mask_v6, &v6_exact, sizeof(mask_v6))) {
+ /* Invalid for both IPv4 and IPv6 */
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "bad external ip %s for nat",
+ nat->external_ip);
+ free(error);
+ continue;
+ }
+ is_v6 = true;
+ }
You can avoid parsing IP addresses here by using the nat_entries on the
ovn_datapath. Example:
const struct ovn_nat *nat = od->nat_entries[i];
if (!lrouter_nat_is_stateless(nat->nb) ||
strcmp(nat->nb->type, "dnat_and_snat")) {
continue;
}
bool is_v6 = nat_entry_is_v6(nat);
+ ds_clear(match);
+ ds_put_format(match, "ip && ip%s.dst == %s", is_v6 ? "6" : "4",
+ nat->external_ip);
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 100,
+ ds_cstr(match), "drop;");
+ }
+
/* Packets are allowed by default. */
ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 0, "1", "next;");
}
@@ -12638,8 +12670,7 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows,
struct ovn_datapath *od,
ds_put_format(match, "ip && ip%s.dst == %s",
is_v6 ? "6" : "4", nat->external_ip);
if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
- ds_put_format(actions, "ip%s.dst=%s; next;",
- is_v6 ? "6" : "4", nat->logical_ip);
+ ds_put_format(actions, "next;");
} else {
ds_put_cstr(actions, "ct_snat;");
}
@@ -12664,8 +12695,7 @@ build_lrouter_in_unsnat_flow(struct hmap *lflows,
struct ovn_datapath *od,
}
if (!strcmp(nat->type, "dnat_and_snat") && stateless) {
- ds_put_format(actions, "ip%s.dst=%s; next;",
- is_v6 ? "6" : "4", nat->logical_ip);
+ ds_put_format(actions, "next;");
} else {
ds_put_cstr(actions, "ct_snat_in_czone;");
}
@@ -12818,8 +12848,7 @@ build_lrouter_out_undnat_flow(struct hmap *lflows,
struct ovn_datapath *od,
if (!strcmp(nat->type, "dnat_and_snat") &&
lrouter_nat_is_stateless(nat)) {
- ds_put_format(actions, "ip%s.src=%s; next;",
- is_v6 ? "6" : "4", nat->external_ip);
+ ds_put_format(actions, "next;");
} else {
ds_put_format(actions,
od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;");
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 17d4f31b3..e5e93b870 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -895,7 +895,7 @@ ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1
echo
echo "IPv4: stateless"
ovn-nbctl --wait=sb --stateless lr-nat-add R1 dnat_and_snat 172.16.1.1
50.0.0.11
-check_flow_match_sets 2 0 0 2 2 0 0
+check_flow_match_sets 2 0 0 1 1 0 0
ovn-nbctl lr-nat-del R1 dnat_and_snat 172.16.1.1
echo
@@ -907,7 +907,7 @@ ovn-nbctl lr-nat-del R1 dnat_and_snat fd01::1
echo
echo "IPv6: stateless"
ovn-nbctl --wait=sb --stateless lr-nat-add R1 dnat_and_snat fd01::1 fd11::2
-check_flow_match_sets 2 0 0 0 0 2 2
+check_flow_match_sets 2 0 0 0 0 1 1
AT_CLEANUP
])
diff --git a/tests/ovn.at b/tests/ovn.at
index 166b5f72e..925949075 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -21708,8 +21708,8 @@ AT_CHECK([for regex in ct_snat ct_dnat ip4.dst=
ip4.src=; do
grep -c "$regex" sbflows;
done], [0], [0
0
-2
-2
+1
+1
])
echo "----------- Post Traffic hv1 dump -----------"
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
