[ovs-dev] [PATCH ovn] ovn-northd: Optimize flows for LB Hairpin traffic.
Numan Siddique
numans at ovn.org
Tue Apr 28 12:19:38 UTC 2020
On Tue, Apr 28, 2020 at 4:14 PM Dumitru Ceara <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>
> 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>
>
Thanks for the fix. I applied this patch to master and branch 20.03.
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.
Thanks
Numan
---
> 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
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list