On 9/18/2020 9:59 AM, Andrew Rybchenko wrote:
On 9/18/20 11:41 AM, Bruce Richardson wrote:
On Thu, Sep 17, 2020 at 08:59:26PM +0300, Andrew Rybchenko wrote:
On 9/16/20 7:44 PM, Bruce Richardson wrote:
As flagged previously on-list, there are a number of macros used to specify
what libs and drivers are enabled in the build which differ from the
equivalents used with make. This patchset is one possible approach to
fixing these, but as part of the investigation some issues were hit where
I'd like additional input to ensure we are ok with the approach taken in
this set.

First, a problem statement:

* While the make build defines generally followed a pattern, there were
    many instances where the defines were unique. These can be seen in the
    values defined in patch 4.

* The NIC PMDs had two separate standards for the defines - some (the
    physical device drivers) tended to have the _PMD at the end of the
    macros, while the virtual drivers had it in the middle. Since the
    majority seemed to go with it at the end, meson chose this option.
    However, as can be seen from patch 4, a number now need special handling
    for compatibility

* This "_PMD" at the end made its way into other device classes, such as
    crypto and event, but it appears that the standard for these classes from
    make is in fact the opposite. Therefore we have had for the last 2+ years
    conflicting macros for crypto, compression and event classes.

* There is also the question of how important compatibility for these
    macros is, especially since we have had numerous incompatibilities
    without it being reported before. There is also the issue of the
    deprecation process for macros, since these are not part of any ABI.

What's done in this set:

* Firstly, and missing dependencies on apps or examples had to be fixed,
    where a dependency was missed because it was never needed due to the code
    being stripped out because of a missing macro.

* Secondly, since it is likely that use of the defines from make is more
    widespread than those from meson, the defines for the crypto, compression
    and event classes are changed to align with the make values. Just in case
    though, temporary code is added to drivers/meson.build to redefine the
    old meson values too, and a deprecation notice is added for these. The
    hope is that we can then remove this temporary code in the next release,
    leaving us with just one macro style for each driver class.

* Thirdly, we add an additional set of backward compatibility macros for
    the ~30 special-cases, where the meson macro template does not match that
    defined for make. Again, this is intended to be temporary and a
    deprecation notice is given for the macros in that file.

* Finally, we replace all instances of the old macros in the codebase with
    the newer versions. While the work done in the first four patches (steps
    1-3 above) should be ok to backport, this final patch is not suitable for
    backporting. However, it is relatively simple to produce a new patch for
    backporting which allow either old or new values to be used in macros.


Open issues/considerations after this patch:

* We still have inconsistencies in our driver macro and naming templates.
    This is just a nice-to-have, but if people are ok with generally having a
    breakage in our macro defines, we could clean this up a lot further.
    Notice how 'net', 'regex' and 'vdpa' have '_PMD' at the end, while most
    others have it before the name. Notice also that many device classes have
    the class at the end of the template, while bbdev has it in the middle.
        $ git grep config_flag_fmt -- drivers/*/meson.build
        drivers/baseband/meson.build:config_flag_fmt = 
'RTE_LIBRTE_PMD_BBDEV_@0@'
        drivers/bus/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_BUS'
        drivers/common/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_COMMON'
        drivers/compress/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@'
        drivers/crypto/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@'
        drivers/event/meson.build:config_flag_fmt = 
'RTE_LIBRTE_PMD_@0@_EVENTDEV'
        drivers/mempool/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_MEMPOOL'
        drivers/net/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
        drivers/raw/meson.build:config_flag_fmt = 'RTE_LIBRTE_PMD_@0@_RAWDEV'
        drivers/regex/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'
        drivers/vdpa/meson.build:config_flag_fmt = 'RTE_LIBRTE_@0@_PMD'

As a generic direction I would vote for standard names which are
based on directory structure:
  - RTE_LIBRTE_ETHDEV
  - RTE_DRIVER_NET_SFC
  - RTE_DRIVER_COMMON_MLX5
  - RTE_DRIVER_BUS_PCI


Definite +1, and it would be good if we standardized the .so names like
that too.

The open question is how much backward compatibility needs to be maintained
for macros (and also too for .so names)? With this patchset, I've aimed
very much to keep strict compatibility for now.

I think it is really good. Technically it does not look hard to
provide backward compatibility for macros, so I'd keep it up to
but not including 21.11 (next LTS?). Same for .so names, if it
is not a problem.

If we approve the decision on naming, all new entities must
follow it from the very beginning.


+1

Reply via email to