[PATCHv3,2/4] Add helper include file with IPSec headers

Signed-off-by: Robbie King <robking@cisco.com>
---
 include/helper/odp_ipsec.h |   73 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)
 create mode 100644 include/helper/odp_ipsec.h

Hi Robbie, and all,

General question: should our examples and tests
use the same odp_ prefix for functions, struct data
types, etc defined by examples and test themselves?

It seems that currently beyond  platform/x/include/api
files only include/helper files use odp_ prefix. But are
they part of normative ODP API?

I was reading ipsec patches and I got quite confused
what constitutes normative ODP api usage and what really
comes from example itself and what is role of helper
data structures.

Thanks,
Victor

On 25 August 2014 13:47, Robbie King <robking@cisco.com> wrote:
> Signed-off-by: Robbie King <robking@cisco.com>
> ---
>  include/helper/odp_ipsec.h |   73 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
>  create mode 100644 include/helper/odp_ipsec.h
>
> diff --git a/include/helper/odp_ipsec.h b/include/helper/odp_ipsec.h
> new file mode 100644
> index 0000000..4cb81a1
> --- /dev/null
> +++ b/include/helper/odp_ipsec.h
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + *
> + * ODP IPSec headers
> + */
> +
> +#ifndef ODP_IPSEC_H_
> +#define ODP_IPSEC_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <odp_std_types.h>
> +#include <odp_byteorder.h>
> +#include <odp_align.h>
> +#include <odp_debug.h>
> +
> +#define ODP_ESPHDR_LEN      8    /**< IPSec ESP header length */
> +#define ODP_ESPTRL_LEN      2    /**< IPSec ESP trailer length */
> +#define ODP_AHHDR_LEN      12    /**< IPSec AH header length */
> +
> +/**
> + * IPSec ESP header
> + */
> +typedef struct ODP_PACKED {
> +       uint32be_t spi;      /**< Security Parameter Index */
> +       uint32be_t seq_no;   /**< Sequence Number */
> +       uint8_t    iv[0];    /**< Initialization vector */
> +} odp_esphdr_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_esphdr_t) == ODP_ESPHDR_LEN, "ODP_ESPHDR_T__SIZE_ERROR");
> +
> +/**
> + * IPSec ESP trailer
> + */
> +typedef struct ODP_PACKED {
> +       uint8_t pad_len;      /**< Padding length (0-255) */
> +       uint8_t next_header;  /**< Next header protocol */
> +       uint8_t icv[0];       /**< Integrity Check Value (optional) */
> +} odp_esptrl_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_esptrl_t) == ODP_ESPTRL_LEN, "ODP_ESPTRL_T__SIZE_ERROR");
> +
> +/**
> + * IPSec AH header
> + */
> +typedef struct ODP_PACKED {
> +       uint8_t    next_header;  /**< Next header protocol */
> +       uint8_t    ah_len;       /**< AH header length */
> +       uint16be_t pad;          /**< Padding (must be 0) */
> +       uint32be_t spi;          /**< Security Parameter Index */
> +       uint32be_t seq_no;       /**< Sequence Number */
> +       uint8_t    icv[0];       /**< Integrity Check Value */
> +} odp_ahhdr_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_ahhdr_t) == ODP_AHHDR_LEN, "ODP_AHHDR_T__SIZE_ERROR");
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> --
> 1.7.7.6
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

On 08/26/2014 09:27 AM, Victor Kamensky wrote:
> Hi Robbie, and all,
>
> General question: should our examples and tests
> use the same odp_ prefix for functions, struct data
> types, etc defined by examples and test themselves?
>
> It seems that currently beyond  platform/x/include/api
> files only include/helper files use odp_ prefix. But are
> they part of normative ODP API?
>
> I was reading ipsec patches and I got quite confused
> what constitutes normative ODP api usage and what really
> comes from example itself and what is role of helper
> data structures.
>
> Thanks,
> Victor

Btw, it's good question. This is not show stopper to that patches
but we should think about things like that.

We have some platform specific functions which named with prefix odp_.
It might be good when in back trace you see all functions with odp_ and  
it's
clear that it's related to odp library. But it's not really clear which 
function are
visible to applications. We can rename implementation internal functions 
to odpplat_
prefix. Or odplg_, odpdpdk_, odpks2_ prefixes. So we can be sure that 
apps do to call them
directly.

