[v3,5/5] event/dlb2: fix port COS initialization

Message ID 20220702162239.1646548-6-timothy.mcdaniel@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Jerin Jacob
Headers
Series DLB2 Bug Fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Timothy McDaniel July 2, 2022, 4:22 p.m. UTC
  Fix cos initialization, handling the default case too.

Substitute the semicolon for the comma
that was expected in the cos_bw command line override.
Commas are not allowed within a multifield option.
The new format is cos_bw=%d:%d:%d:%d, where the sum of
the 4 decimal values must be less than or equal to 100.

Corrected probe-time initialization order.

Fixes: bec8901bfe9f ("event/dlb2: support ldb port specific COS")
Cc: timothy.mcdaniel@intel.com

Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
---
 drivers/event/dlb2/dlb2.c                  | 63 ++++++++++++----------
 drivers/event/dlb2/pf/base/dlb2_resource.c |  3 --
 drivers/event/dlb2/pf/dlb2_pf.c            |  4 ++
 3 files changed, 39 insertions(+), 31 deletions(-)
  

Comments

Jerin Jacob July 4, 2022, 4:11 p.m. UTC | #1
On Sat, Jul 2, 2022 at 9:53 PM Timothy McDaniel
<timothy.mcdaniel@intel.com> wrote:
>
> Fix cos initialization, handling the default case too.
>
> Substitute the semicolon for the comma
> that was expected in the cos_bw command line override.
> Commas are not allowed within a multifield option.
> The new format is cos_bw=%d:%d:%d:%d, where the sum of
> the 4 decimal values must be less than or equal to 100.
>
> Corrected probe-time initialization order.
>
> Fixes: bec8901bfe9f ("event/dlb2: support ldb port specific COS")
> Cc: timothy.mcdaniel@intel.com

No need to CC yourself. Instead, if it is fix Cc: stable@dpdk.org. I
will it on merge.


>
> Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>

Fixed check-git-log.sh issues and Updated the git commit as follows
and applied to dpdk-next-net-eventdev/for-main. Thanks


commit 611ae87343db1ca6fed6a99f5e3c0880e8f0f952
Author: Timothy McDaniel <timothy.mcdaniel@intel.com>
Date:   Sat Jul 2 11:22:39 2022 -0500

    event/dlb2: fix port COS initialization

    Fix cos initialization, handling the default case too.

    Substitute the semicolon for the comma
    that was expected in the cos_bw command line override.
    Commas are not allowed within a multi field option.
    The new format is cos_bw=%d:%d:%d:%d, where the sum of
    the 4 decimal values must be less than or equal to 100.

    Corrected probe-time initialization order.

    Fixes: bec8901bfe9f ("event/dlb2: support ldb port specific COS")
    Cc: stable@dpdk.org

    Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>

commit d98b716ead2878ede3f75049f89375d67ac43f48
Author: Timothy McDaniel <timothy.mcdaniel@intel.com>
Date:   Sat Jul 2 11:22:38 2022 -0500

    event/dlb2: fix CQ depth override credit deadlock

    This commit fixes a bug, where we could encounter a credit
    deadlock due to changing the CQ depth. To remedy this situation,
    the commit reduces the maximum CQ depth from 1024 to 128,
    and also allows configuring the maximum enqueue depth. Maximum
    enqueue depth must be tuned to the CQ depth, if the CQ depth
    is increased.

    Fixes: 86fe66d45667 ("event/dlb2: allow CQ depths up to 1024")
    Cc: stable@dpdk.org

    Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>
commit d0143df3f208e8091b83147c356ba1d3c4f3d97b
Author: Timothy McDaniel <timothy.mcdaniel@intel.com>
Date:   Sat Jul 2 11:22:37 2022 -0500

    event/dlb2: fix CQ depth override

    This commit fixes a bug, where we could assign a CQ depth
    of zero, leading to a subsequent divide-by-zero fault.
    It also fixes an issue where the original default CQ depth
    was returned on a query, instead of the overridden value.

    Fixes: 86fe66d45667 ("event/dlb2: allow CQ depths up to 1024")
    Cc: stable@dpdk.org

    Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>

