On 11/25/22 12:09, Dumitru Ceara wrote: > On 11/22/22 10:29, Ales Musil wrote: >> In order to allow related traffic use the >> new action ct_commit_nat, which ensures that >> the traffic is commited and NATted. In combination >> with match on ct.rel it allows the related traffic >> to go through with correct NAT being applied. >> >> Reported-at: https://bugzilla.redhat.com/2126083 >> Signed-off-by: Ales Musil <[email protected]> >> --- >> v2: Add e2e test case. >> v3: Rebase on current main. >> Address comments from Mark. >> --- >> northd/northd.c | 29 ++++-- >> northd/ovn-northd.8.xml | 29 ++++-- >> tests/ovn-northd.at | 210 +++++++++++++++++++++------------------- >> tests/ovn.at | 10 +- >> tests/system-ovn.at | 135 ++++++++++++++++++++++++++ >> 5 files changed, 292 insertions(+), 121 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index 00ff8f933..9adfd4abb 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -6707,7 +6707,8 @@ build_acls(struct ovn_datapath *od, const struct >> chassis_features *features, >> /* Ingress and Egress ACL Table (Priority 65535). >> * >> * Allow traffic that is related to an existing conntrack entry that >> - * has not been marked for deletion (ct_mark.blocked). >> + * has not been marked for deletion (ct_mark.blocked). At the same >> + * time apply NAT on this traffic. >> * >> * This is enforced at a higher priority than ACLs can be defined. >> * >> @@ -6720,9 +6721,9 @@ build_acls(struct ovn_datapath *od, const struct >> chassis_features *features, >> use_ct_inv_match ? " && !ct.inv" : "", >> ct_blocked_match); >> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3, >> - ds_cstr(&match), "next;"); >> + ds_cstr(&match), "ct_commit_nat;"); >> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, >> - ds_cstr(&match), "next;"); >> + ds_cstr(&match), "ct_commit_nat;"); >> >> /* Ingress and Egress ACL Table (Priority 65532). >> * >> @@ -10249,16 +10250,16 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip >> *lb_vip, >> int prio = 110; >> if (lb_vip->vip_port) { >> prio = 120; >> - new_match = xasprintf("ct.new && %s && %s && " >> + new_match = xasprintf("ct.new && !ct.rel && %s && %s && " >> REG_ORIG_TP_DPORT_ROUTER" == %d", >> ds_cstr(match), lb->proto, lb_vip->vip_port); >> - est_match = xasprintf("ct.est && %s && %s && " >> + est_match = xasprintf("ct.est && !ct.rel && %s && %s && " >> REG_ORIG_TP_DPORT_ROUTER" == %d && %s == 1", >> ds_cstr(match), lb->proto, lb_vip->vip_port, >> ct_natted); > > Can't we just skip the !ct.rel match? I think these flows will always > have lower priority than the ones matching on ct.rel. >
Actually, these flows have priority 110 (or 120) while the ones that match on ct.rel have priority 50. We need the explicit match. It should be OK as is. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
