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

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/16/2020 9:00 PM, Thomas Monjalon wrote:
> 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
> 
> 

dropped the patch from next-net and updated its patchwork status.