[v8,8/8] sched: remove redundant code

Message ID 20201007140915.19491-9-savinay.dharmappa@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Enable dynamic config of subport bandwidth |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Savinay Dharmappa Oct. 7, 2020, 2:09 p.m. UTC
  Remove redundant data structure fields references from
functions and subport level data structures. It also
update the release and deprecation note

Signed-off-by: Savinay Dharmappa <savinay.dharmappa@intel.com>
---
 doc/guides/rel_notes/deprecation.rst   |  6 ----
 doc/guides/rel_notes/release_20_11.rst |  1 +
 lib/librte_sched/rte_sched.c           | 42 ++------------------------
 lib/librte_sched/rte_sched.h           | 12 --------
 4 files changed, 3 insertions(+), 58 deletions(-)
  

Comments

Thomas Monjalon Oct. 9, 2020, 8:28 a.m. UTC | #1
Hi,

> Remove redundant data structure fields references from
> functions and subport level data structures. It also
> update the release and deprecation note
> 
> Signed-off-by: Savinay Dharmappa <savinay.dharmappa@intel.com>
> ---
> 
>  doc/guides/rel_notes/deprecation.rst   |  6 ----
>  doc/guides/rel_notes/release_20_11.rst |  1 +
>  lib/librte_sched/rte_sched.c           | 42 ++------------------------
>  lib/librte_sched/rte_sched.h           | 12 --------

I wonder why this patch exists.

Documentation updates should be done when adding the feature (patch 3).
Please try to conform to the release notes format and recommendations.

Redundant code should be removed when it becomes useless.
Or should it be last because the fields are used in apps?
Anyway it is strange that a test for params->qsize is added.

About the API changes, please update the ABI section of the release notes
in each patch removing some old API.
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 584e72087..f7363a585 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -212,12 +212,6 @@  Deprecation Notices
   in "rte_sched.h". These changes are aligned to improvements suggested in the
   RFC https://mails.dpdk.org/archives/dev/2018-November/120035.html.
 
-* sched: To allow dynamic configuration of the subport bandwidth profile,
-  changes will be made to data structures ``rte_sched_subport_params``,
-  ``rte_sched_port_params`` and new data structure, API functions will be
-  defined in ``rte_sched.h``. These changes are aligned as suggested in the
-  RFC https://mails.dpdk.org/archives/dev/2020-July/175161.html
-
 * metrics: The function ``rte_metrics_init`` will have a non-void return
   in order to notify errors instead of calling ``rte_exit``.
 
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index cdf20404c..45cda64b0 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -116,6 +116,7 @@  New Features
   * Extern objects and functions can be plugged into the pipeline.
   * Transaction-oriented table updates.
 
+* sched: Add support to update subport bandwidth dynamically.
 
 Removed Items
 -------------
diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index c5482d6e0..7c5688068 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -152,16 +152,11 @@  struct rte_sched_grinder {
 struct rte_sched_subport {
 	/* Token bucket (TB) */
 	uint64_t tb_time; /* time of last update */
-	uint64_t tb_period;
-	uint64_t tb_credits_per_period;
-	uint64_t tb_size;
 	uint64_t tb_credits;
 
 	/* Traffic classes (TCs) */
 	uint64_t tc_time; /* time of next update */
-	uint64_t tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint64_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
-	uint64_t tc_period;
 
 	/* TC oversubscription */
 	uint64_t tc_ov_wm;
@@ -837,18 +832,6 @@  rte_sched_subport_check_params(struct rte_sched_subport_params *params,
 		return -EINVAL;
 	}
 
-	if (params->tb_rate == 0 || params->tb_rate > rate) {
-		RTE_LOG(ERR, SCHED,
-			"%s: Incorrect value for tb rate\n", __func__);
-		return -EINVAL;
-	}
-
-	if (params->tb_size == 0) {
-		RTE_LOG(ERR, SCHED,
-			"%s: Incorrect value for tb size\n", __func__);
-		return -EINVAL;
-	}
-
 	/* qsize: if non-zero, power of 2,
 	 * no bigger than 32K (due to 16-bit read/write pointers)
 	 */
@@ -862,29 +845,8 @@  rte_sched_subport_check_params(struct rte_sched_subport_params *params,
 		}
 	}
 
-	for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) {
-		uint64_t tc_rate = params->tc_rate[i];
-		uint16_t qsize = params->qsize[i];
-
-		if ((qsize == 0 && tc_rate != 0) ||
-			(qsize != 0 && tc_rate == 0) ||
-			(tc_rate > params->tb_rate)) {
-			RTE_LOG(ERR, SCHED,
-				"%s: Incorrect value for tc rate\n", __func__);
-			return -EINVAL;
-		}
-	}
-
-	if (params->qsize[RTE_SCHED_TRAFFIC_CLASS_BE] == 0 ||
-		params->tc_rate[RTE_SCHED_TRAFFIC_CLASS_BE] == 0) {
-		RTE_LOG(ERR, SCHED,
-			"%s: Incorrect qsize or tc rate(best effort)\n", __func__);
-		return -EINVAL;
-	}
-
-	if (params->tc_period == 0) {
-		RTE_LOG(ERR, SCHED,
-			"%s: Incorrect value for tc period\n", __func__);
+	if (params->qsize[RTE_SCHED_TRAFFIC_CLASS_BE] == 0) {
+		RTE_LOG(ERR, SCHED, "%s: Incorrect qsize\n", __func__);
 		return -EINVAL;
 	}
 
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index 1506c6487..c1a772b70 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -149,18 +149,6 @@  struct rte_sched_pipe_params {
  * byte.
  */
 struct rte_sched_subport_params {
-	/** Token bucket rate (measured in bytes per second) */
-	uint64_t tb_rate;
-
-	/** Token bucket size (measured in credits) */
-	uint64_t tb_size;
-
-	/** Traffic class rates (measured in bytes per second) */
-	uint64_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
-
-	/** Enforcement period for rates (measured in milliseconds) */
-	uint64_t tc_period;
-
 	/** Number of subport pipes.
 	 * The subport can enable/allocate fewer pipes than the maximum
 	 * number set through struct port_params::n_max_pipes_per_subport,