Re: [RFC v3 0/3] Import Kernel uAPI header files

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

On 9/12/24 15:16, Ferruh Yigit wrote:
> On 9/12/2024 1:08 PM, Maxime Coquelin wrote:
>> Hi Ferruh,
>>
>> On 9/12/24 10:30, Ferruh Yigit wrote:
>>> On 9/11/2024 8:32 PM, Maxime Coquelin wrote:
>>>> This series enables importing Linux Kernel uAPI headers
>>>> into the DPDK repository. It aims at solving alignment
>>>> issues between the build system and the system applications
>>>> linked ot DPDK libraries are run on.
>>>>
>>>
>>> Hi Maxime,
>>>
>>> Is the imported Linux Kernel uAPI headers are always backward compatible?
>>
>> Yes, at least that's what is advertised! :)
>>
>> "
>> The binary interface provided by the kernel to user space cannot be
>> broken except under the most severe circumstances.
>> "
>>
> 
> Ack.
> 
>>
>>> If I build and run on identical platform, lets say has kernel v5.4,
>>> and DPDK has the kernel uAPI header from v6.10, is this has any chance
>>> to create backward compatibility issues?
>>
>> It should not if backward compatibility is preserved as advertised.
>>
>>> Or can it enable some features which are indeed not exist/supported in
>>> the platform that has old version of kernel?
>>
>> It can enable new feature, like for example a new ioctl command.
>>
>> In this case, it should return something like ENOIOCTLCMD if the Kernel
>> does not support it.
>>
> 
> OK. I think this has the same risk with building dpdk application in a
> platform with newer kernel version, and distribute it to the another
> platform with older kernel. If interface is backward compatible, this
> should be OK.

Yes.

> 
> In this approach we are duplicating the kernel headers, which is not a
> great option. Also this requires maintenance, updating kernel headers in
> DPDK as needed, which is not great in long run.
> 
> I guess intention is to enable a feature that comes with newer kernel
> versions, if so why not force enable it in DPDK, like:
> #ifndef FEATURE_MACRO
> #define FEATURE_MACRO
> #define X
> #define Y
> #define Z
> #endif

That's exactly what my proposal aims at fixing!
We do this a lot in Vhost library, and this is a mess.

> If the kernel uAPI is stable, moving these defines to library should be
> OK.

I see some issues with such solution:

1. This is error prone (e.g. involuntary modifications to redefined
    code). It can be also quite complex/big to redefine (look at
    linux/userfaultfd.h)
2. We copy "GPL v2 WITH Linux-syscall-note" definitions into BSD
    licensed code (but IANAL).

> When this feature hits the distribution kernels we can remove
> defines from the DPDK library.

I think this is easier said than done.
Distribution Kernels backport feature on a need basis, it is difficult 
to ensure all the distribution Kernels have a given feature included.

> 
> OR, won't it help if whoever building DPDK, download kernel headers
> package for necessary version, and build DPDK with them?
> For this documenting required versions, and a how to documentation to
> explain meson commands etc.. may be sufficient.

That would add another step to building DPDK.
It would need to be a mandatory step, otherwise we would have to 
continue with all these #ifndef #define..

I think my proposal is the best compromise:
1. No need for extra pre-requisites for building DPDK
2. UAPI included on a need basis (e.g. only vduse.h, only virtio
    headers, etc..)
3. UAPI headers isolated in a dedicated directory, ensuring no
    involuntary modifications are done to these files thanks to provided
    checker that can be integrated into CI.License of the UAPI headers
    are preseved.
4. Helps with builds reproducibility.

> 
>>>
>>>> It can also help simplify spaghetti code done to support
>>>> different versions of the Linux Kernel headers used by
>>>> the build system.
>>>>
>>>> Guidelines and import script are also part of first patch.
>>>> A uAPI checker script is added in this 3rd RFC, goals is to
>>>> use it in CI for any patch touching linux-headers/uapi.
>>>>
>>>> Follow-up patches are an example of imported uAPI inclusion
>>>> of VDUSE header into the Vhost library.
>>>>
>>>> Morten, I did not (again) apply your Ack on patch 1, as it
>>>> has some significant changes and additions.
>>>>
>>>> Changes in RFC v3:
>>>> ==================
>>>> - Support nested headers include
>>>> - Interactive mode to select which headers to include
>>>> - Store version in a file instead of git commit messages
>>>> - All imported headers aligned on same version
>>>> - Improve loops in scripts (David)
>>>> - Update documentation
>>>>
>>>>
>>>> Changes in RFC v2:
>>>> ==================
>>>> - Fix typos in documentation and commit messags (David, Morten)
>>>> - Add uAPI checker script
>>>> - Add uAPI to global_inc
>>>> - Fix build issues on FreeBSD and documentation (CI, David)
>>>> - Simplify import script (David)
>>>>
>>>> Maxime Coquelin (3):
>>>>     uapi: introduce kernel uAPI headers import
>>>>     uapi: import VDUSE header
>>>>     vduse: use imported VDUSE uAPI header
>>>>
>>>>    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 +++++
>>>>    lib/vhost/meson.build                  |   5 +-
>>>>    lib/vhost/vduse.c                      |   2 +-
>>>>    lib/vhost/vduse.h                      |  22 --
>>>>    linux-headers/uapi/.gitignore          |   4 +
>>>>    linux-headers/uapi/linux/vduse.h       | 353 +++++++++++++++++++++++++
>>>>    linux-headers/uapi/version             |   1 +
>>>>    meson.build                            |   8 +-
>>>>    11 files changed, 642 insertions(+), 29 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/linux/vduse.h
>>>>    create mode 100644 linux-headers/uapi/version
>>>>
>>>
>>
>