On 1/20/25 2:43 PM, Lorenzo Bianconi wrote:
> Trigger a full recompute in lb_data_load_balancer_handler() if
> neigh_mode option is updated. since it can't be managed
> incrementally for the moment.
> 
> Reported-at: https://issues.redhat.com/browse/FDP-1054
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> Changes in v2:
> - add ovn-northd self-test
> ---

Hi Lorenzo,

I only have a minor comment in the system test.  I went ahead and fixed
it up and pushed the patch to main, 24.09 and 24.03.

Regards,
Dumitru

>  northd/en-lb-data.c |  5 ++++
>  tests/ovn-northd.at | 27 ++++++++++++++++++
>  tests/system-ovn.at | 68 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 100 insertions(+)
> 
> diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
> index 3f5785c95..0e810eaab 100644
> --- a/northd/en-lb-data.c
> +++ b/northd/en-lb-data.c
> @@ -181,6 +181,7 @@ lb_data_load_balancer_handler(struct engine_node *node, 
> void *data)
>              struct sset old_ips_v6 = SSET_INITIALIZER(&old_ips_v6);
>              sset_swap(&lb->ips_v4, &old_ips_v4);
>              sset_swap(&lb->ips_v6, &old_ips_v6);
> +            enum lb_neighbor_responder_mode neigh_mode = lb->neigh_mode;
>              bool routable = lb->routable;
>              ovn_northd_lb_reinit(lb, tracked_lb);
>              health_checks |= lb->health_checks;
> @@ -215,6 +216,10 @@ lb_data_load_balancer_handler(struct engine_node *node, 
> void *data)
>                   */
>                  return false;
>              }
> +            if (neigh_mode != lb->neigh_mode) {
> +                /* If neigh_mode is updated trigger a full recompute. */
> +                return false;
> +            }
>          }
>      }
>  
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index edfd5764b..f32ae9b23 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -11232,8 +11232,35 @@ check_engine_stats lr_stateful norecompute compute
>  check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set load_balancer lb1 
> options:neighbor_responder=all
> +check_engine_stats lb_data recompute nocompute
> +check_engine_stats northd recompute nocompute
> +check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats lflow recompute nocompute
> +check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set load_balancer lb1 
> options:neighbor_responder=reachable
> +check_engine_stats lb_data recompute nocompute
> +check_engine_stats northd recompute nocompute
> +check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats lflow recompute nocompute
> +check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set load_balancer lb1 
> options:neighbor_responder=none
> +check_engine_stats lb_data recompute nocompute
> +check_engine_stats northd recompute nocompute
> +check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats lflow recompute nocompute
> +check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb -- lb-add lb2 20.0.0.10:80 20.0.0.20:80 -- lb-add 
> lb3 30.0.0.10:80 30.0.0.20:80
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 2e59f425b..ccc228254 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -14859,6 +14859,74 @@ OVS_APP_EXIT_AND_WAIT([ovn-northd])
>  as
>  OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>  /connection dropped.*/d"])
> +AT_CLEANUP
> +])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([neighbor_responder for all])
> +AT_KEYWORDS([neighbor_responder])
> +AT_SKIP_IF([test "$HAVE_ARPING" = no])
> +
> +CHECK_CONNTRACK()
> +CHECK_CONNTRACK_NAT()
> +
> +ovn_start
> +OVS_TRAFFIC_VSWITCHD_START()
> +ADD_BR([br-int])
>  
> +# Set external-ids in br-int needed for ovn-controller
> +ovs-vsctl \

Nit: missing check.

> +        -- set Open_vSwitch . external-ids:system-id=hv1 \
> +        -- set Open_vSwitch . 
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> +        -- set bridge br-int fail-mode=secure 
> other-config:disable-in-band=true
> +
> +# Start ovn-controller
> +start_daemon ovn-controller
> +
> +check_uuid ovn-nbctl create Logical_Router name=lr options:chassis=hv1
> +check ovn-nbctl ls-add ls
> +
> +# Connect alice to R2
> +check ovn-nbctl lrp-add lr lr-ls 00:00:02:01:02:03 192.168.1.254/24
> +check ovn-nbctl lsp-add ls ls-lr -- set Logical_Switch_Port ls-lr \
> +    type=router options:router-port=lr-ls addresses=\"00:00:02:01:02:03\"
> +
> +# Logical port 'foo' in switch 'foo'.
> +ADD_NAMESPACES(foo)
> +ADD_VETH(foo, foo, br-int, "192.168.1.1/24", "f0:00:00:01:02:03", \
> +         "192.168.1.254")
> +check ovn-nbctl lsp-add ls foo \
> +-- lsp-set-addresses foo "f0:00:00:01:02:03 192.168.1.1"
> +
> +check ovn-nbctl lb-add lb0 192.168.1.100 192.168.1.1
> +check ovn-nbctl lr-lb-add lr lb0
> +check ovn-nbctl lb-add lb1 172.16.1.100 192.168.1.1
> +check ovn-nbctl lr-lb-add lr lb1
> +
> +NS_CHECK_EXEC([foo], [ip route add 172.16.1.100 dev foo])
> +
> +check ovn-nbctl set load_balancer lb0 options:neighbor_responder=all
> +check ovn-nbctl set load_balancer lb1 options:neighbor_responder=all
> +check ovn-nbctl --wait=hv sync
> +
> +NS_CHECK_EXEC([foo], [arping -q -c 3 192.168.1.100])
> +NS_CHECK_EXEC([foo], [arping -q -c 3 172.16.1.100])
> +
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
>  AT_CLEANUP
>  ])

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to