Re: [dpdk-dev] [PATCH v1] common/mlx5: remove devx depndency on ibv and dv

[Thomas Monjalon <thomas@monjalon.net>]

16/04/2020 19:35, Ferruh Yigit:
> On 4/9/2020 8:24 AM, David Marchand wrote:
> > On Wed, Apr 8, 2020 at 7:12 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >> On 4/1/2020 10:59 AM, Raslan Darawsheh wrote:
> >>> From: Ophir Munk <ophirmu@mellanox.com>
> >>>>
> >>>> File mlx5_devx_cmds.c should contain pure DevX calls. It must be OS
> >>>> agnostic and not include any references to ibv or dv structs (defined in
> >>>> ibverbs and rdma-core linux libraries).  This commit replaces all ibv and
> >>>> dv references with 'void *'.  Specifically, the following struct were
> >>>> replaced:
> >>>> 1. struct ibv_context *
> >>>> 2. struct ibv_qp *
> >>>> 3. struct mlx5dv_devx_cmd_comp *
> >>>>
> >>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> >>>
> >>> Patch applied to next-net-mlx,
> >>>
> >>
> >> Hi David,
> >>
> >> This patch is failing in the travis for ABI checks [1], since mlx has APIs now
> >> [2], are they public APIs or internal ones, and are they part of the ABI policy,
> >> can you please check this?
> > 
> > - What I see on patchwork and test-report ml for this patch:
> > http://patchwork.dpdk.org/patch/67367/
> > 
> > Ophir proposed a patch on 03/30.
> > 
> > The robot reported an issue on 03/30, and I suppose Ophir got a report.
> > https://mails.dpdk.org/archives/test-report/2020-March/122623.html
> > https://travis-ci.com/github/ovsrobot/dpdk/jobs/308057800#L2337
> > 
> > Matan acked the patch on 03/31.
> > 
> > Rasland merged the patch on 04/01.
> > 
> > I understand that the abi checks are not perfect, and people need help
> > with the new abi checks.
> > Prove me wrong, but here, I get the feeling that it was just ignored
> > by 3 people in a row.
> > 
> > - On the question if these should be public API or internal, that is
> > not for me to reply/investigate.
> > This is a question for Mellanox.
> > 
> 
> Hi Matan, Raslan, Ophir,
> 
> First can you please clarify if these APIs are internal or public?

As most of common drivers, some functions are exported to be
used by some PMDs. So they are not part of the API/ABI and should be skipped
by ABI checks.

> And later if the ABI break issue is not clarified I may need to drop these
> patches. Right now they fail in travis!

Yes, it fails and could it be avoided with some libabigail config.
But the real solution is to mark internal symbols, and we are waiting
for rte_internal patchset to be completed and merged.

Ferruh, please let's not bloat libabigail config,
and reject any patch failing ABI checks.

As a consequence, this patch must be dropped until it uses rte_internal.
Thanks