diff mbox

[API-NEXT,PATCHv4,1/7] api: random: replace use_entropy with odp_rand_kind parameter

Message ID 1478133719-10981-1-git-send-email-bill.fischofer@linaro.org
State Superseded
Headers show

Commit Message

Bill Fischofer Nov. 3, 2016, 12:41 a.m. UTC
Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing the
use_entropy parameter with a more comprehensive enum that specifies the
kind of random data requested.

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

---
Changes in v4:
- Normalize random API signatures with other ODP APIs
- Add new odp_random_seeded_data() API for repeatable random data generation
- Add additional tests for new odp_random_seeded_data() API
- Break out crypto section of User Guide to its own sub-document
- Add User Guide docuemntation for ODP random data API.

Changes in v3:
- Address commments by Petri
- Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the
mathematical term "normal"

 include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

-- 
2.7.4

Comments

Savolainen, Petri (Nokia - FI/Espoo) Nov. 23, 2016, 1:02 p.m. UTC | #1
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bill

> Fischofer

> Sent: Thursday, November 03, 2016 2:42 AM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy

> with odp_rand_kind parameter

> 

> Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing the

> use_entropy parameter with a more comprehensive enum that specifies the

> kind of random data requested.

> 

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

> Changes in v4:

> - Normalize random API signatures with other ODP APIs

> - Add new odp_random_seeded_data() API for repeatable random data

> generation

> - Add additional tests for new odp_random_seeded_data() API

> - Break out crypto section of User Guide to its own sub-document

> - Add User Guide docuemntation for ODP random data API.

> 

> Changes in v3:

> - Address commments by Petri

> - Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the

> mathematical term "normal"

> 

>  include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----

>  1 file changed, 26 insertions(+), 5 deletions(-)

> 

> diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h

> index 00fa15b..62f2376 100644

> --- a/include/odp/api/spec/random.h

> +++ b/include/odp/api/spec/random.h

> @@ -24,18 +24,39 @@ extern "C" {

>   */

> 

>  /**

> + * Random kind selector

> + */

> +typedef enum {

> +	/** Basic random, presumably pseudo-random generated by SW */

> +	ODP_RANDOM_BASIC,

> +	/** Cryptographic quality random */

> +	ODP_RANDOM_CRYPTO,

> +	/** True random, generated from a HW entropy source */

> +	ODP_RANDOM_TRUE,

> +} odp_random_kind_t;

> +

> +/**

>   * Generate random byte data

>   *

> - * @param[out]    buf   Output buffer

> - * @param         size  Size of output buffer

> - * @param use_entropy   Use entropy

> + * The intent in supporting different kinds of random data is to allow

> + * tradeoffs between performance and the quality of random data needed.

> The

> + * assumption is that basic random is cheap while true random is

> relatively

> + * expensive in terms of time to generate, with cryptographic random

> being

> + * something in between. Implementations that support highly efficient

> true

> + * random are free to use this for all requested kinds. So it is always

> + * permissible to "upgrade" a random data request, but never to

> "downgrade"

> + * such requests.

>   *

> - * @todo Define the implication of the use_entropy parameter

> + * @param[out]    buf   Output buffer

> + * @param         len   Length of output buffer in bytes

> + * @param         kind  Specifies the type of random data required.

> Request

> + *                      is expected to fail if the implementation is

> unable to

> + *                      provide the requested type.

>   *

>   * @return Number of bytes written

>   * @retval <0 on failure

>   */

> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t

> use_entropy);

> +int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);


Cannot change int types. Len and return value need to be compared commonly, and signed vs. unsigned comparison produces a compiler warning. Also C spec specify that 'int' is at least 16 bits, which mean that it may the that on some systems. Number of bytes are stored into 32 bit variables throughout the ODP API.
 
-Petri
Savolainen, Petri (Nokia - FI/Espoo) Nov. 24, 2016, 10:55 a.m. UTC | #2
From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

Sent: Wednesday, November 23, 2016 4:05 PM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy with odp_rand_kind parameter



