Skip to content

Commit

Permalink
northd: Avoid matching on ct.dnat flags for load balancers.
Browse files Browse the repository at this point in the history 
Matching on ct.dnat creates openflows that often are not offloadable
to hardware.  ovn-northd uses ct.dnat only for load balancer hairpin
traffic handling and it turns out we don't really need to match on
ct.dnat.

Acked-by: Numan Siddique <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
Signed-off-by: Numan Siddique <[email protected]>
  • Loading branch information
@dceara @numansiddique
dceara authored and numansiddique committed Feb 24, 2021
1 parent e1f896f commit 110e670
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 35 deletions.
32 changes: 17 additions & 15 deletions northd/ovn-northd.8.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -813,19 +813,12 @@
<li>
If the logical switch has load balancer(s) configured, then a
priority-100 flow is added with the match
<code>ip &amp;&amp; ct.trk&amp;&amp; ct.dnat</code> to check if the
<code>ip &amp;&amp; ct.trk</code> to check if the
packet needs to be hairpinned (if after load balancing the destination
IP matches the source IP) or not by executing the action
<code>reg0[6] = chk_lb_hairpin();</code> and advances the packet to
the next table.
</li>

<li>
If the logical switch has load balancer(s) configured, then a
priority-90 flow is added with the match <code>ip</code> to check if
the packet is a reply for a hairpinned connection or not by executing
the action <code>reg0[6] = chk_lb_hairpin_reply();</code> and advances
the packet to the next table.
IP matches the source IP) or not by executing the actions
<code>reg0[6] = chk_lb_hairpin();</code> and
<code>reg0[12] = chk_lb_hairpin_reply();</code> and advances the packet
to the next table.
</li>

<li>
Expand All @@ -838,16 +831,25 @@
<li>
If the logical switch has load balancer(s) configured, then a
priority-100 flow is added with the match
<code>ip &amp;&amp; (ct.new || ct.est) &amp;&amp; ct.trk &amp;&amp;
ct.dnat &amp;&amp; reg0[6] == 1</code> which hairpins the traffic by
<code>ip &amp;&amp; ct.new &amp;&amp; ct.trk &amp;&amp;
reg0[6] == 1</code> which hairpins the traffic by
NATting source IP to the load balancer VIP by executing the action
<code>ct_snat_to_vip</code> and advances the packet to the next table.
</li>

<li>
If the logical switch has load balancer(s) configured, then a
priority-100 flow is added with the match
<code>ip &amp;&amp; ct.est &amp;&amp; ct.trk &amp;&amp;
reg0[6] == 1</code> which hairpins the traffic by
NATting source IP to the load balancer VIP by executing the action
<code>ct_snat</code> and advances the packet to the next table.
</li>

<li>
If the logical switch has load balancer(s) configured, then a
priority-90 flow is added with the match
<code>ip &amp;&amp; reg0[6] == 1</code> which matches on the replies
<code>ip &amp;&amp; reg0[12] == 1</code> which matches on the replies
of hairpinned traffic (i.e., destination IP is VIP,
source IP is the backend IP and source L4 port is backend port for L4
load balancers) and executes <code>ct_snat</code> and advances the
Expand Down
52 changes: 32 additions & 20 deletions northd/ovn-northd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ enum ovn_stage {
#define REGBIT_ACL_HINT_DROP "reg0[9]"
#define REGBIT_ACL_HINT_BLOCK "reg0[10]"
#define REGBIT_LKUP_FDB "reg0[11]"
#define REGBIT_HAIRPIN_REPLY "reg0[12]"

#define REG_ORIG_DIP_IPV4 "reg1"
#define REG_ORIG_DIP_IPV6 "xxreg1"
Expand Down Expand Up @@ -266,7 +267,8 @@ enum ovn_stage {
*
* Logical Switch pipeline:
* +----+----------------------------------------------+---+------------------+
* | R0 | REGBIT_{CONNTRACK/DHCP/DNS/HAIRPIN} | | |
* | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | | |
* | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | X | |
* | | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | X | |
* +----+----------------------------------------------+ X | |
* | R1 | ORIG_DIP_IPV4 (>= IN_STATEFUL) | R | |
Expand Down Expand Up @@ -6036,39 +6038,49 @@ build_lb_hairpin(struct ovn_datapath *od, struct hmap *lflows)
ovn_lflow_add(lflows, od, S_SWITCH_IN_HAIRPIN, 0, "1", "next;");

if (od->has_lb_vip) {
/* Check if the packet needs to be hairpinned. */
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 100,
"ip && ct.trk && ct.dnat",
REGBIT_HAIRPIN " = chk_lb_hairpin(); next;",
/* Check if the packet needs to be hairpinned.
* Set REGBIT_HAIRPIN in the original direction and
* REGBIT_HAIRPIN_REPLY in the reply direction.
*/
ovn_lflow_add_with_hint(
lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 100, "ip && ct.trk",
REGBIT_HAIRPIN " = chk_lb_hairpin(); "
REGBIT_HAIRPIN_REPLY " = chk_lb_hairpin_reply(); "
"next;",
&od->nbs->header_);

/* If packet needs to be hairpinned, snat the src ip with the VIP
* for new sessions. */
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_NAT_HAIRPIN, 100,
"ip && ct.new && ct.trk"
" && "REGBIT_HAIRPIN " == 1",
"ct_snat_to_vip; next;",
&od->nbs->header_);

/* Check if the packet is a reply of hairpinned traffic. */
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 90, "ip",
REGBIT_HAIRPIN " = chk_lb_hairpin_reply(); "
"next;", &od->nbs->header_);

/* If packet needs to be hairpinned, snat the src ip with the VIP. */
/* If packet needs to be hairpinned, for established sessions there
* should already be an SNAT conntrack entry.
*/
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_NAT_HAIRPIN, 100,
"ip && (ct.new || ct.est) && ct.trk && ct.dnat"
"ip && ct.est && ct.trk"
" && "REGBIT_HAIRPIN " == 1",
"ct_snat_to_vip; next;",
"ct_snat;",
&od->nbs->header_);

