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 <amu...@redhat.com>
> ---
> 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.
> } else {
> - new_match = xasprintf("ct.new && %s", ds_cstr(match));
> - est_match = xasprintf("ct.est && %s && %s == 1",
> + new_match = xasprintf("ct.new && !ct.rel && %s", ds_cstr(match));
> + est_match = xasprintf("ct.est && !ct.rel && %s && %s == 1",
> ds_cstr(match), ct_natted);
> }
>
Thanks,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
