[PATCH 0/5] updates for net/ice driver

[PATCH 0/5] updates for net/ice driver

[Bruce Richardson]

This patchset contains a set of updates for the ice driver, a number of
which are in the area of the "rte_tm" APIs for Tx scheduling.

These patches were previously submitted as part of a larger set[1], but
separating them out here for easier review and merge ahead of the more
substantial changes for scheduling in the last couple of patches in that
set.

[1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*

Bruce Richardson (5):
  net/ice: detect stopping a flow-director queue twice
  net/ice: improve Tx scheduler graph output
  net/ice: add option to choose DDP package file
  net/ice: add option to download scheduler topology
  net/ice: limit the number of queues to sched capabilities

 doc/guides/nics/ice.rst        |   9 ++
 drivers/net/ice/base/ice_ddp.c |  18 ++-
 drivers/net/ice/base/ice_ddp.h |   4 +-
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 drivers/net/ice/ice_ethdev.c   |  84 +++++++++++---
 drivers/net/ice/ice_ethdev.h   |   2 +
 drivers/net/ice/ice_rxtx.c     |   5 +
 7 files changed, 172 insertions(+), 146 deletions(-)

--
2.43.0

[PATCH 1/5] net/ice: detect stopping a flow-director queue twice

[Bruce Richardson]

If the flow-director queue is stopped at some point during the running
of an application, the shutdown procedure for the port issues an error
as it tries to stop the queue a second time, and fails to do so. We can
eliminate this error by setting the tail-register pointer to NULL on
stop, and checking for that condition in subsequent stop calls. Since
the register pointer is set on start, any restarting of the queue will
allow a stop call to progress as normal.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ice/ice_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index acd7539b5e..267c1b4a79 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1139,6 +1139,10 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 			    tx_queue_id);
 		return -EINVAL;
 	}
+	if (txq->qtx_tail == NULL) {
+		PMD_DRV_LOG(INFO, "TX queue %u not started", tx_queue_id);
+		return 0;
+	}
 	vsi = txq->vsi;
 
 	q_ids[0] = txq->reg_idx;
@@ -1153,6 +1157,7 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	}
 
 	txq->tx_rel_mbufs(txq);
+	txq->qtx_tail = NULL;
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH 1/5] net/ice: detect stopping a flow-director queue twice

[Stephen Hemminger]

On Wed,  9 Oct 2024 18:08:18 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> If the flow-director queue is stopped at some point during the running
> of an application, the shutdown procedure for the port issues an error
> as it tries to stop the queue a second time, and fails to do so. We can
> eliminate this error by setting the tail-register pointer to NULL on
> stop, and checking for that condition in subsequent stop calls. Since
> the register pointer is set on start, any restarting of the queue will
> allow a stop call to progress as normal.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Acked-by: Stephen Hemminger <stephen@networkplumber.org>

[PATCH 2/5] net/ice: improve Tx scheduler graph output

[Bruce Richardson]

The function to dump the TX scheduler topology only adds to the chart
nodes connected to TX queues or for the flow director VSI. Change the
function to work recursively from the root node and thereby include all
scheduler nodes, whether in use or not, in the dump.

Also, improve the output of the Tx scheduler graphing function:

* Add VSI details to each node in graph
* When number of children is >16, skip middle nodes to reduce size of
  the graph, otherwise dot output is unviewable for large hierarchies
* For VSIs other than zero, use dot's clustering method to put those
  VSIs into subgraphs with borders
* For leaf nodes, display queue numbers for the any nodes assigned to
  ethdev NIC Tx queues

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 1 file changed, 69 insertions(+), 127 deletions(-)

diff --git a/drivers/net/ice/ice_diagnose.c b/drivers/net/ice/ice_diagnose.c
index c357554707..623d84e37d 100644
--- a/drivers/net/ice/ice_diagnose.c
+++ b/drivers/net/ice/ice_diagnose.c
@@ -545,29 +545,15 @@ static void print_rl_profile(struct ice_aqc_rl_profile_elem *prof,
 	fprintf(stream, "\t\t\t\t\t</td>\n");
 }
 
-static
-void print_elem_type(FILE *stream, u8 type)
+static const char *
+get_elem_type(u8 type)
 {
-	switch (type) {
-	case 1:
-		fprintf(stream, "root");
-		break;
-	case 2:
-		fprintf(stream, "tc");
-		break;
-	case 3:
-		fprintf(stream, "se_generic");
-		break;
-	case 4:
-		fprintf(stream, "entry_point");
-		break;
-	case 5:
-		fprintf(stream, "leaf");
-		break;
-	default:
-		fprintf(stream, "%d", type);
-		break;
-	}
+	static const char * const ice_sched_node_types[] = {
+			"Undefined", "Root", "TC", "SE Generic", "SW Entry", "Leaf"
+	};
+	if (type < RTE_DIM(ice_sched_node_types))
+		return ice_sched_node_types[type];
+	return "*UNKNOWN*";
 }
 
 static
@@ -602,7 +588,9 @@ void print_priority_mode(FILE *stream, bool flag)
 }
 
 static
-void print_node(struct ice_aqc_txsched_elem_data *data,
+void print_node(struct ice_sched_node *node,
+		struct rte_eth_dev_data *ethdata,
+		struct ice_aqc_txsched_elem_data *data,
 		struct ice_aqc_rl_profile_elem *cir_prof,
 		struct ice_aqc_rl_profile_elem *eir_prof,
 		struct ice_aqc_rl_profile_elem *shared_prof,
@@ -613,17 +601,19 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 
 	fprintf(stream, "\t\t\t<table>\n");
 
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> teid </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td> %d </td>\n", data->node_teid);
-	fprintf(stream, "\t\t\t\t</tr>\n");
-
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> type </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td>");
-	print_elem_type(stream, data->data.elem_type);
-	fprintf(stream, "</td>\n");
-	fprintf(stream, "\t\t\t\t</tr>\n");
+	fprintf(stream, "\t\t\t\t<tr><td>teid</td><td>%d</td></tr>\n", data->node_teid);
+	fprintf(stream, "\t\t\t\t<tr><td>type</td><td>%s</td></tr>\n",
+			get_elem_type(data->data.elem_type));
+	fprintf(stream, "\t\t\t\t<tr><td>VSI</td><td>%u</td></tr>\n", node->vsi_handle);
+	if (data->data.elem_type == ICE_AQC_ELEM_TYPE_LEAF) {
+		for (uint16_t i = 0; i < ethdata->nb_tx_queues; i++) {
+			struct ice_tx_queue *q = ethdata->tx_queues[i];
+			if (q->q_teid == data->node_teid) {
+				fprintf(stream, "\t\t\t\t<tr><td>TXQ</td><td>%u</td></tr>\n", i);
+				break;
+			}
+		}
+	}
 
 	if (!detail)
 		goto brief;
@@ -705,8 +695,6 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 	fprintf(stream, "\t\tshape=plain\n");
 	fprintf(stream, "\t]\n");
 
-	if (data->parent_teid != 0xFFFFFFFF)
-		fprintf(stream, "\tNODE_%d -> NODE_%d\n", data->parent_teid, data->node_teid);
 }
 
 static
@@ -731,112 +719,92 @@ int query_rl_profile(struct ice_hw *hw,
 	return 0;
 }
 
-static
-int query_node(struct ice_hw *hw, uint32_t child, uint32_t *parent,
-	       uint8_t level, bool detail, FILE *stream)
+static int
+query_node(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	struct ice_aqc_txsched_elem_data data;
+	struct ice_aqc_txsched_elem_data *data = &node->info;
 	struct ice_aqc_rl_profile_elem cir_prof;
 	struct ice_aqc_rl_profile_elem eir_prof;
 	struct ice_aqc_rl_profile_elem shared_prof;
 	struct ice_aqc_rl_profile_elem *cp = NULL;
 	struct ice_aqc_rl_profile_elem *ep = NULL;
 	struct ice_aqc_rl_profile_elem *sp = NULL;
-	int status, ret;
-
-	status = ice_sched_query_elem(hw, child, &data);
-	if (status != ICE_SUCCESS) {
-		if (level == hw->num_tx_sched_layers) {
-			/* ignore the error when a queue has been stopped. */
-			PMD_DRV_LOG(WARNING, "Failed to query queue node %d.", child);
-			*parent = 0xffffffff;
-			return 0;
-		}
-		PMD_DRV_LOG(ERR, "Failed to query scheduling node %d.", child);
-		return -EINVAL;
-	}
-
-	*parent = data.parent_teid;
+	u8 level = node->tx_sched_layer;
+	int ret;
 
-	if (data.data.cir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 0, data.data.cir_bw.bw_profile_idx, &cir_prof);
+	if (data->data.cir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 0, data->data.cir_bw.bw_profile_idx, &cir_prof);
 
 		if (ret)
 			return ret;
 		cp = &cir_prof;
 	}
 
-	if (data.data.eir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 1, data.data.eir_bw.bw_profile_idx, &eir_prof);
+	if (data->data.eir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 1, data->data.eir_bw.bw_profile_idx, &eir_prof);
 
 		if (ret)
 			return ret;
 		ep = &eir_prof;
 	}
 
-	if (data.data.srl_id != 0) {
-		ret = query_rl_profile(hw, level, 2, data.data.srl_id, &shared_prof);
+	if (data->data.srl_id != 0) {
+		ret = query_rl_profile(hw, level, 2, data->data.srl_id, &shared_prof);
 
 		if (ret)
 			return ret;
 		sp = &shared_prof;
 	}
 
-	print_node(&data, cp, ep, sp, detail, stream);
+	print_node(node, ethdata, data, cp, ep, sp, detail, stream);
 
 	return 0;
 }
 
