RE: [EXT] Re: [PATCH v9 13/14] baseband/acc: add PF configure companion function

[Akhil Goyal <gakhil@marvell.com>]

> Hi Maxime, Akhil,
> 
> (replying to myself)
> 
> >
> > Hi Maxime, Akhil,
> >
> > <...>
> > > >>>>> This is same comment as for ACC100 vs. ACC101.
> > > >>>>> Having a single API would be the way to go, given the prototype
> > > >>>>> of the functions are identical.
> > > >>>>>
> > > >>>>> Keep acc200 function, but internal only, rte_acc_configure()
> > > >>>>> would call the acc100/acc101/acc200/accXXX based on the device
> > ID.
> > > >>>>>
> > > >>>> +1 for this.
> > > >>>>
> > > >>>> I believe a bbdev API should be defined to be used by each of the
> > PMD.
> > > >>>> So that application can be agnostic of the underneath device.
> > > >>>>
> > > >>>> I would recommend to send a deprecation notice to remove all the
> > > >>>> pmd APIs going forward. We can take it for now, but these need to
> > > >>>> be replaced with generic API as soon as possible. No new such PMD
> > > >>>> API would be accepted going forward.
> > > >>>>
> > > >>>
> > > >>> OK understood, we can look into this for 23.03.
> > > >>> Are we okay to keep that commit as is for 22.11?
> > > >>>
> > > >> What Maxime is suggesting is adding a wrapper in PMD to hide
> > > >> variant of acc.
> > > >> I believe it is better to do it now as this is long term stable release.
> > > >> You can send a deprecation notice for removing PMD APIs and adding
> > > >> new generic API now which can be done in next cycle.
> > > >
> > > > Updated in the V10 as suggested.
> > >
> > > Thanks.
> > >
> > > > This rte_acc_configure() symbol is marked as experimental. I believe
> > > > we
> > > can remove it without notice (this is already modified in this serie
> > > without notice).
> > >
> > > Yes, no worries since this is experimental.
> > >
> > > > Note that this is only used by bbdev-test so this is all
> > > > self-contained and no
> > > impact to the ecosystem.
> > >
> > > Given it is only meant to be used by the dpdk-test-bbdev application,
> > > maybe it could be an internal API? Avoiding to export it would make our
> > lives easier.
> > >
> >
> > I like that option in case we can build consensus on this.
> > Using bbdev api for this side-band configuration would confuse the
> > ecosystem since this is out of the remit of the intended bbdev api.
> > The current method using a formally exposed PMD API is a bit historical,
> > intention only to expose this from PMD code to the bbdev-test but still within
> > DPDK only.
> > I will push now a patch for further discussion with such a modification (not
> > required for 22.11).
> 
> Actually it would not build in shared lib mode when doing this I believe.
> One option would be to accept not to call these functions from bbdev-test when
> RTE_BUILD_SHARED_LIB is not defined.
> That would be okay with me based on the limited usecase.
> But that may be frown upon. Let me know what you think.
> 
Yes, it will fail in shared build.
We should not add limitation for bbdev to work only in static mode.
People do have use cases for shared build.

For DPDK, anything exposed via version.map is external. Even if you use for bbdev-test app.
In any way, you are not restricting user to not use these pmd APIs.

Hence, I would suggest to remove all the pmd APIs are work on a bbdev API which can be used by all PMDs.

Also, you should send a deprecation notice for removal of the PMD APIs with a definitive timeline.
It is true that these are experimental APIs, but still it would be good to notify this as it was being used since long.

Regards,
Akhil