[EXT] Re: [PATCH v5 0/4] add support for self monitoring

[Tyler Retzlaff]

On Wed, Jan 11, 2023 at 09:39:35AM +0000, Tomasz Duszynski wrote:
> Hi Tyler,
> 
> >-----Original Message-----
> >From: Tyler Retzlaff <roretzla at linux.microsoft.com>
> >Sent: Wednesday, January 11, 2023 1:32 AM
> >To: Tomasz Duszynski <tduszynski at marvell.com>; bruce.richardson at intel.com; mb at smartsharesystems.com
> >Cc: dev at dpdk.org; thomas at monjalon.net; Jerin Jacob Kollanukkaran <jerinj at marvell.com>;
> >mb at smartsharesystems.com; Ruifeng.Wang at arm.com; mattias.ronnblom at ericsson.com; zhoumin at loongson.cn
> >Subject: [EXT] Re: [PATCH v5 0/4] add support for self monitoring
> >
> >External Email
> >
> >----------------------------------------------------------------------
> >hi,
> >
> >don't interpret this as an objection to the functionality but this looks like a clear example of
> >something that doesn't belong in the EAL. has there been a discussion as to whether or not this
> >should be in a separate library?
> 
> No, I don't recall anybody having any concerns about the code placement. Rationale behind 
> making this part of eal was based on the fact that tracing itself is a part of eal and
> since this was meant to be extension to tracing, code placement decision came out naturally. 
> 
> During development phase idea evolved a bit and what initially was supposed to be solely yet
> another tracepoint become generic API to read pmu and tracepoint based on that. Which means
> both can be used independently. 
> 
> That said, since this code has both platform agnostic and platform specific parts this can either be split into: 
> 1. library + eal platform code
> 2. all under eal 
> 
> Either approach seems legit. Thoughts?
> 
> >
> >a basic test is whether or not an implementation exists or can be reasonably provided for all
> >platforms and that isn't strictly evident here. red flag is to see yet more code being added
> >conditionally compiled for a single platform.
> 
> Even libs are not entirely pristine and have platform specific ifdefs lurking so not sure where
> this red flag is coming from. 

i think red flag was probably the wrong term to use sorry for that.
rather i should say it is an indicator that the api probably doesn't
belong in the eal.

fundamentally the purpose of the abstraction library is to relieve the
application from having to do conditional compilation and/or execution for
the subject apis coming from eal. including and exporting apis that work
for only one platform is in direct contradiction.

please explore adding this as a separate library, it is understood that
there are tradeoffs involved.

thanks!