-static
-int query_nodes(struct ice_hw *hw,
-		uint32_t *children, int child_num,
-		uint32_t *parents, int *parent_num,
-		uint8_t level, bool detail,
-		FILE *stream)
+static int
+query_node_recursive(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	uint32_t parent;
-	int i;
-	int j;
-
-	*parent_num = 0;
-	for (i = 0; i < child_num; i++) {
-		bool exist = false;
-		int ret;
+	bool close = false;
+	if (node->parent != NULL && node->vsi_handle != node->parent->vsi_handle) {
+		fprintf(stream, "subgraph cluster_%u {\n", node->vsi_handle);
+		fprintf(stream, "\tlabel = \"VSI %u\";\n", node->vsi_handle);
+		close = true;
+	}
 
-		ret = query_node(hw, children[i], &parent, level, detail, stream);
-		if (ret)
-			return -EINVAL;
+	int ret = query_node(hw, ethdata, node, detail, stream);
+	if (ret != 0)
+		return ret;
 
-		for (j = 0; j < *parent_num; j++) {
-			if (parents[j] == parent) {
-				exist = true;
-				break;
-			}
+	for (uint16_t i = 0; i < node->num_children; i++) {
+		ret = query_node_recursive(hw, ethdata, node->children[i], detail, stream);
+		if (ret != 0)
+			return ret;
+		/* if we have a lot of nodes, skip a bunch in the middle */
+		if (node->num_children > 16 && i == 2) {
+			uint16_t inc = node->num_children - 5;
+			fprintf(stream, "\tn%d_children [label=\"... +%d child nodes ...\"];\n",
+					node->info.node_teid, inc);
+			fprintf(stream, "\tNODE_%d -> n%d_children;\n",
+					node->info.node_teid, node->info.node_teid);
+			i += inc;
 		}
-
-		if (!exist && parent != 0xFFFFFFFF)
-			parents[(*parent_num)++] = parent;
 	}
+	if (close)
+		fprintf(stream, "}\n");
+	if (node->info.parent_teid != 0xFFFFFFFF)
+		fprintf(stream, "\tNODE_%d -> NODE_%d\n",
+				node->info.parent_teid, node->info.node_teid);
 
 	return 0;
 }
 
-int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
+int
+rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 {
 	struct rte_eth_dev *dev;
 	struct ice_hw *hw;
-	struct ice_pf *pf;
-	struct ice_q_ctx *q_ctx;
-	uint16_t q_num;
-	uint16_t i;
-	struct ice_tx_queue *txq;
-	uint32_t buf1[256];
-	uint32_t buf2[256];
-	uint32_t *children = buf1;
-	uint32_t *parents = buf2;
-	int child_num = 0;
-	int parent_num = 0;
-	uint8_t level;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
@@ -846,35 +814,9 @@ int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 
 	dev = &rte_eth_devices[port];
 	hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	level = hw->num_tx_sched_layers;
-
-	q_num = dev->data->nb_tx_queues;
-
-	/* main vsi */
-	for (i = 0; i < q_num; i++) {
-		txq = dev->data->tx_queues[i];
-		q_ctx = ice_get_lan_q_ctx(hw, txq->vsi->idx, 0, i);
-		children[child_num++] = q_ctx->q_teid;
-	}
-
-	/* fdir vsi */
-	q_ctx = ice_get_lan_q_ctx(hw, pf->fdir.fdir_vsi->idx, 0, 0);
-	children[child_num++] = q_ctx->q_teid;
 
 	fprintf(stream, "digraph tx_sched {\n");
-	while (child_num > 0) {
-		int ret;
-		ret = query_nodes(hw, children, child_num,
-				  parents, &parent_num,
-				  level, detail, stream);
-		if (ret)
-			return ret;
-
-		children = parents;
-		child_num = parent_num;
-		level--;
-	}
+	query_node_recursive(hw, dev->data, hw->port_info->root, detail, stream);
 	fprintf(stream, "}\n");
 
 	return 0;
-- 
2.43.0

Re: [PATCH 2/5] net/ice: improve Tx scheduler graph output

[Stephen Hemminger]

On Wed,  9 Oct 2024 18:08:19 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> The function to dump the TX scheduler topology only adds to the chart
> nodes connected to TX queues or for the flow director VSI. Change the
> function to work recursively from the root node and thereby include all
> scheduler nodes, whether in use or not, in the dump.
> 
> Also, improve the output of the Tx scheduler graphing function:
> 
> * Add VSI details to each node in graph
> * When number of children is >16, skip middle nodes to reduce size of
>   the graph, otherwise dot output is unviewable for large hierarchies
> * For VSIs other than zero, use dot's clustering method to put those
>   VSIs into subgraphs with borders
> * For leaf nodes, display queue numbers for the any nodes assigned to
>   ethdev NIC Tx queues
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>


Fixes: ab4eaf9a8a31 ("net/ice: dump Tx scheduling tree")
Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH 2/5] net/ice: improve Tx scheduler graph output

[Bruce Richardson]

On Wed, Oct 09, 2024 at 10:45:43AM -0700, Stephen Hemminger wrote:
> On Wed,  9 Oct 2024 18:08:19 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > The function to dump the TX scheduler topology only adds to the chart
> > nodes connected to TX queues or for the flow director VSI. Change the
> > function to work recursively from the root node and thereby include all
> > scheduler nodes, whether in use or not, in the dump.
> > 
> > Also, improve the output of the Tx scheduler graphing function:
> > 
> > * Add VSI details to each node in graph
> > * When number of children is >16, skip middle nodes to reduce size of
> >   the graph, otherwise dot output is unviewable for large hierarchies
> > * For VSIs other than zero, use dot's clustering method to put those
> >   VSIs into subgraphs with borders
> > * For leaf nodes, display queue numbers for the any nodes assigned to
> >   ethdev NIC Tx queues
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> 
> Fixes: ab4eaf9a8a31 ("net/ice: dump Tx scheduling tree")

This is not really so much a fix as expanding the scope of what is
displayed. Therefore not including the fixes line and Cc of stable in v2.

> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> 
Thanks.

[PATCH 3/5] net/ice: add option to choose DDP package file

[Bruce Richardson]

The "Dynamic Device Personalization" package is loaded at initialization
time by the driver, but the specific package file loaded depends upon
what package file is found first by searching through a hard-coded list
of firmware paths. To enable greater control over the package loading,
we can add a device option to choose a specific DDP package file to
load.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/nics/ice.rst      |  9 +++++++++
 drivers/net/ice/ice_ethdev.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  1 +
 3 files changed, 45 insertions(+)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index 986abb211f..13f4fa1772 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -136,6 +136,15 @@ Runtime Configuration
 
     -a 80:00.0,default-mac-disable=1
 
+- ``DDP Package File``
+
+  Rather than have the driver search for the DDP package to load,
+  or to override what package is used,
+  the ``ddp_pkg_file`` option can be used to provide the path to a specific package file.
+  For example::
+
+    -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg
+
 - ``Protocol extraction for per queue``
 
   Configure the RX queues to do protocol extraction into mbuf for protocol
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 61bff016be..db44f5c4fc 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -37,6 +37,7 @@
 #define ICE_ONE_PPS_OUT_ARG       "pps_out"
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
+#define ICE_DDP_FILENAME          "ddp_pkg_file"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -53,6 +54,7 @@ static const char * const ice_valid_args[] = {
 	ICE_RX_LOW_LATENCY_ARG,
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
+	ICE_DDP_FILENAME,
 	NULL
 };
 
@@ -693,6 +695,18 @@ handle_field_name_arg(__rte_unused const char *key, const char *value,
 	return 0;
 }
 
+static int
+handle_ddp_filename_arg(__rte_unused const char *key, const char *value, void *name_args)
+{
+	const char **filename = name_args;
+	if (strlen(value) >= ICE_MAX_PKG_FILENAME_SIZE) {
+		PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", value);
+		return -1;
+	}
+	*filename = strdup(value);
+	return 0;
+}
+
 static void
 ice_check_proto_xtr_support(struct ice_hw *hw)
 {
@@ -1909,6 +1923,17 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 	size_t bufsz;
 	int err;
 
+	/* first read any explicitly referenced DDP file*/
+	if (adapter->devargs.ddp_filename != NULL) {
+		strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
+		if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+			goto load_fw;
+		} else {
+			PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
+			return -1;
+		}
+	}
+
 	memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
 	snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
 		"ice-%016" PRIx64 ".pkg", dsn);
@@ -2256,6 +2281,13 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 
 	ret = rte_kvargs_process(kvlist, ICE_RX_LOW_LATENCY_ARG,
 				 &parse_bool, &ad->devargs.rx_low_latency);
+	if (ret)
+		goto bail;
+
+	ret = rte_kvargs_process(kvlist, ICE_DDP_FILENAME,
+				 &handle_ddp_filename_arg, &ad->devargs.ddp_filename);
+	if (ret)
+		goto bail;
 
 bail:
 	rte_kvargs_free(kvlist);
@@ -2729,6 +2761,8 @@ ice_dev_close(struct rte_eth_dev *dev)
 	ice_free_hw_tbls(hw);
 	rte_free(hw->port_info);
 	hw->port_info = NULL;
+	free((void *)(uintptr_t)ad->devargs.ddp_filename);
+	ad->devargs.ddp_filename = NULL;
 	ice_shutdown_all_ctrlq(hw, true);
 	rte_free(pf->proto_xtr);
 	pf->proto_xtr = NULL;
@@ -7047,6 +7081,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_FILENAME "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE);
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 8b644ed700..2781362d04 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -569,6 +569,7 @@ struct ice_devargs {
 	/* Name of the field. */
 	char xtr_field_name[RTE_MBUF_DYN_NAMESIZE];
 	uint64_t mbuf_check;
+	const char *ddp_filename;
 };
 
 /**
-- 
2.43.0

Re: [PATCH 3/5] net/ice: add option to choose DDP package file

[Stephen Hemminger]

On Wed,  9 Oct 2024 18:08:20 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> The "Dynamic Device Personalization" package is loaded at initialization
> time by the driver, but the specific package file loaded depends upon
> what package file is found first by searching through a hard-coded list
> of firmware paths. To enable greater control over the package loading,
> we can add a device option to choose a specific DDP package file to
> load.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

[PATCH 4/5] net/ice: add option to download scheduler topology

[Bruce Richardson]

The DDP package file being loaded at init time may contain an
alternative Tx Scheduler topology in it. Add driver option to load this
topology at init time.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ice/base/ice_ddp.c | 18 +++++++++++++++---
 drivers/net/ice/base/ice_ddp.h |  4 ++--
 drivers/net/ice/ice_ethdev.c   | 24 +++++++++++++++---------
 drivers/net/ice/ice_ethdev.h   |  1 +
 4 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ice/base/ice_ddp.c b/drivers/net/ice/base/ice_ddp.c
index 90aaa6b331..1462c2b7fe 100644
--- a/drivers/net/ice/base/ice_ddp.c
+++ b/drivers/net/ice/base/ice_ddp.c
@@ -1333,7 +1333,7 @@ ice_fill_hw_ptype(struct ice_hw *hw)
  * ice_copy_and_init_pkg() instead of directly calling ice_init_pkg() in this
  * case.
  */
-enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
+enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len, bool load_sched)
 {
 	bool already_loaded = false;
 	enum ice_ddp_state state;
@@ -1351,6 +1351,18 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
 		return state;
 	}
 
+	if (load_sched) {
+		enum ice_status res = ice_cfg_tx_topo(hw, buf, len);
+		if (res != ICE_SUCCESS) {
+			ice_debug(hw, ICE_DBG_INIT, "failed to apply sched topology  (err: %d)\n",
+					res);
+			return ICE_DDP_PKG_ERR;
+		}
+		ice_debug(hw, ICE_DBG_INIT, "Topology download successful, reinitializing device\n");
+		ice_deinit_hw(hw);
+		ice_init_hw(hw);
+	}
+
 	/* initialize package info */
 	state = ice_init_pkg_info(hw, pkg);
 	if (state)
@@ -1423,7 +1435,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
  * related routines.
  */
 enum ice_ddp_state
-ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len)
+ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len, bool load_sched)
 {
 	enum ice_ddp_state state;
 	u8 *buf_copy;
@@ -1433,7 +1445,7 @@ ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len)
 
 	buf_copy = (u8 *)ice_memdup(hw, buf, len, ICE_NONDMA_TO_NONDMA);
 
-	state = ice_init_pkg(hw, buf_copy, len);
+	state = ice_init_pkg(hw, buf_copy, len, load_sched);
 	if (!ice_is_init_pkg_successful(state)) {
 		/* Free the copy, since we failed to initialize the package */
 		ice_free(hw, buf_copy);
diff --git a/drivers/net/ice/base/ice_ddp.h b/drivers/net/ice/base/ice_ddp.h
index 5512669f44..d79cdee13a 100644
--- a/drivers/net/ice/base/ice_ddp.h
+++ b/drivers/net/ice/base/ice_ddp.h
@@ -454,9 +454,9 @@ ice_pkg_enum_entry(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
 void *
 ice_pkg_enum_section(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
 		     u32 sect_type);
-enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buff, u32 len);
+enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buff, u32 len, bool load_sched);
 enum ice_ddp_state
-ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len);
+ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len, bool load_sched);
 bool ice_is_init_pkg_successful(enum ice_ddp_state state);
 void ice_free_seg(struct ice_hw *hw);
 
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index db44f5c4fc..ea1ed4fb68 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -38,6 +38,7 @@
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
 #define ICE_DDP_FILENAME          "ddp_pkg_file"
+#define ICE_DDP_LOAD_SCHED        "ddp_load_sched_topo"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -55,6 +56,7 @@ static const char * const ice_valid_args[] = {
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
 	ICE_DDP_FILENAME,
+	ICE_DDP_LOAD_SCHED,
 	NULL
 };
 
@@ -1979,7 +1981,7 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 load_fw:
 	PMD_INIT_LOG(DEBUG, "DDP package name: %s", pkg_file);
 
-	err = ice_copy_and_init_pkg(hw, buf, bufsz);
+	err = ice_copy_and_init_pkg(hw, buf, bufsz, adapter->devargs.ddp_load_sched);
 	if (!ice_is_init_pkg_successful(err)) {
 		PMD_INIT_LOG(ERR, "ice_copy_and_init_hw failed: %d", err);
 		free(buf);
@@ -2012,19 +2014,18 @@ static int
 parse_bool(const char *key, const char *value, void *args)
 {
 	int *i = (int *)args;
-	char *end;
-	int num;
 
-	num = strtoul(value, &end, 10);
-
-	if (num != 0 && num != 1) {
-		PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", "
-			"value must be 0 or 1",
+	if (value == NULL || value[0] == '\0') {
+		PMD_DRV_LOG(WARNING, "key:\"%s\", requires a value, which must be 0 or 1", key);
+		return -1;
+	}
+	if (value[1] != '\0' || (value[0] != '0' && value[0] != '1')) {
+		PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", value must be 0 or 1",
 			value, key);
 		return -1;
 	}
 
-	*i = num;
+	*i = value[0] - '0';
 	return 0;
 }
 
@@ -2289,6 +2290,10 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, ICE_DDP_LOAD_SCHED,
+				 &parse_bool, &ad->devargs.ddp_load_sched);
+	if (ret)
+		goto bail;
 bail:
 	rte_kvargs_free(kvlist);
 	return ret;
