In order for the change to work properly we also had to fallback to
recompute in all cases when load balancers with health check
configuration are added/updated/deleted (through load balancer groups
too). This was currently not happening but didn't really cause issues
before ceea1d7cccad ("northd: Allow LB health checks from router IPs.").
Fixes: ceea1d7cccad ("northd: Allow LB health checks from router IPs.")
Reported-at: https://redhat.atlassian.net/browse/FDP-3453
Signed-off-by: Dumitru Ceara <[email protected]>
---
V3:
- Addressed Ales' comment:
- Split out the pinctrl fix into its own patch.
V2:
- Addressed Mark's comment:
- renamed tests to show the difference between system/non-system tests
- added mac check to the system test
---
northd/en-lb-data.c | 20 +++++++----
northd/lb.c | 13 ++++++-
northd/lb.h | 8 +++--
northd/northd.c | 8 +++--
tests/ovn-northd.at | 78 ++++++++++++++++++++++++++++++++++++++++++
tests/system-ovn.at | 82 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 196 insertions(+), 13 deletions(-)
diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
index a64c06bfda..04ef20bcc6 100644
--- a/northd/en-lb-data.c
+++ b/northd/en-lb-data.c
@@ -268,6 +268,7 @@ lb_data_load_balancer_group_handler(struct engine_node
*node, void *data)
hmapx_add(&clbg->assoc_lbs, lb_group->lbs[i]);
}
+ trk_lb_data->has_health_checks |= lb_group->has_health_checks;
trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
continue;
}
@@ -283,6 +284,7 @@ lb_data_load_balancer_group_handler(struct engine_node
*node, void *data)
if (nbrec_load_balancer_group_is_deleted(tracked_lb_group)) {
hmap_remove(&lb_data->lbgrps, &lb_group->hmap_node);
add_deleted_lbgrp_to_tracked_data(lb_group, trk_lb_data);
+ trk_lb_data->has_health_checks |= lb_group->has_health_checks;
trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
} else {
/* Determine the lbs which are added or deleted for this
@@ -299,6 +301,7 @@ lb_data_load_balancer_group_handler(struct engine_node
*node, void *data)
build_lrouter_lb_ips(lb_group->lb_ips, lb_group->lbs[i]);
}
+ trk_lb_data->has_health_checks |= lb_group->has_health_checks;
trk_lb_data->has_routable_lb |= lb_group->has_routable_lb;
struct crupdated_lbgrp *clbg =
add_crupdated_lbgrp_to_tracked_data(lb_group,
trk_lb_data);
@@ -690,10 +693,12 @@ handle_od_lb_changes(struct nbrec_load_balancer
**nbrec_lbs,
/* Add this lb to the tracked data. */
uuidset_insert(&codlb->assoc_lbs, lb_uuid);
+ struct ovn_northd_lb *lb = ovn_northd_lb_find(&lb_data->lbs,
+ lb_uuid);
+ ovs_assert(lb);
+
+ trk_lb_data->has_health_checks |= lb->health_checks;
if (!trk_lb_data->has_routable_lb) {
- struct ovn_northd_lb *lb =
ovn_northd_lb_find(&lb_data->lbs,
- lb_uuid);
- ovs_assert(lb);
trk_lb_data->has_routable_lb |= lb->routable;
trk_lb_data->has_distributed_lb |= lb->is_distributed;
}
@@ -730,10 +735,12 @@ handle_od_lbgrp_changes(struct
nbrec_load_balancer_group **nbrec_lbgrps,
/* Add this lb group to the tracked data. */
uuidset_insert(&codlb->assoc_lbgrps, lbgrp_uuid);
+ struct ovn_lb_group *lbgrp =
+ ovn_lb_group_find(&lb_data->lbgrps, lbgrp_uuid);
+ ovs_assert(lbgrp);
+
+ trk_lb_data->has_health_checks |= lbgrp->has_health_checks;
if (!trk_lb_data->has_routable_lb) {
- struct ovn_lb_group *lbgrp =
- ovn_lb_group_find(&lb_data->lbgrps, lbgrp_uuid);
- ovs_assert(lbgrp);
trk_lb_data->has_routable_lb |= lbgrp->has_routable_lb;
}
}
@@ -755,6 +762,7 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
lb_data->tracked_lb_data.has_dissassoc_lbs_from_lbgrps = false;
lb_data->tracked_lb_data.has_dissassoc_lbs_from_od = false;
lb_data->tracked_lb_data.has_dissassoc_lbgrps_from_od = false;
+ lb_data->tracked_lb_data.has_health_checks = false;
lb_data->tracked_lb_data.has_routable_lb = false;
struct hmapx_node *node;
diff --git a/northd/lb.c b/northd/lb.c
index 83c1bf97e1..2f683cef1b 100644
--- a/northd/lb.c
+++ b/northd/lb.c
@@ -464,7 +464,8 @@ ovn_northd_lb_init(struct ovn_northd_lb *lb,
* considered. If no matching LRP is found, the 'svc_mon_lrp' is set to
* NULL. */
void
-ovn_northd_lb_backend_set_mon_port(const struct ovn_port *backend_op,
+ovn_northd_lb_backend_set_mon_port(const struct ovn_lb_datapaths *lb_dps,
+ const struct ovn_port *backend_op,
struct ovn_northd_lb_backend
*backend_nb)
{
if (backend_op && !backend_nb->remote_backend) {
@@ -473,6 +474,14 @@ ovn_northd_lb_backend_set_mon_port(const struct
ovn_port *backend_op,
if (!svc_mon_op->peer) {
continue;
}
+
+ /* Skip routers this load balancer is not applied onto. */
+ const struct ovn_datapath *rtr_od = svc_mon_op->peer->od;
+ if (!dynamic_bitmap_is_set(&lb_dps->nb_lr_map,
+ rtr_od->sdp->index)) {
+ continue;
+ }
+
const char *lrp_ip = lrp_find_member_ip(
svc_mon_op->peer,
backend_nb->svc_mon_src_ip);
@@ -564,6 +573,7 @@ ovn_lb_group_init(struct ovn_lb_group *lb_group,
const struct uuid *lb_uuid =
&nbrec_lb_group->load_balancer[i]->header_.uuid;
lb_group->lbs[i] = ovn_northd_lb_find(lbs, lb_uuid);
+ lb_group->has_health_checks |= lb_group->lbs[i]->health_checks;
lb_group->has_routable_lb |= lb_group->lbs[i]->routable;
}
}
@@ -586,6 +596,7 @@ ovn_lb_group_cleanup(struct ovn_lb_group *lb_group)
{
ovn_lb_ip_set_destroy(lb_group->lb_ips);
lb_group->lb_ips = NULL;
+ lb_group->has_health_checks = false;
lb_group->has_routable_lb = false;
free(lb_group->lbs);
}
diff --git a/northd/lb.h b/northd/lb.h
index c1f0c95da1..97d2c4c366 100644
--- a/northd/lb.h
+++ b/northd/lb.h
@@ -108,9 +108,6 @@ struct ovn_northd_lb_backend {
struct ovn_port *svc_mon_lrp;
};
-void ovn_northd_lb_backend_set_mon_port(const struct ovn_port *,
- struct ovn_northd_lb_backend *);
-
struct ovn_northd_lb *ovn_northd_lb_create(const struct
nbrec_load_balancer *);
struct ovn_northd_lb *ovn_northd_lb_find(const struct hmap *,
const struct uuid *);
@@ -136,6 +133,7 @@ struct ovn_lb_group {
size_t n_lbs;
struct ovn_northd_lb **lbs;
struct ovn_lb_ip_set *lb_ips;
+ bool has_health_checks;
bool has_routable_lb;
};
@@ -203,6 +201,10 @@ void ovn_lb_datapaths_add_ls(struct ovn_lb_datapaths
*, size_t n,
struct ovn_datapath **,
size_t n_ls_datapaths);
+void ovn_northd_lb_backend_set_mon_port(const struct ovn_lb_datapaths *,
+ const struct ovn_port *,
+ struct ovn_northd_lb_backend *);
+
struct ovn_lb_group_datapaths {
struct hmap_node hmap_node;
diff --git a/northd/northd.c b/northd/northd.c
index 734efea657..c607c4c08d 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3332,13 +3332,15 @@ ovn_nf_svc_create(struct ovsdb_idl_txn *ovnsb_txn,
static void
ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn,
- const struct ovn_northd_lb *lb,
+ const struct ovn_lb_datapaths *lb_dps,
const struct svc_monitor_addresses
*svc_global_addresses,
struct hmap *ls_ports,
struct sset *svc_monitor_lsps,
struct hmap *local_svc_monitors_map,
struct hmap *ic_learned_svc_monitors_map)
{
+ const struct ovn_northd_lb *lb = lb_dps->lb;
+
if (lb->template) {
return;
}
@@ -3366,7 +3368,7 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn,
continue;
}
- ovn_northd_lb_backend_set_mon_port(op, backend_nb);
+ ovn_northd_lb_backend_set_mon_port(lb_dps, op, backend_nb);
/* If the service monitor is backed by a real port, use its
MAC
* address instead of the default service check MAC. */
@@ -3787,7 +3789,7 @@ build_svc_monitors_data(
struct ovn_lb_datapaths *lb_dps;
HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) {
- ovn_lb_svc_create(ovnsb_txn, lb_dps->lb,
+ ovn_lb_svc_create(ovnsb_txn, lb_dps,
svc_global_addresses,
ls_ports,
svc_monitor_lsps,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 60258a6f25..01ae91a963 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -20170,3 +20170,81 @@ AT_CHECK([ovn-sbctl get Port_Binding $lsp_pb_uuid
external_ids:name],
OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Load balancer health checks - service monitor source MAC - SB
syncing])
+ovn_start
+
+global_svc_mon_mac="11:11:11:11:11:11"
+router_port_mac="00:00:00:00:00:01"
+check ovn-nbctl set NB_Global .
options:svc_monitor_mac="$global_svc_mon_mac"
+
+check ovn-nbctl \
+ -- ls-add ls \
+ -- lsp-add ls lsp1 \
+ -- lsp-add ls lsp2 \
+ -- lr-add lr \
+ -- lrp-add lr lr-ls $router_port_mac 192.168.1.254/24 \
+ -- lsp-add-router-port ls ls-lr lr-ls
+
+check ovn-nbctl \
+ -- lb-add lb0 192.168.5.1:1 192.168.1.1:1,192.168.1.2:1 udp \
+ -- lb-add lb1 192.168.5.1:2 192.168.1.1:2,192.168.1.2:2 udp
+lb0=$(fetch_column nb:Load_Balancer _uuid name=lb0)
+lb1=$(fetch_column nb:Load_Balancer _uuid name=lb1)
+
+check ovn-nbctl
\
+ -- set load_balancer $lb0
ip_port_mappings:192.168.1.1=lsp1:192.168.1.254 \
+ -- set load_balancer $lb1
ip_port_mappings:192.168.1.1=lsp1:192.168.1.254
+
+check_uuid ovn-nbctl --id=@hc create Load_Balancer_Health_Check
vip="192.168.5.1\:1" \
+ -- add Load_Balancer $lb0 health_check @hc
+check_uuid ovn-nbctl --id=@hc create Load_Balancer_Health_Check
vip="192.168.5.1\:2" \
+ -- add Load_Balancer $lb1 health_check @hc
+
+dnl Add LB0 and LB1 to the switch (LB1 via load balancer group).
+check ovn-nbctl ls-lb-add ls lb0
+lbg=$(ovn-nbctl --wait=sb create load_balancer_group name=lbg)
+check ovn-nbctl add load_balancer_group lbg load_balancer $lb1
+check ovn-nbctl add logical_switch ls load_balancer_group $lbg
+
+check ovn-nbctl --wait=sb sync
+check_row_count sb:Service_Monitor 2
+
+AS_BOX([LB0 on switch, LB1 on switch])
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=1
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch + router, LB1 on switch])
+check ovn-nbctl --wait=sb lr-lb-add lr lb0
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=1
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch + router, LB1 on switch + router])
+check ovn-nbctl --wait=sb add logical_router lr load_balancer_group $lbg
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=1
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch + router, LB1 nowhere])
+check ovn-nbctl --wait=sb remove load_balancer_group lbg load_balancer
$lb1
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=1
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch + router, LB1 on switch + router, readd])
+check ovn-nbctl --wait=sb add load_balancer_group lbg load_balancer $lb1
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=1
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch + router, LB1 on switch, remove])
+check ovn-nbctl --wait=sb remove logical_router lr load_balancer_group
$lbg
+check_column "$router_port_mac" sb:Service_Monitor src_mac port=1
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=2
+
+AS_BOX([LB0 on switch, LB1 on switch, remove])
+check ovn-nbctl --wait=sb lr-lb-del lr lb0
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=1
+check_column "$global_svc_mon_mac" sb:Service_Monitor src_mac port=2
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 7edb8a45b0..9e58f39c8b 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -21496,3 +21496,85 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
patch-.*/d
/connection dropped.*/d"])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Load balancer health checks - service monitor source MAC
matching])
+AT_SKIP_IF([test $HAVE_NC = no])
+AT_SKIP_IF([test $HAVE_TCPDUMP = no])
+
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+check ovs-vsctl \
+ -- 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_daemon ovn-controller
+
+global_svc_mon_mac="11:11:11:11:11:11"
+router_port_mac="00:00:00:00:00:01"
+lsp_mac="00:00:00:01:01:01"
+check ovn-nbctl set NB_Global .
options:svc_monitor_mac="$global_svc_mon_mac"
+
+check ovn-nbctl \
+ -- ls-add ls \
+ -- lsp-add ls lsp \
+ -- lsp-set-addresses lsp "$lsp_mac 192.168.1.1" \
+ -- lr-add lr \
+ -- lrp-add lr lr-ls $router_port_mac 192.168.1.254/24 \
+ -- lsp-add-router-port ls ls-lr lr-ls
+
+check ovn-nbctl lb-add lb 192.168.5.1:42 192.168.1.1:42 udp
+lb=$(fetch_column nb:Load_Balancer _uuid name=lb)
+
+lbhc=$(ovn-nbctl \
+ --id=@hc create Load_Balancer_Health_Check vip="192.168.5.1\:42" \
+ -- add Load_Balancer $lb health_check @hc)
+check ovn-nbctl set Load_Balancer_Health_Check $lbhc \
+ options:interval=1 options:timeout=1 \
+ options:success_count=2 \
+ options:failure_count=2
+check ovn-nbctl --wait=sb set load_balancer $lb \
+ ip_port_mappings:192.168.1.1=lsp:192.168.1.254
+
+check ovn-nbctl set Logical_Router lr options:chassis="hv1"
+check ovn-nbctl ls-lb-add ls lb
+
+ADD_NAMESPACES(lsp)
+ADD_VETH(lsp, lsp, br-int, "192.168.1.1/24", "$lsp_mac",
+ "192.168.1.254")
+
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
+dnl Check mac address of probe requests and replies, they should use the
+dnl global svc_monitor_mac.
+NETNS_START_TCPDUMP([lsp], [-nne -i lsp -Q in 'udp'], [lsp_in])
+NETNS_START_TCPDUMP([lsp], [-nne -i lsp -Q out 'udp || icmp'], [lsp_out])
+
+dnl Start a backend server, the monitor should change status to "online".
+NETNS_DAEMONIZE([lsp], [nc -l -u 42], [nc.pid])
+wait_row_count Service_Monitor 1 status=online
+
+dnl Kill the backend server, the monitor should change status to
"offline".
+check kill -9 `cat nc.pid`
+wait_row_count Service_Monitor 1 status=offline
+
+OVS_WAIT_UNTIL([grep -q "$global_svc_mon_mac > $lsp_mac" lsp_in.tcpdump])
+AT_CHECK([grep -v "$global_svc_mon_mac > $lsp_mac" lsp_in.tcpdump], [1])
+
+OVS_WAIT_UNTIL([grep -q "$lsp_mac > $global_svc_mon_mac" lsp_out.tcpdump])
+AT_CHECK([grep -v "$lsp_mac > $global_svc_mon_mac" lsp_out.tcpdump], [1])
+
+OVN_CLEANUP_CONTROLLER([hv1])
+OVN_CLEANUP_NORTHD
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
--
2.53.0