Maxim.
> On 25 August 2014 13:47, Robbie King <robking@cisco.com> wrote:
>> Signed-off-by: Robbie King <robking@cisco.com>
>> ---
>>   include/helper/odp_ipsec.h |   73 ++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 73 insertions(+), 0 deletions(-)
>>   create mode 100644 include/helper/odp_ipsec.h
>>
>> diff --git a/include/helper/odp_ipsec.h b/include/helper/odp_ipsec.h
>> new file mode 100644
>> index 0000000..4cb81a1
>> --- /dev/null
>> +++ b/include/helper/odp_ipsec.h
>> @@ -0,0 +1,73 @@
>> +/* Copyright (c) 2014, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier:     BSD-3-Clause
>> + */
>> +
>> +
>> +/**
>> + * @file
>> + *
>> + * ODP IPSec headers
>> + */
>> +
>> +#ifndef ODP_IPSEC_H_
>> +#define ODP_IPSEC_H_
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include <odp_std_types.h>
>> +#include <odp_byteorder.h>
>> +#include <odp_align.h>
>> +#include <odp_debug.h>
>> +
>> +#define ODP_ESPHDR_LEN      8    /**< IPSec ESP header length */
>> +#define ODP_ESPTRL_LEN      2    /**< IPSec ESP trailer length */
>> +#define ODP_AHHDR_LEN      12    /**< IPSec AH header length */
>> +
>> +/**
>> + * IPSec ESP header
>> + */
>> +typedef struct ODP_PACKED {
>> +       uint32be_t spi;      /**< Security Parameter Index */
>> +       uint32be_t seq_no;   /**< Sequence Number */
>> +       uint8_t    iv[0];    /**< Initialization vector */
>> +} odp_esphdr_t;
>> +
>> +/** @internal Compile time assert */
>> +ODP_STATIC_ASSERT(sizeof(odp_esphdr_t) == ODP_ESPHDR_LEN, "ODP_ESPHDR_T__SIZE_ERROR");
>> +
>> +/**
>> + * IPSec ESP trailer
>> + */
>> +typedef struct ODP_PACKED {
>> +       uint8_t pad_len;      /**< Padding length (0-255) */
>> +       uint8_t next_header;  /**< Next header protocol */
>> +       uint8_t icv[0];       /**< Integrity Check Value (optional) */
>> +} odp_esptrl_t;
>> +
>> +/** @internal Compile time assert */
>> +ODP_STATIC_ASSERT(sizeof(odp_esptrl_t) == ODP_ESPTRL_LEN, "ODP_ESPTRL_T__SIZE_ERROR");
>> +
>> +/**
>> + * IPSec AH header
>> + */
>> +typedef struct ODP_PACKED {
>> +       uint8_t    next_header;  /**< Next header protocol */
>> +       uint8_t    ah_len;       /**< AH header length */
>> +       uint16be_t pad;          /**< Padding (must be 0) */
>> +       uint32be_t spi;          /**< Security Parameter Index */
>> +       uint32be_t seq_no;       /**< Sequence Number */
>> +       uint8_t    icv[0];       /**< Integrity Check Value */
>> +} odp_ahhdr_t;
>> +
>> +/** @internal Compile time assert */
>> +ODP_STATIC_ASSERT(sizeof(odp_ahhdr_t) == ODP_AHHDR_LEN, "ODP_AHHDR_T__SIZE_ERROR");
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif
>> --
>> 1.7.7.6
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Hi Victor, for this file I was just following the existing format
of odp_ip.h (or attempted to anyway).

Definitely in the IPsec example, no functions/structs defined as
part of that begin with "odp_" (not that I could find in quick scan).

-----Original Message-----
From: Victor Kamensky [mailto:victor.kamensky@linaro.org] 
Sent: Tuesday, August 26, 2014 1:27 AM
To: Robbie King (robking)
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec headers

Hi Robbie, and all,

General question: should our examples and tests
use the same odp_ prefix for functions, struct data
types, etc defined by examples and test themselves?

It seems that currently beyond  platform/x/include/api
files only include/helper files use odp_ prefix. But are
they part of normative ODP API?