On Wed, Nov 23, 2016 at 8:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote:


> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bill

> Fischofer

> Sent: Thursday, November 03, 2016 2:42 AM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy

> with odp_rand_kind parameter

>

> Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing the

> use_entropy parameter with a more comprehensive enum that specifies the

> kind of random data requested.

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

> Changes in v4:

> - Normalize random API signatures with other ODP APIs

> - Add new odp_random_seeded_data() API for repeatable random data

> generation

> - Add additional tests for new odp_random_seeded_data() API

> - Break out crypto section of User Guide to its own sub-document

> - Add User Guide docuemntation for ODP random data API.

>

> Changes in v3:

> - Address commments by Petri

> - Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the

> mathematical term "normal"

>

>  include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----

>  1 file changed, 26 insertions(+), 5 deletions(-)

>

> diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h

> index 00fa15b..62f2376 100644

> --- a/include/odp/api/spec/random.h

> +++ b/include/odp/api/spec/random.h

> @@ -24,18 +24,39 @@ extern "C" {

>   */

>

>  /**

> + * Random kind selector

> + */

> +typedef enum {

> +     /** Basic random, presumably pseudo-random generated by SW */

> +     ODP_RANDOM_BASIC,

> +     /** Cryptographic quality random */

> +     ODP_RANDOM_CRYPTO,

> +     /** True random, generated from a HW entropy source */

> +     ODP_RANDOM_TRUE,

> +} odp_random_kind_t;

> +

> +/**

>   * Generate random byte data

>   *

> - * @param[out]    buf   Output buffer

> - * @param         size  Size of output buffer

> - * @param use_entropy   Use entropy

> + * The intent in supporting different kinds of random data is to allow

> + * tradeoffs between performance and the quality of random data needed.

> The

> + * assumption is that basic random is cheap while true random is

> relatively

> + * expensive in terms of time to generate, with cryptographic random

> being

> + * something in between. Implementations that support highly efficient

> true

> + * random are free to use this for all requested kinds. So it is always

> + * permissible to "upgrade" a random data request, but never to

> "downgrade"

> + * such requests.

>   *

> - * @todo Define the implication of the use_entropy parameter

> + * @param[out]    buf   Output buffer

> + * @param         len   Length of output buffer in bytes

> + * @param         kind  Specifies the type of random data required.

> Request

> + *                      is expected to fail if the implementation is

> unable to

> + *                      provide the requested type.

>   *

>   * @return Number of bytes written

>   * @retval <0 on failure

>   */

> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t

> use_entropy);

> +int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);

Cannot change int types. Len and return value need to be compared commonly, and signed vs. unsigned comparison produces a compiler warning. Also C spec specify that 'int' is at least 16 bits, which mean that it may the that on some systems. Number of bytes are stored into 32 bit variables throughout the ODP API.

ODP does not support 16-bit systems. So on any system ODP would actually be implemented on int will be 32-bits. We use int as a general return type for RCs, which is why I made this change. Changing len to uint32_t is consistent with other ODP APIs since you cannot request a negative amount of data (which would be possible with signed input).  The comparison works fine on both GCC and clang. What error do you see?

... HTML comes through still ...

No, ODP API does not dictate which CPU architecture must be used. It may run on a 16 bit system with 16 bit addresses and 32 bit integers, or on a 32 bit system with 32 bit addresses and 16 bit integers, or ...

We do API definition against C spec, which contains this (as an example of int size):
minimum value for an object of type int
INT_MIN -32767 // -(2^15 - 1)
— maximum value for an object of type int
INT_MAX +32767 // 2^15 - 1


int random_data(uint8_t *buf, uint32_t len)
{
	(void)buf;

	return len;
}

int main(int argc, char *argv[])
{
	uint32_t len = 32;
	uint8_t buf[len];

	if (random_data(buf, len) != len)
		printf("error \n");

	...
}

error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  if (random_data(buf, len) != len)


