That seems reasonable.  Just wanted to make sure we weren't limiting the
potential number of classes of service since they can well exceed 256.

On Wed, Jan 14, 2015 at 6:45 AM, Balasubramanian Manoharan <
[email protected]> wrote:

>
> On 14/01/15 5:28 pm, Bill Fischofer wrote:
>
>
>
> On Wed, Jan 14, 2015 at 5:19 AM, Savolainen, Petri (NSN - FI/Espoo) <
> [email protected]> wrote:
>
>> Reviewed-by: Petri Savolainen <[email protected]>
>>
>> A patch set (one type of modification per patch) would have been better
>> approach, but I'm OK with this since changes were trivial.
>>
>>
>> -Petri
>>
>>
>>
>> > -----Original Message-----
>> > From: [email protected] [mailto:lng-odp-
>> > [email protected]] On Behalf Of ext Balasubramanian Manoharan
>> > Sent: Wednesday, January 14, 2015 8:05 AM
>> > To: [email protected]
>> > Subject: [lng-odp] [PATCH v1] api: Move Pktio related APIs to pktio
>> Header
>> > file
>> >
>> > The following APIs are setting parameters to pktio handle and are hence
>> > moved from classification header to pktio header file.
>> >       odp_pktio_default_cos_set()
>> >       odp_pktio_error_cos_set()
>> >       odp_pktio_skip_set()
>> >       odp_pktio_headroom_set()
>> >
>> > This patch also modifies the error return values in API description
>> > from -1 to non-zero value
>> >
>> > Signed-off-by: Balasubramanian Manoharan <[email protected]>
>> > ---
>> >  .../linux-generic/include/api/odp_classification.h | 92
>> ++++-------------
>> > -----
>> >  platform/linux-generic/include/api/odp_packet_io.h | 55 +++++++++++++
>> >  .../linux-generic/include/api/odp_platform_types.h |  3 +
>> >  platform/linux-generic/odp_classification.c        | 22 +++---
>> >  platform/linux-generic/odp_init.c                  |  2 +-
>> >  5 files changed, 86 insertions(+), 88 deletions(-)
>> >
>> > diff --git a/platform/linux-generic/include/api/odp_classification.h
>> > b/platform/linux-generic/include/api/odp_classification.h
>> > index 4c9674b..46189bc 100644
>> > --- a/platform/linux-generic/include/api/odp_classification.h
>> > +++ b/platform/linux-generic/include/api/odp_classification.h
>> > @@ -22,7 +22,6 @@ extern "C" {
>> >  #include <odp_std_types.h>
>> >  #include <odp_buffer_pool.h>
>> >  #include <odp_packet.h>
>> > -#include <odp_packet_io.h>
>> >  #include <odp_queue.h>
>> >
>> >  /** @defgroup odp_classification ODP CLASSIFICATION
>> > @@ -30,11 +29,6 @@ extern "C" {
>> >   *  @{
>> >   */
>> >
>> > -/**
>> > - * Class of service instance type
>> > - */
>> > -typedef uint32_t odp_cos_t;
>> > -
>> >
>> >  /**
>> >   * flow signature type, only used for packet metadata field.
>> > @@ -95,7 +89,7 @@ odp_cos_t odp_cos_create(const char *name);
>> >   *
>> >   * @param[in]        cos_id  class-of-service instance.
>> >   *
>> > - * @return           0 on success, -1 on error.
>> > + * @return           0 on success, non-zero on error.
>> >   */
>> >  int odp_cos_destroy(odp_cos_t cos_id);
>> >
>> > @@ -108,7 +102,7 @@ int odp_cos_destroy(odp_cos_t cos_id);
>> >   *                           of this specific class of service
>> >   *                           will be enqueued.
>> >   *
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   */
>> >  int odp_cos_set_queue(odp_cos_t cos_id, odp_queue_t queue_id);
>> >
>> > @@ -118,67 +112,13 @@ int odp_cos_set_queue(odp_cos_t cos_id,
>> odp_queue_t
>> > queue_id);
>> >   * @param[in]        cos_id          class-of-service instance.
>> >   * @param[in]        drop_policy     Desired packet drop policy for
>> this class.
>> >   *
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   *
>> >   * @note Optional.
>> >   */
>> >  int odp_cos_set_drop(odp_cos_t cos_id, odp_drop_e drop_policy);
>> >
>> >  /**
>> > - * Setup per-port default class-of-service.
>> > - *
>> > - * @param[in]        pktio_in        Ingress port identifier.
>> > - * @param[in]        default_cos     Class-of-service set to all
>> packets arriving
>> > - *                           at the pktio_in ingress port,
>> > - *                           unless overridden by subsequent
>> > - *                           header-based filters.
>> > - *
>> > - * @return                   0 on success, -1 on error.
>> > - */
>> > -int odp_pktio_set_default_cos(odp_pktio_t pktio_in, odp_cos_t
>> > default_cos);
>> > -
>> > -/**
>> > - * Setup per-port error class-of-service
>> > - *
>> > - * @param[in]        pktio_in        Ingress port identifier.
>> > - * @param[in]        error_cos       class-of-service set to all
>> packets arriving
>> > - *                           at the pktio_in ingress port
>> > - *                           that contain an error.
>> > - *
>> > - * @return                   0 on success, -1 on error.
>> > - *
>> > - * @note Optional.
>> > - */
>> > -int odp_pktio_set_error_cos(odp_pktio_t pktio_in, odp_cos_t error_cos);
>> > -
>> > -/**
>> > - * Setup per-port header offset
>> > - *
>> > - * @param[in]        pktio_in        Ingress port identifier.
>> > - * @param[in]        offset          Number of bytes the classifier
>> must
>> > skip.
>> > - *
>> > - * @return                   0 on success, -1 on error.
>> > - * @note  Optional.
>> > - *
>> > - */
>> > -int odp_pktio_set_skip(odp_pktio_t pktio_in, size_t offset);
>> > -
>> > -/**
>> > - * Specify per-port buffer headroom
>> > - *
>> > - * @param[in]        pktio_in        Ingress port identifier.
>> > - * @param[in]        headroom        Number of bytes of space preceding
>> > - *                           packet data to reserve for use as
>> headroom.
>> > - *                           Must not exceed the implementation
>> > - *                           defined ODP_PACKET_MAX_HEADROOM.
>> > - *
>> > - * @return                   0 on success, -1 on error.
>> > - *
>> > - * @note Optional.
>> > - */
>> > -int odp_pktio_set_headroom(odp_pktio_t pktio_in, size_t headroom);
>> > -
>> > -/**
>> >   * Request to override per-port class of service
>> >   * based on Layer-2 priority field if present.
>> >   *
>> > @@ -187,10 +127,10 @@ int odp_pktio_set_headroom(odp_pktio_t pktio_in,
>> > size_t headroom);
>> >   * @param[in]        qos_table       Values of the Layer-2 QoS header
>> field.
>> >   * @param[in]        cos_table       Class-of-service assigned to each
>> of the
>> >   *                           allowed Layer-2 QOS levels.
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   */
>> >  int odp_cos_with_l2_priority(odp_pktio_t pktio_in,
>> > -                          size_t num_qos,
>> > +                          uint8_t num_qos,
>>
>
>  Is the intent here that we're limiting the potential numbers of QoS
> entries to 256 or that we're limiting the number that can be set with a
> single call?  The latter might be reasonable, but it seems the former is an
> unnecessary architectural restriction.
>
> There can only be one CoS associated with each L2 priority value per pktio
> interface.
> Hence 256 seemed a reasonable MAX limit for number of L2 priority which
> can be set in a single call.
> If more than 256 priority value needs to be set for any  proprietary L2
> protocol the same can be done in multiple calls.
>
>
>
>>  >                            uint8_t qos_table[],
>> >                            odp_cos_t cos_table[]);
>> >
>> > @@ -206,15 +146,15 @@ int odp_cos_with_l2_priority(odp_pktio_t pktio_in,
>> >   * @param[in]        l3_preference   when true, Layer-3 QoS overrides
>> >   *                           L2 QoS when present.
>> >   *
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   *
>> >   * @note Optional.
>> >   */
>> >  int odp_cos_with_l3_qos(odp_pktio_t pktio_in,
>> > -                     size_t num_qos,
>> > +                     uint32_t num_qos,
>> >                       uint8_t qos_table[],
>> >                       odp_cos_t cos_table[],
>> > -                     bool l3_preference);
>> > +                     odp_bool_t l3_preference);
>> >
>> >
>> >  /**
>> > @@ -284,7 +224,7 @@ typedef enum odp_pmr_term {
>> >  odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> >                              const void *val,
>> >                              const void *mask,
>> > -                            size_t val_sz);
>> > +                            uint32_t val_sz);
>> >
>> >  /**
>> >   * Create a packet match rule with value range
>> > @@ -302,13 +242,13 @@ odp_pmr_t odp_pmr_create_match(odp_pmr_term_e
>> term,
>> >  odp_pmr_t odp_pmr_create_range(odp_pmr_term_e term,
>> >                              const void *val1,
>> >                              const void *val2,
>> > -                            size_t val_sz);
>> > +                            uint32_t val_sz);
>> >  /**
>> >   * Invalidate a packet match rule and vacate its resources
>> >   *
>> >   * @param[in]        pmr_id  Identifier of the PMR to be destroyed
>> >   *
>> > - * @return           0 on success, -1 or error.
>> > + * @return           0 on success, non-zero or error.
>> >   */
>> >  int odp_pmr_destroy(odp_pmr_t pmr_id);
>> >
>> > @@ -319,7 +259,7 @@ int odp_pmr_destroy(odp_pmr_t pmr_id);
>> >   * @param[in]        src_pktio       pktio to which this PMR is to be
>> applied
>> >   * @param[in]        dst_cos         CoS to be assigned by this PMR
>> >   *
>> > - * @return                   0 on success, -1 or error.
>> > + * @return                   0 on success, non-zero or error.
>> >   */
>> >  int odp_pktio_pmr_cos(odp_pmr_t pmr_id,
>> >                     odp_pktio_t src_pktio, odp_cos_t dst_cos);
>> > @@ -332,7 +272,7 @@ int odp_pktio_pmr_cos(odp_pmr_t pmr_id,
>> >   * @param[in]        dst_cos         CoS to be assigned to packets
>> filtered
>> >   *                           from src_cos that match pmr_id.
>> >   *
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   */
>> >  int odp_cos_pmr_cos(odp_pmr_t pmr_id, odp_cos_t src_cos, odp_cos_t
>> > dst_cos);
>> >
>> > @@ -411,7 +351,7 @@ typedef uint32_t odp_pmr_set_t;
>> >   *                           that have been successfully mapped to the
>> >   *                           underlying platform classification engine
>> and
>> >   *                           may be in the range from 1 to num_terms,
>> > - *                           or -1 for error.
>> > + *                           or non-zero for error.
>> >   */
>> >  int odp_pmr_match_set_create(int num_terms, odp_pmr_match_t *terms,
>> >                            odp_pmr_set_t *pmr_set_id);
>> > @@ -429,7 +369,7 @@ int odp_pmr_match_set_create(int num_terms,
>> > odp_pmr_match_t *terms,
>> >   * @param[in]        pmr_set_id      A composite rule-set handle
>> >   *                           returned when created.
>> >   *
>> > - * @return                   0 on success, -1 on error.
>> > + * @return                   0 on success, non-zero on error.
>> >   */
>> >  int odp_pmr_match_set_destroy(odp_pmr_set_t pmr_set_id);
>> >
>> > @@ -441,7 +381,7 @@ int odp_pmr_match_set_destroy(odp_pmr_set_t
>> > pmr_set_id);
>> >   *                           set is to be applied
>> >   * @param[in]        dst_cos         CoS to be assigned by this PMR
>> match
>> > set
>> >   *
>> > - * @return                   0 on success, -1 or error.
>> > + * @return                   0 on success, non-zero or error.
>> >   */
>> >  int odp_pktio_pmr_match_set_cos(odp_pmr_set_t pmr_set_id, odp_pktio_t
>> > src_pktio,
>> >                               odp_cos_t dst_cos);
>> > diff --git a/platform/linux-generic/include/api/odp_packet_io.h
>> > b/platform/linux-generic/include/api/odp_packet_io.h
>> > index 0c34f29..4835f4d 100644
>> > --- a/platform/linux-generic/include/api/odp_packet_io.h
>> > +++ b/platform/linux-generic/include/api/odp_packet_io.h
>> > @@ -169,6 +169,61 @@ size_t odp_pktio_mac_addr(odp_pktio_t id, void
>> > *mac_addr,
>> >                         size_t addr_size);
>> >
>> >  /**
>> > + * Setup per-port default class-of-service.
>> > + *
>> > + * @param[in]        pktio_in        Ingress port identifier.
>> > + * @param[in]        default_cos     Class-of-service set to all
>> packets arriving
>> > + *                           at the pktio_in ingress port,
>> > + *                           unless overridden by subsequent
>> > + *                           header-based filters.
>> > + *
>> > + * @return                   0 on success, non-zero on error.
>> > + */
>> > +int odp_pktio_default_cos_set(odp_pktio_t pktio_in, odp_cos_t
>> > default_cos);
>> > +
>> > +/**
>> > + * Setup per-port error class-of-service
>> > + *
>> > + * @param[in]        pktio_in        Ingress port identifier.
>> > + * @param[in]        error_cos       class-of-service set to all
>> packets arriving
>> > + *                           at the pktio_in ingress port
>> > + *                           that contain an error.
>> > + *
>> > + * @return                   0 on success, non-zero on error.
>> > + *
>> > + * @note Optional.
>> > + */
>> > +int odp_pktio_error_cos_set(odp_pktio_t pktio_in, odp_cos_t error_cos);
>> > +
>> > +/**
>> > + * Setup per-port header offset
>> > + *
>> > + * @param[in]        pktio_in        Ingress port identifier.
>> > + * @param[in]        offset          Number of bytes the classifier
>> must
>> > skip.
>> > + *
>> > + * @return                   0 on success, non-zero on error.
>> > + * @note  Optional.
>> > + *
>> > + */
>> > +int odp_pktio_skip_set(odp_pktio_t pktio_in, uint32_t offset);
>> > +
>> > +/**
>> > + * Specify per-port buffer headroom
>> > + *
>> > + * @param[in]        pktio_in        Ingress port identifier.
>> > + * @param[in]        headroom        Number of bytes of space preceding
>> > + *                           packet data to reserve for use as
>> headroom.
>> > + *                           Must not exceed the implementation
>> > + *                           defined ODP_PACKET_MAX_HEADROOM.
>> > + *
>> > + * @return                   0 on success, non-zero on error.
>> > + *
>> > + * @note Optional.
>> > + */
>> > +int odp_pktio_headroom_set(odp_pktio_t pktio_in, uint32_t headroom);
>> > +
>> > +
>> > +/**
>> >   * @}
>> >   */
>> >
>> > diff --git a/platform/linux-generic/include/api/odp_platform_types.h
>> > b/platform/linux-generic/include/api/odp_platform_types.h
>> > index 0a00219..6ed9e78 100644
>> > --- a/platform/linux-generic/include/api/odp_platform_types.h
>> > +++ b/platform/linux-generic/include/api/odp_platform_types.h
>> > @@ -74,6 +74,9 @@ typedef uint32_t odp_shm_t;
>> >  #define ODP_SHM_INVALID 0
>> >  #define ODP_SHM_NULL ODP_SHM_INVALID /**< Synonym for buffer pool use
>> */
>> >
>> > +/** ODP Class of service handle */
>> > +typedef uint32_t odp_cos_t;
>> > +
>> >  /**
>> >   * @}
>> >   */
>> > diff --git a/platform/linux-generic/odp_classification.c
>> b/platform/linux-
>> > generic/odp_classification.c
>> > index eeb049a..7d09cce 100644
>> > --- a/platform/linux-generic/odp_classification.c
>> > +++ b/platform/linux-generic/odp_classification.c
>> > @@ -256,7 +256,7 @@ int odp_cos_set_drop(odp_cos_t cos_id, odp_drop_e
>> > drop_policy)
>> >       return 0;
>> >  }
>> >
>> > -int odp_pktio_set_default_cos(odp_pktio_t pktio_in, odp_cos_t
>> > default_cos)
>> > +int odp_pktio_default_cos_set(odp_pktio_t pktio_in, odp_cos_t
>> > default_cos)
>> >  {
>> >       pktio_entry_t *entry;
>> >       cos_t *cos;
>> > @@ -275,7 +275,7 @@ int odp_pktio_set_default_cos(odp_pktio_t pktio_in,
>> > odp_cos_t default_cos)
>> >       return 0;
>> >  }
>> >
>> > -int odp_pktio_set_error_cos(odp_pktio_t pktio_in, odp_cos_t error_cos)
>> > +int odp_pktio_error_cos_set(odp_pktio_t pktio_in, odp_cos_t error_cos)
>> >  {
>> >       pktio_entry_t *entry;
>> >       cos_t *cos;
>> > @@ -296,7 +296,7 @@ int odp_pktio_set_error_cos(odp_pktio_t pktio_in,
>> > odp_cos_t error_cos)
>> >       return 0;
>> >  }
>> >
>> > -int odp_pktio_set_skip(odp_pktio_t pktio_in, size_t offset)
>> > +int odp_pktio_skip_set(odp_pktio_t pktio_in, uint32_t offset)
>> >  {
>> >       pktio_entry_t *entry = get_pktio_entry(pktio_in);
>> >       if (entry == NULL) {
>> > @@ -308,7 +308,7 @@ int odp_pktio_set_skip(odp_pktio_t pktio_in, size_t
>> > offset)
>> >       return 0;
>> >  }
>> >
>> > -int odp_pktio_set_headroom(odp_pktio_t pktio_in, size_t headroom)
>> > +int odp_pktio_headroom_set(odp_pktio_t pktio_in, uint32_t headroom)
>> >  {
>> >       pktio_entry_t *entry = get_pktio_entry(pktio_in);
>> >       if (entry == NULL) {
>> > @@ -320,12 +320,12 @@ int odp_pktio_set_headroom(odp_pktio_t pktio_in,
>> > size_t headroom)
>> >  }
>> >
>> >  int odp_cos_with_l2_priority(odp_pktio_t pktio_in,
>> > -                          size_t num_qos,
>> > +                          uint8_t num_qos,
>> >                            uint8_t qos_table[],
>> >                            odp_cos_t cos_table[])
>> >  {
>> >       pmr_l2_cos_t *l2_cos;
>> > -     size_t i;
>> > +     uint32_t i;
>> >       cos_t *cos;
>> >       pktio_entry_t *entry = get_pktio_entry(pktio_in);
>> >       if (entry == NULL) {
>> > @@ -348,13 +348,13 @@ int odp_cos_with_l2_priority(odp_pktio_t pktio_in,
>> >  }
>> >
>> >  int odp_cos_with_l3_qos(odp_pktio_t pktio_in,
>> > -                     size_t num_qos,
>> > +                     uint32_t num_qos,
>> >                       uint8_t qos_table[],
>> >                       odp_cos_t cos_table[],
>> > -                     bool l3_preference)
>> > +                     odp_bool_t l3_preference)
>> >  {
>> >       pmr_l3_cos_t *l3_cos;
>> > -     size_t i;
>> > +     uint32_t i;
>> >       pktio_entry_t *entry = get_pktio_entry(pktio_in);
>> >       cos_t *cos;
>> >
>> > @@ -382,7 +382,7 @@ int odp_cos_with_l3_qos(odp_pktio_t pktio_in,
>> >  odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> >                              const void *val,
>> >                              const void *mask,
>> > -                            size_t val_sz)
>> > +                            uint32_t val_sz)
>> >  {
>> >       pmr_t *pmr;
>> >       odp_pmr_t id;
>> > @@ -410,7 +410,7 @@ odp_pmr_t odp_pmr_create_match(odp_pmr_term_e term,
>> >  odp_pmr_t odp_pmr_create_range(odp_pmr_term_e term,
>> >                              const void *val1,
>> >                              const void *val2,
>> > -                            size_t val_sz)
>> > +                            uint32_t val_sz)
>> >  {
>> >       pmr_t *pmr;
>> >       odp_pmr_t id;
>> > diff --git a/platform/linux-generic/odp_init.c b/platform/linux-
>> > generic/odp_init.c
>> > index 4d0aa07..77bfd09 100644
>> > --- a/platform/linux-generic/odp_init.c
>> > +++ b/platform/linux-generic/odp_init.c
>> > @@ -55,7 +55,7 @@ int odp_init_global(odp_init_t *params  ODP_UNUSED,
>> >               return -1;
>> >       }
>> >       if (odp_classification_init_global()) {
>> > -             ODP_ERR("ODP crypto init failed.\n");
>> > +             ODP_ERR("ODP classification init failed.\n");
>> >               return -1;
>> >       }
>> >
>> > --
>> > 2.0.1.472.g6f92e5f
>> >
>> >
>> > _______________________________________________
>> > lng-odp mailing list
>> > [email protected]
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>> _______________________________________________
>> lng-odp mailing list
>> [email protected]
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to