/* For the reply of hairpinned traffic, snat the src ip to the VIP. */
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_NAT_HAIRPIN, 90,
"ip && "REGBIT_HAIRPIN " == 1", "ct_snat;",
"ip && "REGBIT_HAIRPIN_REPLY " == 1",
"ct_snat;",
&od->nbs->header_);

/* Ingress Hairpin table.
* - Priority 1: Packets that were SNAT-ed for hairpinning should be
* looped back (i.e., swap ETH addresses and send back on inport).
*/
ovn_lflow_add(lflows, od, S_SWITCH_IN_HAIRPIN, 1,
REGBIT_HAIRPIN " == 1",
"eth.dst <-> eth.src;"
"outport = inport;"
"flags.loopback = 1;"
"output;");
ovn_lflow_add(
lflows, od, S_SWITCH_IN_HAIRPIN, 1,
"("REGBIT_HAIRPIN " == 1 || " REGBIT_HAIRPIN_REPLY " == 1)",
"eth.dst <-> eth.src; outport = inport; flags.loopback = 1; "
"output;");
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/ovn-northd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,35 @@ check_column "$lb0_uuid" sb:datapath_binding load_balancers external_ids:name=sw

AT_CLEANUP

AT_SETUP([ovn -- LS load balancer hairpin logical flows])
ovn_start

check ovn-nbctl \
-- ls-add sw0 \
-- lb-add lb0 10.0.0.10:80 10.0.0.4:8080 \
-- ls-lb-add sw0 lb0

check ovn-nbctl --wait=sb sync

AT_CHECK([ovn-sbctl lflow-list sw0 | grep ls_in_pre_hairpin | sort], [0], [dnl
table=14(ls_in_pre_hairpin ), priority=0 , match=(1), action=(next;)
table=14(ls_in_pre_hairpin ), priority=100 , match=(ip && ct.trk), action=(reg0[[6]] = chk_lb_hairpin(); reg0[[12]] = chk_lb_hairpin_reply(); next;)
])

AT_CHECK([ovn-sbctl lflow-list sw0 | grep ls_in_nat_hairpin | sort], [0], [dnl
table=15(ls_in_nat_hairpin ), priority=0 , match=(1), action=(next;)
table=15(ls_in_nat_hairpin ), priority=100 , match=(ip && ct.est && ct.trk && reg0[[6]] == 1), action=(ct_snat;)
table=15(ls_in_nat_hairpin ), priority=100 , match=(ip && ct.new && ct.trk && reg0[[6]] == 1), action=(ct_snat_to_vip; next;)
table=15(ls_in_nat_hairpin ), priority=90 , match=(ip && reg0[[12]] == 1), action=(ct_snat;)
])

AT_CHECK([ovn-sbctl lflow-list sw0 | grep ls_in_hairpin | sort], [0], [dnl
table=16(ls_in_hairpin ), priority=0 , match=(1), action=(next;)
table=16(ls_in_hairpin ), priority=1 , match=((reg0[[6]] == 1 || reg0[[12]] == 1)), action=(eth.dst <-> eth.src; outport = inport; flags.loopback = 1; output;)
])

AT_CLEANUP

AT_SETUP([ovn -- logical gatapath groups])
AT_KEYWORDS([use_logical_dp_groups])
ovn_start
Expand Down

0 comments on commit 110e670

Please sign in to comment.