-Petri
Bill Fischofer Nov. 28, 2016, 4:25 a.m. UTC | #3
On Thu, Nov 24, 2016 at 4:55 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia-bell-labs.com> wrote:

>

>

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Wednesday, November 23, 2016 4:05 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-

> labs.com>

> Cc: lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace

> use_entropy with odp_rand_kind parameter

>

>

>

> On Wed, Nov 23, 2016 at 8:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <

> petri.savolainen@nokia-bell-labs.com> wrote:

>

>

> > -----Original Message-----

> > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Bill

> > Fischofer

> > Sent: Thursday, November 03, 2016 2:42 AM

> > To: lng-odp@lists.linaro.org

> > Subject: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace

> use_entropy

> > with odp_rand_kind parameter

> >

> > Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing

> the

> > use_entropy parameter with a more comprehensive enum that specifies the

> > kind of random data requested.

> >

> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> > ---

> > Changes in v4:

> > - Normalize random API signatures with other ODP APIs

> > - Add new odp_random_seeded_data() API for repeatable random data

> > generation

> > - Add additional tests for new odp_random_seeded_data() API

> > - Break out crypto section of User Guide to its own sub-document

> > - Add User Guide docuemntation for ODP random data API.

> >

> > Changes in v3:

> > - Address commments by Petri

> > - Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the

> > mathematical term "normal"

> >

> >  include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----

> >  1 file changed, 26 insertions(+), 5 deletions(-)

> >

> > diff --git a/include/odp/api/spec/random.h

> b/include/odp/api/spec/random.h

> > index 00fa15b..62f2376 100644

> > --- a/include/odp/api/spec/random.h

> > +++ b/include/odp/api/spec/random.h

> > @@ -24,18 +24,39 @@ extern "C" {

> >   */

> >

> >  /**

> > + * Random kind selector

> > + */

> > +typedef enum {

> > +     /** Basic random, presumably pseudo-random generated by SW */

> > +     ODP_RANDOM_BASIC,

> > +     /** Cryptographic quality random */

> > +     ODP_RANDOM_CRYPTO,

> > +     /** True random, generated from a HW entropy source */

> > +     ODP_RANDOM_TRUE,

> > +} odp_random_kind_t;

> > +

> > +/**

> >   * Generate random byte data

> >   *

> > - * @param[out]    buf   Output buffer

> > - * @param         size  Size of output buffer

> > - * @param use_entropy   Use entropy

> > + * The intent in supporting different kinds of random data is to allow

> > + * tradeoffs between performance and the quality of random data needed.

> > The

> > + * assumption is that basic random is cheap while true random is

> > relatively

> > + * expensive in terms of time to generate, with cryptographic random

> > being

> > + * something in between. Implementations that support highly efficient

> > true

> > + * random are free to use this for all requested kinds. So it is always

> > + * permissible to "upgrade" a random data request, but never to

> > "downgrade"

> > + * such requests.

> >   *

> > - * @todo Define the implication of the use_entropy parameter

> > + * @param[out]    buf   Output buffer

> > + * @param         len   Length of output buffer in bytes

> > + * @param         kind  Specifies the type of random data required.

> > Request

> > + *                      is expected to fail if the implementation is

> > unable to

> > + *                      provide the requested type.

> >   *

> >   * @return Number of bytes written

> >   * @retval <0 on failure

> >   */

> > -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t

> > use_entropy);

> > +int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);

> Cannot change int types. Len and return value need to be compared

> commonly, and signed vs. unsigned comparison produces a compiler warning.

> Also C spec specify that 'int' is at least 16 bits, which mean that it may

> the that on some systems. Number of bytes are stored into 32 bit variables

> throughout the ODP API.

>

> ODP does not support 16-bit systems. So on any system ODP would actually

> be implemented on int will be 32-bits. We use int as a general return type

> for RCs, which is why I made this change. Changing len to uint32_t is

> consistent with other ODP APIs since you cannot request a negative amount

