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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 9/17/24 13:36, David Marchand wrote:
> On Wed, Sep 11, 2024 at 9:32 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch introduces uAPI headers import into the DPDK
>> repository. This import is possible thanks to Linux Kernel
>> licence exception for syscalls:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/LICENSES/exceptions/Linux-syscall-note
>>
>> Header files are have to be explicitly imported.
>>
>> Guidelines are provided in the documentation, and helper
>> scripts are also provided to ensure proper importation of the
>> header (unmodified content from a released Kernel version):
>>   - import-linux-uapi.sh: used to add and update headers and
>>   their dependencies to linux-headers/uapi/
>>   - check-linux-uapi.sh: used to check all headers are valid
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> I have been trying this script to import linux/vfio.h and cleanup its
> usage in DPDK.
> 
> There is one issue that was raised.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/vfio.h#n1573
> 
> struct vfio_bitmap {
> __u64        pgsize; /* page size for bitmap in bytes */
> __u64        size; /* in bytes */
> __u64 __user *data; /* one bit per page */
> };
> 
> The __user annotation is sanitized by the headers install script in
> the kernel, but the dpdk import script is missing this part.
> Such sanitizations breaks the check script.
> 
> We could invert the logic in the check script: instead of "restoring"
> an imported header, the check would convert a freshly downloaded
> header and compare it against the imported header in dpdk.
> One thing though is that we would need a copy of the "conversion"
> function in the two scripts.
> 
> One idea.. can we have a single script?
> 
> # Interactive mode, with questions about what to import if dependencies exist:
> $ devtools/linux-uapi.sh import linux/vfio.h v6.10
> 
> # Non interactive mode, the script uses the version file and imported headers:
> $ devtools/linux-uapi.sh check
> 
> 
> Regardless of this suggestion, I have some nits about the shell scripts below:
> 
>> ---
>>   devtools/check-linux-uapi.sh           |  85 ++++++++++++++++++
>>   devtools/import-linux-uapi.sh          | 119 +++++++++++++++++++++++++
>>   doc/guides/contributing/index.rst      |   1 +
>>   doc/guides/contributing/linux_uapi.rst |  71 +++++++++++++++
>>   linux-headers/uapi/.gitignore          |   4 +
>>   linux-headers/uapi/version             |   1 +
>>   meson.build                            |   8 +-
>>   7 files changed, 287 insertions(+), 2 deletions(-)
>>   create mode 100755 devtools/check-linux-uapi.sh
>>   create mode 100755 devtools/import-linux-uapi.sh
>>   create mode 100644 doc/guides/contributing/linux_uapi.rst
>>   create mode 100644 linux-headers/uapi/.gitignore
>>   create mode 100644 linux-headers/uapi/version
>>

Thanks for the deep review!
I think I addressed all the comments in upcoming V1

Maxime