[ovs-dev] [PATCH ovn] ovn-northd: Optimize flows for LB Hairpin traffic.
Dumitru Ceara
dceara at redhat.com
Tue Apr 28 12:40:07 UTC 2020
On 4/28/20 2:19 PM, Numan Siddique wrote:
>
>
> On Tue, Apr 28, 2020 at 4:14 PM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
>
> In order to detect that traffic was hairpinned due to logical switch
> load
> balancers we need to match on source and destination IPs of packets (and
> protocol ports potentially) in table ls_in_pre_hairpin.
>
> For this, until now, we created 2 logical flows for each backend of
> a load
> balancer VIP. However, in scenarios where large load balancers (i.e.,
> with large numbers of backends) are applied to multiple logical
> switches, this might generate logical flow count explosion.
>
> One optimization is to generate a single logical flow per VIP that
> combines all conditions generated for each backend. This reduces load on
> the SB DB because of lower number of logical flows and also reduces
> overall DB size because of less overhead due to other fields on the
> logical_flow records.
>
> Comparison of various performance aspects when running OVN with the NB
> database attached to the bug report on a deployment with all VIFs bound
> to a single node (62 load balancer VIPs with 513 load balancer
> backends, applied on 106 logical switches):
>
> Without this patch:
> - SB database size: 60MB
> - # of pre-hairpin logical flows: 109074
> - # of logical flows: 159414
> - ovn-controller max loop iteration time when processing SB DB: 8803ms
> - ovn-northd max loop iteration time: 3988ms
>
> With this patch:
> - SB database size: 29MB (~50% decrease)
> - # of pre-hairpin logical flows: 13250 (~88% decrease)
> - # of logical flows: 63590 (~60% decrease)
> - ovn-controller max loop iteration time when processing SB DB: 5585ms
> - ovn-northd max loop iteration time: 1594ms
>
> Reported-by: Aniket Bhat <anbhat at redhat.com <mailto:anbhat at redhat.com>>
> Reported-at: https://bugzilla.redhat.com/1827403
> Fixes: 1be8ac65bc60 ("ovn-northd: Support hairpinning for logical
> switch load balancing.")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>>
>
>
>
> Thanks for the fix. I applied this patch to master and branch 20.03.
>
Thanks Numan!
> With this patch we will see one logical flow with many ORs. I think to
> address the db explosion issue,
> this patch is fine.
> Although It would be good if we find another way to address the hair pin
> issue, like using learn action as suggested
> by Ilya.
Yes, I'll work on a follow up patch.
OVS "learn" action is an option but it also has some disadvantages: for
example flows generated by learn can't have "resubmit" as action so that
would require a separate OVS table for hairpin detection.
An alternative to "learn" would be to add an "is_hairpin(VIP[:vip-port],
B1[:BP1], ... BN[:BPN])" composite expression to logical flow
expressions. Similarly we'd need an "is_hairpin_reply(...)".
For a LB VIP of the form:
VIP = 42.42.42.42:4200
Backends: [80.80.80.80:8000, 80:80:80:81:8100]
ovn-northd would generate the following two logical flows:
LF1: table=pre-hairpin, priority=2, match="is_hairpin(42.42.42.42:4200,
80.80.80.80:8000, 80:80:80:81:8100)" action="reg0[6]=1;
ct_snat(42.42.42.42);"
LF2: table=pre-hairpin, priority=1,
match="is_hairpin_reply(42.42.42.42:4200, 80.80.80.80:8000,
80:80:80:81:8100)" action="reg0[6]=1; ct_snat;"
ovn-controller, in expr.c, will then expand is_hairpin() and
is_hairpin_reply() to the corresponding expressions, i.e.:
- is_hairpin: (ip.src == B1 && ip.dst == B1 && tcp.dport == BP1) || ...
|| (ip.src == BN && ip.dst == BN && tcp.dport == BPN)
- is_reply_hairpin: ip.dst == VIP && ((ip.src == B1 && tcp.sport == BP1)
|| ... || (ip.src == BN && tcp.sport == BPN))
This approach also lowers the number of logical flows and preserves the
current OVN approach to always have an explicit mapping between SB
entities and openflow entries. It would also follow the current OVN
architecture. Also, adding support for hairpin at router level would
just be a matter of adding logical flows that use the new composite
expressions in the router pipelines.
What do you think?
>
> Thanks
> Numan
Thanks,
Dumitru
>
> ---
> northd/ovn-northd.8.xml | 4 +--
> northd/ovn-northd.c | 79
> ++++++++++++++++++++++++++++++++-----------------
> 2 files changed, 54 insertions(+), 29 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index d39e259..1f81742 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -559,14 +559,14 @@
> <h3>Ingress Table 11: Pre-Hairpin</h3>
> <ul>
> <li>
> - For all configured load balancer backends a priority-2 flow
> that
> + For all configured load balancer VIPs a priority-2 flow that
> matches on traffic that needs to be hairpinned, i.e., after
> load
> balancing the destination IP matches the source IP, which sets
> <code>reg0[6] = 1 </code> and executes
> <code>ct_snat(VIP)</code>
> to force replies to these packets to come back through OVN.
> </li>
> <li>
> - For all configured load balancer backends a priority-1 flow
> that
> + For all configured load balancer VIPs a priority-1 flow that
> matches on replies to hairpinned traffic, i.e., destination
> IP is VIP,
> source IP is the backend IP and source L4 port is backend
> port, which
> sets <code>reg0[6] = 1 </code> and executes
> <code>ct_snat;</code>.
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 63753ac..742aad8 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5550,52 +5550,77 @@ build_lb_hairpin_rules(struct ovn_datapath
> *od, struct hmap *lflows,
> struct ovn_lb *lb, struct lb_vip *lb_vip,
> const char *ip_match, const char *proto)
> {
> + if (lb_vip->n_backends == 0) {
> + return;
> + }
> +
> + struct ds action = DS_EMPTY_INITIALIZER;
> + struct ds match_initiator = DS_EMPTY_INITIALIZER;
> + struct ds match_reply = DS_EMPTY_INITIALIZER;
> + struct ds proto_match = DS_EMPTY_INITIALIZER;
> +
> /* Ingress Pre-Hairpin table.
> - * - Priority 2: SNAT load balanced traffic that needs to be
> hairpinned.
> + * - Priority 2: SNAT load balanced traffic that needs to be
> hairpinned:
> + * - Both SRC and DST IP match backend->ip and destination port
> + * matches backend->port.
> * - Priority 1: unSNAT replies to hairpinned load balanced
> traffic.
> + * - SRC IP matches backend->ip, DST IP matches LB VIP and
> source port
> + * matches backend->port.
> */
> + ds_put_char(&match_reply, '(');
> for (size_t i = 0; i < lb_vip->n_backends; i++) {
> struct lb_vip_backend *backend = &lb_vip->backends[i];
> - struct ds action = DS_EMPTY_INITIALIZER;
> - struct ds match = DS_EMPTY_INITIALIZER;
> - struct ds proto_match = DS_EMPTY_INITIALIZER;
>
> /* Packets that after load balancing have equal source and
> - * destination IPs should be hairpinned. SNAT them so that
> the reply
> - * traffic is directed also through OVN.
> + * destination IPs should be hairpinned.
> */
> if (lb_vip->vip_port) {
> - ds_put_format(&proto_match, " && %s && %s.dst == %"PRIu16,
> - proto, proto, backend->port);
> + ds_put_format(&proto_match, " && %s.dst == %"PRIu16,
> + proto, backend->port);
> }
> - ds_put_format(&match, "%s.src == %s && %s.dst == %s%s",
> + ds_put_format(&match_initiator, "(%s.src == %s && %s.dst ==
> %s%s)",
> ip_match, backend->ip, ip_match, backend->ip,
> ds_cstr(&proto_match));
> - ds_put_format(&action, REGBIT_HAIRPIN " = 1; ct_snat(%s);",
> - lb_vip->vip);
> - ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 2,
> - ds_cstr(&match), ds_cstr(&action),
> - &lb->nlb->header_);
>
> - /* If the packets are replies for hairpinned traffic,
> UNSNAT them. */
> + /* Replies to hairpinned traffic are originated by
> backend->ip:port. */
> ds_clear(&proto_match);
> - ds_clear(&match);
> if (lb_vip->vip_port) {
> - ds_put_format(&proto_match, " && %s && %s.src == %"PRIu16,
> - proto, proto, backend->port);
> + ds_put_format(&proto_match, " && %s.src == %"PRIu16, proto,
> + backend->port);
> }
> - ds_put_format(&match, "%s.src == %s && %s.dst == %s%s",
> - ip_match, backend->ip, ip_match, lb_vip->vip,
> + ds_put_format(&match_reply, "(%s.src == %s%s)", ip_match,
> backend->ip,
> ds_cstr(&proto_match));
> - ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 1,
> - ds_cstr(&match),
> - REGBIT_HAIRPIN " = 1; ct_snat;",
> - &lb->nlb->header_);
> + ds_clear(&proto_match);
>
> - ds_destroy(&action);
> - ds_destroy(&match);
> - ds_destroy(&proto_match);
> + if (i < lb_vip->n_backends - 1) {
> + ds_put_cstr(&match_initiator, " || ");
> + ds_put_cstr(&match_reply, " || ");
> + }
> }
> + ds_put_char(&match_reply, ')');
> +
> + /* SNAT hairpinned initiator traffic so that the reply traffic is
> + * also directed through OVN.
> + */
> + ds_put_format(&action, REGBIT_HAIRPIN " = 1; ct_snat(%s);",
> + lb_vip->vip);
> + ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 2,
> + ds_cstr(&match_initiator),
> ds_cstr(&action),
> + &lb->nlb->header_);
> +
> + /* Replies to hairpinned traffic are destined to the LB VIP. */
> + ds_put_format(&match_reply, " && %s.dst == %s", ip_match,
> lb_vip->vip);
> +
> + /* UNSNAT replies for hairpinned traffic. */
> + ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_HAIRPIN, 1,
> + ds_cstr(&match_reply),
> + REGBIT_HAIRPIN " = 1; ct_snat;",
> + &lb->nlb->header_);
> +
> + ds_destroy(&action);
> + ds_destroy(&match_initiator);
> + ds_destroy(&match_reply);
> + ds_destroy(&proto_match);
> }
>
> static void
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org <mailto:dev at openvswitch.org>
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list