1) Added service monitor type validation during creation to prevent invalid types and avoid subsequent type checks in runtime processing.
2) Implemented ICMP probe handling for logical switch ports health checks. 3) Added tests. Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- controller/pinctrl.c | 90 ++++++++++++++++++----------- tests/ovn-northd.at | 132 +++++++++++++++++++++++++++++++++++++++++++ tests/system-ovn.at | 69 ++++++++++++++++++++++ 3 files changed, 258 insertions(+), 33 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3ccacfa2d..c788acb16 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -6856,6 +6856,8 @@ enum svc_monitor_type { SVC_MON_TYPE_LB, /* network function */ SVC_MON_TYPE_NF, + /* logical switch port */ + SVC_MON_TYPE_LSP, }; /* Service monitor health checks. */ @@ -6980,20 +6982,26 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_service_monitor *sb_svc_mon; SBREC_SERVICE_MONITOR_TABLE_FOR_EACH (sb_svc_mon, svc_mon_table) { enum svc_monitor_type mon_type; - if (sb_svc_mon->type && !strcmp(sb_svc_mon->type, - "network-function")) { + enum svc_monitor_protocol protocol; + + if (sb_svc_mon->type && + !strcmp(sb_svc_mon->type, "network-function")) { mon_type = SVC_MON_TYPE_NF; + } else if (sb_svc_mon->type && + !strcmp(sb_svc_mon->type, "logical-switch-port")) { + mon_type = SVC_MON_TYPE_LSP; } else { mon_type = SVC_MON_TYPE_LB; } - enum svc_monitor_protocol protocol; if (!strcmp(sb_svc_mon->protocol, "udp")) { - protocol = SVC_MON_PROTO_UDP; + protocol = (mon_type == SVC_MON_TYPE_NF) ? + SVC_MON_PROTO_ICMP : SVC_MON_PROTO_UDP; } else if (!strcmp(sb_svc_mon->protocol, "icmp")) { protocol = SVC_MON_PROTO_ICMP; } else { - protocol = SVC_MON_PROTO_TCP; + protocol = (mon_type == SVC_MON_TYPE_NF) ? + SVC_MON_PROTO_ICMP : SVC_MON_PROTO_TCP; } const struct sbrec_port_binding *pb @@ -7030,9 +7038,6 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn, bool mac_found = false; if (mon_type == SVC_MON_TYPE_NF) { - if (protocol != SVC_MON_PROTO_ICMP) { - continue; - } input_pb = lport_lookup_by_name(sbrec_port_binding_by_name, sb_svc_mon->logical_input_port); if (!input_pb) { @@ -7047,11 +7052,6 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn, } } } else { - if (protocol != SVC_MON_PROTO_TCP && - protocol != SVC_MON_PROTO_UDP) { - continue; - } - for (size_t i = 0; i < pb->n_mac && !mac_found; i++) { struct lport_addresses laddrs; @@ -8010,6 +8010,7 @@ static void svc_monitor_send_icmp_health_check__(struct rconn *swconn, struct svc_monitor *svc_mon) { + bool svc_mon_nf = (svc_mon->type == SVC_MON_TYPE_NF) ? true : false; uint64_t packet_stub[128 / 8]; struct dp_packet packet; dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub); @@ -8056,7 +8057,8 @@ svc_monitor_send_icmp_health_check__(struct rconn *swconn, struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub); enum ofp_version version = rconn_get_version(swconn); put_load(svc_mon->dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts); - put_load(svc_mon->input_port_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); + put_load(svc_mon_nf ? svc_mon->input_port_key : svc_mon->port_key, + MFF_LOG_OUTPORT, 0, 32, &ofpacts); put_load(1, MFF_LOG_FLAGS, MLF_LOCAL_ONLY, 1, &ofpacts); struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); resubmit->in_port = OFPP_CONTROLLER; @@ -8338,6 +8340,7 @@ pinctrl_handle_svc_check(struct rconn *swconn, const struct flow *ip_flow, "not found"); return; } + pinctrl_handle_tcp_svc_check(swconn, pkt_in, svc_mon); } else { const char *end = @@ -8350,48 +8353,69 @@ pinctrl_handle_svc_check(struct rconn *swconn, const struct flow *ip_flow, return; } - /* Handle ICMP ECHO REQUEST probes for Network Function services */ + /* Handle ICMP ECHO REQUEST probes for Network Function and + * Logical Switch Port services */ if (in_eth->eth_type == htons(ETH_TYPE_IP)) { struct icmp_header *ih = l4h; /* It's ICMP packet. */ - if (ih->icmp_type == ICMP4_ECHO_REQUEST && ih->icmp_code == 0) { - uint32_t hash = hash_bytes(&dst_ip_addr, sizeof dst_ip_addr, - hash_3words(dp_key, port_key, 0)); - struct svc_monitor *svc_mon = - pinctrl_find_svc_monitor(dp_key, port_key, &dst_ip_addr, 0, + if ((ih->icmp_type == ICMP4_ECHO_REQUEST || + ih->icmp_type == ICMP4_ECHO_REPLY) && ih->icmp_code == 0) { + int is_echo_request = (ih->icmp_type == ICMP4_ECHO_REQUEST); + struct in6_addr *target_addr = is_echo_request + ? &dst_ip_addr : &ip_addr; + uint32_t hash = + hash_bytes(target_addr, sizeof(*target_addr), + hash_3words(dp_key, port_key, 0)); + struct svc_monitor *svc_mon = + pinctrl_find_svc_monitor(dp_key, port_key, target_addr, 0, SVC_MON_PROTO_ICMP, hash); if (!svc_mon) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( - 1, 5); + static struct vlog_rate_limit rl + = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_WARN_RL(&rl, "handle service check: Service monitor " "not found for ICMP request"); return; } - if (svc_mon->type == SVC_MON_TYPE_NF) { - pinctrl_handle_icmp_svc_check(pkt_in, svc_mon); - } + + /* Type validation done during creation - + * asserts on unsupported types. */ + ovs_assert(svc_mon->type != SVC_MON_TYPE_NF || + svc_mon->type != SVC_MON_TYPE_LSP); + + pinctrl_handle_icmp_svc_check(pkt_in, svc_mon); + return; } } else if (in_eth->eth_type == htons(ETH_TYPE_IPV6)) { struct icmp6_data_header *ih6 = l4h; /* It's ICMPv6 packet. */ - if (ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST && + if ((ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST || + ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REPLY) && ih6->icmp6_base.icmp6_code == 0) { - uint32_t hash = hash_bytes(&dst_ip_addr, sizeof dst_ip_addr, + int is_echo_request = + (ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST); + struct in6_addr *target_addr = is_echo_request + ? &dst_ip_addr : &ip_addr; + uint32_t hash = hash_bytes(target_addr, sizeof(*target_addr), hash_3words(dp_key, port_key, 0)); struct svc_monitor *svc_mon = - pinctrl_find_svc_monitor(dp_key, port_key, &dst_ip_addr, 0, + pinctrl_find_svc_monitor(dp_key, port_key, target_addr, 0, SVC_MON_PROTO_ICMP, hash); if (!svc_mon) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( - 1, 5); + static struct vlog_rate_limit rl = + VLOG_RATE_LIMIT_INIT(1, 5); VLOG_WARN_RL(&rl, "handle service check: Service monitor " "not found for ICMPv6 request"); return; } - if (svc_mon->type == SVC_MON_TYPE_NF) { - pinctrl_handle_icmp_svc_check(pkt_in, svc_mon); - } + + /* Type validation done during creation + * - asserts on unsupported types. */ + ovs_assert(svc_mon->type != SVC_MON_TYPE_NF || + svc_mon->type != SVC_MON_TYPE_LSP); + + pinctrl_handle_icmp_svc_check(pkt_in, svc_mon); + return; } } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index d425cfb7a..a42400cf6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -19030,3 +19030,135 @@ check ovn-nbctl --wait=sb sync OVN_CLEANUP_NORTHD AT_CLEANUP ]) +AT_SETUP([Logical Switch Port Health Check - lflow/service monitor synchronization]) +ovn_start + +check ovn-nbctl ls-add ls1 +check ovn-nbctl lsp-add ls1 lport1 -- \ + lsp-set-addresses lport1 "f0:00:0f:01:02:04 192.168.0.10" "f0:00:0f:01:02:06 192.168.0.11" + +check ovn-nbctl lsp-add ls1 lport2 -- \ + lsp-set-addresses lport2 "f0:00:0f:01:02:08 192.168.0.12" + +check ovn-nbctl set NB_Global . options:svc_monitor_mac="11:11:11:11:11:11" + +# Сreate a service monitor for all addresses on one of lsp. +check ovn-nbctl lsp-hc-add lport1 icmp 192.168.0.255 +check_row_count nb:Logical_Switch_Port_Health_Check 1 +lport1_uuid1=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=icmp) + +# Check lflow and service monitor synchronization +AT_CHECK([ovn-sbctl lflow-list ls1 | grep 'ls_in_arp_rsp'| grep "priority=110" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=110 , match=(arp.tpa == 192.168.0.255 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 11:11:11:11:11:11; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 11:11:11:11:11:11; arp.tpa = arp.spa; arp.spa = 192.168.0.255; outport = inport; flags.loopback = 1; output;) +]) + +check_row_count sb:Service_Monitor 2 +check_column "false false" sb:Service_Monitor ic_learned logical_port=lport1 +check_column "false false" sb:Service_Monitor remote logical_port=lport1 +check_column "192.168.0.10 192.168.0.11" sb:Service_Monitor ip logical_port=lport1 +check_column "icmp icmp" sb:Service_Monitor protocol logical_port=lport1 +check_column "192.168.0.255 192.168.0.255" sb:Service_Monitor src_ip logical_port=lport1 +check_column "0 0" sb:Service_Monitor port logical_port=lport1 +check_column "logical-switch-port logical-switch-port" sb:Service_Monitor type logical_port=lport1 +check_column "11:11:11:11:11:11 11:11:11:11:11:11" sb:Service_Monitor src_mac logical_port=lport1 + +# Сreate one more service monitor for all addresses on one of lsp. +check ovn-nbctl lsp-hc-add lport1 tcp 192.168.0.254 80 +check_row_count nb:Logical_Switch_Port_Health_Check 2 +lport1_uuid2=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=tcp) + +# Check that 2 records (2 addresses) have been created for each protocol. +check_row_count sb:Service_Monitor 4 + +AT_CHECK([ovn-sbctl lflow-list ls1 | grep 'ls_in_arp_rsp'| grep "priority=110" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=110 , match=(arp.tpa == 192.168.0.254 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 11:11:11:11:11:11; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 11:11:11:11:11:11; arp.tpa = arp.spa; arp.spa = 192.168.0.254; outport = inport; flags.loopback = 1; output;) + table=??(ls_in_arp_rsp ), priority=110 , match=(arp.tpa == 192.168.0.255 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 11:11:11:11:11:11; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 11:11:11:11:11:11; arp.tpa = arp.spa; arp.spa = 192.168.0.255; outport = inport; flags.loopback = 1; output;) +]) + +# Change addresses on lport - addresses for service monitors should be changed +check ovn-nbctl lsp-set-addresses lport1 "f0:00:0f:01:02:04 192.168.0.20" + +check_row_count sb:Service_Monitor 2 +check_column "false false" sb:Service_Monitor ic_learned logical_port=lport1 +check_column "false false " sb:Service_Monitor remote logical_port=lport1 +check_column "192.168.0.20 192.168.0.20" sb:Service_Monitor ip logical_port=lport1 +check_column "icmp tcp" sb:Service_Monitor protocol logical_port=lport1 +check_column "192.168.0.255 192.168.0.254" sb:Service_Monitor src_ip logical_port=lport1 +check_column "0 80" sb:Service_Monitor port logical_port=lport1 +check_column "logical-switch-port logical-switch-port" sb:Service_Monitor type logical_port=lport1 +check_column "11:11:11:11:11:11 11:11:11:11:11:11" sb:Service_Monitor src_mac logical_port=lport1 + +# Check options propogations +hc_lport1_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="192.168.0.255") + +check ovn-nbctl set logical_switch_port_health_check $hc_lport1_uuid options:interval=3 +check ovn-nbctl set logical_switch_port_health_check $hc_lport1_uuid options:timeout=30 +check ovn-nbctl set logical_switch_port_health_check $hc_lport1_uuid options:success_count=1 +check ovn-nbctl set logical_switch_port_health_check $hc_lport1_uuid options:failure_count=2 + + +sm_lport1_uuid=$(fetch_column sb:service_monitor _uuid protocol=icmp) + +AT_CHECK([ovn-sbctl get Service_Monitor $sm_lport1_uuid options:interval], +[0], ["3" +]) +AT_CHECK([ovn-sbctl get Service_Monitor $sm_lport1_uuid options:failure_count], +[0], ["2" +]) +AT_CHECK([ovn-sbctl get Service_Monitor $sm_lport1_uuid options:success_count], +[0], ["1" +]) +AT_CHECK([ovn-sbctl get Service_Monitor $sm_lport1_uuid options:timeout], +[0], ["30" +]) + +ovn-nbctl list logical_switch_port + +check ovn-nbctl lsp-hc-del lport1 + +check_row_count nb:Logical_Switch_Port_Health_Check 0 + +# Change the service monitor to monitor only one address. +check ovn-nbctl lsp-hc-add lport1 icmp 192.168.0.255 192.168.0.20 +check_row_count nb:Logical_Switch_Port_Health_Check 1 +lport1_uuid3=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=icmp) + +check_row_count sb:Service_Monitor 1 + +AT_CHECK([ovn-sbctl lflow-list ls1 | grep 'ls_in_arp_rsp'| grep "priority=110" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=110 , match=(arp.tpa == 192.168.0.255 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 11:11:11:11:11:11; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 11:11:11:11:11:11; arp.tpa = arp.spa; arp.spa = 192.168.0.255; outport = inport; flags.loopback = 1; output;) +]) + +check_column "false" sb:Service_Monitor ic_learned logical_port=lport1 +check_column "false" sb:Service_Monitor remote logical_port=lport1 +check_column "192.168.0.20" sb:Service_Monitor ip logical_port=lport1 +check_column "icmp" sb:Service_Monitor protocol logical_port=lport1 +check_column "192.168.0.255" sb:Service_Monitor src_ip logical_port=lport1 +check_column "0" sb:Service_Monitor port logical_port=lport1 +check_column "logical-switch-port" sb:Service_Monitor type logical_port=lport1 +check_column "11:11:11:11:11:11" sb:Service_Monitor src_mac logical_port=lport1 + +# Create one more monitor +check ovn-nbctl lsp-hc-add lport2 icmp 192.168.0.255 192.168.0.12 +lport1_uuid4=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid addresses="192.168.0.12") +check_row_count nb:Logical_Switch_Port_Health_Check 2 + +check_row_count sb:Service_Monitor 2 + +# One record for arp replay +AT_CHECK([ovn-sbctl lflow-list ls1 | grep 'ls_in_arp_rsp'| grep "priority=110" | ovn_strip_lflows], [0], [dnl + table=??(ls_in_arp_rsp ), priority=110 , match=(arp.tpa == 192.168.0.255 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 11:11:11:11:11:11; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 11:11:11:11:11:11; arp.tpa = arp.spa; arp.spa = 192.168.0.255; outport = inport; flags.loopback = 1; output;) +]) + +ovn-nbctl list logical_switch_port_health_check + +check ovn-nbctl lsp-hc-del lport1 +check ovn-nbctl lsp-hc-del lport2 +check_row_count nb:Logical_Switch_Port_Health_Check 0 +check_row_count sb:Service_Monitor 0 + +AT_CHECK([ovn-sbctl lflow-list ls1 | grep 'ls_in_arp_rsp'| grep "priority=110" | ovn_strip_lflows], [0], []) + +AT_CLEANUP +]) + diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 303b10894..511e9c517 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -19046,3 +19046,72 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d /Couldn't parse IPv6 prefix nexthop.*/d"]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Logical Switch Port Health Check]) + +ovn_start +OVS_TRAFFIC_VSWITCHD_START() +ADD_BR([br-int]) + +# Set external-ids in br-int needed for ovn-controller +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 ovn-controller +start_daemon ovn-controller + +check ovn-nbctl ls-add ls1 + +ADD_NAMESPACES(lport) +ADD_VETH(lport, lport, br-int, "192.168.0.10", "f0:00:0f:01:02:04", \ + "192.168.0.1") +NS_EXEC([lport], [ip r del default via 192.168.0.1 dev lport]) +NS_EXEC([lport], [ip r add default dev lport]) + +check ovn-nbctl lsp-add ls1 lport -- \ + lsp-set-addresses lport "f0:00:0f:01:02:04 192.168.0.10" + +check ovn-nbctl --wait=hv sync + +check ovn-nbctl lsp-hc-add lport icmp 192.168.0.255 +lport1_uuid1=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=icmp) + +check_row_count sb:Service_Monitor 1 + +NETNS_START_TCPDUMP([lport], [-n -c 2 -i lport], [lport]) +OVS_WAIT_UNTIL([ + total_queries=`grep "ICMP" -c lport.tcpdump` + test "${total_queries}" = "2" +]) + +# Wait until the services are set to online. +wait_row_count Service_Monitor 1 status=online + +# Create one more health check on logical switch port +NETNS_DAEMONIZE([lport], [nc -l -k 192.168.0.10 4041], [lport_tcp.pid]) + +check ovn-nbctl lsp-hc-add lport tcp 192.168.0.255 4041 +lport1_uuid2=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=tcp) + +check_row_count sb:Service_Monitor 2 + +# Wait until the services are set to online. +wait_row_count Service_Monitor 2 status=online + +NETNS_DAEMONIZE([lport], [nc -ulp 4042], [lport_udp.pid]) + +check ovn-nbctl lsp-hc-add lport udp 192.168.0.255 4042 +lport1_uuid3=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=udp) + +check_row_count sb:Service_Monitor 3 +# Wait until the services are set to online. +wait_row_count Service_Monitor 3 status=online + +AT_CLEANUP +]) + -- 2.48.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
