[dpdk-dev] [PATCH v3 4/4] mempool: add in the RTE_NEXT_ABI for ABI breakages

[Hunt, David]

Hi Olivier,

On 3/9/2016 2:59 PM, Olivier MATZ wrote:
> Hi David,
>
> On 03/09/2016 12:30 PM, Hunt, David wrote:
>> Hi Panu,
>>
>> On 3/9/2016 10:46 AM, Panu Matilainen wrote:
>>> On 03/09/2016 11:50 AM, David Hunt wrote:
>>>> This patch is for those people who want to be easily able to switch
>>>> between the new mempool layout and the old. Change the value of
>>>> RTE_NEXT_ABI in common_base config file
>>> I guess the idea here is to document how to switch between the ABIs
>>> but to me this reads as if this patch is supposed to change the value
>>> in common_base. Of course there's  no such change included (nor should
>>> there be) here, but the description could use some fine-tuning perhaps.
>>>
>> You're right, I'll clarify the comments. v4 due soon.
>>
>>>> v3: Updated to take re-work of file layouts into consideration
>>>>
>>>> v2: Kept all the NEXT_ABI defs to this patch so as to make the
>>>> previous patches easier to read, and also to imake it clear what
>>>> code is necessary to keep ABI compatibility when NEXT_ABI is
>>>> disabled.
>>> Maybe its just me, but:
>>> I can see why NEXT_ABI is in a separate patch for review purposes but
>>> for final commit this split doesn't seem right to me. In any case its
>>> quite a large change for NEXT_ABI.
>>>
>> The patch basically re-introduces the old (pre-mempool) code as the
>> refactoring of the code would have made the NEXT_ABI additions totally
>> unreadable. I think this way is the lesser of two evils.
>>
>>> In any case, you should add a deprecation notice for the oncoming ABI
>>> break in 16.07.
>>>
>> Sure, I'll add that in v4.
> Sorry, maybe I wasn't very clear in my previous messages. For me, the
> NEXT_ABI is not the proper solution because, as Panu stated, it makes
> the patch hard to read. My understanding of NEXT_ABI is that it should
> only be used if the changes are small enough. Duplicating the code with
> a big #ifdef NEXT_ABI is not an option to me either.
>
> So that's why the deprecation notice should be used instead. But in this
> case, it means that this patch won't be present in 16.04, but will be
> added in 16.07.
>
> Regards,
> Olivier

Sure, v4 will remove the NEXT_ABI patch , and replace it with just the 
ABI break announcement for 16.07. For anyone who what's to try out the 
patch, they can always get it from patchwork, but not as part 16.04.

Thanks,
David.