From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 9FBE7A0487
	for <public@inbox.dpdk.org>; Mon,  1 Jul 2019 20:59:00 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 9E8B61B9CE;
	Mon,  1 Jul 2019 20:58:59 +0200 (CEST)
Received: from mga12.intel.com (mga12.intel.com [192.55.52.136])
 by dpdk.org (Postfix) with ESMTP id 465401B965
 for <dev@dpdk.org>; Mon,  1 Jul 2019 20:58:58 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga002.fm.intel.com ([10.253.24.26])
 by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 01 Jul 2019 11:58:57 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.63,440,1557212400"; d="scan'208";a="190355224"
Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31])
 by fmsmga002.fm.intel.com with ESMTP; 01 Jul 2019 11:58:56 -0700
Received: from irsmsx108.ger.corp.intel.com ([169.254.11.46]) by
 IRSMSX106.ger.corp.intel.com ([169.254.8.222]) with mapi id 14.03.0439.000;
 Mon, 1 Jul 2019 19:58:55 +0100
From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: "Singh, Jasvinder" <jasvinder.singh@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
CC: "Tovar, AbrahamX" <abrahamx.tovar@intel.com>, "Krakowiak, LukaszX"
 <lukaszx.krakowiak@intel.com>
Thread-Topic: [PATCH v2 02/28] sched: update subport and pipe data structures
Thread-Index: AQHVK2srEzJF8vbtiEOkGQhgnxdn1qa2JWPQ
Date: Mon, 1 Jul 2019 18:58:55 +0000
Message-ID: <3EB4FA525960D640B5BDFFD6A3D891268E8B8C7F@IRSMSX108.ger.corp.intel.com>
References: <20190528120553.2992-2-lukaszx.krakowiak@intel.com>
 <20190625153217.24301-1-jasvinder.singh@intel.com>
 <20190625153217.24301-3-jasvinder.singh@intel.com>
