DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ankur Dwivedi <adwivedi@marvell.com>
To: <dev@dpdk.org>
Cc: <thomas@monjalon.net>, <jerinj@marvell.com>,
	Ankur Dwivedi <adwivedi@marvell.com>
Subject: [PATCH v5 1/1] devtools: add tracepoint check in checkpatch
Date: Tue, 7 Mar 2023 17:35:14 +0530	[thread overview]
Message-ID: <20230307120514.2774917-2-adwivedi@marvell.com> (raw)
In-Reply-To: <20230307120514.2774917-1-adwivedi@marvell.com>

This patch adds a validation in checkpatch tool, to check if a
tracepoint is present in any new function added in cryptodev, ethdev,
eventdev and mempool library.

In this patch, the build_map_changes function is copied from
check-symbol-change.sh to check-tracepoint.sh. The check-tracepoint.sh
script uses build_map_changes function to create a map of functions.
In the map, the newly added functions, added in the experimental section
are identified and their definition are checked for the presence of
tracepoint. The checkpatch return error if the tracepoint is not present.

For functions for which trace is not needed, they can be added to
devtools/trace-skiplist.txt file. The above tracepoint check will be
skipped for them.

Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
---
 devtools/check-tracepoint.sh | 223 +++++++++++++++++++++++++++++++++++
 devtools/checkpatches.sh     |   9 ++
 devtools/trace-skiplist.txt  |   0
 3 files changed, 232 insertions(+)
 create mode 100755 devtools/check-tracepoint.sh
 create mode 100644 devtools/trace-skiplist.txt