> of data (which would be possible with signed input).  The comparison works

> fine on both GCC and clang. What error do you see?

>

> ... HTML comes through still ...

>

> No, ODP API does not dictate which CPU architecture must be used. It may

> run on a 16 bit system with 16 bit addresses and 32 bit integers, or on a

> 32 bit system with 32 bit addresses and 16 bit integers, or ...

>


Actually we do. ODP explicitly does not support 16-bit systems. Assuming
one can find a 16-bit C compiler, attempting to compile it in that way
fails. We made this decision very early in the project as there were no
16-bit target systems of interest and adding support for 16-bit operation
would entail significant structural change to odp-linux well beyond the use
of types. Adding such support would also have significant performance costs
(another reason why we said we'd restrict ODP to 32-bit and 64-bit systems).


>

> We do API definition against C spec, which contains this (as an example of

> int size):

> minimum value for an object of type int

> INT_MIN -32767 // -(2^15 - 1)

> — maximum value for an object of type int

> INT_MAX +32767 // 2^15 - 1

>

>

> int random_data(uint8_t *buf, uint32_t len)

> {

>         (void)buf;

>

>         return len;

> }

>

> int main(int argc, char *argv[])

> {

>         uint32_t len = 32;

>         uint8_t buf[len];

>

>         if (random_data(buf, len) != len)

>                 printf("error \n");

>

>         ...

> }

>

> error: comparison between signed and unsigned integer expressions

> [-Werror=sign-compare]

>   if (random_data(buf, len) != len)

>

>

This same error would occur if the return type was int32_t instead of int.
The correct comparison here is:

if (random_data(buf, sizeof(buf)) != sizeof(buf)) ...

which reports no issues.


> -Petri

>

>
Savolainen, Petri (Nokia - FI/Espoo) Nov. 28, 2016, 8:38 a.m. UTC | #4
From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

Sent: Monday, November 28, 2016 6:25 AM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy with odp_rand_kind parameter



On Thu, Nov 24, 2016 at 4:55 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote:


From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

Sent: Wednesday, November 23, 2016 4:05 PM
To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy with odp_rand_kind parameter



On Wed, Nov 23, 2016 at 8:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote:


> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bill

> Fischofer

> Sent: Thursday, November 03, 2016 2:42 AM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy

> with odp_rand_kind parameter

>

> Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing the

> use_entropy parameter with a more comprehensive enum that specifies the

> kind of random data requested.

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

> Changes in v4:

> - Normalize random API signatures with other ODP APIs

> - Add new odp_random_seeded_data() API for repeatable random data

> generation

> - Add additional tests for new odp_random_seeded_data() API

> - Break out crypto section of User Guide to its own sub-document

> - Add User Guide docuemntation for ODP random data API.

>

> Changes in v3:

> - Address commments by Petri

> - Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the

> mathematical term "normal"

>

>  include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----

>  1 file changed, 26 insertions(+), 5 deletions(-)

>

> diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h

> index 00fa15b..62f2376 100644

> --- a/include/odp/api/spec/random.h

> +++ b/include/odp/api/spec/random.h

> @@ -24,18 +24,39 @@ extern "C" {

>   */

>

>  /**

> + * Random kind selector

> + */

> +typedef enum {

> +     /** Basic random, presumably pseudo-random generated by SW */

> +     ODP_RANDOM_BASIC,

> +     /** Cryptographic quality random */

> +     ODP_RANDOM_CRYPTO,

> +     /** True random, generated from a HW entropy source */

> +     ODP_RANDOM_TRUE,

> +} odp_random_kind_t;

> +

> +/**

>   * Generate random byte data

>   *

> - * @param[out]    buf   Output buffer

> - * @param         size  Size of output buffer

> - * @param use_entropy   Use entropy

> + * The intent in supporting different kinds of random data is to allow

> + * tradeoffs between performance and the quality of random data needed.

> The

> + * assumption is that basic random is cheap while true random is

> relatively

> + * expensive in terms of time to generate, with cryptographic random

> being

> + * something in between. Implementations that support highly efficient

> true

> + * random are free to use this for all requested kinds. So it is always

> + * permissible to "upgrade" a random data request, but never to

> "downgrade"

> + * such requests.

>   *

> - * @todo Define the implication of the use_entropy parameter

> + * @param[out]    buf   Output buffer

> + * @param         len   Length of output buffer in bytes

> + * @param         kind  Specifies the type of random data required.

> Request

> + *                      is expected to fail if the implementation is

> unable to

> + *                      provide the requested type.

>   *

>   * @return Number of bytes written

>   * @retval <0 on failure

>   */

> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t

> use_entropy);

> +int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);

