On Wed, Nov 18, 2015 at 6:51 AM, Zoltan Kiss <[email protected]> wrote:
> > > On 18/11/15 12:05, Bill Fischofer wrote: > >> >> >> On Wed, Nov 18, 2015 at 4:58 AM, Zoltan Kiss <[email protected] >> <mailto:[email protected]>> wrote: >> >> >> On 17/11/15 20:46, Bill Fischofer wrote: >> >> I vote for B. The idea is that if the application wishes to >> override it >> can do so otherwise let the implementation take its defaults >> from the >> environment or however else it wishes to support platform-specific >> >> >> I think your analogy fails a bit here because not passing >> odp_platform_init_t doesn't mean you are using "defaults". It just >> means you are using an another way to provide the configuration. >> >> >> That "other way" by definition is a platform-specific default mechanism, >> which might be an environment variable or something else entirely. The >> point is either the application has an application-centric preference or >> it does not. Option B handles those options simply and cleanly. >> > > I see. If you phrase it like this, it explains why we want to have two > ways to do the same things, I like that. My original thought was to > explicitly define ODP_PLATFORM_PARAMS, but it's better to leave it out. > I would phrase option B then like this: > > --- a/include/odp/api/init.h > +++ b/include/odp/api/init.h > @@ -152,6 +152,10 @@ typedef struct odp_platform_init_t { > * > * @see odp_term_global() > * @see odp_init_local() which is required per thread before use. > + * @note The underlying implementation may have an another way to get > typo: an another. Otherwise I like the note. > + * configuration related to platform_params (e.g. environmental variable, > + * configuration file), but if the application passes it, it should always > + * take precedence. > */ > int odp_init_global(const odp_init_t *params, > const odp_platform_init_t *platform_params); > > > > >> >> configuration options. This is in keeping with how we handle >> other >> overridable defaults for things like log and abort functions. >> Specify >> NULL == accept whatever default behavior is in effect, otherwise >> specify >> what you want. Simpler for everyone. >> >> On Tue, Nov 17, 2015 at 1:05 PM, Mike Holmes >> <[email protected] <mailto:[email protected]> >> <mailto:[email protected] <mailto:[email protected]>>> >> wrote: >> >> >> >> On 17 November 2015 at 13:54, Zoltan Kiss >> <[email protected] <mailto:[email protected]> >> <mailto:[email protected] >> >> <mailto:[email protected]>>> wrote: >> >> Hi all, >> >> Our odp_init_global() has a second parameter with type >> odp_platform_init_t. It's supposed to convey platform >> specific >> init parameters, however neither linux-generic nor >> linux-dpdk >> uses that. In the latter we use instead the >> ODP_PLATFORM_PARAMS >> environment variable. It has the little advantage that >> you don't >> have to modify your application's code, but it limits >> you to >> strings. In case of ODP-OVS we store this in OVSDB and >> retrieve >> it from the startup script (or specify it manually if >> you don't >> use the startup script.) >> I'm tempted to change ODP-OVS and ODP-DPDK to use >> odp_platform_init_t, would be cleaner for OVS and for >> any bigger >> application which have a nice, full-fledged config >> database >> solution. But that would immediately bring us a bigger >> problem: >> none of our unit tests or example applications supports >> passing >> this parameter at all. They are small applications, >> implementing >> a proper config management would be an overkill. I have >> two >> options to solve this: >> >> Apart from keeping the odp_platform_init_t type to be >> passed >> odp_init_global() >> >> A) change our code in linux-generic for examples and >> tests (>20 >> places in the repo) to get the platform parameters from >> ODP_PLATFORM_PARAMS env variable, and pass it down to >> odp_init_global() as a string. >> >> B) Or just allow a platform to use both ways, document >> this, and >> require that if both are present, the >> odp_platform_init_t passed >> as parameter should take precedence. So smaller >> applications >> using simply configurable platforms (like ODP-DPDK) >> don't have >> to figure out a way to deal with this problem. >> >> I have a slight preference towards B), but I could be >> convinced >> that it's a bad idea to have 2 ways to do the same thing. >> >> >> I like A, two ways always feels bad to me. >> I like that it is explicitly passed in and you know what >> you have. >> Env vars can change without the difference being seen in >> the code as >> easily and not all OS'es have env vars. Env vars are a >> great way to >> do system dependent things but I think the application should >> translate them into the odp_platform_init_t so that the >> fudging is >> the apps problem and the app knows more about the portability >> compromises it is making when using platform specifics. >> >> >> Thoughts? >> >> Zoltan >> _______________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected]> >> <mailto:[email protected] <mailto:[email protected] >> >> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source >> software for >> ARM SoCs >> >> __ >> >> >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected]> >> <mailto:[email protected] <mailto:[email protected] >> >> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >>
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