@@ -7081,6 +7086,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_LOAD_SCHED "=<0|1>"
 			      ICE_DDP_FILENAME "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 2781362d04..76310f2c99 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -564,6 +564,7 @@ struct ice_devargs {
 	uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
 	uint8_t pin_idx;
 	uint8_t pps_out_ena;
+	int ddp_load_sched;
 	int xtr_field_offs;
 	uint8_t xtr_flag_offs[PROTO_XTR_MAX];
 	/* Name of the field. */
-- 
2.43.0

Re: [PATCH 4/5] net/ice: add option to download scheduler topology

[Stephen Hemminger]

On Wed,  9 Oct 2024 18:08:21 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
> index 2781362d04..76310f2c99 100644
> --- a/drivers/net/ice/ice_ethdev.h
> +++ b/drivers/net/ice/ice_ethdev.h
> @@ -564,6 +564,7 @@ struct ice_devargs {
>  	uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
>  	uint8_t pin_idx;
>  	uint8_t pps_out_ena;
> +	int ddp_load_sched;

Since this is a boolean option why is the flag not uint8_t like the other flags
in ice_devargs?

Re: [PATCH 4/5] net/ice: add option to download scheduler topology

[Bruce Richardson]

On Wed, Oct 09, 2024 at 10:49:00AM -0700, Stephen Hemminger wrote:
> On Wed,  9 Oct 2024 18:08:21 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
> > index 2781362d04..76310f2c99 100644
> > --- a/drivers/net/ice/ice_ethdev.h
> > +++ b/drivers/net/ice/ice_ethdev.h
> > @@ -564,6 +564,7 @@ struct ice_devargs {
> >  	uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
> >  	uint8_t pin_idx;
> >  	uint8_t pps_out_ena;
> > +	int ddp_load_sched;
> 
> Since this is a boolean option why is the flag not uint8_t like the other flags
> in ice_devargs?

Good point. Will update.

[PATCH 5/5] net/ice: limit the number of queues to sched capabilities

[Bruce Richardson]

Rather than assuming that each VSI can hold up to 256 queue pairs,
or the reported device limit, query the available nodes in the scheduler
tree to check that we are not overflowing the limit for number of
child scheduling nodes at each level. Do this by multiplying
max_children for each level beyond the VSI and using that as an
additional cap on the number of queues.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index ea1ed4fb68..2d5e8b27d7 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -914,7 +914,7 @@ ice_vsi_config_default_rss(struct ice_aqc_vsi_props *info)
 }
 
 static int
-ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
+ice_vsi_config_tc_queue_mapping(struct ice_hw *hw, struct ice_vsi *vsi,
 				struct ice_aqc_vsi_props *info,
 				uint8_t enabled_tcmap)
 {
@@ -930,13 +930,28 @@ ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
 	}
 
 	/* vector 0 is reserved and 1 vector for ctrl vsi */
-	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2)
+	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2) {
 		vsi->nb_qps = 0;
-	else
+	} else {
 		vsi->nb_qps = RTE_MIN
 			((uint16_t)vsi->adapter->hw.func_caps.common_cap.num_msix_vectors - 2,
 			RTE_MIN(vsi->nb_qps, ICE_MAX_Q_PER_TC));
 
+		/* cap max QPs to what the HW reports as num-children for each layer.
+		 * Multiply num_children for each layer from the entry_point layer to
+		 * the qgroup, or second-last layer.
+		 * Avoid any potential overflow by using uint32_t type and breaking loop
+		 * once we have a number greater than the already configured max.
+		 */
+		uint32_t max_sched_vsi_nodes = 1;
+		for (uint8_t i = hw->sw_entry_point_layer; i < hw->num_tx_sched_layers - 1; i++) {
+			max_sched_vsi_nodes *= hw->max_children[i];
+			if (max_sched_vsi_nodes >= vsi->nb_qps)
+				break;
+		}
+		vsi->nb_qps = RTE_MIN(vsi->nb_qps, max_sched_vsi_nodes);
+	}
+
 	/* nb_qps(hex)  -> fls */
 	/* 0000		-> 0 */
 	/* 0001		-> 0 */
@@ -1708,7 +1723,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 			rte_cpu_to_le_16(hw->func_caps.fd_fltr_best_effort);
 
 		/* Enable VLAN/UP trip */
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
@@ -1732,7 +1747,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 		vsi_ctx.info.fd_options = rte_cpu_to_le_16(cfg);
 		vsi_ctx.info.sw_id = hw->port_info->sw_id;
 		vsi_ctx.info.sw_flags2 = ICE_AQ_VSI_SW_FLAG_LAN_ENA;
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
-- 
2.43.0

Re: [PATCH 5/5] net/ice: limit the number of queues to sched capabilities

[Stephen Hemminger]

On Wed,  9 Oct 2024 18:08:22 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> Rather than assuming that each VSI can hold up to 256 queue pairs,
> or the reported device limit, query the available nodes in the scheduler
> tree to check that we are not overflowing the limit for number of
> child scheduling nodes at each level. Do this by multiplying
> max_children for each level beyond the VSI and using that as an
> additional cap on the number of queues.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

[PATCH v2 0/5] updates for net/ice driver

[Bruce Richardson]

This patchset contains a set of updates for the ice driver, a number of
which are in the area of the "rte_tm" APIs for Tx scheduling.

These patches were previously submitted as part of a larger set[1], but
separating them out here for easier review and merge ahead of the more
substantial changes for scheduling in the last couple of patches in that
set.

[1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*

---
v2: small update to patch 4 following review by Stephen.

Bruce Richardson (5):
  net/ice: detect stopping a flow-director queue twice
  net/ice: improve Tx scheduler graph output
  net/ice: add option to choose DDP package file
  net/ice: add option to download scheduler topology
  net/ice: limit the number of queues to sched capabilities

 doc/guides/nics/ice.rst        |   9 ++
 drivers/net/ice/base/ice_ddp.c |  18 ++-
 drivers/net/ice/base/ice_ddp.h |   4 +-
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 drivers/net/ice/ice_ethdev.c   |  84 +++++++++++---
 drivers/net/ice/ice_ethdev.h   |   2 +
 drivers/net/ice/ice_rxtx.c     |   5 +
 7 files changed, 172 insertions(+), 146 deletions(-)

--
2.43.0

[PATCH v2 1/5] net/ice: detect stopping a flow-director queue twice

[Bruce Richardson]

If the flow-director queue is stopped at some point during the running
of an application, the shutdown procedure for the port issues an error
as it tries to stop the queue a second time, and fails to do so. We can
eliminate this error by setting the tail-register pointer to NULL on
stop, and checking for that condition in subsequent stop calls. Since
the register pointer is set on start, any restarting of the queue will
allow a stop call to progress as normal.

Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index d2f9edc221..024d97cb46 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1139,6 +1139,10 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 			    tx_queue_id);
 		return -EINVAL;
 	}
+	if (txq->qtx_tail == NULL) {
+		PMD_DRV_LOG(INFO, "TX queue %u not started", tx_queue_id);
+		return 0;
+	}
 	vsi = txq->vsi;
 
 	q_ids[0] = txq->reg_idx;
@@ -1153,6 +1157,7 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	}
 
 	txq->tx_rel_mbufs(txq);
+	txq->qtx_tail = NULL;
 
 	return 0;
 }
-- 
2.43.0

[PATCH v2 2/5] net/ice: improve Tx scheduler graph output

[Bruce Richardson]

The function to dump the TX scheduler topology only adds to the chart
nodes connected to TX queues or for the flow director VSI. Change the
function to work recursively from the root node and thereby include all
scheduler nodes, whether in use or not, in the dump.

Also, improve the output of the Tx scheduler graphing function:

* Add VSI details to each node in graph
* When number of children is >16, skip middle nodes to reduce size of
  the graph, otherwise dot output is unviewable for large hierarchies
* For VSIs other than zero, use dot's clustering method to put those
  VSIs into subgraphs with borders
* For leaf nodes, display queue numbers for the any nodes assigned to
  ethdev NIC Tx queues

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 1 file changed, 69 insertions(+), 127 deletions(-)

diff --git a/drivers/net/ice/ice_diagnose.c b/drivers/net/ice/ice_diagnose.c
index c357554707..623d84e37d 100644
--- a/drivers/net/ice/ice_diagnose.c
+++ b/drivers/net/ice/ice_diagnose.c
@@ -545,29 +545,15 @@ static void print_rl_profile(struct ice_aqc_rl_profile_elem *prof,
 	fprintf(stream, "\t\t\t\t\t</td>\n");
 }
 
-static
-void print_elem_type(FILE *stream, u8 type)
+static const char *
+get_elem_type(u8 type)
 {
-	switch (type) {
-	case 1:
-		fprintf(stream, "root");
-		break;
-	case 2:
-		fprintf(stream, "tc");
-		break;
-	case 3:
-		fprintf(stream, "se_generic");
-		break;
-	case 4:
-		fprintf(stream, "entry_point");
-		break;
-	case 5:
-		fprintf(stream, "leaf");
-		break;
-	default:
-		fprintf(stream, "%d", type);
-		break;
-	}
+	static const char * const ice_sched_node_types[] = {
+			"Undefined", "Root", "TC", "SE Generic", "SW Entry", "Leaf"
+	};
+	if (type < RTE_DIM(ice_sched_node_types))
+		return ice_sched_node_types[type];
+	return "*UNKNOWN*";
 }
 
 static
@@ -602,7 +588,9 @@ void print_priority_mode(FILE *stream, bool flag)
 }
 
 static
-void print_node(struct ice_aqc_txsched_elem_data *data,
+void print_node(struct ice_sched_node *node,
+		struct rte_eth_dev_data *ethdata,
+		struct ice_aqc_txsched_elem_data *data,
 		struct ice_aqc_rl_profile_elem *cir_prof,
 		struct ice_aqc_rl_profile_elem *eir_prof,
 		struct ice_aqc_rl_profile_elem *shared_prof,
@@ -613,17 +601,19 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 
 	fprintf(stream, "\t\t\t<table>\n");
 
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> teid </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td> %d </td>\n", data->node_teid);
-	fprintf(stream, "\t\t\t\t</tr>\n");
-
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> type </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td>");
-	print_elem_type(stream, data->data.elem_type);
-	fprintf(stream, "</td>\n");
-	fprintf(stream, "\t\t\t\t</tr>\n");
+	fprintf(stream, "\t\t\t\t<tr><td>teid</td><td>%d</td></tr>\n", data->node_teid);
+	fprintf(stream, "\t\t\t\t<tr><td>type</td><td>%s</td></tr>\n",
+			get_elem_type(data->data.elem_type));
+	fprintf(stream, "\t\t\t\t<tr><td>VSI</td><td>%u</td></tr>\n", node->vsi_handle);
+	if (data->data.elem_type == ICE_AQC_ELEM_TYPE_LEAF) {
+		for (uint16_t i = 0; i < ethdata->nb_tx_queues; i++) {
+			struct ice_tx_queue *q = ethdata->tx_queues[i];
+			if (q->q_teid == data->node_teid) {
+				fprintf(stream, "\t\t\t\t<tr><td>TXQ</td><td>%u</td></tr>\n", i);
+				break;
+			}
+		}
+	}
 
 	if (!detail)
 		goto brief;
@@ -705,8 +695,6 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 	fprintf(stream, "\t\tshape=plain\n");
 	fprintf(stream, "\t]\n");
 
-	if (data->parent_teid != 0xFFFFFFFF)
-		fprintf(stream, "\tNODE_%d -> NODE_%d\n", data->parent_teid, data->node_teid);
 }
 
 static
@@ -731,112 +719,92 @@ int query_rl_profile(struct ice_hw *hw,
 	return 0;
 }
 
-static
-int query_node(struct ice_hw *hw, uint32_t child, uint32_t *parent,
-	       uint8_t level, bool detail, FILE *stream)
+static int
+query_node(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	struct ice_aqc_txsched_elem_data data;
+	struct ice_aqc_txsched_elem_data *data = &node->info;
 	struct ice_aqc_rl_profile_elem cir_prof;
 	struct ice_aqc_rl_profile_elem eir_prof;
 	struct ice_aqc_rl_profile_elem shared_prof;
 	struct ice_aqc_rl_profile_elem *cp = NULL;
 	struct ice_aqc_rl_profile_elem *ep = NULL;
 	struct ice_aqc_rl_profile_elem *sp = NULL;
-	int status, ret;
-
-	status = ice_sched_query_elem(hw, child, &data);
-	if (status != ICE_SUCCESS) {
-		if (level == hw->num_tx_sched_layers) {
-			/* ignore the error when a queue has been stopped. */
-			PMD_DRV_LOG(WARNING, "Failed to query queue node %d.", child);
-			*parent = 0xffffffff;
-			return 0;
-		}
-		PMD_DRV_LOG(ERR, "Failed to query scheduling node %d.", child);
-		return -EINVAL;
-	}
-
-	*parent = data.parent_teid;
+	u8 level = node->tx_sched_layer;
+	int ret;
 
-	if (data.data.cir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 0, data.data.cir_bw.bw_profile_idx, &cir_prof);
+	if (data->data.cir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 0, data->data.cir_bw.bw_profile_idx, &cir_prof);
 
 		if (ret)
 			return ret;
 		cp = &cir_prof;
 	}
 
-	if (data.data.eir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 1, data.data.eir_bw.bw_profile_idx, &eir_prof);
+	if (data->data.eir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 1, data->data.eir_bw.bw_profile_idx, &eir_prof);
 
 		if (ret)
 			return ret;
 		ep = &eir_prof;
 	}
 