I was reading ipsec patches and I got quite confused
what constitutes normative ODP api usage and what really
comes from example itself and what is role of helper
data structures.

Thanks,
Victor

On 25 August 2014 13:47, Robbie King <robking@cisco.com> wrote:
> Signed-off-by: Robbie King <robking@cisco.com>
> ---
>  include/helper/odp_ipsec.h |   73 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
>  create mode 100644 include/helper/odp_ipsec.h
>
> diff --git a/include/helper/odp_ipsec.h b/include/helper/odp_ipsec.h
> new file mode 100644
> index 0000000..4cb81a1
> --- /dev/null
> +++ b/include/helper/odp_ipsec.h
> @@ -0,0 +1,73 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + *
> + * ODP IPSec headers
> + */
> +
> +#ifndef ODP_IPSEC_H_
> +#define ODP_IPSEC_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <odp_std_types.h>
> +#include <odp_byteorder.h>
> +#include <odp_align.h>
> +#include <odp_debug.h>
> +
> +#define ODP_ESPHDR_LEN      8    /**< IPSec ESP header length */
> +#define ODP_ESPTRL_LEN      2    /**< IPSec ESP trailer length */
> +#define ODP_AHHDR_LEN      12    /**< IPSec AH header length */
> +
> +/**
> + * IPSec ESP header
> + */
> +typedef struct ODP_PACKED {
> +       uint32be_t spi;      /**< Security Parameter Index */
> +       uint32be_t seq_no;   /**< Sequence Number */
> +       uint8_t    iv[0];    /**< Initialization vector */
> +} odp_esphdr_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_esphdr_t) == ODP_ESPHDR_LEN, "ODP_ESPHDR_T__SIZE_ERROR");
> +
> +/**
> + * IPSec ESP trailer
> + */
> +typedef struct ODP_PACKED {
> +       uint8_t pad_len;      /**< Padding length (0-255) */
> +       uint8_t next_header;  /**< Next header protocol */
> +       uint8_t icv[0];       /**< Integrity Check Value (optional) */
> +} odp_esptrl_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_esptrl_t) == ODP_ESPTRL_LEN, "ODP_ESPTRL_T__SIZE_ERROR");
> +
> +/**
> + * IPSec AH header
> + */
> +typedef struct ODP_PACKED {
> +       uint8_t    next_header;  /**< Next header protocol */
> +       uint8_t    ah_len;       /**< AH header length */
> +       uint16be_t pad;          /**< Padding (must be 0) */
> +       uint32be_t spi;          /**< Security Parameter Index */
> +       uint32be_t seq_no;       /**< Sequence Number */
> +       uint8_t    icv[0];       /**< Integrity Check Value */
> +} odp_ahhdr_t;
> +
> +/** @internal Compile time assert */
> +ODP_STATIC_ASSERT(sizeof(odp_ahhdr_t) == ODP_AHHDR_LEN, "ODP_AHHDR_T__SIZE_ERROR");
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> --
> 1.7.7.6
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Hi Victor,

Some clarification on helpers. Helper files/structures are there so that examples (and other simple apps) can reuse typical building blocks of a networking app (e.g. protocol header structs and constants). A full featured, production quality, application would not use helper files, but use its own definitions. Helpers are delivered "as is", those will not include every possible protocol or protocol option, etc.

There should not be many functions in helper files, and those in there are not expected to be HW accelerated (otherwise they should be under the normative ODP API). E.g. all applications running on Linux have to do process/thread creation, but Linux processes/threads won't be part of ODP API. Also e.g. IP protocol stack is not part of ODP: a real application would use some other project/product ported on top of ODP, a toy application may use helpers for simple IP stack implementation.

We could use odp_helper_ prefix (in all helper definitions) to make the distinction clear. Also we could move everything related to tests under "test" directory:

odp/test
     |
     +--helper
     +--example
     +--helper

-Petri

> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Robbie King (robking)
> Sent: Tuesday, August 26, 2014 4:07 PM
> To: Victor Kamensky
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> headers
> 
> Hi Victor, for this file I was just following the existing format
> of odp_ip.h (or attempted to anyway).
> 
> Definitely in the IPsec example, no functions/structs defined as
> part of that begin with "odp_" (not that I could find in quick scan).
> 
> -----Original Message-----
> From: Victor Kamensky [mailto:victor.kamensky@linaro.org]
> Sent: Tuesday, August 26, 2014 1:27 AM
> To: Robbie King (robking)
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> headers
> 
> Hi Robbie, and all,
> 
> General question: should our examples and tests
> use the same odp_ prefix for functions, struct data
> types, etc defined by examples and test themselves?
> 
> It seems that currently beyond  platform/x/include/api
> files only include/helper files use odp_ prefix. But are
> they part of normative ODP API?
> 
> I was reading ipsec patches and I got quite confused
> what constitutes normative ODP api usage and what really
> comes from example itself and what is role of helper
> data structures.
> 
> Thanks,
> Victor
> 
> On 25 August 2014 13:47, Robbie King <robking@cisco.com> wrote:
> > Signed-off-by: Robbie King <robking@cisco.com>
> > ---
> >  include/helper/odp_ipsec.h |   73
> ++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 73 insertions(+), 0 deletions(-)
> >  create mode 100644 include/helper/odp_ipsec.h
> >
> > diff --git a/include/helper/odp_ipsec.h b/include/helper/odp_ipsec.h
> > new file mode 100644
> > index 0000000..4cb81a1
> > --- /dev/null
> > +++ b/include/helper/odp_ipsec.h
> > @@ -0,0 +1,73 @@
> > +/* Copyright (c) 2014, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier:     BSD-3-Clause
> > + */
> > +
> > +
> > +/**
> > + * @file
> > + *
> > + * ODP IPSec headers
> > + */
> > +
> > +#ifndef ODP_IPSEC_H_
> > +#define ODP_IPSEC_H_
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <odp_std_types.h>
> > +#include <odp_byteorder.h>
> > +#include <odp_align.h>
> > +#include <odp_debug.h>
> > +
> > +#define ODP_ESPHDR_LEN      8    /**< IPSec ESP header length */
> > +#define ODP_ESPTRL_LEN      2    /**< IPSec ESP trailer length */
> > +#define ODP_AHHDR_LEN      12    /**< IPSec AH header length */
> > +
> > +/**
> > + * IPSec ESP header
> > + */
> > +typedef struct ODP_PACKED {
> > +       uint32be_t spi;      /**< Security Parameter Index */
> > +       uint32be_t seq_no;   /**< Sequence Number */
> > +       uint8_t    iv[0];    /**< Initialization vector */
> > +} odp_esphdr_t;
> > +
> > +/** @internal Compile time assert */
> > +ODP_STATIC_ASSERT(sizeof(odp_esphdr_t) == ODP_ESPHDR_LEN,
> "ODP_ESPHDR_T__SIZE_ERROR");
> > +
> > +/**
> > + * IPSec ESP trailer
> > + */
> > +typedef struct ODP_PACKED {
> > +       uint8_t pad_len;      /**< Padding length (0-255) */
> > +       uint8_t next_header;  /**< Next header protocol */
> > +       uint8_t icv[0];       /**< Integrity Check Value (optional) */
> > +} odp_esptrl_t;
> > +
> > +/** @internal Compile time assert */
> > +ODP_STATIC_ASSERT(sizeof(odp_esptrl_t) == ODP_ESPTRL_LEN,
> "ODP_ESPTRL_T__SIZE_ERROR");
> > +
> > +/**
> > + * IPSec AH header
> > + */
> > +typedef struct ODP_PACKED {
> > +       uint8_t    next_header;  /**< Next header protocol */
> > +       uint8_t    ah_len;       /**< AH header length */
> > +       uint16be_t pad;          /**< Padding (must be 0) */
> > +       uint32be_t spi;          /**< Security Parameter Index */
> > +       uint32be_t seq_no;       /**< Sequence Number */
> > +       uint8_t    icv[0];       /**< Integrity Check Value */
> > +} odp_ahhdr_t;
> > +
> > +/** @internal Compile time assert */
> > +ODP_STATIC_ASSERT(sizeof(odp_ahhdr_t) == ODP_AHHDR_LEN,
> "ODP_AHHDR_T__SIZE_ERROR");
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > --
> > 1.7.7.6
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

odp/test
     |
     +--api_test
     +--example
     +--helper

