From bdab795534363f4e72a9fb7df5db5eecad8553ad Mon Sep 17 00:00:00 2001 From: Tao Date: Fri, 8 Sep 2023 17:22:34 +0200 Subject: [PATCH] roll back when necessary --- include/monitoring/dp_graphtrace.h | 2 +- include/rte_flow/dp_rte_flow_init.h | 2 +- src/dp_port.c | 4 +- src/monitoring/dp_graphtrace.c | 9 ++-- src/rte_flow/dp_rte_flow_init.c | 4 +- tools/dp_graphtrace.c | 65 +++++++++++++++++++---------- 6 files changed, 51 insertions(+), 35 deletions(-) diff --git a/include/monitoring/dp_graphtrace.h b/include/monitoring/dp_graphtrace.h index e47c9170f..263cea999 100644 --- a/include/monitoring/dp_graphtrace.h +++ b/include/monitoring/dp_graphtrace.h @@ -19,7 +19,7 @@ enum dp_graphtrace_loglevel { int dp_graphtrace_init(void); void dp_graphtrace_free(void); -void _dp_graphtrace_send(int type, struct rte_node *node, struct rte_node *next_node, void **objs, uint16_t nb_objs); +void _dp_graphtrace_send(enum dp_graphtrace_pkt_type type, struct rte_node *node, struct rte_node *next_node, void **objs, uint16_t nb_objs); // Logging the trace for debugging #ifdef ENABLE_PYTEST diff --git a/include/rte_flow/dp_rte_flow_init.h b/include/rte_flow/dp_rte_flow_init.h index 0eceee36e..9e35610b7 100644 --- a/include/rte_flow/dp_rte_flow_init.h +++ b/include/rte_flow/dp_rte_flow_init.h @@ -9,7 +9,7 @@ extern "C" { int dp_install_isolated_mode_ipip(int port_id, uint8_t proto_id); -int dp_install_jump_rule_int_default_group(uint16_t port_id, uint32_t group_id); +int dp_install_jump_rule_in_default_group(uint16_t port_id, uint32_t group_id); int dp_install_default_rule_in_monitoring_group(uint16_t port_id); int dp_install_default_capture_rule_in_vnet_group(uint16_t port_id); diff --git a/src/dp_port.c b/src/dp_port.c index 5cef57895..baa772ae8 100644 --- a/src/dp_port.c +++ b/src/dp_port.c @@ -487,9 +487,9 @@ int dp_port_start(uint16_t port_id) if (port->port_type == DP_PORT_VF) { if (graphtrace_enabled) - ret = dp_install_jump_rule_int_default_group(port_id, DP_RTE_FLOW_MONITORING_GROUP); + ret = dp_install_jump_rule_in_default_group(port_id, DP_RTE_FLOW_MONITORING_GROUP); else - ret = dp_install_jump_rule_int_default_group(port_id, DP_RTE_FLOW_VNET_GROUP); + ret = dp_install_jump_rule_in_default_group(port_id, DP_RTE_FLOW_VNET_GROUP); if (DP_FAILED(ret)) { DPS_LOG_ERR("Cannot install default jump rule", DP_LOG_PORTID(port_id), DP_LOG_RET(ret)); diff --git a/src/monitoring/dp_graphtrace.c b/src/monitoring/dp_graphtrace.c index c7c77ffb8..efc937a17 100644 --- a/src/monitoring/dp_graphtrace.c +++ b/src/monitoring/dp_graphtrace.c @@ -82,6 +82,7 @@ void dp_handle_graphtrace_request(const struct rte_mp_msg *mp_msg, struct dp_gra if (offload_enabled) { if (DP_FAILED(dp_send_event_hardware_capture_start_msg())) { DPS_LOG_ERR("Cannot send hardware capture start message"); + dp_graphtrace_disable(); // roll back if failed on this side, and error code cannot be sent back reply->error_code = DP_ERROR; return; } @@ -155,7 +156,7 @@ void dp_graphtrace_free(void) } -void _dp_graphtrace_send(int type, struct rte_node *node, struct rte_node *next_node, void **objs, uint16_t nb_objs) +void _dp_graphtrace_send(enum dp_graphtrace_pkt_type type, struct rte_node *node, struct rte_node *next_node, void **objs, uint16_t nb_objs) { uint16_t nb_dups = 0; struct rte_mbuf *dups[nb_objs]; @@ -173,11 +174,7 @@ void _dp_graphtrace_send(int type, struct rte_node *node, struct rte_node *next_ dups[nb_dups++] = dup; pktinfo = dp_get_graphtrace_pktinfo(dup); pktinfo->pktid = dp_get_pkt_mark(objs[i])->id; - if (type == DP_GRAPHTRACE_PKT_TYPE_SOFTWARE) - pktinfo->pkt_type = DP_GRAPHTRACE_PKT_TYPE_SOFTWARE; - else if (type == DP_GRAPHTRACE_PKT_TYPE_OFFLOAD) - pktinfo->pkt_type = DP_GRAPHTRACE_PKT_TYPE_OFFLOAD; - + pktinfo->pkt_type = type; pktinfo->node = node; pktinfo->next_node = next_node; } diff --git a/src/rte_flow/dp_rte_flow_init.c b/src/rte_flow/dp_rte_flow_init.c index 1c38d6b66..51b6e6b08 100644 --- a/src/rte_flow/dp_rte_flow_init.c +++ b/src/rte_flow/dp_rte_flow_init.c @@ -62,7 +62,7 @@ int dp_install_isolated_mode_ipip(int port_id, uint8_t proto_id) return DP_OK; } -int dp_install_jump_rule_int_default_group(uint16_t port_id, uint32_t dst_group) +int dp_install_jump_rule_in_default_group(uint16_t port_id, uint32_t dst_group) { struct rte_flow_item pattern[2]; // first is a NULL ethernet header matching, second is the end int pattern_cnt = 0; @@ -184,7 +184,7 @@ static int dp_change_all_vf_default_jump_rte_flow_group(uint32_t dst_group) continue; } - if (DP_FAILED(dp_install_jump_rule_int_default_group(port->port_id, dst_group))) { + if (DP_FAILED(dp_install_jump_rule_in_default_group(port->port_id, dst_group))) { DPS_LOG_WARNING("Failed to install default jump flow", DP_LOG_PORTID(port->port_id)); continue; } diff --git a/tools/dp_graphtrace.c b/tools/dp_graphtrace.c index facd94e95..d2e8bb116 100644 --- a/tools/dp_graphtrace.c +++ b/tools/dp_graphtrace.c @@ -46,7 +46,7 @@ static bool capture_hw_pkt = false; static const struct option longopts[] = { { "help", 0, 0, DP_GRAPHTRACE_OPT_HELP }, - { "capture-hw-pkt", 0, 0, DP_GRAPHTRACE_OPT_CAPTURE_HW_PKT }, + { "hw-packet", 0, 0, DP_GRAPHTRACE_OPT_CAPTURE_HW_PKT }, }; @@ -181,7 +181,7 @@ static int dp_graphtrace_request(enum dp_graphtrace_action action, struct dp_gra if (DP_FAILED(ret)) { fprintf(stderr, "Cannot send graphtrace request %s\n", dp_strerror_verbose(ret)); return DP_ERROR; - } + } if (DP_FAILED(reply->error_code)) { fprintf(stderr, "Graphtrace request failed %s\n", dp_strerror_verbose(reply->error_code)); @@ -195,8 +195,10 @@ static int dp_graphtrace_start(void) { struct dp_graphtrace_mp_reply reply; - if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_START, &reply))) + if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_START, &reply))) { + fprintf(stderr, "Failed to request graph tracing\n"); return DP_ERROR; + } primary_alive = true; return DP_OK; @@ -210,15 +212,22 @@ static int dp_graphtrace_stop(void) if (!primary_alive) return DP_OK; - return dp_graphtrace_request(DP_GRAPHTRACE_ACTION_STOP, &reply); + if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_STOP, &reply))) { + fprintf(stderr, "Failed to request graph tracing termination\n"); + return DP_ERROR; + } + + return DP_OK; } static int dp_graphtrace_enable_hw_capture(void) { struct dp_graphtrace_mp_reply reply; - if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_ENABLE_HW_CAPTURE, &reply))) + if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_ENABLE_HW_CAPTURE, &reply))) { + fprintf(stderr, "Failed to enable hardware packet capture\n"); return DP_ERROR; + } return DP_OK; } @@ -227,8 +236,10 @@ static int dp_graphtrace_disable_hw_capture(void) { struct dp_graphtrace_mp_reply reply; - if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_DISABLE_HW_CAPTURE, &reply))) + if (DP_FAILED(dp_graphtrace_request(DP_GRAPHTRACE_ACTION_DISABLE_HW_CAPTURE, &reply))) { + fprintf(stderr, "Failed to disable hardware packet capture\n"); return DP_ERROR; + } return DP_OK; } @@ -257,6 +268,20 @@ static void dp_graphtrace_monitor_primary(void *arg __rte_unused) fprintf(stderr, "Warning: Cannot re-schedule primary process monitor %s\n", dp_strerror_verbose(ret)); } +static int dp_graphtrace_init_hw_pkt_capture(struct dp_graphtrace *graphtrace) +{ + if (!capture_hw_pkt) + return DP_OK; + + if (DP_FAILED(dp_graphtrace_enable_hw_capture())) { + fprintf(stderr, "Failed to enable hardware packet capture\n"); + rte_eal_alarm_cancel(dp_graphtrace_monitor_primary, (void *)-1); + return DP_ERROR; + } + + return DP_OK; +} + static int do_graphtrace(struct dp_graphtrace *graphtrace) { int ret; @@ -274,34 +299,28 @@ static int do_graphtrace(struct dp_graphtrace *graphtrace) } if (DP_FAILED(dp_graphtrace_start())) { - fprintf(stderr, "Failed to request graph tracing\n"); rte_eal_alarm_cancel(dp_graphtrace_monitor_primary, (void *)-1); return DP_ERROR; } - if (capture_hw_pkt) { - if (DP_FAILED(dp_graphtrace_enable_hw_capture())) { - fprintf(stderr, "Failed to enable hardware packet capture\n"); - rte_eal_alarm_cancel(dp_graphtrace_monitor_primary, (void *)-1); - return DP_ERROR; - } + if (DP_FAILED(dp_graphtrace_init_hw_pkt_capture(graphtrace))) { + if (dp_graphtrace_stop()) // rollback if failed + fprintf(stderr, "Failed to stop graph tracing after failing to init hw pkt capture\n"); + rte_eal_alarm_cancel(dp_graphtrace_monitor_primary, (void *)-1); + return DP_ERROR; } ret = dp_graphtrace_dump(graphtrace); - if (capture_hw_pkt) { - if (DP_FAILED(dp_graphtrace_disable_hw_capture())) { - fprintf(stderr, "Failed to disable hardware packet capture\n"); + if (capture_hw_pkt) + if (DP_FAILED(dp_graphtrace_disable_hw_capture())) ret = DP_ERROR; - } - } - if (DP_FAILED(dp_graphtrace_stop())) { - fprintf(stderr, "Failed to request graph tracing termination\n"); + if (DP_FAILED(dp_graphtrace_stop())) ret = DP_ERROR; - } rte_eal_alarm_cancel(dp_graphtrace_monitor_primary, (void *)-1); + return ret; } @@ -314,7 +333,7 @@ static void dp_print_usage(const char *prgname) { fprintf(stderr, " --help show this help message and exit\n" - " --capture-hw-pkt capture pkts after offloading rules are installed (experimental feature, only vf's pkts are supported)\n", + " --hw-packet capture pkts after offloading rules are installed (experimental feature, only VF's outgoing packets are supported)\n", prgname); } @@ -350,7 +369,7 @@ int main(int argc, char **argv) ret = parse_args(argc, argv); switch (ret) { case DP_CONF_RUNMODE_ERROR: - fprintf(stderr, "Cannot parse cmd options %s\n", dp_strerror_verbose(ret)); + fprintf(stderr, "Cannot parse command-line options %s\n", dp_strerror_verbose(ret)); return EXIT_FAILURE; case DP_CONF_RUNMODE_EXIT: return EXIT_SUCCESS;