commit d45ba4380684a86f4c723b9da2f854de22389001
Author: Timothy McDaniel <timothy.mcdaniel@intel.com>
Date:   Sat Jul 2 11:22:36 2022 -0500

    event/dlb2: fix initialization of COS bandwidth args

    This commit fixes a typo and resultant bug that triggered a
    coverity warning.

    Coverity issue: 4607286
    Fixes: bec8901bfe9f ("event/dlb2: support ldb port specific COS")
    Cc: stable@dpdk.org

    Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>

commit 9274d2236be09989c4d18a4a852e77fb87cf53f1
Author: Timothy McDaniel <timothy.mcdaniel@intel.com>
Date:   Sat Jul 2 11:22:35 2022 -0500

    event/dlb2: fix array sizing

    This commit fixes a segfault that resulted from reading
    beyond the end of the port_cos array. The root cause was using
    the DLB num ports define instead of the eventdev num ports define.

    Fixes: bec8901bfe9f ("event/dlb2: support ldb port specific COS")
    Cc: stable@dpdk.org

    Signed-off-by: Timothy McDaniel <timothy.mcdaniel@intel.com>

> ---
>  drivers/event/dlb2/dlb2.c                  | 63 ++++++++++++----------
>  drivers/event/dlb2/pf/base/dlb2_resource.c |  3 --
>  drivers/event/dlb2/pf/dlb2_pf.c            |  4 ++
>  3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
> index 8a68c25c93..26af75beb8 100644
> --- a/drivers/event/dlb2/dlb2.c
> +++ b/drivers/event/dlb2/dlb2.c
> @@ -170,10 +170,11 @@ dlb2_init_port_cos(struct dlb2_eventdev *dlb2, int *port_cos)
>  {
>         int q;
>
> -       for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++) {
> -               dlb2->ev_ports[q].cos_id = port_cos[q];
> -               dlb2->cos_ports[port_cos[q]]++;
> -       }
> +       for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++)
> +               if (port_cos[q] != DLB2_COS_DEFAULT) {
> +                       dlb2->ev_ports[q].cos_id = port_cos[q];
> +                       dlb2->cos_ports[port_cos[q]]++;
> +               }
>  }
>
>  static void
> @@ -181,6 +182,17 @@ dlb2_init_cos_bw(struct dlb2_eventdev *dlb2,
>                  struct dlb2_cos_bw *cos_bw)
>  {
>         int q;
> +
> +
> +       /* If cos_bw not set, then split evenly */
> +       if (cos_bw->val[0] == 0 && cos_bw->val[1] == 0 &&
> +               cos_bw->val[2] == 0 && cos_bw->val[3] == 0) {
> +               cos_bw->val[0] = 25;
> +               cos_bw->val[1] = 25;
> +               cos_bw->val[2] = 25;
> +               cos_bw->val[3] = 25;
> +       }
> +
>         for (q = 0; q < DLB2_COS_NUM_VALS; q++)
>                 dlb2->cos_bw[q] = cos_bw->val[q];
>
> @@ -464,19 +476,15 @@ set_port_cos(const char *key __rte_unused,
>         }
>
>         /* command line override may take one of the following 3 forms:
> -        * port_cos=all:<cos_id> ... all ports
>          * port_cos=port-port:<cos_id> ... a range of ports
>          * port_cos=port:<cos_id> ... just one port
>          */
> -       if (sscanf(value, "all:%d", &cos_id) == 1) {
> -               first = 0;
> -               last = DLB2_MAX_NUM_LDB_PORTS - 1;
> -       } else if (sscanf(value, "%d-%d:%d", &first, &last, &cos_id) == 3) {
> +       if (sscanf(value, "%d-%d:%d", &first, &last, &cos_id) == 3) {
>                 /* we have everything we need */
>         } else if (sscanf(value, "%d:%d", &first, &cos_id) == 2) {
>                 last = first;
>         } else {
> -               DLB2_LOG_ERR("Error parsing ldb port port_cos devarg. Should be all:val, port-port:val, or port:val\n");
> +               DLB2_LOG_ERR("Error parsing ldb port port_cos devarg. Should be port-port:val, or port:val\n");
>                 return -EINVAL;
>         }
>
> @@ -511,13 +519,13 @@ set_cos_bw(const char *key __rte_unused,
>
>         /* format must be %d,%d,%d,%d */
>
> -       if (sscanf(value, "%d,%d,%d,%d", &cos_bw->val[0], &cos_bw->val[1],
> +       if (sscanf(value, "%d:%d:%d:%d", &cos_bw->val[0], &cos_bw->val[1],
>                    &cos_bw->val[2], &cos_bw->val[3]) != 4) {
> -               DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0,bw1,bw2,bw3 where all values combined are <= 100\n");
> +               DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0:bw1:bw2:bw3 where all values combined are <= 100\n");
>                 return -EINVAL;
>         }
>         if (cos_bw->val[0] + cos_bw->val[1] + cos_bw->val[2] + cos_bw->val[3] > 100) {
> -               DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0,bw1,bw2,bw3  where all values combined are <= 100\n");
> +               DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0:bw1:bw2:bw3  where all values combined are <= 100\n");
>                 return -EINVAL;
>         }
>
> @@ -781,9 +789,9 @@ dlb2_hw_create_sched_domain(struct dlb2_eventdev *dlb2,
>
>         /* LDB ports */
>
> -       /* tally of ports with non default COS */
> -       cos_ports = dlb2->cos_ports[1] + dlb2->cos_ports[2] +
> -                   dlb2->cos_ports[3];
> +       /* tally of COS ports from cmd line */
> +       cos_ports = dlb2->cos_ports[0] + dlb2->cos_ports[1] +
> +                   dlb2->cos_ports[2] + dlb2->cos_ports[3];
>
>         if (cos_ports > resources_asked->num_ldb_ports) {
>                 DLB2_LOG_ERR("dlb2: num_ldb_ports < nonzero cos_ports\n");
> @@ -4552,6 +4560,17 @@ dlb2_primary_eventdev_probe(struct rte_eventdev *dev,
>         evdev_dlb2_default_info.max_event_port_enqueue_depth =
>                 dlb2->max_enq_depth;
>
> +       dlb2_init_queue_depth_thresholds(dlb2,
> +                                        dlb2_args->qid_depth_thresholds.val);
> +
> +       dlb2_init_cq_weight(dlb2,
> +                           dlb2_args->cq_weight.limit);
> +
> +       dlb2_init_port_cos(dlb2,
> +                          dlb2_args->port_cos.cos_id);
> +
> +       dlb2_init_cos_bw(dlb2,
> +                        &dlb2_args->cos_bw);
>
>         err = dlb2_iface_open(&dlb2->qm_instance, name);
>         if (err < 0) {
> @@ -4623,18 +4642,6 @@ dlb2_primary_eventdev_probe(struct rte_eventdev *dev,
>
>         dlb2_entry_points_init(dev);
>
> -       dlb2_init_queue_depth_thresholds(dlb2,
> -                                        dlb2_args->qid_depth_thresholds.val);
> -
> -       dlb2_init_cq_weight(dlb2,
> -                           dlb2_args->cq_weight.limit);
> -
> -       dlb2_init_port_cos(dlb2,
> -                          dlb2_args->port_cos.cos_id);
> -
> -       dlb2_init_cos_bw(dlb2,
> -                        &dlb2_args->cos_bw);
> -
>         return 0;
>  }
>
> diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
> index da1949c763..e73d289445 100644
> --- a/drivers/event/dlb2/pf/base/dlb2_resource.c
> +++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
> @@ -236,9 +236,6 @@ int dlb2_resource_init(struct dlb2_hw *hw, enum dlb2_hw_ver ver)
>                 hw->rsrcs.sn_groups[i].slot_use_bitmap = 0;
>         }
>
> -       for (i = 0; i < DLB2_NUM_COS_DOMAINS; i++)
> -               hw->cos_reservation[i] = 100 / DLB2_NUM_COS_DOMAINS;
> -
>         return 0;
>
>  unwind:
> diff --git a/drivers/event/dlb2/pf/dlb2_pf.c b/drivers/event/dlb2/pf/dlb2_pf.c
> index 086d4a1cc7..dd3f2b8ece 100644
> --- a/drivers/event/dlb2/pf/dlb2_pf.c
> +++ b/drivers/event/dlb2/pf/dlb2_pf.c
> @@ -712,10 +712,14 @@ dlb2_eventdev_pci_init(struct rte_eventdev *eventdev)
>                 .max_enq_depth = DLB2_MAX_ENQUEUE_DEPTH
>         };
>         struct dlb2_eventdev *dlb2;
> +       int q;
>
>         DLB2_LOG_DBG("Enter with dev_id=%d socket_id=%d",
>                      eventdev->data->dev_id, eventdev->data->socket_id);
>
> +       for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++)
> +               dlb2_args.port_cos.cos_id[q] = DLB2_COS_DEFAULT;
> +
>         dlb2_pf_iface_fn_ptrs_init();
>
>         pci_dev = RTE_DEV_TO_PCI(eventdev->dev);
> --
> 2.25.1
>
  

Patch

diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
index 8a68c25c93..26af75beb8 100644
--- a/drivers/event/dlb2/dlb2.c
+++ b/drivers/event/dlb2/dlb2.c
@@ -170,10 +170,11 @@  dlb2_init_port_cos(struct dlb2_eventdev *dlb2, int *port_cos)
 {
 	int q;
 
-	for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++) {
-		dlb2->ev_ports[q].cos_id = port_cos[q];
-		dlb2->cos_ports[port_cos[q]]++;
-	}
+	for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++)
+		if (port_cos[q] != DLB2_COS_DEFAULT) {
+			dlb2->ev_ports[q].cos_id = port_cos[q];
+			dlb2->cos_ports[port_cos[q]]++;
+		}
 }
 
 static void
@@ -181,6 +182,17 @@  dlb2_init_cos_bw(struct dlb2_eventdev *dlb2,
 		 struct dlb2_cos_bw *cos_bw)
 {
 	int q;
+
+
+	/* If cos_bw not set, then split evenly */
+	if (cos_bw->val[0] == 0 && cos_bw->val[1] == 0 &&
+		cos_bw->val[2] == 0 && cos_bw->val[3] == 0) {
+		cos_bw->val[0] = 25;
+		cos_bw->val[1] = 25;
+		cos_bw->val[2] = 25;
+		cos_bw->val[3] = 25;
+	}
+
 	for (q = 0; q < DLB2_COS_NUM_VALS; q++)
 		dlb2->cos_bw[q] = cos_bw->val[q];
 
@@ -464,19 +476,15 @@  set_port_cos(const char *key __rte_unused,
 	}
 
 	/* command line override may take one of the following 3 forms:
-	 * port_cos=all:<cos_id> ... all ports
 	 * port_cos=port-port:<cos_id> ... a range of ports
 	 * port_cos=port:<cos_id> ... just one port
 	 */
-	if (sscanf(value, "all:%d", &cos_id) == 1) {
-		first = 0;
-		last = DLB2_MAX_NUM_LDB_PORTS - 1;
-	} else if (sscanf(value, "%d-%d:%d", &first, &last, &cos_id) == 3) {
+	if (sscanf(value, "%d-%d:%d", &first, &last, &cos_id) == 3) {
 		/* we have everything we need */
 	} else if (sscanf(value, "%d:%d", &first, &cos_id) == 2) {
 		last = first;
 	} else {
-		DLB2_LOG_ERR("Error parsing ldb port port_cos devarg. Should be all:val, port-port:val, or port:val\n");
+		DLB2_LOG_ERR("Error parsing ldb port port_cos devarg. Should be port-port:val, or port:val\n");
 		return -EINVAL;
 	}
 
@@ -511,13 +519,13 @@  set_cos_bw(const char *key __rte_unused,
 
 	/* format must be %d,%d,%d,%d */
 
-	if (sscanf(value, "%d,%d,%d,%d", &cos_bw->val[0], &cos_bw->val[1],
+	if (sscanf(value, "%d:%d:%d:%d", &cos_bw->val[0], &cos_bw->val[1],
 		   &cos_bw->val[2], &cos_bw->val[3]) != 4) {
-		DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0,bw1,bw2,bw3 where all values combined are <= 100\n");
+		DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0:bw1:bw2:bw3 where all values combined are <= 100\n");
 		return -EINVAL;
 	}
 	if (cos_bw->val[0] + cos_bw->val[1] + cos_bw->val[2] + cos_bw->val[3] > 100) {
-		DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0,bw1,bw2,bw3  where all values combined are <= 100\n");
+		DLB2_LOG_ERR("Error parsing cos bandwidth devarg. Should be bw0:bw1:bw2:bw3  where all values combined are <= 100\n");
 		return -EINVAL;
 	}
 
@@ -781,9 +789,9 @@  dlb2_hw_create_sched_domain(struct dlb2_eventdev *dlb2,
 
 	/* LDB ports */
 
-	/* tally of ports with non default COS */
-	cos_ports = dlb2->cos_ports[1] + dlb2->cos_ports[2] +
-		    dlb2->cos_ports[3];
+	/* tally of COS ports from cmd line */
+	cos_ports = dlb2->cos_ports[0] + dlb2->cos_ports[1] +
+		    dlb2->cos_ports[2] + dlb2->cos_ports[3];
 
 	if (cos_ports > resources_asked->num_ldb_ports) {
 		DLB2_LOG_ERR("dlb2: num_ldb_ports < nonzero cos_ports\n");
@@ -4552,6 +4560,17 @@  dlb2_primary_eventdev_probe(struct rte_eventdev *dev,
 	evdev_dlb2_default_info.max_event_port_enqueue_depth =
 		dlb2->max_enq_depth;
 
+	dlb2_init_queue_depth_thresholds(dlb2,
+					 dlb2_args->qid_depth_thresholds.val);
+
+	dlb2_init_cq_weight(dlb2,
+			    dlb2_args->cq_weight.limit);
+
+	dlb2_init_port_cos(dlb2,
+			   dlb2_args->port_cos.cos_id);
+
+	dlb2_init_cos_bw(dlb2,
+			 &dlb2_args->cos_bw);
 
 	err = dlb2_iface_open(&dlb2->qm_instance, name);
 	if (err < 0) {
@@ -4623,18 +4642,6 @@  dlb2_primary_eventdev_probe(struct rte_eventdev *dev,
 
 	dlb2_entry_points_init(dev);
 
-	dlb2_init_queue_depth_thresholds(dlb2,
-					 dlb2_args->qid_depth_thresholds.val);
-
-	dlb2_init_cq_weight(dlb2,
-			    dlb2_args->cq_weight.limit);
-
-	dlb2_init_port_cos(dlb2,
-			   dlb2_args->port_cos.cos_id);
-
-	dlb2_init_cos_bw(dlb2,
-			 &dlb2_args->cos_bw);
-
 	return 0;
 }
 
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index da1949c763..e73d289445 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -236,9 +236,6 @@  int dlb2_resource_init(struct dlb2_hw *hw, enum dlb2_hw_ver ver)
 		hw->rsrcs.sn_groups[i].slot_use_bitmap = 0;
 	}
 
-	for (i = 0; i < DLB2_NUM_COS_DOMAINS; i++)
-		hw->cos_reservation[i] = 100 / DLB2_NUM_COS_DOMAINS;
-
 	return 0;
 
 unwind:
diff --git a/drivers/event/dlb2/pf/dlb2_pf.c b/drivers/event/dlb2/pf/dlb2_pf.c
index 086d4a1cc7..dd3f2b8ece 100644
--- a/drivers/event/dlb2/pf/dlb2_pf.c
+++ b/drivers/event/dlb2/pf/dlb2_pf.c
@@ -712,10 +712,14 @@  dlb2_eventdev_pci_init(struct rte_eventdev *eventdev)
 		.max_enq_depth = DLB2_MAX_ENQUEUE_DEPTH
 	};
 	struct dlb2_eventdev *dlb2;
+	int q;
 
 	DLB2_LOG_DBG("Enter with dev_id=%d socket_id=%d",
 		     eventdev->data->dev_id, eventdev->data->socket_id);
 
+	for (q = 0; q < DLB2_MAX_NUM_PORTS_ALL; q++)
+		dlb2_args.port_cos.cos_id[q] = DLB2_COS_DEFAULT;
+
 	dlb2_pf_iface_fn_ptrs_init();
 
 	pci_dev = RTE_DEV_TO_PCI(eventdev->dev);