-	if (data.data.srl_id != 0) {
-		ret = query_rl_profile(hw, level, 2, data.data.srl_id, &shared_prof);
+	if (data->data.srl_id != 0) {
+		ret = query_rl_profile(hw, level, 2, data->data.srl_id, &shared_prof);
 
 		if (ret)
 			return ret;
 		sp = &shared_prof;
 	}
 
-	print_node(&data, cp, ep, sp, detail, stream);
+	print_node(node, ethdata, data, cp, ep, sp, detail, stream);
 
 	return 0;
 }
 
-static
-int query_nodes(struct ice_hw *hw,
-		uint32_t *children, int child_num,
-		uint32_t *parents, int *parent_num,
-		uint8_t level, bool detail,
-		FILE *stream)
+static int
+query_node_recursive(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	uint32_t parent;
-	int i;
-	int j;
-
-	*parent_num = 0;
-	for (i = 0; i < child_num; i++) {
-		bool exist = false;
-		int ret;
+	bool close = false;
+	if (node->parent != NULL && node->vsi_handle != node->parent->vsi_handle) {
+		fprintf(stream, "subgraph cluster_%u {\n", node->vsi_handle);
+		fprintf(stream, "\tlabel = \"VSI %u\";\n", node->vsi_handle);
+		close = true;
+	}
 
-		ret = query_node(hw, children[i], &parent, level, detail, stream);
-		if (ret)
-			return -EINVAL;
+	int ret = query_node(hw, ethdata, node, detail, stream);
+	if (ret != 0)
+		return ret;
 
-		for (j = 0; j < *parent_num; j++) {
-			if (parents[j] == parent) {
-				exist = true;
-				break;
-			}
+	for (uint16_t i = 0; i < node->num_children; i++) {
+		ret = query_node_recursive(hw, ethdata, node->children[i], detail, stream);
+		if (ret != 0)
+			return ret;
+		/* if we have a lot of nodes, skip a bunch in the middle */
+		if (node->num_children > 16 && i == 2) {
+			uint16_t inc = node->num_children - 5;
+			fprintf(stream, "\tn%d_children [label=\"... +%d child nodes ...\"];\n",
+					node->info.node_teid, inc);
+			fprintf(stream, "\tNODE_%d -> n%d_children;\n",
+					node->info.node_teid, node->info.node_teid);
+			i += inc;
 		}
-
-		if (!exist && parent != 0xFFFFFFFF)
-			parents[(*parent_num)++] = parent;
 	}
+	if (close)
+		fprintf(stream, "}\n");
+	if (node->info.parent_teid != 0xFFFFFFFF)
+		fprintf(stream, "\tNODE_%d -> NODE_%d\n",
+				node->info.parent_teid, node->info.node_teid);
 
 	return 0;
 }
 
-int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
+int
+rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 {
 	struct rte_eth_dev *dev;
 	struct ice_hw *hw;
-	struct ice_pf *pf;
-	struct ice_q_ctx *q_ctx;
-	uint16_t q_num;
-	uint16_t i;
-	struct ice_tx_queue *txq;
-	uint32_t buf1[256];
-	uint32_t buf2[256];
-	uint32_t *children = buf1;
-	uint32_t *parents = buf2;
-	int child_num = 0;
-	int parent_num = 0;
-	uint8_t level;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
@@ -846,35 +814,9 @@ int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 
 	dev = &rte_eth_devices[port];
 	hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	level = hw->num_tx_sched_layers;
-
-	q_num = dev->data->nb_tx_queues;
-
-	/* main vsi */
-	for (i = 0; i < q_num; i++) {
-		txq = dev->data->tx_queues[i];
-		q_ctx = ice_get_lan_q_ctx(hw, txq->vsi->idx, 0, i);
-		children[child_num++] = q_ctx->q_teid;
-	}
-
-	/* fdir vsi */
-	q_ctx = ice_get_lan_q_ctx(hw, pf->fdir.fdir_vsi->idx, 0, 0);
-	children[child_num++] = q_ctx->q_teid;
 
 	fprintf(stream, "digraph tx_sched {\n");
-	while (child_num > 0) {
-		int ret;
-		ret = query_nodes(hw, children, child_num,
-				  parents, &parent_num,
-				  level, detail, stream);
-		if (ret)
-			return ret;
-
-		children = parents;
-		child_num = parent_num;
-		level--;
-	}
+	query_node_recursive(hw, dev->data, hw->port_info->root, detail, stream);
 	fprintf(stream, "}\n");
 
 	return 0;
-- 
2.43.0

[PATCH v2 3/5] net/ice: add option to choose DDP package file

[Bruce Richardson]

The "Dynamic Device Personalization" package is loaded at initialization
time by the driver, but the specific package file loaded depends upon
what package file is found first by searching through a hard-coded list
of firmware paths. To enable greater control over the package loading,
we can add a device option to choose a specific DDP package file to
load.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/nics/ice.rst      |  9 +++++++++
 drivers/net/ice/ice_ethdev.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  1 +
 3 files changed, 45 insertions(+)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index 5e9363cec6..4a62c3daea 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -138,6 +138,15 @@ Runtime Configuration
 
     -a 80:00.0,default-mac-disable=1
 
+- ``DDP Package File``
+
+  Rather than have the driver search for the DDP package to load,
+  or to override what package is used,
+  the ``ddp_pkg_file`` option can be used to provide the path to a specific package file.
+  For example::
+
+    -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg
+
 - ``Protocol extraction for per queue``
 
   Configure the RX queues to do protocol extraction into mbuf for protocol
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 70298ac330..cb792e0119 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -38,6 +38,7 @@
 #define ICE_ONE_PPS_OUT_ARG       "pps_out"
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
+#define ICE_DDP_FILENAME          "ddp_pkg_file"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -54,6 +55,7 @@ static const char * const ice_valid_args[] = {
 	ICE_RX_LOW_LATENCY_ARG,
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
+	ICE_DDP_FILENAME,
 	NULL
 };
 
@@ -696,6 +698,18 @@ handle_field_name_arg(__rte_unused const char *key, const char *value,
 	return 0;
 }
 
+static int
+handle_ddp_filename_arg(__rte_unused const char *key, const char *value, void *name_args)
+{
+	const char **filename = name_args;
+	if (strlen(value) >= ICE_MAX_PKG_FILENAME_SIZE) {
+		PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", value);
+		return -1;
+	}
+	*filename = strdup(value);
+	return 0;
+}
+
 static void
 ice_check_proto_xtr_support(struct ice_hw *hw)
 {
@@ -1912,6 +1926,17 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 	size_t bufsz;
 	int err;
 
+	/* first read any explicitly referenced DDP file*/
+	if (adapter->devargs.ddp_filename != NULL) {
+		strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
+		if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+			goto load_fw;
+		} else {
+			PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
+			return -1;
+		}
+	}
+
 	memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
 	snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
 		"ice-%016" PRIx64 ".pkg", dsn);
@@ -2259,6 +2284,13 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 
 	ret = rte_kvargs_process(kvlist, ICE_RX_LOW_LATENCY_ARG,
 				 &parse_bool, &ad->devargs.rx_low_latency);
+	if (ret)
+		goto bail;
+
+	ret = rte_kvargs_process(kvlist, ICE_DDP_FILENAME,
+				 &handle_ddp_filename_arg, &ad->devargs.ddp_filename);
+	if (ret)
+		goto bail;
 
 bail:
 	rte_kvargs_free(kvlist);
@@ -2762,6 +2794,8 @@ ice_dev_close(struct rte_eth_dev *dev)
 	ice_free_hw_tbls(hw);
 	rte_free(hw->port_info);
 	hw->port_info = NULL;
+	free((void *)(uintptr_t)ad->devargs.ddp_filename);
+	ad->devargs.ddp_filename = NULL;
 	ice_shutdown_all_ctrlq(hw, true);
 	rte_free(pf->proto_xtr);
 	pf->proto_xtr = NULL;
@@ -7135,6 +7169,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_FILENAME "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE);
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 57087c98ed..076cf595e8 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -569,6 +569,7 @@ struct ice_devargs {
 	/* Name of the field. */
 	char xtr_field_name[RTE_MBUF_DYN_NAMESIZE];
 	uint64_t mbuf_check;
+	const char *ddp_filename;
 };
 
 /**
-- 
2.43.0

[PATCH v2 4/5] net/ice: add option to download scheduler topology

[Bruce Richardson]

The DDP package file being loaded at init time may contain an
alternative Tx Scheduler topology in it. Add driver option to load this
topology at init time.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
V2: use uint8_t rather than int for boolean flag for consistency.
---
 drivers/net/ice/base/ice_ddp.c | 18 +++++++++++++++---
 drivers/net/ice/base/ice_ddp.h |  4 ++--
 drivers/net/ice/ice_ethdev.c   | 24 +++++++++++++++---------
 drivers/net/ice/ice_ethdev.h   |  1 +
 4 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ice/base/ice_ddp.c b/drivers/net/ice/base/ice_ddp.c
index c17a58eab8..ab9645f3a2 100644
--- a/drivers/net/ice/base/ice_ddp.c
+++ b/drivers/net/ice/base/ice_ddp.c
@@ -1333,7 +1333,7 @@ ice_fill_hw_ptype(struct ice_hw *hw)
  * ice_copy_and_init_pkg() instead of directly calling ice_init_pkg() in this
  * case.
  */
-enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
+enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len, bool load_sched)
 {
 	bool already_loaded = false;
 	enum ice_ddp_state state;
@@ -1351,6 +1351,18 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
 		return state;
 	}
 
+	if (load_sched) {
+		enum ice_status res = ice_cfg_tx_topo(hw, buf, len);
+		if (res != ICE_SUCCESS) {
+			ice_debug(hw, ICE_DBG_INIT, "failed to apply sched topology  (err: %d)\n",
+					res);
+			return ICE_DDP_PKG_ERR;
+		}
+		ice_debug(hw, ICE_DBG_INIT, "Topology download successful, reinitializing device\n");
+		ice_deinit_hw(hw);
+		ice_init_hw(hw);
+	}
+
 	/* initialize package info */
 	state = ice_init_pkg_info(hw, pkg);
 	if (state)
@@ -1423,7 +1435,7 @@ enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buf, u32 len)
  * related routines.
  */
 enum ice_ddp_state
-ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len)
+ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len, bool load_sched)
 {
 	enum ice_ddp_state state;
 	u8 *buf_copy;
@@ -1433,7 +1445,7 @@ ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len)
 
 	buf_copy = (u8 *)ice_memdup(hw, buf, len, ICE_NONDMA_TO_NONDMA);
 