> -----Original Message-----
> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> bounces@lists.linaro.org] On Behalf Of ext Savolainen, Petri (NSN -
> FI/Espoo)
> Sent: Wednesday, August 27, 2014 10:48 AM
> To: ext Robbie King (robking); Victor Kamensky
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> headers
> 
> Hi Victor,
> 
> Some clarification on helpers. Helper files/structures are there so that
> examples (and other simple apps) can reuse typical building blocks of a
> networking app (e.g. protocol header structs and constants). A full
> featured, production quality, application would not use helper files, but
> use its own definitions. Helpers are delivered "as is", those will not
> include every possible protocol or protocol option, etc.
> 
> There should not be many functions in helper files, and those in there are
> not expected to be HW accelerated (otherwise they should be under the
> normative ODP API). E.g. all applications running on Linux have to do
> process/thread creation, but Linux processes/threads won't be part of ODP
> API. Also e.g. IP protocol stack is not part of ODP: a real application
> would use some other project/product ported on top of ODP, a toy
> application may use helpers for simple IP stack implementation.
> 
> We could use odp_helper_ prefix (in all helper definitions) to make the
> distinction clear. Also we could move everything related to tests under
> "test" directory:
> 
> odp/test
>      |
>      +--helper
>      +--example
>      +--helper
> 
> -Petri
> 
> > -----Original Message-----
> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp-
> > bounces@lists.linaro.org] On Behalf Of ext Robbie King (robking)
> > Sent: Tuesday, August 26, 2014 4:07 PM
> > To: Victor Kamensky
> > Cc: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> > headers
> >
> > Hi Victor, for this file I was just following the existing format
> > of odp_ip.h (or attempted to anyway).
> >
> > Definitely in the IPsec example, no functions/structs defined as
> > part of that begin with "odp_" (not that I could find in quick scan).
> >
> > -----Original Message-----
> > From: Victor Kamensky [mailto:victor.kamensky@linaro.org]
> > Sent: Tuesday, August 26, 2014 1:27 AM
> > To: Robbie King (robking)
> > Cc: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> > headers
> >
> > Hi Robbie, and all,
> >
> > General question: should our examples and tests
> > use the same odp_ prefix for functions, struct data
> > types, etc defined by examples and test themselves?
> >
> > It seems that currently beyond  platform/x/include/api
> > files only include/helper files use odp_ prefix. But are
> > they part of normative ODP API?
> >
> > I was reading ipsec patches and I got quite confused
> > what constitutes normative ODP api usage and what really
> > comes from example itself and what is role of helper
> > data structures.
> >
> > Thanks,
> > Victor
> >
> > On 25 August 2014 13:47, Robbie King <robking@cisco.com> wrote:
> > > Signed-off-by: Robbie King <robking@cisco.com>
> > > ---
> > >  include/helper/odp_ipsec.h |   73
> > ++++++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 73 insertions(+), 0 deletions(-)
> > >  create mode 100644 include/helper/odp_ipsec.h
> > >
> > > diff --git a/include/helper/odp_ipsec.h b/include/helper/odp_ipsec.h
> > > new file mode 100644
> > > index 0000000..4cb81a1
> > > --- /dev/null
> > > +++ b/include/helper/odp_ipsec.h
> > > @@ -0,0 +1,73 @@
> > > +/* Copyright (c) 2014, Linaro Limited
> > > + * All rights reserved.
> > > + *
> > > + * SPDX-License-Identifier:     BSD-3-Clause
> > > + */
> > > +
> > > +
> > > +/**
> > > + * @file
> > > + *
> > > + * ODP IPSec headers
> > > + */
> > > +
> > > +#ifndef ODP_IPSEC_H_
> > > +#define ODP_IPSEC_H_
> > > +
> > > +#ifdef __cplusplus
> > > +extern "C" {
> > > +#endif
> > > +
> > > +#include <odp_std_types.h>
> > > +#include <odp_byteorder.h>
> > > +#include <odp_align.h>
> > > +#include <odp_debug.h>
> > > +
> > > +#define ODP_ESPHDR_LEN      8    /**< IPSec ESP header length */
> > > +#define ODP_ESPTRL_LEN      2    /**< IPSec ESP trailer length */
> > > +#define ODP_AHHDR_LEN      12    /**< IPSec AH header length */
> > > +
> > > +/**
> > > + * IPSec ESP header
> > > + */
> > > +typedef struct ODP_PACKED {
> > > +       uint32be_t spi;      /**< Security Parameter Index */
> > > +       uint32be_t seq_no;   /**< Sequence Number */
> > > +       uint8_t    iv[0];    /**< Initialization vector */
> > > +} odp_esphdr_t;
> > > +
> > > +/** @internal Compile time assert */
> > > +ODP_STATIC_ASSERT(sizeof(odp_esphdr_t) == ODP_ESPHDR_LEN,
> > "ODP_ESPHDR_T__SIZE_ERROR");
> > > +
> > > +/**
> > > + * IPSec ESP trailer
> > > + */
> > > +typedef struct ODP_PACKED {
> > > +       uint8_t pad_len;      /**< Padding length (0-255) */
> > > +       uint8_t next_header;  /**< Next header protocol */
> > > +       uint8_t icv[0];       /**< Integrity Check Value (optional) */
> > > +} odp_esptrl_t;
> > > +
> > > +/** @internal Compile time assert */
> > > +ODP_STATIC_ASSERT(sizeof(odp_esptrl_t) == ODP_ESPTRL_LEN,
> > "ODP_ESPTRL_T__SIZE_ERROR");
> > > +
> > > +/**
> > > + * IPSec AH header
> > > + */
> > > +typedef struct ODP_PACKED {
> > > +       uint8_t    next_header;  /**< Next header protocol */
> > > +       uint8_t    ah_len;       /**< AH header length */
> > > +       uint16be_t pad;          /**< Padding (must be 0) */
> > > +       uint32be_t spi;          /**< Security Parameter Index */
> > > +       uint32be_t seq_no;       /**< Sequence Number */
> > > +       uint8_t    icv[0];       /**< Integrity Check Value */
> > > +} odp_ahhdr_t;
> > > +
> > > +/** @internal Compile time assert */
> > > +ODP_STATIC_ASSERT(sizeof(odp_ahhdr_t) == ODP_AHHDR_LEN,
> > "ODP_AHHDR_T__SIZE_ERROR");
> > > +
> > > +#ifdef __cplusplus
> > > +}
> > > +#endif
> > > +
> > > +#endif
> > > --
> > > 1.7.7.6
> > >
> > >
> > > _______________________________________________
> > > lng-odp mailing list
> > > lng-odp@lists.linaro.org
> > > http://lists.linaro.org/mailman/listinfo/lng-odp
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

