[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Hi Ferruh, > -Original Message- > From: Ferruh Yigit [mailto:ferruh.yigit at intel.com] > Sent: Tuesday, October 18, 2016 2:53 PM > To: Shreyansh Jain ; Thomas Monjalon > > Cc: dev at dpdk.org; viktorin at rehivetech.com; David Marchand > ; Hemant Agrawal > Subject: Re: [dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device > generalization > > On 10/17/2016 6:29 PM, Shreyansh Jain wrote: > > Hi Ferruh, > > > >> -Original Message- > >> From: Ferruh Yigit [mailto:ferruh.yigit at intel.com] > >> Sent: Monday, October 17, 2016 7:13 PM > >> To: Shreyansh Jain ; Thomas Monjalon > >> > >> Cc: dev at dpdk.org; viktorin at rehivetech.com; David Marchand > >> ; Hemant Agrawal > >> Subject: Re: [dpdk-dev] [PATCH v11 00/24] Introducing > rte_driver/rte_device > >> generalization > >> > >> On 10/5/2016 12:57 PM, Shreyansh Jain wrote: > >>> Hi Thomas, > >>> > >>> On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote: > >>>> 2016-10-04 12:21, Shreyansh Jain: > >>>>> Hi Thomas, > >>>>> > >>>>> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: > >>>>>> Applied, thanks everybody for the great (re)work! > >>>>> > >>>>> Thanks! > >>>>> > >>> [...] > >>> [...] > >>>>> > >>>>> It can be merged with changes for: > >>>>> - drv_name > >>>>> - EAL_ before _REGISTER_ macros > >>>>> - eth_driver => rte_driver naming > >>>> > >>>> Good. > >>>> Could you make it this week, please? > >>>> > >>> > >>> Certainly. At least some of those I can send within this week :) > >>> > >> > >> > >> I caught while running ABI validation script today, I think this patch > >> should increase LIBABIVER of: > >> - lib/librte_cryptodev > >> - lib/librte_eal > >> - lib/librte_ether > > > > Should I be referring to [1] for understanding how/when to change the > LIBABIVER? > > > > [1] http://dpdk.org/doc/guides/contributing/versioning.html > > Yes, this is the document. > > Briefly, if library becomes incompatible with existing applications, > LIBABIVER needs to be increased to indicate this. > > Increasing LIBABIVER changes dynamic library name and so_name, and this > cause existing application do not work with this new library. Not > increasing the LIBABIVER, app may start running but can have segfault or > can generate wrong values. So increasing LIBABIVER is to inform user and > prevent surprises. Thanks for explanation. I understand. I will send across another patch for this. - Shreyansh
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
2016-10-18 09:04, Neil Horman: > On Mon, Oct 17, 2016 at 02:43:12PM +0100, Ferruh Yigit wrote: > > I caught while running ABI validation script today, I think this patch > > should increase LIBABIVER of: > > - lib/librte_cryptodev > > - lib/librte_eal > > - lib/librte_ether > > > It should, and it in fact should wait a release if it hadn't been documented > as > a comming change. It was announced in release 16.04: http://dpdk.org/commit/c7970af While bumping LIBABIVER, the deprecation notices must be removed, and the API/ABI changes noted in the release notes, please.
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
On Mon, Oct 17, 2016 at 02:43:12PM +0100, Ferruh Yigit wrote: > On 10/5/2016 12:57 PM, Shreyansh Jain wrote: > > Hi Thomas, > > > > On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote: > >> 2016-10-04 12:21, Shreyansh Jain: > >>> Hi Thomas, > >>> > >>> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: > Applied, thanks everybody for the great (re)work! > >>> > >>> Thanks! > >>> > > [...] > > [...] > >>> > >>> It can be merged with changes for: > >>> - drv_name > >>> - EAL_ before _REGISTER_ macros > >>> - eth_driver => rte_driver naming > >> > >> Good. > >> Could you make it this week, please? > >> > > > > Certainly. At least some of those I can send within this week :) > > > > > I caught while running ABI validation script today, I think this patch > should increase LIBABIVER of: > - lib/librte_cryptodev > - lib/librte_eal > - lib/librte_ether > It should, and it in fact should wait a release if it hadn't been documented as a comming change. Neil > Thanks, > ferruh >
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Hi Ferruh, > -Original Message- > From: Ferruh Yigit [mailto:ferruh.yigit at intel.com] > Sent: Monday, October 17, 2016 7:13 PM > To: Shreyansh Jain ; Thomas Monjalon > > Cc: dev at dpdk.org; viktorin at rehivetech.com; David Marchand > ; Hemant Agrawal > Subject: Re: [dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device > generalization > > On 10/5/2016 12:57 PM, Shreyansh Jain wrote: > > Hi Thomas, > > > > On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote: > >> 2016-10-04 12:21, Shreyansh Jain: > >>> Hi Thomas, > >>> > >>> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: > >>>> Applied, thanks everybody for the great (re)work! > >>> > >>> Thanks! > >>> > > [...] > > [...] > >>> > >>> It can be merged with changes for: > >>> - drv_name > >>> - EAL_ before _REGISTER_ macros > >>> - eth_driver => rte_driver naming > >> > >> Good. > >> Could you make it this week, please? > >> > > > > Certainly. At least some of those I can send within this week :) > > > > > I caught while running ABI validation script today, I think this patch > should increase LIBABIVER of: > - lib/librte_cryptodev > - lib/librte_eal > - lib/librte_ether Should I be referring to [1] for understanding how/when to change the LIBABIVER? [1] http://dpdk.org/doc/guides/contributing/versioning.html > > Thanks, > ferruh
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
On 10/5/2016 12:57 PM, Shreyansh Jain wrote: > Hi Thomas, > > On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote: >> 2016-10-04 12:21, Shreyansh Jain: >>> Hi Thomas, >>> >>> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: Applied, thanks everybody for the great (re)work! >>> >>> Thanks! >>> > [...] > [...] >>> >>> It can be merged with changes for: >>> - drv_name >>> - EAL_ before _REGISTER_ macros >>> - eth_driver => rte_driver naming >> >> Good. >> Could you make it this week, please? >> > > Certainly. At least some of those I can send within this week :) > I caught while running ABI validation script today, I think this patch should increase LIBABIVER of: - lib/librte_cryptodev - lib/librte_eal - lib/librte_ether Thanks, ferruh
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Hi Thomas, On Tuesday 04 October 2016 01:12 PM, Thomas Monjalon wrote: > 2016-10-04 12:21, Shreyansh Jain: >> Hi Thomas, >> >> On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: >>> Applied, thanks everybody for the great (re)work! >> >> Thanks! >> [...] [...] >> >> It can be merged with changes for: >> - drv_name >> - EAL_ before _REGISTER_ macros >> - eth_driver => rte_driver naming > > Good. > Could you make it this week, please? > Certainly. At least some of those I can send within this week :) - Shreyansh
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Hi Thomas, On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: > Applied, thanks everybody for the great (re)work! Thanks! > > 2016-09-20 18:11, Shreyansh Jain: >> Future Work/Pending: >> === >> - Presently eth_driver, rte_eth_dev are not aligned to the rte_driver/ >>rte_device model. eth_driver still is a PCI specific entity. This >>has been highlighted by comments from Ferruh in [9]. >> - Some variables, like drv_name (as highlighted by Ferruh), are getting >>duplicated across rte_xxx_driver/device and rte_driver/device. Both the above are already part of my todo list. > > What about those pending work? > > I would add more remaining issues: > - probe/remove naming could be applied to vdev for consistency Is that for uniformity reasons? I still feel 'probe/remove' are not appropriate for a virtual device. init/deinit are more appropriate. As for PCI, probe/remove are standard parlance and hence suit it better than init/deinit. Nevertheless, uniform naming convention can have its benefits - ease of code understanding being one. Change is simple once we come to a conclusion. > - rte_eal_device_insert must be called in vdev Ok. > - REGISTER macros should be prefixed with RTE_ That would include: DRIVER_REGISTER_VDEV DRIVER_REGISTER_PCI_TABLE DRIVER_REGISTER_PCI I will publish a patch soon. This would be fairly straightforward change. > - Some functions in EAL does not need eal_ in their prefix: > rte_eal_pci_ -> rte_pci_ > rte_eal_dev_ -> rte_dev_ > rte_eal_vdev_ -> rte_vdev_ > rte_eal_driver -> rte_drv_ > rte_eal_vdrv -> rte_vdrv_ > > It can be merged with changes for: - drv_name - EAL_ before _REGISTER_ macros - eth_driver => rte_driver naming -- - Shreyansh
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
2016-10-04 12:21, Shreyansh Jain: > Hi Thomas, > > On Monday 03 October 2016 07:58 PM, Thomas Monjalon wrote: > > Applied, thanks everybody for the great (re)work! > > Thanks! > > > > > 2016-09-20 18:11, Shreyansh Jain: > >> Future Work/Pending: > >> === > >> - Presently eth_driver, rte_eth_dev are not aligned to the rte_driver/ > >>rte_device model. eth_driver still is a PCI specific entity. This > >>has been highlighted by comments from Ferruh in [9]. > >> - Some variables, like drv_name (as highlighted by Ferruh), are getting > >>duplicated across rte_xxx_driver/device and rte_driver/device. > > Both the above are already part of my todo list. > > > What about those pending work? > > > > I would add more remaining issues: > > - probe/remove naming could be applied to vdev for consistency > > Is that for uniformity reasons? I still feel 'probe/remove' are not > appropriate for a virtual device. init/deinit are more appropriate. As > for PCI, probe/remove are standard parlance and hence suit it better > than init/deinit. PCI probe is "scan + checks + init". The vdev requires "args parsing + checks + init". The device will not be initialized if checks fail, e.g. missing support or name conflict. I think it could fit in "probe" rather than "init". The remove word looks appropriate in both cases. > Nevertheless, uniform naming convention can have its benefits - ease of > code understanding being one. Yes that's the other pro for "probe/remove". > Change is simple once we come to a conclusion. > > > - rte_eal_device_insert must be called in vdev > > Ok. > > > - REGISTER macros should be prefixed with RTE_ > > That would include: > DRIVER_REGISTER_VDEV > DRIVER_REGISTER_PCI_TABLE > DRIVER_REGISTER_PCI > > I will publish a patch soon. This would be fairly straightforward change. > > > - Some functions in EAL does not need eal_ in their prefix: > > rte_eal_pci_ -> rte_pci_ > > rte_eal_dev_ -> rte_dev_ > > rte_eal_vdev_ -> rte_vdev_ > > rte_eal_driver -> rte_drv_ > > rte_eal_vdrv -> rte_vdrv_ > > > > > > It can be merged with changes for: > - drv_name > - EAL_ before _REGISTER_ macros > - eth_driver => rte_driver naming Good. Could you make it this week, please?
[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Applied, thanks everybody for the great (re)work! 2016-09-20 18:11, Shreyansh Jain: > Future Work/Pending: > === > - Presently eth_driver, rte_eth_dev are not aligned to the rte_driver/ >rte_device model. eth_driver still is a PCI specific entity. This >has been highlighted by comments from Ferruh in [9]. > - Some variables, like drv_name (as highlighted by Ferruh), are getting >duplicated across rte_xxx_driver/device and rte_driver/device. What about those pending work? I would add more remaining issues: - probe/remove naming could be applied to vdev for consistency - rte_eal_device_insert must be called in vdev - REGISTER macros should be prefixed with RTE_ - Some functions in EAL does not need eal_ in their prefix: rte_eal_pci_ -> rte_pci_ rte_eal_dev_ -> rte_dev_ rte_eal_vdev_ -> rte_vdev_ rte_eal_driver -> rte_drv_ rte_eal_vdrv -> rte_vdrv_
