[v2,1/4] trace: fixup CTF event description at registration

Message ID 20201028210249.9021-2-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series Rework CTF event description storage |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

David Marchand Oct. 28, 2020, 9:02 p.m. UTC
  CTF event description is currently built by appending all fields in a
single string at trace point registration.
When dumping the metadata, this string is split again and inspected to
fixup reserved keywords and special tokens like "." or "->".

Move this fixup per field at trace point registration time so that there
is no need for inspecting / string parsing when dumping metadata.
Use dynamic allocations to remove an artificial size limit on the CTF
event description manipulations.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_trace.c     |   5 +
 lib/librte_eal/common/eal_common_trace_ctf.c | 159 +++++--------------
 lib/librte_eal/common/eal_trace.h            |   1 +
 3 files changed, 46 insertions(+), 119 deletions(-)
  

Comments

Sunil Kumar Kori Oct. 29, 2020, 8:35 a.m. UTC | #1
>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Thursday, October 29, 2020 2:33 AM
>To: dev@dpdk.org
>Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Sunil Kumar Kori
><skori@marvell.com>
>Subject: [EXT] [PATCH v2 1/4] trace: fixup CTF event description at registration
>
>External Email
>
>----------------------------------------------------------------------
>CTF event description is currently built by appending all fields in a single string
>at trace point registration.
>When dumping the metadata, this string is split again and inspected to fixup
>reserved keywords and special tokens like "." or "->".
>
>Move this fixup per field at trace point registration time so that there is no
>need for inspecting / string parsing when dumping metadata.
>Use dynamic allocations to remove an artificial size limit on the CTF event
>description manipulations.
>
>Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Sunil Kumar Kori <skori@mavell.com>

[snipped]

>--
>2.23.0
  

Patch

diff --git a/lib/librte_eal/common/eal_common_trace.c b/lib/librte_eal/common/eal_common_trace.c
index b6da5537fe..80b458edb6 100644
--- a/lib/librte_eal/common/eal_common_trace.c
+++ b/lib/librte_eal/common/eal_common_trace.c
@@ -432,11 +432,16 @@  __rte_trace_point_emit_field(size_t sz, const char *in, const char *datatype)
 	char *field = RTE_PER_LCORE(ctf_field);
 	int count = RTE_PER_LCORE(ctf_count);
 	size_t size;
+	char *fixup;
 	int rc;
 
 	size = RTE_MAX(0, TRACE_CTF_FIELD_SIZE - 1 - count);
 	RTE_PER_LCORE(trace_point_sz) += sz;
+	fixup = trace_metadata_fixup_field(in);
+	if (fixup != NULL)
+		in = fixup;
 	rc = snprintf(RTE_PTR_ADD(field, count), size, "%s %s;", datatype, in);
+	free(fixup);
 	if (rc <= 0 || (size_t)rc >= size) {
 		RTE_PER_LCORE(trace_point_sz) = 0;
 		trace_crit("CTF field is too long");
diff --git a/lib/librte_eal/common/eal_common_trace_ctf.c b/lib/librte_eal/common/eal_common_trace_ctf.c
index 9dc91df0fb..174cdac1b0 100644
--- a/lib/librte_eal/common/eal_common_trace_ctf.c
+++ b/lib/librte_eal/common/eal_common_trace_ctf.c
@@ -220,131 +220,12 @@  meta_stream_emit(char **meta, int *offset)
 	return meta_copy(meta, offset, str, rc);
 }
 