-	state = ice_init_pkg(hw, buf_copy, len);
+	state = ice_init_pkg(hw, buf_copy, len, load_sched);
 	if (!ice_is_init_pkg_successful(state)) {
 		/* Free the copy, since we failed to initialize the package */
 		ice_free(hw, buf_copy);
diff --git a/drivers/net/ice/base/ice_ddp.h b/drivers/net/ice/base/ice_ddp.h
index 5512669f44..d79cdee13a 100644
--- a/drivers/net/ice/base/ice_ddp.h
+++ b/drivers/net/ice/base/ice_ddp.h
@@ -454,9 +454,9 @@ ice_pkg_enum_entry(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
 void *
 ice_pkg_enum_section(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
 		     u32 sect_type);
-enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buff, u32 len);
+enum ice_ddp_state ice_init_pkg(struct ice_hw *hw, u8 *buff, u32 len, bool load_sched);
 enum ice_ddp_state
-ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len);
+ice_copy_and_init_pkg(struct ice_hw *hw, const u8 *buf, u32 len, bool load_sched);
 bool ice_is_init_pkg_successful(enum ice_ddp_state state);
 void ice_free_seg(struct ice_hw *hw);
 
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index cb792e0119..422f600e3c 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -39,6 +39,7 @@
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
 #define ICE_DDP_FILENAME          "ddp_pkg_file"
+#define ICE_DDP_LOAD_SCHED        "ddp_load_sched_topo"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -56,6 +57,7 @@ static const char * const ice_valid_args[] = {
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
 	ICE_DDP_FILENAME,
+	ICE_DDP_LOAD_SCHED,
 	NULL
 };
 
@@ -1982,7 +1984,7 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 load_fw:
 	PMD_INIT_LOG(DEBUG, "DDP package name: %s", pkg_file);
 
-	err = ice_copy_and_init_pkg(hw, buf, bufsz);
+	err = ice_copy_and_init_pkg(hw, buf, bufsz, adapter->devargs.ddp_load_sched);
 	if (!ice_is_init_pkg_successful(err)) {
 		PMD_INIT_LOG(ERR, "ice_copy_and_init_hw failed: %d", err);
 		free(buf);
@@ -2015,19 +2017,18 @@ static int
 parse_bool(const char *key, const char *value, void *args)
 {
 	int *i = (int *)args;
-	char *end;
-	int num;
 
-	num = strtoul(value, &end, 10);
-
-	if (num != 0 && num != 1) {
-		PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", "
-			"value must be 0 or 1",
+	if (value == NULL || value[0] == '\0') {
+		PMD_DRV_LOG(WARNING, "key:\"%s\", requires a value, which must be 0 or 1", key);
+		return -1;
+	}
+	if (value[1] != '\0' || (value[0] != '0' && value[0] != '1')) {
+		PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", value must be 0 or 1",
 			value, key);
 		return -1;
 	}
 
-	*i = num;
+	*i = value[0] - '0';
 	return 0;
 }
 
@@ -2292,6 +2293,10 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 	if (ret)
 		goto bail;
 
+	ret = rte_kvargs_process(kvlist, ICE_DDP_LOAD_SCHED,
+				 &parse_bool, &ad->devargs.ddp_load_sched);
+	if (ret)
+		goto bail;
 bail:
 	rte_kvargs_free(kvlist);
 	return ret;
@@ -7169,6 +7174,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_LOAD_SCHED "=<0|1>"
 			      ICE_DDP_FILENAME "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 076cf595e8..2794a76096 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -564,6 +564,7 @@ struct ice_devargs {
 	uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
 	uint8_t pin_idx;
 	uint8_t pps_out_ena;
+	uint8_t ddp_load_sched;
 	int xtr_field_offs;
 	uint8_t xtr_flag_offs[PROTO_XTR_MAX];
 	/* Name of the field. */
-- 
2.43.0

[PATCH v2 5/5] net/ice: limit the number of queues to sched capabilities

[Bruce Richardson]

Rather than assuming that each VSI can hold up to 256 queue pairs,
or the reported device limit, query the available nodes in the scheduler
tree to check that we are not overflowing the limit for number of
child scheduling nodes at each level. Do this by multiplying
max_children for each level beyond the VSI and using that as an
additional cap on the number of queues.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_ethdev.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 422f600e3c..190ec00571 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -917,7 +917,7 @@ ice_vsi_config_default_rss(struct ice_aqc_vsi_props *info)
 }
 
 static int
-ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
+ice_vsi_config_tc_queue_mapping(struct ice_hw *hw, struct ice_vsi *vsi,
 				struct ice_aqc_vsi_props *info,
 				uint8_t enabled_tcmap)
 {
@@ -933,13 +933,28 @@ ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
 	}
 
 	/* vector 0 is reserved and 1 vector for ctrl vsi */
-	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2)
+	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2) {
 		vsi->nb_qps = 0;
-	else
+	} else {
 		vsi->nb_qps = RTE_MIN
 			((uint16_t)vsi->adapter->hw.func_caps.common_cap.num_msix_vectors - 2,
 			RTE_MIN(vsi->nb_qps, ICE_MAX_Q_PER_TC));
 
+		/* cap max QPs to what the HW reports as num-children for each layer.
+		 * Multiply num_children for each layer from the entry_point layer to
+		 * the qgroup, or second-last layer.
+		 * Avoid any potential overflow by using uint32_t type and breaking loop
+		 * once we have a number greater than the already configured max.
+		 */
+		uint32_t max_sched_vsi_nodes = 1;
+		for (uint8_t i = hw->sw_entry_point_layer; i < hw->num_tx_sched_layers - 1; i++) {
+			max_sched_vsi_nodes *= hw->max_children[i];
+			if (max_sched_vsi_nodes >= vsi->nb_qps)
+				break;
+		}
+		vsi->nb_qps = RTE_MIN(vsi->nb_qps, max_sched_vsi_nodes);
+	}
+
 	/* nb_qps(hex)  -> fls */
 	/* 0000		-> 0 */
 	/* 0001		-> 0 */
@@ -1711,7 +1726,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 			rte_cpu_to_le_16(hw->func_caps.fd_fltr_best_effort);
 
 		/* Enable VLAN/UP trip */
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
@@ -1735,7 +1750,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 		vsi_ctx.info.fd_options = rte_cpu_to_le_16(cfg);
 		vsi_ctx.info.sw_id = hw->port_info->sw_id;
 		vsi_ctx.info.sw_flags2 = ICE_AQ_VSI_SW_FLAG_LAN_ENA;
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
-- 
2.43.0

[PATCH v3 0/4] updates for net/ice driver

[Bruce Richardson]

This patchset contains a set of updates for the ice driver, a number of
which are in the area of the "rte_tm" APIs for Tx scheduling.

These patches were previously submitted as part of a larger set[1], but
separating them out here for easier review and merge ahead of the more
substantial changes for scheduling in the last couple of patches in that
set.

[1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*

---
v3: dropped patch 4 completely from set, it was missing doc updates and
    it fits better with out sched changes. Will include in a later set
v2: small update to patch 4 following review by Stephen.


Bruce Richardson (4):
  net/ice: detect stopping a flow-director queue twice
  net/ice: improve Tx scheduler graph output
  net/ice: add option to choose DDP package file
  net/ice: limit the number of queues to sched capabilities

 doc/guides/nics/ice.rst        |   9 ++
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 drivers/net/ice/ice_ethdev.c   |  60 +++++++++-
 drivers/net/ice/ice_ethdev.h   |   1 +
 drivers/net/ice/ice_rxtx.c     |   5 +
 5 files changed, 139 insertions(+), 132 deletions(-)

--
2.43.0

[PATCH v3 1/4] net/ice: detect stopping a flow-director queue twice

[Bruce Richardson]

If the flow-director queue is stopped at some point during the running
of an application, the shutdown procedure for the port issues an error
as it tries to stop the queue a second time, and fails to do so. We can
eliminate this error by setting the tail-register pointer to NULL on
stop, and checking for that condition in subsequent stop calls. Since
the register pointer is set on start, any restarting of the queue will
allow a stop call to progress as normal.

Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index d2f9edc221..024d97cb46 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1139,6 +1139,10 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 			    tx_queue_id);
 		return -EINVAL;
 	}
+	if (txq->qtx_tail == NULL) {
+		PMD_DRV_LOG(INFO, "TX queue %u not started", tx_queue_id);
+		return 0;
+	}
 	vsi = txq->vsi;
 
 	q_ids[0] = txq->reg_idx;
@@ -1153,6 +1157,7 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	}
 
 	txq->tx_rel_mbufs(txq);
+	txq->qtx_tail = NULL;
 
 	return 0;
 }
-- 
2.43.0

[PATCH v3 2/4] net/ice: improve Tx scheduler graph output

[Bruce Richardson]

The function to dump the TX scheduler topology only adds to the chart
nodes connected to TX queues or for the flow director VSI. Change the
function to work recursively from the root node and thereby include all
scheduler nodes, whether in use or not, in the dump.

Also, improve the output of the Tx scheduler graphing function:

* Add VSI details to each node in graph
* When number of children is >16, skip middle nodes to reduce size of
  the graph, otherwise dot output is unviewable for large hierarchies
* For VSIs other than zero, use dot's clustering method to put those
  VSIs into subgraphs with borders
* For leaf nodes, display queue numbers for the any nodes assigned to
  ethdev NIC Tx queues

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_diagnose.c | 196 ++++++++++++---------------------
 1 file changed, 69 insertions(+), 127 deletions(-)

diff --git a/drivers/net/ice/ice_diagnose.c b/drivers/net/ice/ice_diagnose.c
index c357554707..623d84e37d 100644
--- a/drivers/net/ice/ice_diagnose.c
+++ b/drivers/net/ice/ice_diagnose.c
@@ -545,29 +545,15 @@ static void print_rl_profile(struct ice_aqc_rl_profile_elem *prof,
 	fprintf(stream, "\t\t\t\t\t</td>\n");
 }
 
-static
-void print_elem_type(FILE *stream, u8 type)
+static const char *
+get_elem_type(u8 type)
 {
-	switch (type) {
-	case 1:
-		fprintf(stream, "root");
-		break;
-	case 2:
-		fprintf(stream, "tc");
-		break;
-	case 3:
-		fprintf(stream, "se_generic");
-		break;
-	case 4:
-		fprintf(stream, "entry_point");
-		break;
-	case 5:
-		fprintf(stream, "leaf");
-		break;
-	default:
-		fprintf(stream, "%d", type);
-		break;
-	}
+	static const char * const ice_sched_node_types[] = {
+			"Undefined", "Root", "TC", "SE Generic", "SW Entry", "Leaf"
+	};
+	if (type < RTE_DIM(ice_sched_node_types))
+		return ice_sched_node_types[type];
+	return "*UNKNOWN*";
 }
 
 static
@@ -602,7 +588,9 @@ void print_priority_mode(FILE *stream, bool flag)
 }
 
 static
