roll back when necessary

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

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);
 

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));

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;
 		}

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;
 			}

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;