On Wed, Aug 27, 2014 at 08:48:23AM +0100, Savolainen, Petri (NSN - FI/Espoo) wrote:
> Hi Victor,
> 
> Some clarification on helpers. Helper files/structures are there so
> that examples (and other simple apps) can reuse typical building
> blocks of a networking app (e.g. protocol header structs and
> constants). A full featured, production quality, application would not
> use helper files, but use its own definitions. Helpers are delivered
> "as is", those will not include every possible protocol or protocol
> option, etc.
> 
> There should not be many functions in helper files, and those in there
> are not expected to be HW accelerated (otherwise they should be under
> the normative ODP API). E.g. all applications running on Linux have to
> do process/thread creation, but Linux processes/threads won't be part
> of ODP API. Also e.g. IP protocol stack is not part of ODP: a real
> application would use some other project/product ported on top of ODP,
> a toy application may use helpers for simple IP stack implementation.
> 

So helpers are really just part of the ODP test/example code, packaged
and delivered as a convenience, but with no guarantee of stability or
functionality. We definitely need to make this clearer than it currently
is. If they aren't part of the ODP API (which I agree they are not) then
they shouldn't be documented in the API reference manual. Perhaps they
should go into a separate helper manual, or none at all.

> We could use odp_helper_ prefix (in all helper definitions) to make
> the distinction clear. Also we could move everything related to tests
> under "test" directory:
> 
> odp/test
>      |
>      +--api_test
>      +--example
>      +--helper
> 

+1 from me, they should also build into a separate library.

Looking at what's in the helpers at the minute, perhaps some of
odp_packet_helper.h should be merged into odp_packet.h (why is
odp_packet_is_valid() a helper?), and odp_ring.h should at some
point move to being an internal header.

