* Add service monitor type validation during creation to prevent invalid types and avoid subsequent type checks in runtime processing. * Implement ICMP probe handling for logical switch ports health checks. * Add unit tests for the new functionality.
Signed-off-by: Alexandra Rukomoinikova <[email protected]> --- controller/pinctrl.c | 90 ++++++++++++++++++----------- tests/ovn-northd.at | 132 +++++++++++++++++++++++++++++++++++++++++++ tests/system-ovn.at | 68 ++++++++++++++++++++++ 3 files changed, 257 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..9225f0646 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" + +# Create service monitor for all lsp addresses. +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 + +# Create one more service monitor for all lsp addresses. +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..15b7491bb 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -19046,3 +19046,71 @@ 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
