From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hemant Agrawal <hemant.agrawal@nxp.com>
Subject: Re: [PATCH V3 3/5] ethdev: add new api for traffic
 metering and policing
Date: Thu, 12 Oct 2017 16:24:10 +0530
Message-ID: <293766e1-462b-70e2-9fba-7f9cab5f24ee@nxp.com>
References: <1507208974-180500-2-git-send-email-cristian.dumitrescu@intel.com>
 <1507301136-131382-1-git-send-email-cristian.dumitrescu@intel.com>
 <1507301136-131382-4-git-send-email-cristian.dumitrescu@intel.com>
 <c22ea29e-4793-ef67-6fbd-f2ca447717bb@nxp.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: <thomas@monjalon.net>, <adrien.mazarguil@6wind.com>,
 <jingjing.wu@intel.com>, <jerin.jacob@caviumnetworks.com>,
 <jasvinder.singh@intel.com>
To: Cristian Dumitrescu <cristian.dumitrescu@intel.com>, <dev@dpdk.org>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM02-SN1-obe.outbound.protection.outlook.com
 (mail-sn1nam02on0081.outbound.protection.outlook.com [104.47.36.81])
 by dpdk.org (Postfix) with ESMTP id 3EA571B204
 for <dev@dpdk.org>; Thu, 12 Oct 2017 12:54:16 +0200 (CEST)
In-Reply-To: <c22ea29e-4793-ef67-6fbd-f2ca447717bb@nxp.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 10/12/2017 4:18 PM, Hemant Agrawal wrote:
> On 10/6/2017 8:15 PM, Cristian Dumitrescu wrote:
>
> Overall the patch looks ok. Few small comments.
>
>> diff --git a/lib/librte_ether/rte_mtr.h b/lib/librte_ether/rte_mtr.h
>> new file mode 100644
>> index 0000000..f0f897f
>> --- /dev/null
>> +++ b/lib/librte_ether/rte_mtr.h
>> @@ -0,0 +1,723 @@
>> +/*-
>> + *   BSD LICENSE
>> + *
>> + *   Copyright(c) 2017 Intel Corporation.
>> + *   Copyright(c) 2017 NXP.
>
> can you make it "Copyright 2017 NXP"
>> + *   Copyright(c) 2017 Cavium.
>> + *   All rights reserved.
>> + *
>
> <snip>...
>
>> +/**
>> + * Statistics counter type
>> + */
>> +enum rte_mtr_stats_type {
>> +    /** Number of packets passed as green by the policer. */
>> +    RTE_MTR_STATS_N_PKTS_GREEN = 1 << 0,
>> +
>> +    /** Number of bytes passed as green by the policer. */
>> +    RTE_MTR_STATS_N_BYTES_GREEN = 1 << 1,
>> +
>> +    /** Number of packets passed as yellow by the policer. */
>> +    RTE_MTR_STATS_N_PKTS_YELLOW = 1 << 2,
>> +
>> +    /** Number of bytes passed as yellow by the policer. */
>> +    RTE_MTR_STATS_N_BYTES_YELLOW = 1 << 3,
>> +
>> +    /** Number of packets passed as red by the policer. */
>> +    RTE_MTR_STATS_N_PKTS_RED = 1 << 4,
>> +
>> +    /** Number of bytes passed as red by the policer. */
>> +    RTE_MTR_STATS_N_BYTES_RED = 1 << 5,
>> +
>> +    /** Number of packets dropped by the policer. */
>> +    RTE_MTR_STATS_N_PKTS_DROPPED = 1 << 6,
>> +
>> +    /** Number of bytes dropped by the policer. */
>> +    RTE_MTR_STATS_N_BYTES_DROPPED = 1 << 7,
>
> Trivial one.
> do you want to group the pkts and bytes in separate group respectively?
>
>> +};
>> +
>> +/**
>> + * Statistics counters
>> + */
>> +struct rte_mtr_stats {
>> +    /** Number of packets passed by the policer (per color). */
>> +    uint64_t n_pkts[RTE_MTR_COLORS];
>> +
>> +    /** Number of bytes passed by the policer (per color). */
>> +    uint64_t n_bytes[RTE_MTR_COLORS];
>> +
>> +    /** Number of packets dropped by the policer. */
>> +    uint64_t n_pkts_dropped;
>> +
>> +    /** Number of bytes passed by the policer. */
>> +    uint64_t n_bytes_dropped;
>> +};
>> +
>> +/**
>> + * Traffic metering algorithms
>> + */
>> +enum rte_mtr_algorithm {
>> +    /** Single Rate Three Color Marker (srTCM) - IETF RFC 2697. */
>> +    RTE_MTR_SRTCM_RFC2697,
>> +
>> +    /** Two Rate Three Color Marker (trTCM) - IETF RFC 2698. */
>> +    RTE_MTR_TRTCM_RFC2698,
>> +
>> +    /** Two Rate Three Color Marker (trTCM) - IETF RFC 4115. */
>> +    RTE_MTR_TRTCM_RFC4115,
>
> I will still prefer if you still add "PASS_THROUGH" as explicit algo.
>
>> +};
>> +
>> +/**
>> + * Meter profile
>> + */
>
>
>
Reviewed-by:  Hemant Agrawal <hemant.agrawal@nxp.com>