Re: [dpdk-dev] [PATCH] pci: Add the class_id support in pci probe

[Panu Matilainen <pmatilai@redhat.com>]

On 01/29/2016 11:34 AM, Thomas Monjalon wrote:
> 2016-01-29 11:21, Panu Matilainen:
>> On 01/28/2016 11:38 PM, Thomas Monjalon wrote:
>>> 2016-01-13 14:22, Panu Matilainen:
>>>> On 01/13/2016 01:55 PM, Bruce Richardson wrote:
>>>>> On Thu, Dec 31, 2015 at 09:12:14AM -0800, Stephen Hemminger wrote:
>>>>>> On Tue, 29 Dec 2015 10:53:26 +0800
>>>>>> Ziye Yang <ziye.yang@intel.com> wrote:
>>>>>>
>>>>>>> This patch is used to add the class_id support
>>>>>>> for pci_probe since some devices need the class_info
>>>>>>> (class_code, subclass_code, programming_interface)
>>>>>>>
>>>>>>> Signed-off-by: Ziye Yang <ziye.yang@intel.com>
>>>>>>
>>>>>> Since rte_pci is exposed to application this breaks the ABI.
>>>>>
>>>>> But applications are not going to be defining rte_pci_ids values internally, are
>>>>> they? That is for drivers to use. Is this really an ABI breakage for applications that we
>>>>> need to be concerned about?
>>>>
>>>> There might not be applications using it but drivers are ABI consumers
>>>> too - think of 3rd party drivers and such.
>>>
>>> Drivers are not ABI consumers in the sense that ABI means
>>> Application Binary Interface.
>>> We are talking about drivers interface here.
>>> When establishing the ABI policy we were discussing about applications only.
>>
>> Generally speaking an ABI is an interface between two program (or
>> software if you like) modules, its not specific to "applications".
>> Looking at http://dpdk.org/doc/guides/contributing/versioning.html I see
>> it does only talk about applications, but an ABI consumer can also be
>> another library. A driver calling rte_malloc() is just as much
>> librte_eal ABI consumer as anything else.
>>
>> Now, I understand that drivers use and need interface(s) that
>> applications have no use for or simply cannot use, and those interfaces
>> could be subject to different policies. As an extreme example, the Linux
>> kernel has two isolated ABIs, one is the userland system call interface
>> which is guaranteed to stay forever and the other is kernel module
>> interface, guarantees nothing at all.
>>
>> In DPDK the difference is far muddier than that since all the interfaces
>> live in common, versioned userland DSOs. So if there are two different
>> interfaces following two different policies, it's all the more important
>> to clearly document them. One simple way could be using a different
>> prefix than rte_.
>
> Good suggestion. Or we can simply have different files with a clear notice
> in their headers and in the versioning doc.
> It was well split in rte_cryptodev_pmd.h

Using separate headers is also good. Optimally both? :)

>>> I agree we must allow 3rd party drivers but there is no good reason
>>> to try to upgrade DPDK without upgrading/porting the external drivers.
>>> If someone does not want to release its driver and keep upgrading DPDK,
>>> it is acceptable IMHO to force an upgrade of its driver.
>>
>> Note that I've no particular sympathy for 3rd party drivers as such.
>> What I *do* care about is that breakage is made explicit, as in drivers
>> built for an incompatible version refuse to load at all, instead of
>> silently corrupting memory etc.
>
> OK I agree.

Cool, the rest is just details then.

> Anyway the ABI versionning does not cover the structure changes.
> What about making a DPDK version check when registering a driver?
> So a binary driver would be clearly bound to a DPDK version.

That's one possibility. Another way to achieve essentially the same is 
to make rte_eal_driver_register() symbol version follow the DPDK 
version, in which case a driver built for another version will fail at 
dlopen() already.

> And we should change or remove the .so version which never change for
> most of drivers.

Yup, so-versioning DSOs which do not offer any ABI is kinda pointless.

	- Panu -