Cannot change int types. Len and return value need to be compared commonly, and signed vs. unsigned comparison produces a compiler warning. Also C spec specify that 'int' is at least 16 bits, which mean that it may the that on some systems. Number of bytes are stored into 32 bit variables throughout the ODP API.

ODP does not support 16-bit systems. So on any system ODP would actually be implemented on int will be 32-bits. We use int as a general return type for RCs, which is why I made this change. Changing len to uint32_t is consistent with other ODP APIs since you cannot request a negative amount of data (which would be possible with signed input).  The comparison works fine on both GCC and clang. What error do you see?
... HTML comes through still ...

No, ODP API does not dictate which CPU architecture must be used. It may run on a 16 bit system with 16 bit addresses and 32 bit integers, or on a 32 bit system with 32 bit addresses and 16 bit integers, or ...

Actually we do. ODP explicitly does not support 16-bit systems. Assuming one can find a 16-bit C compiler, attempting to compile it in that way fails. We made this decision very early in the project as there were no 16-bit target systems of interest and adding support for 16-bit operation would entail significant structural change to odp-linux well beyond the use of types. Adding such support would also have significant performance costs (another reason why we said we'd restrict ODP to 32-bit and 64-bit systems).


... HTML comes through still ...

No, *API* does not dictate int size. Odp-linux implementation may fail on 16 bit system, but that does not limit anybody else from writing an ODP implementation to such a system. That's the whole idea of single, portable API spec - it allows non-Linux, special purpose CPU implementations where needed.




We do API definition against C spec, which contains this (as an example of int size):
minimum value for an object of type int
INT_MIN -32767 // -(2^15 - 1)
— maximum value for an object of type int
INT_MAX +32767 // 2^15 - 1


int random_data(uint8_t *buf, uint32_t len)
{
        (void)buf;

        return len;
}

int main(int argc, char *argv[])
{
        uint32_t len = 32;
        uint8_t buf[len];

        if (random_data(buf, len) != len)
                printf("error \n");

        ...
}

error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  if (random_data(buf, len) != len)

This same error would occur if the return type was int32_t instead of int. The correct comparison here is:

if (random_data(buf, sizeof(buf)) != sizeof(buf)) ...

which reports no issues.


... HTML comes through still ...

API cannot not dictate that sizeof(buf) must be used (directly) for comparison. There's no problem when correct types are used:

int32_t random_data(uint8_t *buf, int32_t len);

int32_t len = sizeof(buf);

if (random_data(buf, len) != len) ... or 

if (random_data(buf, sizeof(buf)) != len)

These are OK with the current spec.


-Petri
Bill Fischofer Nov. 28, 2016, 5:55 p.m. UTC | #5
On Mon, Nov 28, 2016 at 2:38 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia-bell-labs.com> wrote:
>

>

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Monday, November 28, 2016 6:25 AM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>

> Cc: lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy with odp_rand_kind parameter

>

>

>

> On Thu, Nov 24, 2016 at 4:55 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote:

>

>

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Wednesday, November 23, 2016 4:05 PM

> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com>

> Cc: lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy with odp_rand_kind parameter

>

>

>

> On Wed, Nov 23, 2016 at 8:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-labs.com> wrote:

>

>

>> -----Original Message-----

>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Bill

>> Fischofer

>> Sent: Thursday, November 03, 2016 2:42 AM

>> To: lng-odp@lists.linaro.org

>> Subject: [lng-odp] [API-NEXT PATCHv4 1/7] api: random: replace use_entropy

>> with odp_rand_kind parameter

>>

>> Address bug https://bugs.linaro.org/show_bug.cgi?id=2557 by replacing the

>> use_entropy parameter with a more comprehensive enum that specifies the

>> kind of random data requested.

>>

>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

>> ---

>> Changes in v4:

>> - Normalize random API signatures with other ODP APIs

>> - Add new odp_random_seeded_data() API for repeatable random data

>> generation

>> - Add additional tests for new odp_random_seeded_data() API

>> - Break out crypto section of User Guide to its own sub-document

>> - Add User Guide docuemntation for ODP random data API.

>>

>> Changes in v3:

>> - Address commments by Petri

>> - Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the

>> mathematical term "normal"

>>

>>  include/odp/api/spec/random.h | 31 ++++++++++++++++++++++++++-----

>>  1 file changed, 26 insertions(+), 5 deletions(-)

>>

>> diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h

>> index 00fa15b..62f2376 100644

>> --- a/include/odp/api/spec/random.h

>> +++ b/include/odp/api/spec/random.h

>> @@ -24,18 +24,39 @@ extern "C" {

>>   */

>>

>>  /**

>> + * Random kind selector

>> + */

>> +typedef enum {

>> +     /** Basic random, presumably pseudo-random generated by SW */

>> +     ODP_RANDOM_BASIC,

>> +     /** Cryptographic quality random */

>> +     ODP_RANDOM_CRYPTO,

>> +     /** True random, generated from a HW entropy source */

>> +     ODP_RANDOM_TRUE,

>> +} odp_random_kind_t;

>> +

>> +/**

>>   * Generate random byte data

>>   *

>> - * @param[out]    buf   Output buffer

>> - * @param         size  Size of output buffer

>> - * @param use_entropy   Use entropy

>> + * The intent in supporting different kinds of random data is to allow

>> + * tradeoffs between performance and the quality of random data needed.

>> The

>> + * assumption is that basic random is cheap while true random is

>> relatively

>> + * expensive in terms of time to generate, with cryptographic random

>> being

>> + * something in between. Implementations that support highly efficient

>> true

>> + * random are free to use this for all requested kinds. So it is always

>> + * permissible to "upgrade" a random data request, but never to

>> "downgrade"

>> + * such requests.

>>   *

>> - * @todo Define the implication of the use_entropy parameter

>> + * @param[out]    buf   Output buffer

>> + * @param         len   Length of output buffer in bytes

>> + * @param         kind  Specifies the type of random data required.

>> Request

>> + *                      is expected to fail if the implementation is

>> unable to

>> + *                      provide the requested type.

>>   *

>>   * @return Number of bytes written

>>   * @retval <0 on failure

>>   */

>> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t

>> use_entropy);

>> +int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);

