From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6C8E24591C; Fri, 6 Sep 2024 10:29:52 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5BD3842ED4; Fri, 6 Sep 2024 10:29:52 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 9CE224029E for ; Fri, 6 Sep 2024 10:29:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1725611390; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:autocrypt:autocrypt; bh=ALGGHj7ARkpV9w2uUVrHnkrieH19CPjsCGeZXiTAPW4=; b=WDXBatYmdQZvvmtRgBLz/Qf7oxPY9EYLRRlLHYIcIs+0YY1b1V9o7cRSVd47OfmT2yomey FF78Wfy0c4+2Qge+3mWxyuH3EGZrbEs3BABypghLKUn0S/famKsGKelC5hKRAwS0jkKpgS OK4YCNMTim0BZlG2MQNF8sIq+v4paiE= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-640-691WFOmcNPqgg-LMxAPGwQ-1; Fri, 06 Sep 2024 04:29:48 -0400 X-MC-Unique: 691WFOmcNPqgg-LMxAPGwQ-1 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 12C3819560BD; Fri, 6 Sep 2024 08:29:47 +0000 (UTC) Received: from [10.39.208.34] (unknown [10.39.208.34]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 338C919560AA; Fri, 6 Sep 2024 08:29:44 +0000 (UTC) Message-ID: <49b4e187-caac-4fb8-bfae-0182bb3635db@redhat.com> Date: Fri, 6 Sep 2024 10:29:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC 1/3] uapi: introduce kernel uAPI headers importation To: David Marchand Cc: dev@dpdk.org, techboard@dpdk.org, thomas@monjalon.net References: <20240905221528.1861323-1-maxime.coquelin@redhat.com> <20240905221528.1861323-2-maxime.coquelin@redhat.com> From: Maxime Coquelin Autocrypt: addr=maxime.coquelin@redhat.com; keydata= xsFNBFOEQQIBEADjNLYZZqghYuWv1nlLisptPJp+TSxE/KuP7x47e1Gr5/oMDJ1OKNG8rlNg kLgBQUki3voWhUbMb69ybqdMUHOl21DGCj0BTU3lXwapYXOAnsh8q6RRM+deUpasyT+Jvf3a gU35dgZcomRh5HPmKMU4KfeA38cVUebsFec1HuJAWzOb/UdtQkYyZR4rbzw8SbsOemtMtwOx YdXodneQD7KuRU9IhJKiEfipwqk2pufm2VSGl570l5ANyWMA/XADNhcEXhpkZ1Iwj3TWO7XR uH4xfvPl8nBsLo/EbEI7fbuUULcAnHfowQslPUm6/yaGv6cT5160SPXT1t8U9QDO6aTSo59N jH519JS8oeKZB1n1eLDslCfBpIpWkW8ZElGkOGWAN0vmpLfdyiqBNNyS3eGAfMkJ6b1A24un /TKc6j2QxM0QK4yZGfAxDxtvDv9LFXec8ENJYsbiR6WHRHq7wXl/n8guyh5AuBNQ3LIK44x0 KjGXP1FJkUhUuruGyZsMrDLBRHYi+hhDAgRjqHgoXi5XGETA1PAiNBNnQwMf5aubt+mE2Q5r qLNTgwSo2dpTU3+mJ3y3KlsIfoaxYI7XNsPRXGnZi4hbxmeb2NSXgdCXhX3nELUNYm4ArKBP LugOIT/zRwk0H0+RVwL2zHdMO1Tht1UOFGfOZpvuBF60jhMzbQARAQABzSxNYXhpbWUgQ29x dWVsaW4gPG1heGltZS5jb3F1ZWxpbkByZWRoYXQuY29tPsLBeAQTAQIAIgUCV3u/5QIbAwYL CQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQyjiNKEaHD4ma2g/+P+Hg9WkONPaY1J4AR7Uf kBneosS4NO3CRy0x4WYmUSLYMLx1I3VH6SVjqZ6uBoYy6Fs6TbF6SHNc7QbB6Qjo3neqnQR1 71Ua1MFvIob8vUEl3jAR/+oaE1UJKrxjWztpppQTukIk4oJOmXbL0nj3d8dA2QgHdTyttZ1H xzZJWWz6vqxCrUqHU7RSH9iWg9R2iuTzii4/vk1oi4Qz7y/q8ONOq6ffOy/t5xSZOMtZCspu Mll2Szzpc/trFO0pLH4LZZfz/nXh2uuUbk8qRIJBIjZH3ZQfACffgfNefLe2PxMqJZ8mFJXc RQO0ONZvwoOoHL6CcnFZp2i0P5ddduzwPdGsPq1bnIXnZqJSl3dUfh3xG5ArkliZ/++zGF1O wvpGvpIuOgLqjyCNNRoR7cP7y8F24gWE/HqJBXs1qzdj/5Hr68NVPV1Tu/l2D1KMOcL5sOrz 2jLXauqDWn1Okk9hkXAP7+0Cmi6QwAPuBT3i6t2e8UdtMtCE4sLesWS/XohnSFFscZR6Vaf3 gKdWiJ/fW64L6b9gjkWtHd4jAJBAIAx1JM6xcA1xMbAFsD8gA2oDBWogHGYcScY/4riDNKXi lw92d6IEHnSf6y7KJCKq8F+Jrj2BwRJiFKTJ6ChbOpyyR6nGTckzsLgday2KxBIyuh4w+hMq TGDSp2rmWGJjASrOwU0EVPSbkwEQAMkaNc084Qvql+XW+wcUIY+Dn9A2D1gMr2BVwdSfVDN7 0ZYxo9PvSkzh6eQmnZNQtl8WSHl3VG3IEDQzsMQ2ftZn2sxjcCadexrQQv3Lu60Tgj7YVYRM H+fLYt9W5YuWduJ+FPLbjIKynBf6JCRMWr75QAOhhhaI0tsie3eDsKQBA0w7WCuPiZiheJaL 4MDe9hcH4rM3ybnRW7K2dLszWNhHVoYSFlZGYh+MGpuODeQKDS035+4H2rEWgg+iaOwqD7bg CQXwTZ1kSrm8NxIRVD3MBtzp9SZdUHLfmBl/tLVwDSZvHZhhvJHC6Lj6VL4jPXF5K2+Nn/Su CQmEBisOmwnXZhhu8ulAZ7S2tcl94DCo60ReheDoPBU8PR2TLg8rS5f9w6mLYarvQWL7cDtT d2eX3Z6TggfNINr/RTFrrAd7NHl5h3OnlXj7PQ1f0kfufduOeCQddJN4gsQfxo/qvWVB7PaE 1WTIggPmWS+Xxijk7xG6x9McTdmGhYaPZBpAxewK8ypl5+yubVsE9yOOhKMVo9DoVCjh5To5 aph7CQWfQsV7cd9PfSJjI2lXI0dhEXhQ7lRCFpf3V3mD6CyrhpcJpV6XVGjxJvGUale7+IOp sQIbPKUHpB2F+ZUPWds9yyVxGwDxD8WLqKKy0WLIjkkSsOb9UBNzgRyzrEC9lgQ/ABEBAAHC wV8EGAECAAkFAlT0m5MCGwwACgkQyjiNKEaHD4nU8hAAtt0xFJAy0sOWqSmyxTc7FUcX+pbD KVyPlpl6urKKMk1XtVMUPuae/+UwvIt0urk1mXi6DnrAN50TmQqvdjcPTQ6uoZ8zjgGeASZg jj0/bJGhgUr9U7oG7Hh2F8vzpOqZrdd65MRkxmc7bWj1k81tOU2woR/Gy8xLzi0k0KUa8ueB iYOcZcIGTcs9CssVwQjYaXRoeT65LJnTxYZif2pfNxfINFzCGw42s3EtZFteczClKcVSJ1+L +QUY/J24x0/ocQX/M1PwtZbB4c/2Pg/t5FS+s6UB1Ce08xsJDcwyOPIH6O3tccZuriHgvqKP yKz/Ble76+NFlTK1mpUlfM7PVhD5XzrDUEHWRTeTJSvJ8TIPL4uyfzhjHhlkCU0mw7Pscyxn DE8G0UYMEaNgaZap8dcGMYH/96EfE5s/nTX0M6MXV0yots7U2BDb4soLCxLOJz4tAFDtNFtA wLBhXRSvWhdBJZiig/9CG3dXmKfi2H+wdUCSvEFHRpgo7GK8/Kh3vGhgKmnnxhl8ACBaGy9n fxjSxjSO6rj4/MeenmlJw1yebzkX8ZmaSi8BHe+n6jTGEFNrbiOdWpJgc5yHIZZnwXaW54QT UhhSjDL1rV2B4F28w30jYmlRmm2RdN7iCZfbyP3dvFQTzQ4ySquuPkIGcOOHrvZzxbRjzMx1 Mwqu3GQ= In-Reply-To: X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 9/6/24 09:13, David Marchand wrote: > On Fri, Sep 6, 2024 at 12:15 AM Maxime Coquelin > wrote: >> >> This patch introduces uAPI headers importation 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, and >> libraries and drivers have to explicitly enable their >> inclusion. >> >> Guidelines are provided in the documentation, and a helper >> script is also provided to ensure proper importation of the >> header (unmodified content from a released Kernel version). >> >> Next version will introduce a script to check headers are >> valids. >> >> Signed-off-by: Maxime Coquelin >> --- >> devtools/import-linux-uapi.sh | 48 ++++++++++++++++++++ >> doc/guides/contributing/index.rst | 1 + >> doc/guides/contributing/linux_uapi.rst | 63 ++++++++++++++++++++++++++ >> meson.build | 4 ++ >> 4 files changed, 116 insertions(+) >> create mode 100755 devtools/import-linux-uapi.sh >> create mode 100644 doc/guides/contributing/linux_uapi.rst >> >> diff --git a/devtools/import-linux-uapi.sh b/devtools/import-linux-uapi.sh >> new file mode 100755 >> index 0000000000..efeffdd332 >> --- /dev/null >> +++ b/devtools/import-linux-uapi.sh >> @@ -0,0 +1,48 @@ >> +#!/bin/sh -e >> +# SPDX-License-Identifier: BSD-3-Clause >> +# Copyright (c) 2024 Red Hat, Inc. >> + >> +# >> +# Import Linux Kernel uAPI header file >> +# >> + >> +base_url="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/include/uapi/" >> +base_path="linux-headers/uapi/" >> + >> +print_usage() >> +{ >> + echo "Usage: $(basename $0) [-h] [file] [version]" > > file and version are not optional. > So they should not be surrounded with []. Ok > > >> + echo "Example of valid file is linux/vfio.h" >> + echo "Example of valid version is v6.10" >> +} >> + >> +while getopts hv ARG ; do >> + case $ARG in >> + h ) print_usage; exit 0 ;; >> + ? ) print_usage; exit 1 ;; >> + esac >> +done >> +shift $(($OPTIND - 1)) >> + >> +if [ $# -ne 2 ]; then >> + print_usage; exit 1; > > For consistency with the rest of the script, don't use ; Ok > >> +fi >> + >> +file=$1 >> +version=$2 >> + >> +url="${base_url}${file}?h=${version}" >> +path="${base_path}${file}" >> + >> +# Move to the root of the DPDK tree >> +cd $(dirname $0)/.. >> + >> +# Check file and version are valid >> +curl -s -o /dev/null -w "%{http_code}" $url | grep -q "200" > > Can we rely on curl to report such errors? > -f is probably the right option. > > @@ -37,12 +37,9 @@ path="${base_path}${file}" > # Move to the root of the DPDK tree > cd $(dirname $0)/.. > > -# Check file and version are valid > -curl -s -o /dev/null -w "%{http_code}" $url | grep -q "200" > - > # Create path if needed > mkdir -p $(dirname $path) > > # Download the file > -curl -s -o $path $url > +curl -s -f -o $path $url > > $ ./devtools/import-linux-uapi.sh linux/vdplop.h v6.10; echo $? > 22 OK, what about this to get rid of the mkdir? diff --git a/devtools/import-linux-uapi.sh b/devtools/import-linux-uapi.sh index efeffdd332..3769da80bb 100755 --- a/devtools/import-linux-uapi.sh +++ b/devtools/import-linux-uapi.sh @@ -37,12 +37,6 @@ path="${base_path}${file}" # Move to the root of the DPDK tree cd $(dirname $0)/.. -# Check file and version are valid -curl -s -o /dev/null -w "%{http_code}" $url | grep -q "200" - -# Create path if needed -mkdir -p $(dirname $path) - # Download the file -curl -s -o $path $url +curl -s -f --create-dirs -o $path $url The only downside in both your version and this one is that versus initial one is that the directory gets created if curl failed. We can though combine the best of both worlds: $ git diff diff --git a/devtools/import-linux-uapi.sh b/devtools/import-linux-uapi.sh index efeffdd332..857d3dd33b 100755 --- a/devtools/import-linux-uapi.sh +++ b/devtools/import-linux-uapi.sh @@ -34,15 +34,9 @@ version=$2 url="${base_url}${file}?h=${version}" path="${base_path}${file}" -# Move to the root of the DPDK tree -cd $(dirname $0)/.. - -# Check file and version are valid -curl -s -o /dev/null -w "%{http_code}" $url | grep -q "200" - -# Create path if needed -mkdir -p $(dirname $path) +# Check URL is valid +curl -s -f -o /dev/null $url # Download the file -curl -s -o $path $url +curl -s -f --create-dirs -o $path $url $ ./devtools/import-linux-uapi.sh linux/vduse.h v6.10 $ ./devtools/import-linux-uapi.sh linuxxx/vduse.h v6.10 $ find linux-headers/ linux-headers/ linux-headers/uapi linux-headers/uapi/.gitignore linux-headers/uapi/linux linux-headers/uapi/linux/vduse.h What do you prefer? > > >> + >> +# Create path if needed >> +mkdir -p $(dirname $path) >> + >> +# Download the file >> +curl -s -o $path $url >> + > > No need for a blank line at the end of the file. Ack > >> diff --git a/doc/guides/contributing/index.rst b/doc/guides/contributing/index.rst >> index dcb9b1fbf0..603dc72654 100644 >> --- a/doc/guides/contributing/index.rst >> +++ b/doc/guides/contributing/index.rst >> @@ -19,3 +19,4 @@ Contributor's Guidelines >> vulnerability >> stable >> cheatsheet >> + linux_uapi >> diff --git a/doc/guides/contributing/linux_uapi.rst b/doc/guides/contributing/linux_uapi.rst >> new file mode 100644 >> index 0000000000..3bfd05eb62 >> --- /dev/null >> +++ b/doc/guides/contributing/linux_uapi.rst >> @@ -0,0 +1,63 @@ >> +.. SPDX-License-Identifier: BSD-3-Clause >> + Copyright(c) 2024 Red Hat, Inc. >> + >> +Linux uAPI header files >> +======================= >> + >> + > > Single empty line. Ack > >> +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 > > Please start sentences on a new line. > It won't affect the generated documentation and it slightly enhance > readability, code churn when updating another sentence etc... > > >> +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 >> +`_ >> +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 ID:`` >> +tag and title MUST be prefixed with uapi keywork. For example:: >> + >> + uapi: import VDUSE header file > > For the very first import of header lambda.h, ok. > > But can we make sure people will do better titles? > Importing such headers must be done only when needed (maybe put some > guidelines on this topic in the doc too?), and so the commit title > should reflect the reason why DPDK needs an update. > uapi: import awesome VDUSE feature definitions I agree, I will update the guidelines to reflect this. > > >> + >> + This patch imports VDUSE uAPI header file. >> + >> + uAPI Version: v6.10 > > uAPI ID ? Will fix. Prefer version than ID. > >> + >> + Signed-off-by: Alex Smith >> + >> +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 > > Missing an empty line here. OK, I think CI reported it. > > >> + includes += linux_uapi_inc > > I would rather have this hidden in global_inc meson.build. Ok, so to make it clear, you would be in favor of having these uAPI available to all libs and drivers as soon as it is a Linux build. I'm fine with it, but wanted to make sure other reviewers are aware and agree. Note that the includes will have to be prefixed with uapi/, so it will not change the default behaviour for existing linux/ includes >> + >> +Then, it can be included with ``uapi/`` prefix in C files. For example to >> +include VDUSE uAPI: >> + >> +.. code-block:: c > > Missing an empty line here. Ack > >> + #include >> + >> diff --git a/meson.build b/meson.build >> index 8b248d4505..53cdaef558 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -77,6 +77,10 @@ global_inc = include_directories('.', 'config', >> subdir('buildtools') >> subdir('config') >> >> +if is_linux >> + linux_uapi_inc = include_directories('linux-headers') >> +endif > > s/linux_uapi_inc/global_inc/. Ok. I think I have some materials to post a second revision of the RFC, so that build failures are fixed. Thanks, Maxime > >> + >> # build libs and drivers >> subdir('lib') >> subdir('drivers') >> -- >> 2.46.0 >> > >