> -----Original Message-----
> From: ext Stuart Haslam [mailto:stuart.haslam@arm.com]
> Sent: Wednesday, August 27, 2014 2:58 PM
> To: Savolainen, Petri (NSN - FI/Espoo)
> Cc: ext Robbie King (robking); Victor Kamensky; lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> headers
> 
> On Wed, Aug 27, 2014 at 08:48:23AM +0100, Savolainen, Petri (NSN -
> FI/Espoo) wrote:
> > Hi Victor,
> >
> > Some clarification on helpers. Helper files/structures are there so
> > that examples (and other simple apps) can reuse typical building
> > blocks of a networking app (e.g. protocol header structs and
> > constants). A full featured, production quality, application would not
> > use helper files, but use its own definitions. Helpers are delivered
> > "as is", those will not include every possible protocol or protocol
> > option, etc.
> >
> > There should not be many functions in helper files, and those in there
> > are not expected to be HW accelerated (otherwise they should be under
> > the normative ODP API). E.g. all applications running on Linux have to
> > do process/thread creation, but Linux processes/threads won't be part
> > of ODP API. Also e.g. IP protocol stack is not part of ODP: a real
> > application would use some other project/product ported on top of ODP,
> > a toy application may use helpers for simple IP stack implementation.
> >
> 
> So helpers are really just part of the ODP test/example code, packaged
> and delivered as a convenience, but with no guarantee of stability or
> functionality. We definitely need to make this clearer than it currently
> is. If they aren't part of the ODP API (which I agree they are not) then
> they shouldn't be documented in the API reference manual. Perhaps they
> should go into a separate helper manual, or none at all.
> 
> > We could use odp_helper_ prefix (in all helper definitions) to make
> > the distinction clear. Also we could move everything related to tests
> > under "test" directory:
> >
> > odp/test
> >      |
> >      +--api_test
> >      +--example
> >      +--helper
> >
> 
> +1 from me, they should also build into a separate library.
> 
> Looking at what's in the helpers at the minute, perhaps some of
> odp_packet_helper.h should be merged into odp_packet.h (why is
> odp_packet_is_valid() a helper?), and odp_ring.h should at some
> point move to being an internal header.
> 

True, packet helper needs cleaning. About ring I'm not sure, do we think that rings can be HW accelerated? We have already HW accelerated queues.

-Petri

 

> --
> Stuart.

On Wed, Aug 27, 2014 at 01:10:13PM +0100, Savolainen, Petri (NSN - FI/Espoo) wrote:
> 
> 
> > -----Original Message-----
> > From: ext Stuart Haslam [mailto:stuart.haslam@arm.com]
> > Sent: Wednesday, August 27, 2014 2:58 PM
> > To: Savolainen, Petri (NSN - FI/Espoo)
> > Cc: ext Robbie King (robking); Victor Kamensky; lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> > headers
> > 
> > On Wed, Aug 27, 2014 at 08:48:23AM +0100, Savolainen, Petri (NSN -
> > FI/Espoo) wrote:
> > > Hi Victor,
> > >
> > > Some clarification on helpers. Helper files/structures are there so
> > > that examples (and other simple apps) can reuse typical building
> > > blocks of a networking app (e.g. protocol header structs and
> > > constants). A full featured, production quality, application would not
> > > use helper files, but use its own definitions. Helpers are delivered
> > > "as is", those will not include every possible protocol or protocol
> > > option, etc.
> > >
> > > There should not be many functions in helper files, and those in there
> > > are not expected to be HW accelerated (otherwise they should be under
> > > the normative ODP API). E.g. all applications running on Linux have to
> > > do process/thread creation, but Linux processes/threads won't be part
> > > of ODP API. Also e.g. IP protocol stack is not part of ODP: a real
> > > application would use some other project/product ported on top of ODP,
> > > a toy application may use helpers for simple IP stack implementation.
> > >
> > 
> > So helpers are really just part of the ODP test/example code, packaged
> > and delivered as a convenience, but with no guarantee of stability or
> > functionality. We definitely need to make this clearer than it currently
> > is. If they aren't part of the ODP API (which I agree they are not) then
> > they shouldn't be documented in the API reference manual. Perhaps they
> > should go into a separate helper manual, or none at all.
> > 
> > > We could use odp_helper_ prefix (in all helper definitions) to make
> > > the distinction clear. Also we could move everything related to tests
> > > under "test" directory:
> > >
> > > odp/test
> > >      |
> > >      +--api_test
> > >      +--example
> > >      +--helper
> > >
> > 
> > +1 from me, they should also build into a separate library.
> > 
> > Looking at what's in the helpers at the minute, perhaps some of
> > odp_packet_helper.h should be merged into odp_packet.h (why is
> > odp_packet_is_valid() a helper?), and odp_ring.h should at some
> > point move to being an internal header.
> > 
> 
> True, packet helper needs cleaning. About ring I'm not sure, do we
> think that rings can be HW accelerated? We have already HW accelerated
> queues.
> 
> -Petri