-static void
-string_fixed_replace(char *input, const char *search, const char *replace)
-{
-	char *found;
-	size_t len;
-
-	found = strstr(input, search);
-	if (found == NULL)
-		return;
-
-	if (strlen(found) != strlen(search))
-		return;
-
-	len = strlen(replace);
-	memcpy(found, replace, len);
-	found[len] = '\0';
-}
-
-static void
-ctf_fixup_align(char *str)
-{
-	string_fixed_replace(str, "align", "_align");
-}
-
-static void
-ctf_fixup_arrow_deref(char *str)
-{
-	const char *replace = "_";
-	const char *search = "->";
-	char *found;
-	size_t len;
-
-	found = strstr(str, search);
-	if (found == NULL)
-		return;
-
-	do {
-		memcpy(found, replace, strlen(replace));
-		len = strlen(found + 2);
-		memcpy(found + 1, found + 2, len);
-		found[len + 1] = '\0';
-		found = strstr(str, search);
-	} while (found != NULL);
-}
-
-static void
-ctf_fixup_dot_deref(char *str)
-{
-	const char *replace = "_";
-	const char *search = ".";
-	char *found;
-	size_t len;
-
-	found = strstr(str, search);
-	if (found == NULL)
-		return;
-
-	len = strlen(replace);
-	do {
-		memcpy(found, replace, len);
-		found = strstr(str, search);
-	} while (found != NULL);
-}
-
-static void
-ctf_fixup_event(char *str)
-{
-	string_fixed_replace(str, "event", "_event");
-}
-
-static int
-ctf_fixup_keyword(char *str)
-{
-	char dup_str[TRACE_CTF_FIELD_SIZE];
-	char input[TRACE_CTF_FIELD_SIZE];
-	const char *delim = ";";
-	char *from;
-	int len;
-
-	if (str == NULL)
-		return 0;
-
-	len = strlen(str);
-	if (len >= TRACE_CTF_FIELD_SIZE) {
-		trace_err("ctf_field reached its maximum limit");
-		return -EMSGSIZE;
-	}
-
-	/* Create duplicate string */
-	strcpy(dup_str, str);
-
-	len = 0;
-	from = strtok(dup_str, delim);
-	while (from != NULL) {
-		strcpy(input, from);
-		ctf_fixup_align(input);
-		ctf_fixup_dot_deref(input);
-		ctf_fixup_arrow_deref(input);
-		ctf_fixup_event(input);
-
-		strcpy(&input[strlen(input)], delim);
-		if ((len + strlen(input)) >= TRACE_CTF_FIELD_SIZE) {
-			trace_err("ctf_field reached its maximum limit");
-			return -EMSGSIZE;
-		}
-
-		strcpy(str + len, input);
-		len += strlen(input);
-		from = strtok(NULL, delim);
-	}
-
-	return 0;
-}
-
 static int
 meta_event_emit(char **meta, int *offset, struct trace_point *tp)
 {
 	char *str = NULL;
 	int rc;
 
-	/* Fixup ctf field string in case it using reserved ctf keywords */
-	rc = ctf_fixup_keyword(tp->ctf_field);
-	if (rc)
-		return rc;
-
 	rc = metadata_printf(&str,
 		"event {\n"
 		"    id = %d;\n"
@@ -491,3 +372,43 @@  rte_trace_metadata_dump(FILE *f)
 	rc = fprintf(f, "%s", ctf_meta);
 	return rc < 0 ? rc : 0;
 }
+
+char *trace_metadata_fixup_field(const char *field)
+{
+	const char *ctf_reserved_words[] = {
+		"align",
+		"event",
+	};
+	unsigned int i;
+	char *out;
+	char *p;
+
+	/* reserved keywords */
+	for (i = 0; i < RTE_DIM(ctf_reserved_words); i++) {
+		if (strcmp(field, ctf_reserved_words[i]) != 0)
+			continue;
+		if (asprintf(&out, "_%s", ctf_reserved_words[i]) == -1)
+			out = NULL;
+		return out;
+	}
+
+	/* nothing to replace, return early */
+	if (strstr(field, ".") == NULL && strstr(field, "->") == NULL)
+		return NULL;
+
+	out = strdup(field);
+	if (out == NULL)
+		return NULL;
+	p = out;
+	while ((p = strstr(p, ".")) != NULL) {
+		p[0] = '_';
+		p++;
+	}
+	p = out;
+	while ((p = strstr(p, "->")) != NULL) {
+		p[0] = '_';
+		p++;
+		memmove(p, p + 1, strlen(p));
+	}
+	return out;
+}
diff --git a/lib/librte_eal/common/eal_trace.h b/lib/librte_eal/common/eal_trace.h
index 438c2b73f6..8a3f6c5359 100644
--- a/lib/librte_eal/common/eal_trace.h
+++ b/lib/librte_eal/common/eal_trace.h
@@ -104,6 +104,7 @@  bool trace_has_duplicate_entry(void);
 void trace_uuid_generate(void);
 int trace_metadata_create(void);
 void trace_metadata_destroy(void);
+char *trace_metadata_fixup_field(const char *field);
 int trace_mkdir(void);
 int trace_epoch_time_save(void);
 void trace_mem_free(void);