In-Reply-To: <20190625153217.24301-3-jasvinder.singh@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTM1ZjFhMWUtMmU2ZS00OGEwLTkxYjUtODRlMTQxM2ZlNGZiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZVhcL214RzVEZjA2MXM1TVNFTk5kUUdPU1ZQVmp3aEJYSXdZcGtXWTZvY1lqWGVWdFwvQ2ZWQnhxSEg3UVwvdVZTciJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.2.0.6
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v2 02/28] sched: update subport and pipe data
	structures
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Tuesday, June 25, 2019 4:32 PM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Tovar, AbrahamX
> <abrahamx.tovar@intel.com>; Krakowiak, LukaszX
> <lukaszx.krakowiak@intel.com>
> Subject: [PATCH v2 02/28] sched: update subport and pipe data structures
>=20
> Update subport and pipe data structures to allow configuration
> flexiblity for pipe traffic classes and queues, and subport level
> configuration of the pipe parameters.
>=20
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Signed-off-by: Abraham Tovar <abrahamx.tovar@intel.com>
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> ---
>  app/test/test_sched.c        |   2 +-
>  examples/qos_sched/init.c    |   2 +-
>  lib/librte_sched/rte_sched.h | 126 +++++++++++++++++++++++------------
>  3 files changed, 85 insertions(+), 45 deletions(-)
>=20
> diff --git a/app/test/test_sched.c b/app/test/test_sched.c
> index 49bb9ea6f..d6651d490 100644
> --- a/app/test/test_sched.c
> +++ b/app/test/test_sched.c
> @@ -40,7 +40,7 @@ static struct rte_sched_pipe_params pipe_profile[] =3D =
{
>  		.tc_rate =3D {305175, 305175, 305175, 305175},
>  		.tc_period =3D 40,
>=20
> -		.wrr_weights =3D {1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1},
> +		.wrr_weights =3D {1, 1, 1, 1,  1, 1, 1, 1},
>  	},
>  };
>=20
> diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
> index 1209bd7ce..f6e9af16b 100644
> --- a/examples/qos_sched/init.c
> +++ b/examples/qos_sched/init.c
> @@ -186,7 +186,7 @@ static struct rte_sched_pipe_params
> pipe_profiles[RTE_SCHED_PIPE_PROFILES_PER_PO
>  		.tc_ov_weight =3D 1,
>  #endif
>=20
> -		.wrr_weights =3D {1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1,  1, 1, 1, 1},
> +		.wrr_weights =3D {1, 1, 1, 1,  1, 1, 1, 1},
>  	},
>  };
>=20
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index 470a0036a..ebde07669 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -114,6 +114,35 @@ extern "C" {
>  #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT      24
>  #endif
>=20
> +/*
> + * Pipe configuration parameters. The period and credits_per_period
> + * parameters are measured in bytes, with one byte meaning the time
> + * duration associated with the transmission of one byte on the
> + * physical medium of the output port, with pipe or pipe traffic class
> + * rate (measured as percentage of output port rate) determined as
> + * credits_per_period divided by period. One credit represents one
> + * byte.
> + */
> +struct rte_sched_pipe_params {
> +	/** Token bucket rate (measured in bytes per second) */
> +	uint32_t tb_rate;
> +	/** Token bucket size (measured in credits) */
> +	uint32_t tb_size;
> +
> +	/** Traffic class rates (measured in bytes per second) */
> +	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> +
> +	/** Enforcement period (measured in milliseconds) */
> +	uint32_t tc_period;
> +#ifdef RTE_SCHED_SUBPORT_TC_OV
> +	/** Best-effort traffic class oversubscription weight */
> +	uint8_t tc_ov_weight;
> +#endif

We should always enable the Best Effort traffic class oversubscription feat=
ure on the API side, at least. In case this feature is disabled through the=
 build time option (RTE_SCHED_SUBPORT_TC_OV), the values for these params c=
an be ignored.

We should also consider always enabling the run-time part for this feature,=
 as the oversubscription is the typical configuration used by the service p=
roviders. Do you see significant performance drop when this feature is enab=
led?

> +
> +	/** WRR weights of best-effort traffic class queues */
> +	uint8_t wrr_weights[RTE_SCHED_BE_QUEUES_PER_PIPE];
> +};
> +
>  /*
>   * Subport configuration parameters. The period and credits_per_period
>   * parameters are measured in bytes, with one byte meaning the time
> @@ -124,15 +153,44 @@ extern "C" {
>   * byte.
>   */
>  struct rte_sched_subport_params {
> -	/* Subport token bucket */
> -	uint32_t tb_rate;                /**< Rate (measured in bytes per secon=
d)
> */
> -	uint32_t tb_size;                /**< Size (measured in credits) */
> +	/** Token bucket rate (measured in bytes per second) */
> +	uint32_t tb_rate;
> +
> +	/** Token bucket size (measured in credits) */
> +	uint32_t tb_size;
>=20
> -	/* Subport traffic classes */
> +	/** Traffic class rates (measured in bytes per second) */
>  	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Traffic class rates (measured in bytes per second) */
> +
> +	/** Enforcement period for rates (measured in milliseconds) */
>  	uint32_t tc_period;
> -	/**< Enforcement period for rates (measured in milliseconds) */
> +
> +	/** Number of subport_pipes */
> +	uint32_t n_subport_pipes;

Minor issue: Any reason why not keeping the initial name of n_pipes_per_sub=
port? The initial name looks more intuitive to me, I vote to keep it; it is=
 also inline with other naming conventions in this library.

> +
> +	/** Packet queue size for each traffic class.
> +	 * All the pipes within the same subport share the similar
> +	 * configuration for the queues. Queues which are not needed, have
> +	 * zero size.
> +	 */
> +	uint16_t qsize[RTE_SCHED_QUEUES_PER_PIPE];
> +
> +	/** Pipe profile table.
> +	 * Every pipe is configured using one of the profiles from this table.
> +	 */
> +	struct rte_sched_pipe_params *pipe_profiles;
> +
> +	/** Profiles in the pipe profile table */
> +	uint32_t n_pipe_profiles;
> +
> +	/** Max profiles allowed in the pipe profile table */
> +	uint32_t n_max_pipe_profiles;
> +#ifdef RTE_SCHED_RED
> +	/** RED parameters */
> +	struct rte_red_params
> +
> 	red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS
> ];
> +
> +#endif
>  };
>=20
>  /** Subport statistics */
> @@ -155,33 +213,6 @@ struct rte_sched_subport_stats {
>  #endif
>  };
>=20
> -/*
> - * Pipe configuration parameters. The period and credits_per_period
> - * parameters are measured in bytes, with one byte meaning the time
> - * duration associated with the transmission of one byte on the
> - * physical medium of the output port, with pipe or pipe traffic class
> - * rate (measured as percentage of output port rate) determined as
> - * credits_per_period divided by period. One credit represents one
> - * byte.
> - */
> -struct rte_sched_pipe_params {
> -	/* Pipe token bucket */
> -	uint32_t tb_rate;                /**< Rate (measured in bytes per secon=
d)
> */
> -	uint32_t tb_size;                /**< Size (measured in credits) */
> -
> -	/* Pipe traffic classes */
> -	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Traffic class rates (measured in bytes per second) */
> -	uint32_t tc_period;
> -	/**< Enforcement period (measured in milliseconds) */
> -#ifdef RTE_SCHED_SUBPORT_TC_OV
> -	uint8_t tc_ov_weight;		 /**< Weight Traffic class 3
> oversubscription */
> -#endif
> -
> -	/* Pipe queues */
> -	uint8_t  wrr_weights[RTE_SCHED_QUEUES_PER_PIPE]; /**< WRR
> weights */
> -};
> -
>  /** Queue statistics */
>  struct rte_sched_queue_stats {
>  	/* Packets */
> @@ -198,16 +229,25 @@ struct rte_sched_queue_stats {
>=20
>  /** Port configuration parameters. */
>  struct rte_sched_port_params {
> -	const char *name;                /**< String to be associated */
> -	int socket;                      /**< CPU socket ID */
> -	uint32_t rate;                   /**< Output port rate
> -					  * (measured in bytes per second) */
> -	uint32_t mtu;                    /**< Maximum Ethernet frame size
> -					  * (measured in bytes).
> -					  * Should not include the framing
> overhead. */
> -	uint32_t frame_overhead;         /**< Framing overhead per packet
> -					  * (measured in bytes) */
> -	uint32_t n_subports_per_port;    /**< Number of subports */
> +	/** Name of the port to be associated */
> +	const char *name;
> +
> +	/** CPU socket ID */
> +	int socket;
> +
> +	/** Output port rate (measured in bytes per second) */
> +	uint32_t rate;
> +
> +	/** Maximum Ethernet frame size (measured in bytes).
> +	 * Should not include the framing overhead.
> +	 */
> +	uint32_t mtu;
> +
> +	/** Framing overhead per packet (measured in bytes) */
> +	uint32_t frame_overhead;
> +
> +	/** Number of subports */
> +	uint32_t n_subports_per_port;
>  	uint32_t n_pipes_per_subport;    /**< Number of pipes per subport
> */
>  	uint16_t qsize[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>  	/**< Packet queue size for each traffic class.
> --
> 2.21.0