Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Bill Fischofer]

On Wed, Nov 18, 2015 at 4:58 AM, Zoltan Kiss  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.


>
> 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 > > wrote:
>>
>>
>>
>> On 17 November 2015 at 13:54, Zoltan Kiss > > 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
>> lng-odp@lists.linaro.org 
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>>
>>
>> --
>> Mike Holmes
>> Technical Manager - Linaro Networking Group
>> Linaro.org ***│ *Open source software for
>> ARM SoCs
>>
>> __
>>
>>
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org 
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>>
>>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Zoltan Kiss]

On 18/11/15 12:05, Bill Fischofer wrote:



On Wed, Nov 18, 2015 at 4:58 AM, Zoltan Kiss > 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
+ * 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

>>
wrote:



 On 17 November 2015 at 13:54, Zoltan Kiss

 >> 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
   

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Bill Fischofer]

On Wed, Nov 18, 2015 at 6:51 AM, Zoltan Kiss  wrote:

>
>
> On 18/11/15 12:05, Bill Fischofer wrote:
>
>>
>>
>> On Wed, Nov 18, 2015 at 4:58 AM, Zoltan Kiss > > 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
>> 
>> >>
>> wrote:
>>
>>
>>
>>  On 17 November 2015 at 13:54, Zoltan Kiss
>> 
>>  >
>> >> 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
>> 

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Savolainen, Petri (Nokia - FI/Espoo)]

> 
> --- 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
> + * 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);
> 

Otherwise OK, but add it into the body of the documentation text (instead of 
@note).

-Petri
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Mike Holmes]

On 18 November 2015 at 08:08, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

> >
> > --- 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
> > + * 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);
> >
>
> Otherwise OK, but add it into the body of the documentation text (instead
> of @note).
>

I like this but I'd like a clarification.

If a platform may have another way, implicitly we saying platform must
always accept  platform_params for all the items that may have been passed
another way.


>
> -Petri
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Zoltan Kiss]

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.


Thoughts?

Zoltan
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Bill Fischofer]

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
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  wrote:

>
>
> On 17 November 2015 at 13:54, Zoltan Kiss  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
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org  *│ *Open source software for ARM SoCs
>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_platform_init_t vs ODP_PLATFORM_PARAMS

[Mike Holmes]

On 17 November 2015 at 13:54, Zoltan Kiss  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
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp