Skip to content

Commit

Permalink
northd: Allow related traffic through LB
Browse files Browse the repository at this point in the history
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 <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 650f06b)
  • Loading branch information
almusil authored and dceara committed Jan 23, 2023
1 parent 3ce06e6 commit 06f92e6
Show file tree
Hide file tree
Showing 5 changed files with 283 additions and 112 deletions.
29 changes: 22 additions & 7 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -6734,7 +6734,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.
*
Expand All @@ -6747,9 +6748,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).
*
Expand Down Expand Up @@ -9950,16 +9951,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);
} 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);
}

Expand Down Expand Up @@ -13449,6 +13450,20 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows,
ovn_lflow_add(lflows, od, S_ROUTER_OUT_EGR_LOOP, 0, "1", "next;");
ovn_lflow_add(lflows, od, S_ROUTER_IN_ECMP_STATEFUL, 0, "1", "next;");

/* Ingress DNAT Table (Priority 50).
*
* Allow traffic that is related to an existing conntrack entry.
* At the same time apply NAT for this traffic.
*
* NOTE: This does not support related data sessions (eg,
* a dynamically negotiated FTP data channel), but will allow
* related traffic such as an ICMP Port Unreachable through
* that's generated from a non-listening UDP port. */
if (od->has_lb_vip) {
ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50,
"ct.rel && !ct.est && !ct.new", "ct_commit_nat;");
}

/* If the router has load balancer or DNAT rules, re-circulate every packet
* through the DNAT zone so that packets that need to be unDNATed in the
* reverse direction get unDNATed.
Expand Down
29 changes: 20 additions & 9 deletions northd/ovn-northd.8.xml
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,8 @@
related to a committed flow in the connection tracker (e.g., an
ICMP Port Unreachable from a non-listening UDP port), as long
as the committed flow does not have <code>ct_mark.blocked</code> set.
This flow also applies NAT to the related traffic so that ICMP headers
and the inner packet have correct addresses.
If ACL logging and logging of related packets is enabled, then a
companion priority-65533 flow will be installed that accomplishes the
same thing but also logs the traffic.
Expand Down Expand Up @@ -3152,10 +3154,10 @@ icmp6 {
Router with gateway port in <code>OVN_Northbound</code> database that
includes a L4 port <var>PORT</var> of protocol <var>P</var> and IPv4
or IPv6 address <var>VIP</var>, a priority-120 flow that matches on
<code>ct.new &amp;&amp; ip &amp;&amp; reg0 == <var>VIP</var>
&amp;&amp; <var>P</var> &amp;&amp; reg9[16..31] == </code>
<code><var>PORT</var></code> (<code>xxreg0 == <var>VIP</var></code>
in the IPv6 case) with an action of
<code>ct.new &amp;&amp; !ct.rel &amp;&amp; ip &amp;&amp; reg0 ==
<var>VIP</var> &amp;&amp; <var>P</var> &amp;&amp; reg9[16..31] ==
</code> <code><var>PORT</var></code> (<code>xxreg0 == <var>VIP</var>
</code> in the IPv6 case) with an action of
<code>ct_lb_mark(<var>args</var>)</code>, where <var>args</var> contains
comma separated IPv4 or IPv6 addresses (and optional port numbers) to
load balance to. If the router is configured to force SNAT any
Expand Down Expand Up @@ -3184,9 +3186,9 @@ icmp6 {
<code>OVN_Northbound</code> database that includes a L4 port
<var>PORT</var> of protocol <var>P</var> and IPv4 or IPv6 address
<var>VIP</var>, a priority-120 flow that matches on
<code>ct.est &amp;&amp; ip4 &amp;&amp; reg0 == <var>VIP</var>
&amp;&amp; <var>P</var> &amp;&amp; reg9[16..31] == </code>
<code><var>PORT</var></code> (<code>ip6</code> and
<code>ct.est &amp;&amp; !ct.rel &amp;&amp; ip4 &amp;&amp; reg0 ==
<var>VIP</var> &amp;&amp; <var>P</var> &amp;&amp; reg9[16..31] ==
</code> <code><var>PORT</var></code> (<code>ip6</code> and
<code>xxreg0 == <var>VIP</var></code> in the IPv6 case) with an
action of <code>next;</code>. If the router is configured to force
SNAT any load-balanced packets, the above action will be replaced by
Expand All @@ -3209,7 +3211,7 @@ icmp6 {
For all the configured load balancing rules for a router in
<code>OVN_Northbound</code> database that includes just an IP address
<var>VIP</var> to match on, a priority-110 flow that matches on
<code>ct.new &amp;&amp; ip4 &amp;&amp; reg0 ==
<code>ct.new &amp;&amp; !ct.rel &amp;&amp; ip4 &amp;&amp; reg0 ==
<var>VIP</var></code> (<code>ip6</code> and <code>xxreg0 ==
<var>VIP</var></code> in the IPv6 case) with an action of
<code>ct_lb_mark(<var>args</var>)</code>, where <var>args</var> contains
Expand All @@ -3236,7 +3238,7 @@ icmp6 {
For all the configured load balancing rules for a router in
<code>OVN_Northbound</code> database that includes just an IP address
<var>VIP</var> to match on, a priority-110 flow that matches on
<code>ct.est &amp;&amp; ip4 &amp;&amp; reg0 ==
<code>ct.est &amp;&amp; !ct.rel &amp;&amp; ip4 &amp;&amp; reg0 ==
<var>VIP</var></code> (or <code>ip6</code> and
<code>xxreg0 == <var>VIP</var></code>) with an action of
<code>next;</code>. If the router is configured to force SNAT any
Expand All @@ -3263,6 +3265,15 @@ icmp6 {
Please note using <code>--reject</code> option will disable
empty_lb SB controller event for this load balancer.
</li>

<li>
<p>
For the related traffic, a priority 50 flow that matches
<code>ct.rel &amp;&amp; !ct.est &amp;&amp; !ct.new </code>
with an action of <code>ct_commit_nat;</code>, if the router
has load balancer assigned to it.
</p>
</li>
</ul>

<p>Ingress Table 6: DNAT on Gateway Routers</p>
Expand Down

0 comments on commit 06f92e6

Please sign in to comment.