Re: [RFC v2 1/3] uapi: introduce kernel uAPI headers import

[David Marchand <david.marchand@redhat.com>]

On Fri, Sep 6, 2024 at 5:23 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:

[snip]

> diff --git a/devtools/check-linux-uapi.sh b/devtools/check-linux-uapi.sh
> new file mode 100755
> index 0000000000..76111d78ce
> --- /dev/null
> +++ b/devtools/check-linux-uapi.sh
> @@ -0,0 +1,74 @@
> +#!/bin/sh

-e maybe?

> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2024 Red Hat, Inc.
> +
> +#
> +# Import Linux Kernel uAPI header file

# Check Linux Kernel uAPI headers

> +#
> +
> +base_url="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/include/uapi/"
> +base_path="linux-headers/uapi/"
> +errors=0
> +
> +print_usage()
> +{
> +       echo "Usage: $(basename $0) [-h]"
> +}
> +
> +check_uapi_header() {
> +       path=$1
> +       file=${1//"$base_path"/}

I suspect it is a bashism.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
${parameter#[word]}
Remove Smallest Prefix Pattern. The word shall be expanded to produce
a pattern. The parameter expansion shall then result in parameter,
with the smallest portion of the prefix matched by the pattern
deleted. If present, word shall not begin with an unquoted '#'.

So a POSIX alternative is:
file=${1#$base_path/}


> +       version=$(git log --format=%b -1 $path | sed -ne 's/^uAPI Version: \(.*\)$/\1/p')

I would add a check and raise a warning (error?) if extracting version
fails (iow -z "$version").


> +
> +       url="${base_url}${file}?h=${version}"
> +       echo -n "Checking $file for version $version... "
> +       curl -s -f -o $tmpinput $url
> +       if [ $? -ne 0 ]; then
> +               echo "Failed to download $url"
> +               exit 1
> +       fi
> +
> +       diff -q $path $tmpinput >/dev/null
> +       if [ $? -ne 0 ]; then
> +               echo "KO"
> +               diff -u $path $tmpinput
> +               errors=$((errors+1))
> +       else
> +               echo "OK"
> +       fi
> +}
> +
> +while getopts hv ARG ; do
> +       case $ARG in
> +               h )
> +                       print_usage
> +                       exit 0
> +                       ;;
> +               ? )
> +                       print_usage
> +                       exit 1
> +                       ;;
> +       esac
> +done
> +
> +shift $(($OPTIND - 1))
> +if [ $# -ne 0 ]; then
> +       print_usage
> +       exit 1
> +fi
> +
> +cd $(dirname $0)/..
> +
> +tmpinput=$(mktemp -t dpdk.checkuapi.XXXXXX)
> +trap "rm -f '$tmpinput'" INT
> +
> +while IFS= read -d '' -r filename; do

Simpler:

for filename in $(find $base_path -name "*.h" -type f); do
     check_uapi_header "${filename}" </dev/null
done

> +       check_uapi_header "${filename}" </dev/null
> +done < <(find $base_path -name "*.h" -type f -print0)


> +
> +echo "$errors error(s) found"
> +
> +rm -f $tmpinput
> +trap - INT
> +
> +exit $errors

[snip]

> diff --git a/doc/guides/contributing/linux_uapi.rst b/doc/guides/contributing/linux_uapi.rst
> new file mode 100644
> index 0000000000..a3f684013a
> --- /dev/null
> +++ b/doc/guides/contributing/linux_uapi.rst
> @@ -0,0 +1,77 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +   Copyright(c) 2024 Red Hat, Inc.
> +
> +Linux uAPI header files
> +=======================
> +
> +Rationale
> +---------
> +
> +The system a DPDK library or driver is built on is not necessarily running the
> +same Kernel version than the system that will run it.
> +Importing Linux Kernel uAPI headers enable to build features that are not
> +supported yet by the build system.
> +
> +For example, the build system runs upstream Kernel v5.19 and we would like to
> +build a VDUSE application that will use VDUSE_IOTLB_GET_INFO ioctl() introduced
> +in Linux Kernel v6.0.
> +
> +`Linux Kernel licence exception regarding syscalls
> +<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/LICENSES/exceptions/Linux-syscall-note>`_
> +enable importing unmodified Linux Kernel uAPI header files.
> +
> +Importing or updating an uAPI header file
> +-----------------------------------------
> +
> +In order to ensure the imported uAPI headers are both unmodified and from a
> +released version of the linux Kernel, a helper script is made available and
> +MUST be used.
> +Below is an example to import ``linux/vduse.h`` file from Linux ``v6.10``:
> +
> +.. code-block:: console
> +
> +   ./devtools/import-linux-uapi.sh linux/vduse.h v6.10
> +
> +Once imported, the header files should be committed without any other change,
> +and the commit message MUST specify the imported version using
> +``uAPI Version:`` tag and title MUST be prefixed with uapi keyword.
> +For example::
> +
> +  uapi: import VDUSE header file
> +
> +  This patch imports VDUSE uAPI header file for inclusion
> +  into the Vhost library.
> +
> +  uAPI Version: v6.10
> +
> +  Signed-off-by: Alex Smith <alex.smith@example.com>
> +
> +Updating an already imported header to a newer released version should only
> +be done on a need basis.
> +The commit message should reflect why updating the header is necessary.

+1.


> +
> +Once committed, user can check headers and commit message are valid by using
> +the Linux uAPI checker tool:
> +
> +.. code-block:: console
> +
> +   ./devtools/check-linux-uapi.sh

And thanks for adding this check.
It will help maintainers.


> +
> +Header inclusion into library or driver
> +---------------------------------------
> +
> +The library or driver willing to make use of imported uAPI headers needs to
> +explicitly add uAPI headers path to the ``includes`` var in its ``meson.build``
> +file:
> +
> +.. code-block:: python
> +
> +   includes += linux_uapi_inc

Now that the uapi headers directory is pushed to global_inc, there is
no need for this part in the doc.


> +
> +Then, it can be included with ``uapi/`` prefix in C files.
> +For example to include VDUSE uAPI:
> +
> +.. code-block:: c
> +
> +   #include <uapi/linux/vduse.h>
> +
> diff --git a/linux-headers/uapi/.gitignore b/linux-headers/uapi/.gitignore
> new file mode 100644
> index 0000000000..88829d04e9
> --- /dev/null
> +++ b/linux-headers/uapi/.gitignore
> @@ -0,0 +1,3 @@
> +**
> +!**/
> +!**/*.h

Nice trick to solve the per patch build issue :-).


-- 
David Marchand