[v5,3/3] devtools: check flow API doc tables
Checks
Commit Message
The script check-doc-vs-code.sh may be used to add
some automatic checks of the doc.
If run without any argument, a complete check is done.
The optional argument is a git history reference point
to check faster only what has changed since this commit.
In this commit, the only check is for rte_flow tables,
achieved through the script parse-flow-support.sh.
If run without a .ini reference, it prints rte_flow tables.
Note: detected features are marked with the value Y,
while the real .ini file could have special values like I.
The script allow parsing exceptions (exclude or include),
like for bnxt code which lists unsupported items and actions.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
devtools/check-doc-vs-code.sh | 79 ++++++++++++++++++++++++++++++++++
devtools/parse-flow-support.sh | 76 ++++++++++++++++++++++++++++++++
2 files changed, 155 insertions(+)
create mode 100755 devtools/check-doc-vs-code.sh
create mode 100755 devtools/parse-flow-support.sh
Comments
On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> The script check-doc-vs-code.sh may be used to add
> some automatic checks of the doc.
>
> If run without any argument, a complete check is done.
> The optional argument is a git history reference point
> to check faster only what has changed since this commit.
>
> In this commit, the only check is for rte_flow tables,
> achieved through the script parse-flow-support.sh.
> If run without a .ini reference, it prints rte_flow tables.
> Note: detected features are marked with the value Y,
> while the real .ini file could have special values like I.
> The script allow parsing exceptions (exclude or include),
> like for bnxt code which lists unsupported items and actions.
>
Thanks for the scripts.
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> devtools/check-doc-vs-code.sh | 79 ++++++++++++++++++++++++++++++++++
> devtools/parse-flow-support.sh | 76 ++++++++++++++++++++++++++++++++
> 2 files changed, 155 insertions(+)
> create mode 100755 devtools/check-doc-vs-code.sh
> create mode 100755 devtools/parse-flow-support.sh
>
> diff --git a/devtools/check-doc-vs-code.sh b/devtools/check-doc-vs-code.sh
> new file mode 100755
> index 0000000000..6e53d66899
> --- /dev/null
> +++ b/devtools/check-doc-vs-code.sh
> @@ -0,0 +1,79 @@
> +#! /bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2021 Mellanox Technologies, Ltd
> +
> +# Check whether doc & code are in sync.
> +# Optional argument: check only what changed since a commit.
> +trusted_commit=$1 # example: origin/main
> +
> +selfdir=$(dirname $(readlink -f $0))
> +rootdir=$(readlink -f $selfdir/..)
> +
> +result=0
> +
> +# speed up by ignoring Unicode details
> +export LC_COLLATE=C
> +
> +changed_files()
> +{
> + [ -n "$files" ] ||
> + files=$(git diff-tree --name-only -r $trusted_commit..)
> + echo "$files"
> +}
> +
> +has_code_change() # <pattern>
> +{
> + test -n "$(git log --format='%h' -S"$1" $trusted_commit..)"
> +}
> +
> +has_file_change() # <pattern>
> +{
> + changed_files | grep -q "$1"
> +}
> +
> +changed_net_drivers()
> +{
> + net_paths='drivers/net/|doc/guides/nics/features/'
> + [ -n "$drivers" ] ||
> + drivers=$(changed_files |
> + sed -rn "s,^($net_paths)([^./]*).*,\2,p")
> + echo "$drivers"
> +}
I will not reviewed in details yet, but first observation,
when 'trusted_commit' argument is used, the drivers list has many duplicated
entries which makes the output redundant and makes script take too much time.
Getting only unique list may help on it.
On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> The script check-doc-vs-code.sh may be used to add
> some automatic checks of the doc.
>
> If run without any argument, a complete check is done.
> The optional argument is a git history reference point
> to check faster only what has changed since this commit.
>
> In this commit, the only check is for rte_flow tables,
> achieved through the script parse-flow-support.sh.
> If run without a .ini reference, it prints rte_flow tables.
> Note: detected features are marked with the value Y,
> while the real .ini file could have special values like I.
> The script allow parsing exceptions (exclude or include),
> like for bnxt code which lists unsupported items and actions.
>
Overall great to be able to generate and check document against code, also good
to have this by relatively small/simple scripts, thanks for the work.
This helps to remove the maintenance concerns I had.
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> devtools/check-doc-vs-code.sh | 79 ++++++++++++++++++++++++++++++++++
> devtools/parse-flow-support.sh | 76 ++++++++++++++++++++++++++++++++
Will there be automated checks as part of the build system? Presumably in
'developer_mode'?
btw, scripts points out some new features not documented in .ini files, those
are the recently added ones, patch requires a rebase on top of latest code.
> +rte_flow_support() # <category>
> +{
> + title="rte_flow $1s"
> + pattern=$(echo "RTE_FLOW_$1_TYPE_" | awk '{print toupper($0)}')
> + list "$title" "$pattern" | grep -vwE 'void|end'
Should 'RTE_FLOW_ITEM_TYPE_ANY' also excluded, does it have benefit to have it
as listed?
14/05/2021 12:51, Ferruh Yigit:
> On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> > The script check-doc-vs-code.sh may be used to add
> > some automatic checks of the doc.
> >
> > If run without any argument, a complete check is done.
> > The optional argument is a git history reference point
> > to check faster only what has changed since this commit.
> >
> > In this commit, the only check is for rte_flow tables,
> > achieved through the script parse-flow-support.sh.
> > If run without a .ini reference, it prints rte_flow tables.
> > Note: detected features are marked with the value Y,
> > while the real .ini file could have special values like I.
> > The script allow parsing exceptions (exclude or include),
> > like for bnxt code which lists unsupported items and actions.
> >
>
> Overall great to be able to generate and check document against code, also good
> to have this by relatively small/simple scripts, thanks for the work.
> This helps to remove the maintenance concerns I had.
>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > devtools/check-doc-vs-code.sh | 79 ++++++++++++++++++++++++++++++++++
> > devtools/parse-flow-support.sh | 76 ++++++++++++++++++++++++++++++++
>
> Will there be automated checks as part of the build system? Presumably in
> 'developer_mode'?
I think we should discuss what could enter in developer mode or not.
I'm afraid it will become a mini-CI and will make compilation longer for everybody.
> btw, scripts points out some new features not documented in .ini files, those
> are the recently added ones, patch requires a rebase on top of latest code.
OK I am rebasing.
> > +rte_flow_support() # <category>
> > +{
> > + title="rte_flow $1s"
> > + pattern=$(echo "RTE_FLOW_$1_TYPE_" | awk '{print toupper($0)}')
> > + list "$title" "$pattern" | grep -vwE 'void|end'
>
> Should 'RTE_FLOW_ITEM_TYPE_ANY' also excluded, does it have benefit to have it
> as listed?
This item may be unsupported by some PMDs,
so I think we need to report it.
13/05/2021 20:40, Ferruh Yigit:
> On 4/7/2021 11:33 PM, Thomas Monjalon wrote:
> > +changed_files()
> > +{
> > + [ -n "$files" ] ||
> > + files=$(git diff-tree --name-only -r $trusted_commit..)
> > + echo "$files"
> > +}
> > +
> > +has_code_change() # <pattern>
> > +{
> > + test -n "$(git log --format='%h' -S"$1" $trusted_commit..)"
> > +}
> > +
> > +has_file_change() # <pattern>
> > +{
> > + changed_files | grep -q "$1"
> > +}
> > +
> > +changed_net_drivers()
> > +{
> > + net_paths='drivers/net/|doc/guides/nics/features/'
> > + [ -n "$drivers" ] ||
> > + drivers=$(changed_files |
> > + sed -rn "s,^($net_paths)([^./]*).*,\2,p")
> > + echo "$drivers"
> > +}
>
> I will not reviewed in details yet, but first observation,
> when 'trusted_commit' argument is used, the drivers list has many duplicated
> entries which makes the output redundant and makes script take too much time.
> Getting only unique list may help on it.
Yes good catch, thanks.
new file mode 100755
@@ -0,0 +1,79 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2021 Mellanox Technologies, Ltd
+
+# Check whether doc & code are in sync.
+# Optional argument: check only what changed since a commit.
+trusted_commit=$1 # example: origin/main
+
+selfdir=$(dirname $(readlink -f $0))
+rootdir=$(readlink -f $selfdir/..)
+
+result=0
+
+# speed up by ignoring Unicode details
+export LC_COLLATE=C
+
+changed_files()
+{
+ [ -n "$files" ] ||
+ files=$(git diff-tree --name-only -r $trusted_commit..)
+ echo "$files"
+}
+
+has_code_change() # <pattern>
+{
+ test -n "$(git log --format='%h' -S"$1" $trusted_commit..)"
+}
+
+has_file_change() # <pattern>
+{
+ changed_files | grep -q "$1"
+}
+
+changed_net_drivers()
+{
+ net_paths='drivers/net/|doc/guides/nics/features/'
+ [ -n "$drivers" ] ||
+ drivers=$(changed_files |
+ sed -rn "s,^($net_paths)([^./]*).*,\2,p")
+ echo "$drivers"
+}
+
+all_net_drivers()
+{
+ find $rootdir/drivers/net -mindepth 1 -maxdepth 1 -type d |
+ sed 's,.*/,,' |
+ sort
+}
+
+check_rte_flow() # <driver>
+{
+ code=$rootdir/drivers/net/$1
+ doc=$rootdir/doc/guides/nics/features/$1.ini
+ [ -d $code ] || return 0
+ [ -f $doc ] || return 0
+ report=$($selfdir/parse-flow-support.sh $code $doc)
+ if [ -n "$report" ]; then
+ echo "rte_flow doc out of sync for $1"
+ echo "$report" | sed 's,^,\t,'
+ result=$(($result + 1))
+ fi
+}
+
+if [ -z "$trusted_commit" ]; then
+ # check all
+ for driver in $(all_net_drivers); do
+ check_rte_flow $driver
+ done
+ exit $result
+fi
+
+# find what changed and check
+if has_code_change 'RTE_FLOW_.*_TYPE_' ||
+ has_file_change 'doc/guides/nics/features'; then
+ for driver in $(changed_net_drivers); do
+ check_rte_flow $driver
+ done
+fi
+exit $result
new file mode 100755
@@ -0,0 +1,76 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2021 Mellanox Technologies, Ltd
+
+# Parse rte_flow support of a driver directory,
+# and optionally show difference with a doc file in .ini format.
+
+dir=$1 # drivers/net/foo
+ref=$2 # doc/guides/nics/features/foo.ini
+
+if [ -z "$dir" ]; then
+ echo "directory argument is required" >&2
+ exit 1
+fi
+
+# sorting order
+export LC_COLLATE=C
+
+# exclude exceptions
+exclude() # <pattern>
+{
+ case $(basename $dir) in
+ bnxt)
+ filter=$(sed -n "/$1/{N;/TYPE_NOT_SUPPORTED/P;}" \
+ $dir/tf_ulp/ulp_template_db{,_tbl}.c |
+ grep -wo "$1[[:alnum:]_]*" | sort -u |
+ tr '\n' '|' | sed 's,.$,\n,')
+ grep -vE "$filter";;
+ *) cat
+ esac
+}
+
+# include exceptions
+include() # <pattern>
+{
+ case $(basename $dir) in
+ esac
+}
+
+# generate INI section
+list() # <title> <pattern>
+{
+ echo "[$1]"
+ git grep -who "$2[[:alnum:]_]*" $dir |
+ (exclude $2; include $2) | sort -u |
+ awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}'
+}
+
+rte_flow_support() # <category>
+{
+ title="rte_flow $1s"
+ pattern=$(echo "RTE_FLOW_$1_TYPE_" | awk '{print toupper($0)}')
+ list "$title" "$pattern" | grep -vwE 'void|end'
+}
+
+if [ -z "$ref" ]; then
+ rte_flow_support item
+ echo
+ rte_flow_support action
+ exit 0
+fi
+
+rte_flow_compare() # <category>
+{
+ section="rte_flow $1s]"
+ {
+ rte_flow_support $1
+ sed -n "/$section/,/]/p" "$ref" | sed '/^$/d'
+ } |
+ sed '/]/d' | # ignore section title
+ sed 's, *=.*,,' | # ignore value (better in doc than generated one)
+ sort | uniq -u # show differences
+}
+
+rte_flow_compare item
+rte_flow_compare action