> Cannot change int types. Len and return value need to be compared commonly, and signed vs. unsigned comparison produces a compiler warning. Also C spec specify that 'int' is at least 16 bits, which mean that it may the that on some systems. Number of bytes are stored into 32 bit variables throughout the ODP API.

>

> ODP does not support 16-bit systems. So on any system ODP would actually be implemented on int will be 32-bits. We use int as a general return type for RCs, which is why I made this change. Changing len to uint32_t is consistent with other ODP APIs since you cannot request a negative amount of data (which would be possible with signed input).  The comparison works fine on both GCC and clang. What error do you see?

> ... HTML comes through still ...

>

> No, ODP API does not dictate which CPU architecture must be used. It may run on a 16 bit system with 16 bit addresses and 32 bit integers, or on a 32 bit system with 32 bit addresses and 16 bit integers, or ...

>

> Actually we do. ODP explicitly does not support 16-bit systems. Assuming one can find a 16-bit C compiler, attempting to compile it in that way fails. We made this decision very early in the project as there were no 16-bit target systems of interest and adding support for 16-bit operation would entail significant structural change to odp-linux well beyond the use of types. Adding such support would also have significant performance costs (another reason why we said we'd restrict ODP to 32-bit and 64-bit systems).

>

>

> ... HTML comes through still ...


Hopefully this one works better and should be plain text.

>

> No, *API* does not dictate int size. Odp-linux implementation may fail on 16 bit system, but that does not limit anybody else from writing an ODP implementation to such a system. That's the whole idea of single, portable API spec - it allows non-Linux, special purpose CPU implementations where needed.


That's fine, however on a 16-bit system you're not going to be
generating megabyte-sized random buffers. a 64K buffer limit would be
more than sufficient on such systems, so this point is sort of moot.

>

>

>

>

> We do API definition against C spec, which contains this (as an example of int size):

> minimum value for an object of type int

> INT_MIN -32767 // -(2^15 - 1)

> — maximum value for an object of type int

> INT_MAX +32767 // 2^15 - 1

>

>

> int random_data(uint8_t *buf, uint32_t len)

> {

>         (void)buf;

>

>         return len;

> }

>

> int main(int argc, char *argv[])

> {

>         uint32_t len = 32;

>         uint8_t buf[len];

>

>         if (random_data(buf, len) != len)

>                 printf("error \n");

>

>         ...

> }

>

> error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]