diff --git a/devtools/check-tracepoint.sh b/devtools/check-tracepoint.sh
new file mode 100755
index 0000000000..88d6b1fd53
--- /dev/null
+++ b/devtools/check-tracepoint.sh
@@ -0,0 +1,223 @@
+#!/bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Neil Horman <nhorman@tuxdriver.com>
+# Copyright(C) 2023 Marvell.
+
+selfdir=$(dirname $(readlink -f $0))
+
+# Library for trace check
+libdir="cryptodev ethdev eventdev mempool"
+
+# Functions for which the trace check can be skipped
+skiplist="$selfdir/trace-skiplist.txt"
+
+build_map_changes()
+{
+	local fname="$1"
+	local mapdb="$2"
+
+	cat "$fname" | awk '
+		# Initialize our variables
+		BEGIN {map="";sym="";ar="";sec=""; in_sec=0; in_map=0}
+
+		# Anything that starts with + or -, followed by an a
+		# and ends in the string .map is the name of our map file
+		# This may appear multiple times in a patch if multiple
+		# map files are altered, and all section/symbol names
+		# appearing between a triggering of this rule and the
+		# next trigger of this rule are associated with this file
+		/[-+] [ab]\/.*\.map/ {map=$2; in_map=1; next}
+
+		# The previous rule catches all .map files, anything else
+		# indicates we left the map chunk.
+		/[-+] [ab]\// {in_map=0}
+
+		# Triggering this rule, which starts a line and ends it
+		# with a { identifies a versioned section.  The section name is
+		# the rest of the line with the + and { symbols removed.
+		# Triggering this rule sets in_sec to 1, which actives the
+		# symbol rule below
+		/^.*{/ {
+			gsub("+", "");
+			if (in_map == 1) {
+				sec=$(NF-1); in_sec=1;
+			}
+		}
+
+		# This rule identifies the end of a section, and disables the
+		# symbol rule
+		/.*}/ {in_sec=0}
+
+		# This rule matches on a + followed by any characters except a :
+		# (which denotes a global vs local segment), and ends with a ;.
+		# The semicolon is removed and the symbol is printed with its
+		# association file name and version section, along with an
+		# indicator that the symbol is a new addition.  Note this rule
+		# only works if we have found a version section in the rule
+		# above (hence the in_sec check) And found a map file (the
+		# in_map check).  If we are not in a map chunk, do nothing.  If
+		# we are in a map chunk but not a section chunk, record it as
+		# unknown.
+		/^+[^}].*[^:*];/ {gsub(";","");sym=$2;
+			if (in_map == 1) {
+				if (in_sec == 1) {
+					print map " " sym " " sec " add"
+				} else {
+					print map " " sym " unknown add"
+				}
+			}
+		}
+
+		# This is the same rule as above, but the rule matches on a
+		# leading - rather than a +, denoting that the symbol is being
+		# removed.
+		/^-[^}].*[^:*];/ {gsub(";","");sym=$2;
+			if (in_map == 1) {
+				if (in_sec == 1) {
+					print map " " sym " " sec " del"
+				} else {
+					print map " " sym " unknown del"
+				}
+			}
+		}' > "$mapdb"
+
+		sort -u "$mapdb" > "$mapdb.2"
+		mv -f "$mapdb.2" "$mapdb"
+
+}
+
+find_trace_fn()
+{
+	local fname="$1"
+
+	cat "$fname" | awk -v symname="$2 *\\\(" '
+		# Initialize the variables.
+		# The variable symname provides a pattern to match
+		# "function_name(", zero or more spaces can be present
+		# between function_name and (.
+		BEGIN {state=0; ln_pth=0}
+
+		# Matches the function name, set state=1.
+		$0 ~ symname {state=1}
+
+		# If closing parentheses with semicolon ");" is found, then it
+		# is not the function definition.
+		/) *;/ {
+			if (state == 1) {
+				state=0
+			}
+		}
+
+		/)/ {
+			if (state == 1) {
+				state=2
+				ln_pth=NR
+				next
+			}
+		}
+
+		# If closing parentheses and then opening braces is found in
+		# adjacent line, then this is the start of function
+		# definition. Check for tracepoint in the function definition.
+		# The tracepoint has _trace_ in its name.
+		/{/ {
+			if (state == 2) {
+				if (ln_pth != NR - 1) {
+					state=0
+					next
+				}
+				while (index($0, "}") != 2) {
+					if (index($0, "_trace_") != 0) {
+						exit 0
+					}
+					if (getline <= 0) {
+						break
+					}
+				}
+				exit 1
+			}
+		}
+	'
+}
+
+check_for_tracepoint()
+{
+	local patch="$1"
+	local mapdb="$2"
+	local skip_sym
+	local libname
+	local secname
+	local mname
+	local ret=0
+	local pdir
+	local libp
+	local libn
+	local sym
+	local ar
+
+	while read -r mname symname secname ar; do
+		pdir=$(echo $mname | awk 'BEGIN {FS="/"};{print $2}')
+		libname=$(echo $mname | awk 'BEGIN {FS="/"};{print $3}')
+		skip_sym=0
+		libp=0
+
+		if [ "$pdir" != "lib" ]; then
+			continue
+		fi
+
+		for libn in $libdir; do
+			if [ $libn = $libname ]; then
+				libp=1
+				break
+			fi
+		done
+
+		if [ $libp -eq 0 ]; then
+			continue
+		fi
+
+		for sym in $(cat $skiplist); do
+			if [ $sym = $symname ]; then
+				skip_sym=1
+				break
+			fi
+		done
+
+		if [ $skip_sym -eq 1 ]; then
+			continue
+		fi
+
+		if [ "$ar" = "add" ] && [ "$secname" = "EXPERIMENTAL" ]; then
+			# Check if new API is added with tracepoint
+			find_trace_fn $patch $symname
+			if [ $? -eq 1 ]; then
+				ret=1
+				echo -n "ERROR: New function $symname is added "
+				echo -n "without a tracepoint. Please add a tracepoint "
+				echo -n "or add the function $symname in "
+				echo "devtools/trace-skiplist.txt to skip this error."
+			fi
+		fi
+	done < "$mapdb"
+
+	return $ret
+}
+
+trap clean_and_exit_on_sig EXIT
+
+mapfile=`mktemp -t dpdk.mapdb.XXXXXX`
+patch=$1
+exit_code=1
+
+clean_and_exit_on_sig()
+{
+	rm -f "$mapfile"
+	exit $exit_code
+}
+
+build_map_changes "$patch" "$mapfile"
+check_for_tracepoint "$patch" "$mapfile"
+exit_code=$?
+rm -f "$mapfile"
+
+exit $exit_code
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 1dee094c7a..471db1d21b 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -10,6 +10,7 @@
 . $(dirname $(readlink -f $0))/load-devel-config
 
 VALIDATE_NEW_API=$(dirname $(readlink -f $0))/check-symbol-change.sh