-void print_node(struct ice_aqc_txsched_elem_data *data,
+void print_node(struct ice_sched_node *node,
+		struct rte_eth_dev_data *ethdata,
+		struct ice_aqc_txsched_elem_data *data,
 		struct ice_aqc_rl_profile_elem *cir_prof,
 		struct ice_aqc_rl_profile_elem *eir_prof,
 		struct ice_aqc_rl_profile_elem *shared_prof,
@@ -613,17 +601,19 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 
 	fprintf(stream, "\t\t\t<table>\n");
 
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> teid </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td> %d </td>\n", data->node_teid);
-	fprintf(stream, "\t\t\t\t</tr>\n");
-
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> type </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td>");
-	print_elem_type(stream, data->data.elem_type);
-	fprintf(stream, "</td>\n");
-	fprintf(stream, "\t\t\t\t</tr>\n");
+	fprintf(stream, "\t\t\t\t<tr><td>teid</td><td>%d</td></tr>\n", data->node_teid);
+	fprintf(stream, "\t\t\t\t<tr><td>type</td><td>%s</td></tr>\n",
+			get_elem_type(data->data.elem_type));
+	fprintf(stream, "\t\t\t\t<tr><td>VSI</td><td>%u</td></tr>\n", node->vsi_handle);
+	if (data->data.elem_type == ICE_AQC_ELEM_TYPE_LEAF) {
+		for (uint16_t i = 0; i < ethdata->nb_tx_queues; i++) {
+			struct ice_tx_queue *q = ethdata->tx_queues[i];
+			if (q->q_teid == data->node_teid) {
+				fprintf(stream, "\t\t\t\t<tr><td>TXQ</td><td>%u</td></tr>\n", i);
+				break;
+			}
+		}
+	}
 
 	if (!detail)
 		goto brief;
@@ -705,8 +695,6 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 	fprintf(stream, "\t\tshape=plain\n");
 	fprintf(stream, "\t]\n");
 
-	if (data->parent_teid != 0xFFFFFFFF)
-		fprintf(stream, "\tNODE_%d -> NODE_%d\n", data->parent_teid, data->node_teid);
 }
 
 static
@@ -731,112 +719,92 @@ int query_rl_profile(struct ice_hw *hw,
 	return 0;
 }
 
-static
-int query_node(struct ice_hw *hw, uint32_t child, uint32_t *parent,
-	       uint8_t level, bool detail, FILE *stream)
+static int
+query_node(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	struct ice_aqc_txsched_elem_data data;
+	struct ice_aqc_txsched_elem_data *data = &node->info;
 	struct ice_aqc_rl_profile_elem cir_prof;
 	struct ice_aqc_rl_profile_elem eir_prof;
 	struct ice_aqc_rl_profile_elem shared_prof;
 	struct ice_aqc_rl_profile_elem *cp = NULL;
 	struct ice_aqc_rl_profile_elem *ep = NULL;
 	struct ice_aqc_rl_profile_elem *sp = NULL;
-	int status, ret;
-
-	status = ice_sched_query_elem(hw, child, &data);
-	if (status != ICE_SUCCESS) {
-		if (level == hw->num_tx_sched_layers) {
-			/* ignore the error when a queue has been stopped. */
-			PMD_DRV_LOG(WARNING, "Failed to query queue node %d.", child);
-			*parent = 0xffffffff;
-			return 0;
-		}
-		PMD_DRV_LOG(ERR, "Failed to query scheduling node %d.", child);
-		return -EINVAL;
-	}
-
-	*parent = data.parent_teid;
+	u8 level = node->tx_sched_layer;
+	int ret;
 
-	if (data.data.cir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 0, data.data.cir_bw.bw_profile_idx, &cir_prof);
+	if (data->data.cir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 0, data->data.cir_bw.bw_profile_idx, &cir_prof);
 
 		if (ret)
 			return ret;
 		cp = &cir_prof;
 	}
 
-	if (data.data.eir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 1, data.data.eir_bw.bw_profile_idx, &eir_prof);
+	if (data->data.eir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 1, data->data.eir_bw.bw_profile_idx, &eir_prof);
 
 		if (ret)
 			return ret;
 		ep = &eir_prof;
 	}
 
-	if (data.data.srl_id != 0) {
-		ret = query_rl_profile(hw, level, 2, data.data.srl_id, &shared_prof);
+	if (data->data.srl_id != 0) {
+		ret = query_rl_profile(hw, level, 2, data->data.srl_id, &shared_prof);
 
 		if (ret)
 			return ret;
 		sp = &shared_prof;
 	}
 
-	print_node(&data, cp, ep, sp, detail, stream);
+	print_node(node, ethdata, data, cp, ep, sp, detail, stream);
 
 	return 0;
 }
 
-static
-int query_nodes(struct ice_hw *hw,
-		uint32_t *children, int child_num,
-		uint32_t *parents, int *parent_num,
-		uint8_t level, bool detail,
-		FILE *stream)
+static int
+query_node_recursive(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	uint32_t parent;
-	int i;
-	int j;
-
-	*parent_num = 0;
-	for (i = 0; i < child_num; i++) {
-		bool exist = false;
-		int ret;
+	bool close = false;
+	if (node->parent != NULL && node->vsi_handle != node->parent->vsi_handle) {
+		fprintf(stream, "subgraph cluster_%u {\n", node->vsi_handle);
+		fprintf(stream, "\tlabel = \"VSI %u\";\n", node->vsi_handle);
+		close = true;
+	}
 
-		ret = query_node(hw, children[i], &parent, level, detail, stream);
-		if (ret)
-			return -EINVAL;
+	int ret = query_node(hw, ethdata, node, detail, stream);
+	if (ret != 0)
+		return ret;
 
-		for (j = 0; j < *parent_num; j++) {
-			if (parents[j] == parent) {
-				exist = true;
-				break;
-			}
+	for (uint16_t i = 0; i < node->num_children; i++) {
+		ret = query_node_recursive(hw, ethdata, node->children[i], detail, stream);
+		if (ret != 0)
+			return ret;
+		/* if we have a lot of nodes, skip a bunch in the middle */
+		if (node->num_children > 16 && i == 2) {
+			uint16_t inc = node->num_children - 5;
+			fprintf(stream, "\tn%d_children [label=\"... +%d child nodes ...\"];\n",
+					node->info.node_teid, inc);
+			fprintf(stream, "\tNODE_%d -> n%d_children;\n",
+					node->info.node_teid, node->info.node_teid);
+			i += inc;
 		}
-
-		if (!exist && parent != 0xFFFFFFFF)
-			parents[(*parent_num)++] = parent;
 	}
+	if (close)
+		fprintf(stream, "}\n");
+	if (node->info.parent_teid != 0xFFFFFFFF)
+		fprintf(stream, "\tNODE_%d -> NODE_%d\n",
+				node->info.parent_teid, node->info.node_teid);
 
 	return 0;
 }
 
-int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
+int
+rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 {
 	struct rte_eth_dev *dev;
 	struct ice_hw *hw;
-	struct ice_pf *pf;
-	struct ice_q_ctx *q_ctx;
-	uint16_t q_num;
-	uint16_t i;
-	struct ice_tx_queue *txq;
-	uint32_t buf1[256];
-	uint32_t buf2[256];
-	uint32_t *children = buf1;
-	uint32_t *parents = buf2;
-	int child_num = 0;
-	int parent_num = 0;
-	uint8_t level;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
@@ -846,35 +814,9 @@ int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 
 	dev = &rte_eth_devices[port];
 	hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	level = hw->num_tx_sched_layers;
-
-	q_num = dev->data->nb_tx_queues;
-
-	/* main vsi */
-	for (i = 0; i < q_num; i++) {
-		txq = dev->data->tx_queues[i];
-		q_ctx = ice_get_lan_q_ctx(hw, txq->vsi->idx, 0, i);
-		children[child_num++] = q_ctx->q_teid;
-	}
-
-	/* fdir vsi */
-	q_ctx = ice_get_lan_q_ctx(hw, pf->fdir.fdir_vsi->idx, 0, 0);
-	children[child_num++] = q_ctx->q_teid;
 
 	fprintf(stream, "digraph tx_sched {\n");
-	while (child_num > 0) {
-		int ret;
-		ret = query_nodes(hw, children, child_num,
-				  parents, &parent_num,
-				  level, detail, stream);
-		if (ret)
-			return ret;
-
-		children = parents;
-		child_num = parent_num;
-		level--;
-	}
+	query_node_recursive(hw, dev->data, hw->port_info->root, detail, stream);
 	fprintf(stream, "}\n");
 
 	return 0;
-- 
2.43.0

[PATCH v3 3/4] net/ice: add option to choose DDP package file

[Bruce Richardson]

The "Dynamic Device Personalization" package is loaded at initialization
time by the driver, but the specific package file loaded depends upon
what package file is found first by searching through a hard-coded list
of firmware paths. To enable greater control over the package loading,
we can add a device option to choose a specific DDP package file to
load.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/nics/ice.rst      |  9 +++++++++
 drivers/net/ice/ice_ethdev.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  1 +
 3 files changed, 45 insertions(+)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index 91b5a9b461..6c66dc8008 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -138,6 +138,15 @@ Runtime Configuration
 
     -a 80:00.0,default-mac-disable=1
 
+- ``DDP Package File``
+
+  Rather than have the driver search for the DDP package to load,
+  or to override what package is used,
+  the ``ddp_pkg_file`` option can be used to provide the path to a specific package file.
+  For example::
+
+    -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg
+
 - ``Protocol extraction for per queue``
 
   Configure the RX queues to do protocol extraction into mbuf for protocol
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 70298ac330..97092c8550 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -38,6 +38,7 @@
 #define ICE_ONE_PPS_OUT_ARG       "pps_out"
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
+#define ICE_DDP_FILENAME_ARG      "ddp_pkg_file"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -54,6 +55,7 @@ static const char * const ice_valid_args[] = {
 	ICE_RX_LOW_LATENCY_ARG,
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
+	ICE_DDP_FILENAME_ARG,
 	NULL
 };
 
@@ -696,6 +698,18 @@ handle_field_name_arg(__rte_unused const char *key, const char *value,
 	return 0;
 }
 
+static int
+handle_ddp_filename_arg(__rte_unused const char *key, const char *value, void *name_args)
+{
+	const char **filename = name_args;
+	if (strlen(value) >= ICE_MAX_PKG_FILENAME_SIZE) {
+		PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", value);
+		return -1;
+	}
+	*filename = strdup(value);
+	return 0;
+}
+
 static void
 ice_check_proto_xtr_support(struct ice_hw *hw)
 {
@@ -1912,6 +1926,17 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 	size_t bufsz;
 	int err;
 
+	/* first read any explicitly referenced DDP file*/
+	if (adapter->devargs.ddp_filename != NULL) {
+		strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
+		if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+			goto load_fw;
+		} else {
+			PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
+			return -1;
+		}
+	}
+
 	memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
 	snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
 		"ice-%016" PRIx64 ".pkg", dsn);
@@ -2259,6 +2284,13 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 
 	ret = rte_kvargs_process(kvlist, ICE_RX_LOW_LATENCY_ARG,
 				 &parse_bool, &ad->devargs.rx_low_latency);
+	if (ret)
+		goto bail;
+
+	ret = rte_kvargs_process(kvlist, ICE_DDP_FILENAME_ARG,
+				 &handle_ddp_filename_arg, &ad->devargs.ddp_filename);
+	if (ret)
+		goto bail;
 
 bail:
 	rte_kvargs_free(kvlist);
@@ -2762,6 +2794,8 @@ ice_dev_close(struct rte_eth_dev *dev)
 	ice_free_hw_tbls(hw);
 	rte_free(hw->port_info);
 	hw->port_info = NULL;
+	free((void *)(uintptr_t)ad->devargs.ddp_filename);
+	ad->devargs.ddp_filename = NULL;
 	ice_shutdown_all_ctrlq(hw, true);
 	rte_free(pf->proto_xtr);
 	pf->proto_xtr = NULL;
@@ -7135,6 +7169,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_FILENAME_ARG "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE);
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 57087c98ed..076cf595e8 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -569,6 +569,7 @@ struct ice_devargs {
 	/* Name of the field. */
 	char xtr_field_name[RTE_MBUF_DYN_NAMESIZE];
 	uint64_t mbuf_check;
+	const char *ddp_filename;
 };
 
 /**
-- 
2.43.0

[PATCH v3 4/4] net/ice: limit the number of queues to sched capabilities

[Bruce Richardson]

Rather than assuming that each VSI can hold up to 256 queue pairs,
or the reported device limit, query the available nodes in the scheduler
tree to check that we are not overflowing the limit for number of
child scheduling nodes at each level. Do this by multiplying
max_children for each level beyond the VSI and using that as an
additional cap on the number of queues.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_ethdev.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 97092c8550..d5e94a6685 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -915,7 +915,7 @@ ice_vsi_config_default_rss(struct ice_aqc_vsi_props *info)
 }
 
 static int
-ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
+ice_vsi_config_tc_queue_mapping(struct ice_hw *hw, struct ice_vsi *vsi,
 				struct ice_aqc_vsi_props *info,
 				uint8_t enabled_tcmap)
 {
@@ -931,13 +931,28 @@ ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
 	}
 
 	/* vector 0 is reserved and 1 vector for ctrl vsi */
-	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2)
+	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2) {
 		vsi->nb_qps = 0;
-	else
+	} else {
 		vsi->nb_qps = RTE_MIN
 			((uint16_t)vsi->adapter->hw.func_caps.common_cap.num_msix_vectors - 2,
 			RTE_MIN(vsi->nb_qps, ICE_MAX_Q_PER_TC));
 
+		/* cap max QPs to what the HW reports as num-children for each layer.
+		 * Multiply num_children for each layer from the entry_point layer to
+		 * the qgroup, or second-last layer.
+		 * Avoid any potential overflow by using uint32_t type and breaking loop
+		 * once we have a number greater than the already configured max.
+		 */
+		uint32_t max_sched_vsi_nodes = 1;
+		for (uint8_t i = hw->sw_entry_point_layer; i < hw->num_tx_sched_layers - 1; i++) {
+			max_sched_vsi_nodes *= hw->max_children[i];
+			if (max_sched_vsi_nodes >= vsi->nb_qps)
+				break;
+		}
+		vsi->nb_qps = RTE_MIN(vsi->nb_qps, max_sched_vsi_nodes);
+	}
+
 	/* nb_qps(hex)  -> fls */
 	/* 0000		-> 0 */
 	/* 0001		-> 0 */
