Re: [dpdk-dev] [PATCH v3 00/14] Make shared memory config non-public

[David Marchand <david.marchand@redhat.com>]

On Wed, Jul 3, 2019 at 11:38 AM David Marchand <david.marchand@redhat.com>
wrote:

> Hello Anatoly,
>
> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov <anatoly.burakov@intel.com>
> wrote:
>
>> This patchset removes the shared memory config from public
>> API, and replaces all usages of said config with new API
>> calls.
>>
>> A lot of the patchset is a search-and-replace job and should
>> be pretty easy to review. The rest are pretty trivial EAL
>> changes.
>>
>> This patchset depends on FreeBSD fixes patchset:
>>
>> http://patches.dpdk.org/project/dpdk/list/?series=5196
>>
>> v3:
>> - Rebase on top of latest master
>>
>> v2:
>> - Collapsed all changes into fewer patches
>> - Addressed review comments
>> - Created a new file to store the code
>> - Changed namespace to "rte_mcfg_"
>> - Added some unification around config init
>> - Removed "packed" attribute from mem config
>> - Removed unnecessary inlining
>> - Added a check to explicitly forbid running multiprocess
>>   applications that differ in their DPDK versions
>>
>
>
> For the parts I already had a look at, I still think the changes are in
> too many patches.
> A lot of this is just search/replace we can have it with the patch that
> introduces it.
> - patch 1, 2 and 3 could be squashed as a single one (plus removing the
> unused macro from patch 8)
> - idem with patch 4 and 5
> - idem with patch 6 and 7 (plus removing the unused macro from patch 8)
>
>
Overall, I am ok with the changes, once the patch 13 is fixed.
You can add my ack on the n+1 patchset.


I just want to state two approaches to merge these changes:
- the first one would be to split this series in two
  - take the first part of this series now, but mark the new API
"experimental"
  - postpone the merge to 19.11 of the second part, which starts at the
hiding rte_mem_config patch
- the second one is taking these changes in one go

The second one is the quicker and the more straightforward but it leaves
the risk of having missed something and we must break the ABI again in
19.11.
The risk is quite low, given the changes.


Thomas, comments?

-- 
David Marchand