+VALIDATE_TRACEPOINT=$(dirname $(readlink -f $0))/check-tracepoint.sh
 
 # Enable codespell by default. This can be overwritten from a config file.
 # Codespell can also be enabled by setting DPDK_CHECKPATCH_CODESPELL to a valid path
@@ -386,6 +387,14 @@ check () { # <patch-file> <commit>
 		ret=1
 	fi
 
+	! $verbose || printf '\nChecking API additions with tracepoint :\n'
+	report=$($VALIDATE_TRACEPOINT "$tmpinput")
+	if [ $? -ne 0 ] ; then
+		$headline_printed || print_headline "$subject"
+		printf '%s\n' "$report"
+		ret=1
+	fi
+
 	if [ "$tmpinput" != "$1" ]; then
 		rm -f "$tmpinput"
 		trap - INT
diff --git a/devtools/trace-skiplist.txt b/devtools/trace-skiplist.txt
new file mode 100644
index 0000000000..e69de29bb2
-- 
2.25.1


  reply	other threads:[~2023-03-07 12:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-12  9:23 [PATCH] " Ankur Dwivedi
2022-10-12 11:45 ` [PATCH v2] " Ankur Dwivedi
2022-10-12 13:08   ` Thomas Monjalon
2022-10-12 15:16     ` [EXT] " Ankur Dwivedi
2022-10-12 16:19       ` Thomas Monjalon
2022-10-15 12:58   ` [PATCH v3 0/2] " Ankur Dwivedi
2022-10-15 12:58     ` [PATCH v3 1/2] devtools: move build symbol map function Ankur Dwivedi
2022-10-15 12:58     ` [PATCH v3 2/2] devtools: add tracepoint check in checkpatch Ankur Dwivedi
2022-11-02  4:08     ` [PATCH v3 0/2] " Ankur Dwivedi
2023-03-03 15:58     ` [PATCH v4 " Ankur Dwivedi
2023-03-03 15:58       ` [PATCH v4 1/2] devtools: move build symbol map function Ankur Dwivedi
2023-03-03 15:58       ` [PATCH v4 2/2] devtools: add tracepoint check in checkpatch Ankur Dwivedi
2023-03-07 12:05       ` [PATCH v5 0/1] " Ankur Dwivedi
2023-03-07 12:05         ` Ankur Dwivedi [this message]
2023-05-18 13:45           ` [PATCH v5 1/1] " Ankur Dwivedi
2023-05-18 15:33             ` Stephen Hemminger
2023-08-21 13:53               ` [EXT] " Ankur Dwivedi
2023-08-21 14:46                 ` Morten Brørup
2023-08-30 16:23                   ` Thomas Monjalon
2023-08-30 18:38                     ` Morten Brørup
2023-09-01  2:32                       ` Jerin Jacob
2023-09-01  7:28                         ` Thomas Monjalon
2023-11-14 13:15                           ` Ankur Dwivedi
2023-11-28 13:18         ` [PATCH v5 0/1] " Thomas Monjalon
2023-11-28 14:07           ` [EXT] " Ankur Dwivedi
2023-11-28 15:55             ` Thomas Monjalon
2023-11-30  5:56               ` Ankur Dwivedi
2023-11-30  8:40                 ` Thomas Monjalon
2023-11-30 13:16                   ` Ankur Dwivedi
2023-12-15  6:43         ` [PATCH v6 0/2] " Ankur Dwivedi
2023-12-15  6:43           ` [PATCH v6 1/2] devtools: move build map changes function Ankur Dwivedi
2023-12-15  6:43           ` [PATCH v6 2/2] devtools: add tracepoint check in checkpatch Ankur Dwivedi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230307120514.2774917-2-adwivedi@marvell.com \
    --to=adwivedi@marvell.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).