[ovs-dev] [PATCH ovn 20.09] northd: Fix lb_action when there are no active backends for lb health_check.

[Numan Siddique]

On Thu, Nov 12, 2020 at 4:15 PM Lorenzo Bianconi
<lorenzo.bianconi at redhat.com> wrote:
>
> Fix the following warning reported by ovn-controller when there are no
> active backends for lb health_check and selection_fields has been
> configured for hash computation.
>
> flow|WARN|error parsing actions "drop; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");":
> Syntax error at `hash_fields' expecting end of input.
>
> Tested-by: Flavio Fernandes <flavio at flaviof.com>
> Fixes: 5af304e747 ("Support selection fields in load balancer.")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Signed-off-by: Numan Siddique <numans at ovn.org>

Thanks. I applied to branch-20.09.

Numan

> ---
>  northd/ovn-northd.c | 5 ++++-
>  tests/ovn.at        | 6 ++++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index ec99fd3bd..c97cbcf73 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3613,6 +3613,8 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip,
>                                         struct ds *action,
>                                         char *selection_fields)
>  {
> +    bool skip_hash_fields = false;
> +
>      if (lb_vip->health_check) {
>          ds_put_cstr(action, "ct_lb(backends=");
>
> @@ -3631,6 +3633,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip,
>          }
>
>          if (!n_active_backends) {
> +            skip_hash_fields = true;
>              ds_clear(action);
>              ds_put_cstr(action, "drop;");
>          } else {
> @@ -3641,7 +3644,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip,
>          ds_put_format(action, "ct_lb(backends=%s);", lb_vip->backend_ips);
>      }
>
> -    if (selection_fields && selection_fields[0]) {
> +    if (!skip_hash_fields && selection_fields && selection_fields[0]) {
>          ds_chomp(action, ';');
>          ds_chomp(action, ')');
>          ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 596239133..92841b902 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -19223,6 +19223,8 @@ ovn-nbctl lsp-set-addresses sw1-lr0 router
>  ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
>
>  ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80
> +OVN_LB_ID=$(ovn-nbctl --bare --column _uuid find load_balancer name=lb1)
> +ovn-nbctl set load_balancer ${OVN_LB_ID} selection_fields="ip_dst,ip_src,tp_dst,tp_src"
>
>  ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2
>  ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:20.0.0.3=sw1-p1:20.0.0.2
> @@ -19259,12 +19261,12 @@ service_monitor | sed '/^$/d' | wc -l`])
>
>  ovn-sbctl dump-flows sw0 | grep ct_lb | grep priority=120 > lflows.txt
>  AT_CHECK([cat lflows.txt], [0], [dnl
> -  table=11(ls_in_stateful     ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
> +  table=11(ls_in_stateful     ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
>  ])
>
>  ovn-sbctl dump-flows lr0 | grep ct_lb | grep priority=120 > lflows.txt
>  AT_CHECK([cat lflows.txt], [0], [dnl
> -  table=6 (lr_in_dnat         ), priority=120  , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80 && is_chassis_resident("cr-lr0-public")), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
> +  table=6 (lr_in_dnat         ), priority=120  , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80 && is_chassis_resident("cr-lr0-public")), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
>  ])
>
>  # get the svc monitor mac.
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>