Re: [dpdk-dev] [PATCH v2] doc: define qualification criteria for external library

[Thomas Monjalon <thomas@monjalon.net>]

20/11/2023 18:46, Jerin Jacob:
> On Sun, Nov 19, 2023 at 2:23 PM Morten Brørup <mb@smartsharesystems.com> wrote:
> > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> > > On Fri, Nov 17, 2023 at 1:57 PM Morten Brørup
> > > > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> > > > > On Thu, Sep 28, 2023 at 11:10 AM <jerinj@marvell.com> wrote:
> > > > > > From: Jerin Jacob <jerinj@marvell.com>
> > > > > > +- **Free availability**: The library must be freely available to
> > > > > build in either source or binary
> > > > > > +  form, with a preference for source form.
> > > >
> > > > Suggest adding:
> > > >
> > > > - **Free use and distribution license**: The library must be freely
> > > available to use and distribute without any attached conditions.
> > > >
> > > > We must require a BSD-like license, to ensure that DPDK as a whole
> > > (including 3rd party libraries) remains BSD licensed, and can be used
> > > in commercial (closed source) applications.
> > >
> > > As far as I understand, The initial scope of was “free availability”
> > > for building.
> > > Free distribution is much wider scope. I don't think, current external
> > > libraries[1] have free distribution rights.
> > > [1]
> > > https://github.com/DPDK/dpdk/blob/main/doc/guides/gpus/cuda.rst
> > > https://gitlab.com/nvidia/headers/cuda-individual/cudart/-
> > > /blob/main/LICENSE?ref_type=heads
> >
> > I didn't mean the library source code; I only meant the library in binary form.
> >
> > It is nice if the library's header files may be distributed too, but I don't see it as a requirement, especially if they are freely available elsewhere (preferably without imposing additional restrictions on the ability to use and distribute the library in binary form).
> >
> > How about this instead:
> >
> > - **License permitting free use and distribution in binary form**:
> > The library's license must allow free and unconditional use and distribution of the library in binary form.
> 
> Distribution and unconditional use is not the case for existing
> library dependencies such as
> https://gitlab.com/nvidia/headers/cuda-individual/cudart/-/blob/main/LICENSE?ref_type=heads
> 
> So I am not sure, Which is the correct thing to do. Maybe we can
> discuss more in tech board meeting if there are no other comments in
> mailing list on this topic.

I don't think we should make mandatory to have rights of redistribution.
To me, being to download and install the dependency without any restriction
is enough *for a driver*.
If distribution of the dependency is restricted,
then the driver will be disabled by the distro.
It is not our problem I think.

And I agree we must have stricter requirements for libraries dependencies.
I am OK to mandate free distribution for such library dependencies.


> > We might want lawyers to verify the wording when we have agreed on our intentions.
> >
> > > I am fine with either way, Feedback from others?

[...]

> > > > > > +- **Code readability**: When the depended library is optional,
> > > use
> > > > > stubs to reduce the ``ifdef``
> > > > > > +  clutter to enable better code readability.
> > > >
> > > > Why does everyone keep insisting that stubs make code more readable?
> > > Sometimes #ifdef is better.
> > >
> > > Could you share a case where when #ifdefs is better(Just to understand
> > > the view).?
> >
> > If an external library provides some simple functions, and a group (i.e. a subset) of functions in a DPDK library depends on that external library, then it might be more readable if those functions are enabled/disabled as a group in the DPDK library rather than individually.
> >
> > E.g. the external library provides functions for statistical processing, and the DPDK library can be built with or without statistics, depending on using the external library or not. If the DPDK library is built without statistics, it should not register statistics availability towards a management module (e.g. telemetry) if it is not really implemented within the DPDK library (because the underlying functions are stubs).
> >
> > It might also be a matter of personal preferences. When reviewing some source code, an #ifdef makes the availability of an underlying function perfectly clear. Blindly calling a function (of an external library) doesn't really show if the function is implemented for real, or just a stub.
> 
> In general theme in DPDK code base that we are trying to avoid a  lot
> of #ifdef in a given C file.
> Instead, we are doing following scheme.
> 
> https://github.com/DPDK/dpdk/blob/main/drivers/ml/cnxk/meson.build#L84
> https://github.com/DPDK/dpdk/blob/main/drivers/ml/cnxk/meson.build#L60
> https://github.com/DPDK/dpdk/blob/main/drivers/ml/cnxk/mvtvm_ml_stubs.c
> 
> > My opinion on this might be tainted by my preference for building from scratch. The distro people might see it very differently!
> 
> Yeah. I don't have a strong opinion, I can change to following, if
> there are no comments on this. Or we can discuss more in TB meeting if
> there are no review comments in mailing list on this topic.
> 
> **Code readability**: When the depended library is optional, use
> either stubs or ``#ifdef`` consistently, not a mix of both, to ensure
> code readability.
> 
> > > > Please use something like this instead:
> > > >
> > > > - **Code readability**: When the depended library is optional, use
> > > either stubs or ``#ifdef`` consistently, not a mix of both, to ensure
> > > code readability.

We should better focus on code organisation for technical reasons:
we don't want to mix OS-specific code in a common file.
That's why stubs are preferred to implement functions with OS-specific includes, etc.
If it's just a matter of disabling some code, #ifdef is fine.