@@ -1709,7 +1724,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 			rte_cpu_to_le_16(hw->func_caps.fd_fltr_best_effort);
 
 		/* Enable VLAN/UP trip */
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
@@ -1733,7 +1748,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 		vsi_ctx.info.fd_options = rte_cpu_to_le_16(cfg);
 		vsi_ctx.info.sw_id = hw->port_info->sw_id;
 		vsi_ctx.info.sw_flags2 = ICE_AQ_VSI_SW_FLAG_LAN_ENA;
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
-- 
2.43.0

Re: [PATCH v3 0/4] updates for net/ice driver

[Bruce Richardson]

On Fri, Oct 18, 2024 at 03:38:18PM +0100, Bruce Richardson wrote:
> This patchset contains a set of updates for the ice driver, a number of
> which are in the area of the "rte_tm" APIs for Tx scheduling.
> 
> These patches were previously submitted as part of a larger set[1], but
> separating them out here for easier review and merge ahead of the more
> substantial changes for scheduling in the last couple of patches in that
> set.
> 
> [1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*
> 
> ---
> v3: dropped patch 4 completely from set, it was missing doc updates and
>     it fits better with out sched changes. Will include in a later set
> v2: small update to patch 4 following review by Stephen.
> 

Recheck-request: rebase=main,iol-compile-amd64-testing,iol-compile-arm64-testing

[PATCH v4 0/4] updates for net/ice driver

[Bruce Richardson]

This patchset contains a set of updates for the ice driver, a number of
which are in the area of the "rte_tm" APIs for Tx scheduling.

These patches were previously submitted as part of a larger set[1], but
separating them out here for easier review and merge ahead of the more
substantial changes for scheduling in the last couple of patches in that
set.

[1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*

---
V4: removed VSI printing in patch 2 - the VSI is documented by the groups
    on the dot graph.
v3: dropped patch 4 completely from set, it was missing doc updates and
    it fits better with out sched changes. Will include in a later set
v2: small update to patch 4 following review by Stephen.


Bruce Richardson (4):
  net/ice: detect stopping a flow-director queue twice
  net/ice: improve Tx scheduler graph output
  net/ice: add option to choose DDP package file
  net/ice: limit the number of queues to sched capabilities

 doc/guides/nics/ice.rst        |   9 ++
 drivers/net/ice/ice_diagnose.c | 202 ++++++++++++---------------------
 drivers/net/ice/ice_ethdev.c   |  60 +++++++++-
 drivers/net/ice/ice_ethdev.h   |   1 +
 drivers/net/ice/ice_rxtx.c     |   5 +
 5 files changed, 141 insertions(+), 136 deletions(-)

--
2.43.0

[PATCH v4 1/4] net/ice: detect stopping a flow-director queue twice

[Bruce Richardson]

If the flow-director queue is stopped at some point during the running
of an application, the shutdown procedure for the port issues an error
as it tries to stop the queue a second time, and fails to do so. We can
eliminate this error by setting the tail-register pointer to NULL on
stop, and checking for that condition in subsequent stop calls. Since
the register pointer is set on start, any restarting of the queue will
allow a stop call to progress as normal.

Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index d2f9edc221..024d97cb46 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1139,6 +1139,10 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 			    tx_queue_id);
 		return -EINVAL;
 	}
+	if (txq->qtx_tail == NULL) {
+		PMD_DRV_LOG(INFO, "TX queue %u not started", tx_queue_id);
+		return 0;
+	}
 	vsi = txq->vsi;
 
 	q_ids[0] = txq->reg_idx;
@@ -1153,6 +1157,7 @@ ice_fdir_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id)
 	}
 
 	txq->tx_rel_mbufs(txq);
+	txq->qtx_tail = NULL;
 
 	return 0;
 }
-- 
2.43.0

[PATCH v4 2/4] net/ice: improve Tx scheduler graph output

[Bruce Richardson]

The function to dump the TX scheduler topology only adds to the chart
nodes connected to TX queues or for the flow director VSI. Change the
function to work recursively from the root node and thereby include all
scheduler nodes, whether in use or not, in the dump.

Also, improve the output of the Tx scheduler graphing function:

* When number of children is >16, skip middle nodes to reduce size of
  the graph, otherwise dot output is unviewable for large hierarchies
* For VSIs other than zero, use dot's clustering method to put those
  VSIs into subgraphs with borders
* For leaf nodes, display queue numbers for the any nodes assigned to
  ethdev NIC Tx queues

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_diagnose.c | 202 ++++++++++++---------------------
 1 file changed, 71 insertions(+), 131 deletions(-)

diff --git a/drivers/net/ice/ice_diagnose.c b/drivers/net/ice/ice_diagnose.c
index c357554707..5bec9d00ad 100644
--- a/drivers/net/ice/ice_diagnose.c
+++ b/drivers/net/ice/ice_diagnose.c
@@ -510,7 +510,7 @@ int rte_pmd_ice_dump_switch(uint16_t port, uint8_t **buff, uint32_t *size)
 	return ice_dump_switch(dev, buff, size);
 }