I think consensus is that rings can't be accelerated, so rings may be used
internally but the public API should be queues.

In the Cisco DP work I'm doing, I'm currently using some polled queues
as "free lists" for objects.  I assume that platforms that can do
scheduling in HW but not necessarily polling that there will
be an architecture efficient SW implementation available.

-----Original Message-----
From: Stuart Haslam [mailto:stuart.haslam@arm.com] 
Sent: Wednesday, August 27, 2014 8:37 AM
To: Savolainen, Petri (NSN - FI/Espoo)
Cc: Robbie King (robking); Victor Kamensky; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec headers

On Wed, Aug 27, 2014 at 01:10:13PM +0100, Savolainen, Petri (NSN - FI/Espoo) wrote:
> 
> 
> > -----Original Message-----
> > From: ext Stuart Haslam [mailto:stuart.haslam@arm.com]
> > Sent: Wednesday, August 27, 2014 2:58 PM
> > To: Savolainen, Petri (NSN - FI/Espoo)
> > Cc: ext Robbie King (robking); Victor Kamensky; lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCHv3 2/4] Add helper include file with IPSec
> > headers
> > 
> > On Wed, Aug 27, 2014 at 08:48:23AM +0100, Savolainen, Petri (NSN -
> > FI/Espoo) wrote:
> > > Hi Victor,
> > >
> > > Some clarification on helpers. Helper files/structures are there so
> > > that examples (and other simple apps) can reuse typical building
> > > blocks of a networking app (e.g. protocol header structs and
> > > constants). A full featured, production quality, application would not
> > > use helper files, but use its own definitions. Helpers are delivered
> > > "as is", those will not include every possible protocol or protocol
> > > option, etc.
> > >
> > > There should not be many functions in helper files, and those in there
> > > are not expected to be HW accelerated (otherwise they should be under
> > > the normative ODP API). E.g. all applications running on Linux have to
> > > do process/thread creation, but Linux processes/threads won't be part
> > > of ODP API. Also e.g. IP protocol stack is not part of ODP: a real
> > > application would use some other project/product ported on top of ODP,
> > > a toy application may use helpers for simple IP stack implementation.
> > >
> > 
> > So helpers are really just part of the ODP test/example code, packaged
> > and delivered as a convenience, but with no guarantee of stability or
> > functionality. We definitely need to make this clearer than it currently
> > is. If they aren't part of the ODP API (which I agree they are not) then
> > they shouldn't be documented in the API reference manual. Perhaps they
> > should go into a separate helper manual, or none at all.
> > 
> > > We could use odp_helper_ prefix (in all helper definitions) to make
> > > the distinction clear. Also we could move everything related to tests
> > > under "test" directory:
> > >
> > > odp/test
> > >      |
> > >      +--api_test
> > >      +--example
> > >      +--helper
> > >
> > 
> > +1 from me, they should also build into a separate library.
> > 
> > Looking at what's in the helpers at the minute, perhaps some of
> > odp_packet_helper.h should be merged into odp_packet.h (why is
> > odp_packet_is_valid() a helper?), and odp_ring.h should at some
> > point move to being an internal header.
> > 
> 
> True, packet helper needs cleaning. About ring I'm not sure, do we
> think that rings can be HW accelerated? We have already HW accelerated
> queues.
> 
> -Petri

I think consensus is that rings can't be accelerated, so rings may be used
internally but the public API should be queues.