>   if (random_data(buf, len) != len)

>

> This same error would occur if the return type was int32_t instead of int. The correct comparison here is:

>

> if (random_data(buf, sizeof(buf)) != sizeof(buf)) ...

>

> which reports no issues.

>

>

> ... HTML comes through still ...

>

> API cannot not dictate that sizeof(buf) must be used (directly) for comparison. There's no problem when correct types are used:

>

> int32_t random_data(uint8_t *buf, int32_t len);


The problem with specifying the len parameter as int32_t rather than
uint32_t is that the API would then permit negative lengths to be
used, which would clearly be in error. A length is always a
non-negative value.

>

> int32_t len = sizeof(buf);

>

> if (random_data(buf, len) != len) ... or

>

> if (random_data(buf, sizeof(buf)) != len)

>

> These are OK with the current spec.

>

>

> -Petri

>
diff mbox

Patch

diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h
index 00fa15b..62f2376 100644
--- a/include/odp/api/spec/random.h
+++ b/include/odp/api/spec/random.h
@@ -24,18 +24,39 @@  extern "C" {
  */
 
 /**
+ * Random kind selector
+ */
+typedef enum {
+	/** Basic random, presumably pseudo-random generated by SW */
+	ODP_RANDOM_BASIC,
+	/** Cryptographic quality random */
+	ODP_RANDOM_CRYPTO,
+	/** True random, generated from a HW entropy source */
+	ODP_RANDOM_TRUE,
+} odp_random_kind_t;
+
+/**
  * Generate random byte data
  *
- * @param[out]    buf   Output buffer
- * @param         size  Size of output buffer
- * @param use_entropy   Use entropy
+ * The intent in supporting different kinds of random data is to allow
+ * tradeoffs between performance and the quality of random data needed. The
+ * assumption is that basic random is cheap while true random is relatively
+ * expensive in terms of time to generate, with cryptographic random being
+ * something in between. Implementations that support highly efficient true
+ * random are free to use this for all requested kinds. So it is always
+ * permissible to "upgrade" a random data request, but never to "downgrade"
+ * such requests.
  *
- * @todo Define the implication of the use_entropy parameter
+ * @param[out]    buf   Output buffer
+ * @param         len   Length of output buffer in bytes
+ * @param         kind  Specifies the type of random data required. Request
+ *                      is expected to fail if the implementation is unable to
+ *                      provide the requested type.
  *
  * @return Number of bytes written
  * @retval <0 on failure
  */
-int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t use_entropy);
+int odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);
 
 /**
  * @}