-static void print_rl_profile(struct ice_aqc_rl_profile_elem *prof,
+static void print_rl_profile(const struct ice_aqc_rl_profile_elem *prof,
 			     FILE *stream)
 {
 	fprintf(stream, "\t\t\t\t\t<td>\n");
@@ -545,29 +545,15 @@ static void print_rl_profile(struct ice_aqc_rl_profile_elem *prof,
 	fprintf(stream, "\t\t\t\t\t</td>\n");
 }

-static
-void print_elem_type(FILE *stream, u8 type)
+static const char *
+get_elem_type(u8 type)
 {
-	switch (type) {
-	case 1:
-		fprintf(stream, "root");
-		break;
-	case 2:
-		fprintf(stream, "tc");
-		break;
-	case 3:
-		fprintf(stream, "se_generic");
-		break;
-	case 4:
-		fprintf(stream, "entry_point");
-		break;
-	case 5:
-		fprintf(stream, "leaf");
-		break;
-	default:
-		fprintf(stream, "%d", type);
-		break;
-	}
+	static const char * const ice_sched_node_types[] = {
+			"Undefined", "Root", "TC", "SE Generic", "SW Entry", "Leaf"
+	};
+	if (type < RTE_DIM(ice_sched_node_types))
+		return ice_sched_node_types[type];
+	return "*UNKNOWN*";
 }

 static
@@ -602,10 +588,11 @@ void print_priority_mode(FILE *stream, bool flag)
 }

 static
-void print_node(struct ice_aqc_txsched_elem_data *data,
-		struct ice_aqc_rl_profile_elem *cir_prof,
-		struct ice_aqc_rl_profile_elem *eir_prof,
-		struct ice_aqc_rl_profile_elem *shared_prof,
+void print_node(const struct rte_eth_dev_data *ethdata,
+		const struct ice_aqc_txsched_elem_data *data,
+		const struct ice_aqc_rl_profile_elem *cir_prof,
+		const struct ice_aqc_rl_profile_elem *eir_prof,
+		const struct ice_aqc_rl_profile_elem *shared_prof,
 		bool detail, FILE *stream)
 {
 	fprintf(stream, "\tNODE_%d [\n", data->node_teid);
@@ -613,17 +600,18 @@ void print_node(struct ice_aqc_txsched_elem_data *data,

 	fprintf(stream, "\t\t\t<table>\n");

-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> teid </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td> %d </td>\n", data->node_teid);
-	fprintf(stream, "\t\t\t\t</tr>\n");
-
-	fprintf(stream, "\t\t\t\t<tr>\n");
-	fprintf(stream, "\t\t\t\t\t<td> type </td>\n");
-	fprintf(stream, "\t\t\t\t\t<td>");
-	print_elem_type(stream, data->data.elem_type);
-	fprintf(stream, "</td>\n");
-	fprintf(stream, "\t\t\t\t</tr>\n");
+	fprintf(stream, "\t\t\t\t<tr><td>teid</td><td>%d</td></tr>\n", data->node_teid);
+	fprintf(stream, "\t\t\t\t<tr><td>type</td><td>%s</td></tr>\n",
+			get_elem_type(data->data.elem_type));
+	if (data->data.elem_type == ICE_AQC_ELEM_TYPE_LEAF) {
+		for (uint16_t i = 0; i < ethdata->nb_tx_queues; i++) {
+			struct ice_tx_queue *q = ethdata->tx_queues[i];
+			if (q->q_teid == data->node_teid) {
+				fprintf(stream, "\t\t\t\t<tr><td>TXQ</td><td>%u</td></tr>\n", i);
+				break;
+			}
+		}
+	}

 	if (!detail)
 		goto brief;
@@ -705,8 +693,6 @@ void print_node(struct ice_aqc_txsched_elem_data *data,
 	fprintf(stream, "\t\tshape=plain\n");
 	fprintf(stream, "\t]\n");

-	if (data->parent_teid != 0xFFFFFFFF)
-		fprintf(stream, "\tNODE_%d -> NODE_%d\n", data->parent_teid, data->node_teid);
 }

 static
@@ -731,112 +717,92 @@ int query_rl_profile(struct ice_hw *hw,
 	return 0;
 }

-static
-int query_node(struct ice_hw *hw, uint32_t child, uint32_t *parent,
-	       uint8_t level, bool detail, FILE *stream)
+static int
+query_node(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	struct ice_aqc_txsched_elem_data data;
+	struct ice_aqc_txsched_elem_data *data = &node->info;
 	struct ice_aqc_rl_profile_elem cir_prof;
 	struct ice_aqc_rl_profile_elem eir_prof;
 	struct ice_aqc_rl_profile_elem shared_prof;
 	struct ice_aqc_rl_profile_elem *cp = NULL;
 	struct ice_aqc_rl_profile_elem *ep = NULL;
 	struct ice_aqc_rl_profile_elem *sp = NULL;
-	int status, ret;
-
-	status = ice_sched_query_elem(hw, child, &data);
-	if (status != ICE_SUCCESS) {
-		if (level == hw->num_tx_sched_layers) {
-			/* ignore the error when a queue has been stopped. */
-			PMD_DRV_LOG(WARNING, "Failed to query queue node %d.", child);
-			*parent = 0xffffffff;
-			return 0;
-		}
-		PMD_DRV_LOG(ERR, "Failed to query scheduling node %d.", child);
-		return -EINVAL;
-	}
-
-	*parent = data.parent_teid;
+	u8 level = node->tx_sched_layer;
+	int ret;

-	if (data.data.cir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 0, data.data.cir_bw.bw_profile_idx, &cir_prof);
+	if (data->data.cir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 0, data->data.cir_bw.bw_profile_idx, &cir_prof);

 		if (ret)
 			return ret;
 		cp = &cir_prof;
 	}

-	if (data.data.eir_bw.bw_profile_idx != 0) {
-		ret = query_rl_profile(hw, level, 1, data.data.eir_bw.bw_profile_idx, &eir_prof);
+	if (data->data.eir_bw.bw_profile_idx != 0) {
+		ret = query_rl_profile(hw, level, 1, data->data.eir_bw.bw_profile_idx, &eir_prof);

 		if (ret)
 			return ret;
 		ep = &eir_prof;
 	}

-	if (data.data.srl_id != 0) {
-		ret = query_rl_profile(hw, level, 2, data.data.srl_id, &shared_prof);
+	if (data->data.srl_id != 0) {
+		ret = query_rl_profile(hw, level, 2, data->data.srl_id, &shared_prof);

 		if (ret)
 			return ret;
 		sp = &shared_prof;
 	}

-	print_node(&data, cp, ep, sp, detail, stream);
+	print_node(ethdata, data, cp, ep, sp, detail, stream);

 	return 0;
 }

-static
-int query_nodes(struct ice_hw *hw,
-		uint32_t *children, int child_num,
-		uint32_t *parents, int *parent_num,
-		uint8_t level, bool detail,
-		FILE *stream)
+static int
+query_node_recursive(struct ice_hw *hw, struct rte_eth_dev_data *ethdata,
+		struct ice_sched_node *node, bool detail, FILE *stream)
 {
-	uint32_t parent;
-	int i;
-	int j;
-
-	*parent_num = 0;
-	for (i = 0; i < child_num; i++) {
-		bool exist = false;
-		int ret;
+	bool close = false;
+	if (node->parent != NULL && node->vsi_handle != node->parent->vsi_handle) {
+		fprintf(stream, "subgraph cluster_%u {\n", node->vsi_handle);
+		fprintf(stream, "\tlabel = \"VSI %u\";\n", node->vsi_handle);
+		close = true;
+	}

-		ret = query_node(hw, children[i], &parent, level, detail, stream);
-		if (ret)
-			return -EINVAL;
+	int ret = query_node(hw, ethdata, node, detail, stream);
+	if (ret != 0)
+		return ret;

-		for (j = 0; j < *parent_num; j++) {
-			if (parents[j] == parent) {
-				exist = true;
-				break;
-			}
+	for (uint16_t i = 0; i < node->num_children; i++) {
+		ret = query_node_recursive(hw, ethdata, node->children[i], detail, stream);
+		if (ret != 0)
+			return ret;
+		/* if we have a lot of nodes, skip a bunch in the middle */
+		if (node->num_children > 16 && i == 2) {
+			uint16_t inc = node->num_children - 5;
+			fprintf(stream, "\tn%d_children [label=\"... +%d child nodes ...\"];\n",
+					node->info.node_teid, inc);
+			fprintf(stream, "\tNODE_%d -> n%d_children;\n",
+					node->info.node_teid, node->info.node_teid);
+			i += inc;
 		}
-
-		if (!exist && parent != 0xFFFFFFFF)
-			parents[(*parent_num)++] = parent;
 	}
+	if (close)
+		fprintf(stream, "}\n");
+	if (node->info.parent_teid != 0xFFFFFFFF)
+		fprintf(stream, "\tNODE_%d -> NODE_%d\n",
+				node->info.parent_teid, node->info.node_teid);

 	return 0;
 }

-int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
+int
+rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)
 {
 	struct rte_eth_dev *dev;
 	struct ice_hw *hw;
-	struct ice_pf *pf;
-	struct ice_q_ctx *q_ctx;
-	uint16_t q_num;
-	uint16_t i;
-	struct ice_tx_queue *txq;
-	uint32_t buf1[256];
-	uint32_t buf2[256];
-	uint32_t *children = buf1;
-	uint32_t *parents = buf2;
-	int child_num = 0;
-	int parent_num = 0;
-	uint8_t level;

 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);

@@ -846,35 +812,9 @@ int rte_pmd_ice_dump_txsched(uint16_t port, bool detail, FILE *stream)

 	dev = &rte_eth_devices[port];
 	hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	level = hw->num_tx_sched_layers;
-
-	q_num = dev->data->nb_tx_queues;
-
-	/* main vsi */
-	for (i = 0; i < q_num; i++) {
-		txq = dev->data->tx_queues[i];
-		q_ctx = ice_get_lan_q_ctx(hw, txq->vsi->idx, 0, i);
-		children[child_num++] = q_ctx->q_teid;
-	}
-
-	/* fdir vsi */
-	q_ctx = ice_get_lan_q_ctx(hw, pf->fdir.fdir_vsi->idx, 0, 0);
-	children[child_num++] = q_ctx->q_teid;

 	fprintf(stream, "digraph tx_sched {\n");
-	while (child_num > 0) {
-		int ret;
-		ret = query_nodes(hw, children, child_num,
-				  parents, &parent_num,
-				  level, detail, stream);
-		if (ret)
-			return ret;
-
-		children = parents;
-		child_num = parent_num;
-		level--;
-	}
+	query_node_recursive(hw, dev->data, hw->port_info->root, detail, stream);
 	fprintf(stream, "}\n");

 	return 0;
--
2.43.0

[PATCH v4 3/4] net/ice: add option to choose DDP package file

[Bruce Richardson]

The "Dynamic Device Personalization" package is loaded at initialization
time by the driver, but the specific package file loaded depends upon
what package file is found first by searching through a hard-coded list
of firmware paths. To enable greater control over the package loading,
we can add a device option to choose a specific DDP package file to
load.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/nics/ice.rst      |  9 +++++++++
 drivers/net/ice/ice_ethdev.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  1 +
 3 files changed, 45 insertions(+)

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index 91b5a9b461..6c66dc8008 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -138,6 +138,15 @@ Runtime Configuration
 
     -a 80:00.0,default-mac-disable=1
 
+- ``DDP Package File``
+
+  Rather than have the driver search for the DDP package to load,
+  or to override what package is used,
+  the ``ddp_pkg_file`` option can be used to provide the path to a specific package file.
+  For example::
+
+    -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg
+
 - ``Protocol extraction for per queue``
 
   Configure the RX queues to do protocol extraction into mbuf for protocol
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 70298ac330..97092c8550 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -38,6 +38,7 @@
 #define ICE_ONE_PPS_OUT_ARG       "pps_out"
 #define ICE_RX_LOW_LATENCY_ARG    "rx_low_latency"
 #define ICE_MBUF_CHECK_ARG       "mbuf_check"
+#define ICE_DDP_FILENAME_ARG      "ddp_pkg_file"
 
 #define ICE_CYCLECOUNTER_MASK  0xffffffffffffffffULL
 
@@ -54,6 +55,7 @@ static const char * const ice_valid_args[] = {
 	ICE_RX_LOW_LATENCY_ARG,
 	ICE_DEFAULT_MAC_DISABLE,
 	ICE_MBUF_CHECK_ARG,
+	ICE_DDP_FILENAME_ARG,
 	NULL
 };
 
@@ -696,6 +698,18 @@ handle_field_name_arg(__rte_unused const char *key, const char *value,
 	return 0;
 }
 
+static int
+handle_ddp_filename_arg(__rte_unused const char *key, const char *value, void *name_args)
+{
+	const char **filename = name_args;
+	if (strlen(value) >= ICE_MAX_PKG_FILENAME_SIZE) {
+		PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", value);
+		return -1;
+	}
+	*filename = strdup(value);
+	return 0;
+}
+
 static void
 ice_check_proto_xtr_support(struct ice_hw *hw)
 {
@@ -1912,6 +1926,17 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 	size_t bufsz;
 	int err;
 
+	/* first read any explicitly referenced DDP file*/
+	if (adapter->devargs.ddp_filename != NULL) {
+		strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
+		if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+			goto load_fw;
+		} else {
+			PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
+			return -1;
+		}
+	}
+
 	memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
 	snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
 		"ice-%016" PRIx64 ".pkg", dsn);
@@ -2259,6 +2284,13 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
 
 	ret = rte_kvargs_process(kvlist, ICE_RX_LOW_LATENCY_ARG,
 				 &parse_bool, &ad->devargs.rx_low_latency);
+	if (ret)
+		goto bail;
+
+	ret = rte_kvargs_process(kvlist, ICE_DDP_FILENAME_ARG,
+				 &handle_ddp_filename_arg, &ad->devargs.ddp_filename);
+	if (ret)
+		goto bail;
 
 bail:
 	rte_kvargs_free(kvlist);
@@ -2762,6 +2794,8 @@ ice_dev_close(struct rte_eth_dev *dev)
 	ice_free_hw_tbls(hw);
 	rte_free(hw->port_info);
 	hw->port_info = NULL;
+	free((void *)(uintptr_t)ad->devargs.ddp_filename);
+	ad->devargs.ddp_filename = NULL;
 	ice_shutdown_all_ctrlq(hw, true);
 	rte_free(pf->proto_xtr);
 	pf->proto_xtr = NULL;
@@ -7135,6 +7169,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice,
 			      ICE_PROTO_XTR_ARG "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>"
 			      ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>"
 			      ICE_DEFAULT_MAC_DISABLE "=<0|1>"
+			      ICE_DDP_FILENAME_ARG "=</path/to/file>"
 			      ICE_RX_LOW_LATENCY_ARG "=<0|1>");
 
 RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE);
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 57087c98ed..076cf595e8 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -569,6 +569,7 @@ struct ice_devargs {
 	/* Name of the field. */
 	char xtr_field_name[RTE_MBUF_DYN_NAMESIZE];
 	uint64_t mbuf_check;
+	const char *ddp_filename;
 };
 
 /**
-- 
2.43.0

[PATCH v4 4/4] net/ice: limit the number of queues to sched capabilities

[Bruce Richardson]

Rather than assuming that each VSI can hold up to 256 queue pairs,
or the reported device limit, query the available nodes in the scheduler
tree to check that we are not overflowing the limit for number of
child scheduling nodes at each level. Do this by multiplying
max_children for each level beyond the VSI and using that as an
additional cap on the number of queues.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ice/ice_ethdev.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 97092c8550..d5e94a6685 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -915,7 +915,7 @@ ice_vsi_config_default_rss(struct ice_aqc_vsi_props *info)
 }

 static int
-ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
+ice_vsi_config_tc_queue_mapping(struct ice_hw *hw, struct ice_vsi *vsi,
 				struct ice_aqc_vsi_props *info,
 				uint8_t enabled_tcmap)
 {
@@ -931,13 +931,28 @@ ice_vsi_config_tc_queue_mapping(struct ice_vsi *vsi,
 	}

 	/* vector 0 is reserved and 1 vector for ctrl vsi */
-	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2)
+	if (vsi->adapter->hw.func_caps.common_cap.num_msix_vectors < 2) {
 		vsi->nb_qps = 0;
-	else
+	} else {
 		vsi->nb_qps = RTE_MIN
 			((uint16_t)vsi->adapter->hw.func_caps.common_cap.num_msix_vectors - 2,
 			RTE_MIN(vsi->nb_qps, ICE_MAX_Q_PER_TC));

+		/* cap max QPs to what the HW reports as num-children for each layer.
+		 * Multiply num_children for each layer from the entry_point layer to
+		 * the qgroup, or second-last layer.
+		 * Avoid any potential overflow by using uint32_t type and breaking loop
+		 * once we have a number greater than the already configured max.
+		 */
+		uint32_t max_sched_vsi_nodes = 1;
+		for (uint8_t i = hw->sw_entry_point_layer; i < hw->num_tx_sched_layers - 1; i++) {
+			max_sched_vsi_nodes *= hw->max_children[i];
+			if (max_sched_vsi_nodes >= vsi->nb_qps)
+				break;
+		}
+		vsi->nb_qps = RTE_MIN(vsi->nb_qps, max_sched_vsi_nodes);
+	}
+
 	/* nb_qps(hex)  -> fls */
 	/* 0000		-> 0 */
 	/* 0001		-> 0 */
@@ -1709,7 +1724,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 			rte_cpu_to_le_16(hw->func_caps.fd_fltr_best_effort);

 		/* Enable VLAN/UP trip */
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
@@ -1733,7 +1748,7 @@ ice_setup_vsi(struct ice_pf *pf, enum ice_vsi_type type)
 		vsi_ctx.info.fd_options = rte_cpu_to_le_16(cfg);
 		vsi_ctx.info.sw_id = hw->port_info->sw_id;
 		vsi_ctx.info.sw_flags2 = ICE_AQ_VSI_SW_FLAG_LAN_ENA;
-		ret = ice_vsi_config_tc_queue_mapping(vsi,
+		ret = ice_vsi_config_tc_queue_mapping(hw, vsi,
 						      &vsi_ctx.info,
 						      ICE_DEFAULT_TCMAP);
 		if (ret) {
--
2.43.0

Re: [PATCH v4 0/4] updates for net/ice driver

[Bruce Richardson]

On Tue, Oct 22, 2024 at 05:39:40PM +0100, Bruce Richardson wrote:
> This patchset contains a set of updates for the ice driver, a number of
> which are in the area of the "rte_tm" APIs for Tx scheduling.
> 
> These patches were previously submitted as part of a larger set[1], but
> separating them out here for easier review and merge ahead of the more
> substantial changes for scheduling in the last couple of patches in that
> set.
> 
> [1] https://patches.dpdk.org/project/dpdk/list/?series=32758&state=*
> 
> ---
> V4: removed VSI printing in patch 2 - the VSI is documented by the groups
>     on the dot graph.
> v3: dropped patch 4 completely from set, it was missing doc updates and
>     it fits better with out sched changes. Will include in a later set
> v2: small update to patch 4 following review by Stephen.
> 
> 
Applied to dpdk-next-net-intel

Regards,
/Bruce