[v8,10/10] devtools: check event device doc tables

  From: Sunil Kumar Kori <skori@marvell.com>

In this commit, check is added for event device, Rx,
Tx, Crypto and Timer adapters tables for all supported
drivers.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v8:
 - Fix script dump for dsw and SW driver
 - Review comments incorporated
v7:
 - Rebased to 21.11 rc3
 - Fix Tx adapter capabilities for SW driver
v6:
 - Rebased to 21.11 rc2
 - Added feature matrices for all supported drivers
 - Added doc vs code check script
 
v5:
 - Rebased to 21.11
 - Added feature matrix for cnxk.
v4:
 - Rebased to 20.02
v3:
 - Removed .txt files to generate tables.
 - Use conf.py script to generate tables.
 - Add .ini file for all supported PMDs.
v2:
 - Review comments incorporated

 devtools/check-doc-vs-code.sh   |  35 +++++++
 devtools/parse-event-support.sh | 165 ++++++++++++++++++++++++++++++++
 2 files changed, 200 insertions(+)
 create mode 100755 devtools/parse-event-support.sh
  

23/11/2021 12:07, skori@marvell.com:
> --- a/devtools/check-doc-vs-code.sh
> +++ b/devtools/check-doc-vs-code.sh
> +all_event_drivers()
> +{
> +	find $rootdir/drivers/event -mindepth 1 -maxdepth 1 -type d |
> +	sed 's,.*/,,' |
> +	sort
> +}
> +
> +check_event_dev() # <driver>
> +{
> +	code=$rootdir/drivers/event/$1
> +	doc=$rootdir/doc/guides/eventdevs/features/$1.ini
> +	[ -d $code ] || return 0
> +	[ -f $doc ] || return 0
> +	report=$($selfdir/parse-event-support.sh $code $doc)
> +	if [ -n "$report" ]; then
> +		error "doc out of sync for $1"
> +		echo "$report" | sed 's,^,\t,'
> +	fi
> +}

These 2 functions are mostly copy/paste of rte_flow functions.
Given there will be more in future, I would prefer code being factorized.

>  if [ -z "$trusted_commit" ]; then
>  	# check all
>  	for driver in $(all_net_drivers); do
>  		check_rte_flow $driver
>  	done
> +

I would remove this blank line.

> +	for driver in $(all_event_drivers); do
> +		check_event_dev $driver
> +	done
>  	exit $result
>  fi
[...]
> +if has_code_change 'RTE_EVENT_DEV_CAP_*' ||
> +		has_code_change 'RTE_EVENT_ETH_RX_ADAPTER_CAP_*' ||
> +		has_code_change 'RTE_EVENT_ETH_TX_ADAPTER_CAP_*' ||
> +		has_code_change 'RTE_EVENT_CRYPTO_ADAPTER_CAP_*' ||
> +		has_code_change 'RTE_EVENT_TIMER_ADAPTER_CAP_*' ||

Can it be a single query?

> +		has_file_change 'doc/guides/eventdevs/features'; then
> +	for driver in $(all_event_drivers); do

No need to check all drivers.
For rte_flow, only changed drivers are checked.

> +		check_event_dev $driver
> +	done
> +fi
[...]
> +# generate INI section
> +list() # <title> <pattern> <extra_patterns>
> +{
> +	echo "[$1]"
> +	word0=$(git grep -who "$2[[:alnum:]_]*" $dir)
> +	word1=$(echo "$3")

Why echo?

> +	words="$word0""$word1"

Why so many quotes?

> +	echo "$words" | sort -u |
> +		awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}'
> +}
[...]
> +event_dev_rx_adptr_support()
> +{
> +	title="Eth Rx adapter Features"
> +	pattern=$(echo "RTE_EVENT_ETH_RX_ADAPTER_CAP_" |
> +		awk '{print toupper($0)}')
> +	check_rx_adptr_sw_capa || extra='RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID
> +				RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
> +				RTE_EVENT_ETH_RX_ADAPTER_CAP_EVENT_VECTOR'

Why having extra parameter, instead of updating the pattern?
By the way, the pattern RTE_EVENT_ETH_RX_ADAPTER_CAP_ already include all of this.

> +	list "$title" "$pattern" "$extra"
> +}
[...]
> +# compare with reference input
> +event_dev_sched_compare()
> +{
> +	section="Scheduling Features]"
> +	{
> +		event_dev_sched_support
> +		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
> +	sed "s,^,Scheduling Features ," # prefix with category name
> +}
> +
> +event_dev_rx_adptr_compare()
> +{
> +	section="Eth Rx adapter Features]"
> +	{
> +		event_dev_rx_adptr_support
> +		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
> +	sed "s,^,Eth Rx adapter Features ," # prefix with category name
> +}
> +
> +event_dev_tx_adptr_compare()
> +{
> +	section="Eth Tx adapter Features]"
> +	{
> +		event_dev_tx_adptr_support
> +		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
> +	sed "s,^,Eth Tx adapter Features ," # prefix with category name
> +}
> +
> +event_dev_crypto_adptr_compare()
> +{
> +	section="Crypto adapter Features]"
> +	{
> +		event_dev_crypto_adptr_support
> +		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
> +	sed "s,^,Crypto adapter Features ," # prefix with category name
> +}
> +
> +event_dev_timer_adptr_compare()
> +{
> +	section="Timer adapter Features]"
> +	{
> +		event_dev_timer_adptr_support
> +		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
> +	sed "s,^,Timer adapter Features ," # prefix with category name
> +}

I think these functions can be factorized.
  

Hi Thomas,

It will take some time to handle all the comments for this patch. So I would request you to defer this patch for next release and take other patches in series. 
Also I will send one patch to add feature matrix for dlb2 platform and fix minor review comments given by You. 

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Wednesday, November 24, 2021 4:23 PM
>To: Sunil Kumar Kori <skori@marvell.com>
>Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; nikhil.rao@intel.com;
>Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>hemant.agrawal@nxp.com; nipun.gupta@nxp.com;
>harry.van.haaren@intel.com; mattias.ronnblom@ericsson.com;
>liang.j.ma@intel.com; dev@dpdk.org
>Subject: [EXT] Re: [PATCH v8 10/10] devtools: check event device doc tables
>
>External Email
>
>----------------------------------------------------------------------
>23/11/2021 12:07, skori@marvell.com:
>> --- a/devtools/check-doc-vs-code.sh
>> +++ b/devtools/check-doc-vs-code.sh
>> +all_event_drivers()
>> +{
>> +	find $rootdir/drivers/event -mindepth 1 -maxdepth 1 -type d |
>> +	sed 's,.*/,,' |
>> +	sort
>> +}
>> +
>> +check_event_dev() # <driver>
>> +{
>> +	code=$rootdir/drivers/event/$1
>> +	doc=$rootdir/doc/guides/eventdevs/features/$1.ini
>> +	[ -d $code ] || return 0
>> +	[ -f $doc ] || return 0
>> +	report=$($selfdir/parse-event-support.sh $code $doc)
>> +	if [ -n "$report" ]; then
>> +		error "doc out of sync for $1"
>> +		echo "$report" | sed 's,^,\t,'
>> +	fi
>> +}
>
>These 2 functions are mostly copy/paste of rte_flow functions.
>Given there will be more in future, I would prefer code being factorized.
>
>>  if [ -z "$trusted_commit" ]; then
>>  	# check all
>>  	for driver in $(all_net_drivers); do
>>  		check_rte_flow $driver
>>  	done
>> +
>
>I would remove this blank line.
>
>> +	for driver in $(all_event_drivers); do
>> +		check_event_dev $driver
>> +	done
>>  	exit $result
>>  fi
>[...]
>> +if has_code_change 'RTE_EVENT_DEV_CAP_*' ||
>> +		has_code_change 'RTE_EVENT_ETH_RX_ADAPTER_CAP_*' ||
>> +		has_code_change 'RTE_EVENT_ETH_TX_ADAPTER_CAP_*' ||
>> +		has_code_change 'RTE_EVENT_CRYPTO_ADAPTER_CAP_*' ||
>> +		has_code_change 'RTE_EVENT_TIMER_ADAPTER_CAP_*' ||
>
>Can it be a single query?
>
>> +		has_file_change 'doc/guides/eventdevs/features'; then
>> +	for driver in $(all_event_drivers); do
>
>No need to check all drivers.
>For rte_flow, only changed drivers are checked.
>
>> +		check_event_dev $driver
>> +	done
>> +fi
>[...]
>> +# generate INI section
>> +list() # <title> <pattern> <extra_patterns> {
>> +	echo "[$1]"
>> +	word0=$(git grep -who "$2[[:alnum:]_]*" $dir)
>> +	word1=$(echo "$3")
>
>Why echo?
>
>> +	words="$word0""$word1"
>
>Why so many quotes?
>
>> +	echo "$words" | sort -u |
>> +		awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}'
>> +}
>[...]
>> +event_dev_rx_adptr_support()
>> +{
>> +	title="Eth Rx adapter Features"
>> +	pattern=$(echo "RTE_EVENT_ETH_RX_ADAPTER_CAP_" |
>> +		awk '{print toupper($0)}')
>> +	check_rx_adptr_sw_capa ||
>extra='RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID
>> +
>	RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
>> +
>	RTE_EVENT_ETH_RX_ADAPTER_CAP_EVENT_VECTOR'
>
>Why having extra parameter, instead of updating the pattern?
>By the way, the pattern RTE_EVENT_ETH_RX_ADAPTER_CAP_ already include
>all of this.
>
>> +	list "$title" "$pattern" "$extra"
>> +}
>[...]
>> +# compare with reference input
>> +event_dev_sched_compare()
>> +{
>> +	section="Scheduling Features]"
>> +	{
>> +		event_dev_sched_support
>> +		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
>> +	sed "s,^,Scheduling Features ," # prefix with category name }
>> +
>> +event_dev_rx_adptr_compare()
>> +{
>> +	section="Eth Rx adapter Features]"
>> +	{
>> +		event_dev_rx_adptr_support
>> +		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
>> +	sed "s,^,Eth Rx adapter Features ," # prefix with category name }
>> +
>> +event_dev_tx_adptr_compare()
>> +{
>> +	section="Eth Tx adapter Features]"
>> +	{
>> +		event_dev_tx_adptr_support
>> +		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
>> +	sed "s,^,Eth Tx adapter Features ," # prefix with category name }
>> +
>> +event_dev_crypto_adptr_compare()
>> +{
>> +	section="Crypto adapter Features]"
>> +	{
>> +		event_dev_crypto_adptr_support
>> +		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
>> +	sed "s,^,Crypto adapter Features ," # prefix with category name }
>> +
>> +event_dev_timer_adptr_compare()
>> +{
>> +	section="Timer adapter Features]"
>> +	{
>> +		event_dev_timer_adptr_support
>> +		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
>> +	sed "s,^,Timer adapter Features ," # prefix with category name }
>
>I think these functions can be factorized.
>
  

24/11/2021 12:16, Sunil Kumar Kori:
> Hi Thomas,
> 
> It will take some time to handle all the comments for this patch. So I would request you to defer this patch for next release and take other patches in series. 
> Also I will send one patch to add feature matrix for dlb2 platform and fix minor review comments given by You. 

This is just documentation, so it can be merged
until the last day of the release.
Please try to get the script ready.
If the script is not ready on time, I will merge only the doc.