diff mbox series

[net-next] octeontx2-pf: Reuse Transmit queue/Send queue index of HTB class

Message ID 20240508070935.11501-1-hkelam@marvell.com (mailing list archive)
State Accepted
Commit 04fb71cc5f1866f391a9c21ea634217e065ae525
Delegated to: Netdev Maintainers
Headers show
Series [net-next] octeontx2-pf: Reuse Transmit queue/Send queue index of HTB class | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 937 this patch: 937
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 110 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-10--18-00 (tests: 1014)

Commit Message

Hariprasad Kelam May 8, 2024, 7:09 a.m. UTC
Real number of Transmit queues are incremented when user enables HTB
class and vice versa. Depending on SKB priority driver returns transmit
queue (Txq). Transmit queues and Send queues are one-to-one mapped.

In few scenarios, Driver is returning transmit queue value which is
greater than real number of transmit queue and Stack detects this as
error and overwrites transmit queue value.

For example
user has added two classes and real number of queues are incremented
accordingly
- tc class add dev eth1 parent 1: classid 1:1 htb
      rate 100Mbit ceil 100Mbit prio 1 quantum 1024
- tc class add dev eth1 parent 1: classid 1:2 htb
      rate 100Mbit ceil 200Mbit prio 7 quantum 1024

now if user deletes the class with id 1:1, driver decrements the real
number of queues
- tc class del dev eth1 classid 1:1

But for the class with id 1:2, driver is returning transmit queue
value which is higher than real number of transmit queue leading
to below error

eth1 selects TX queue x, but real number of TX queues is x

This patch solves the problem by assigning deleted class transmit
queue/send queue to active class.

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/nic/qos.c  | 80 ++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

Comments

Markus Elfring May 8, 2024, 8:32 a.m. UTC | #1
> This patch solves the problem by assigning deleted class transmit
> queue/send queue to active class.

* How do you think about to add the tag “Fixes”?

* Would you like to use imperative wordings for an improved change description?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9-rc7#n94

Regards,
Markus
Hariprasad Kelam May 8, 2024, 9:49 a.m. UTC | #2
> …
> > This patch solves the problem by assigning deleted class transmit
> > queue/send queue to active class.
> 
> * How do you think about to add the tag “Fixes”?
> 
      This patch is targeted to "net-next" ,reason being it implements a new way of assigning transmit queues to the HTB classes. 
Though the commit description mentions solving a problem, but the implementation went beyond a simple fix.

Thanks,
Hariprasad k 

> * Would you like to use imperative wordings for an improved change
> description?
>   https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_Docum
> entation_process_submitting-2Dpatches.rst-3Fh-3Dv6.9-2Drc7-
> 23n94&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=2bd4kP44ECYFgf-
> KoNSJWqEipEtpxXnNBKy0vyoJJ8A&m=MOvcAghhRCElPpsRn5uxNJhBQZu4Fl3-
> ddvU7WhIbz5A7suWUSas__inrZk8Mf__&s=Cq1cWWlB5WUn1OnoW8aG_ghi
> w7xXjNVqh2DsIaumLRU&e=
> 
> Regards,
> Markus
Simon Horman May 10, 2024, 10:39 a.m. UTC | #3
On Wed, May 08, 2024 at 12:39:35PM +0530, Hariprasad Kelam wrote:
> Real number of Transmit queues are incremented when user enables HTB
> class and vice versa. Depending on SKB priority driver returns transmit
> queue (Txq). Transmit queues and Send queues are one-to-one mapped.
> 
> In few scenarios, Driver is returning transmit queue value which is
> greater than real number of transmit queue and Stack detects this as
> error and overwrites transmit queue value.
> 
> For example
> user has added two classes and real number of queues are incremented
> accordingly
> - tc class add dev eth1 parent 1: classid 1:1 htb
>       rate 100Mbit ceil 100Mbit prio 1 quantum 1024
> - tc class add dev eth1 parent 1: classid 1:2 htb
>       rate 100Mbit ceil 200Mbit prio 7 quantum 1024
> 
> now if user deletes the class with id 1:1, driver decrements the real
> number of queues
> - tc class del dev eth1 classid 1:1
> 
> But for the class with id 1:2, driver is returning transmit queue
> value which is higher than real number of transmit queue leading
> to below error
> 
> eth1 selects TX queue x, but real number of TX queues is x
> 
> This patch solves the problem by assigning deleted class transmit
> queue/send queue to active class.
> 
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>

Reviewed-by: Simon Horman <horms@kernel.org>
patchwork-bot+netdevbpf@kernel.org May 11, 2024, 2 a.m. UTC | #4
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 8 May 2024 12:39:35 +0530 you wrote:
> Real number of Transmit queues are incremented when user enables HTB
> class and vice versa. Depending on SKB priority driver returns transmit
> queue (Txq). Transmit queues and Send queues are one-to-one mapped.
> 
> In few scenarios, Driver is returning transmit queue value which is
> greater than real number of transmit queue and Stack detects this as
> error and overwrites transmit queue value.
> 
> [...]

Here is the summary with links:
  - [net-next] octeontx2-pf: Reuse Transmit queue/Send queue index of HTB class
    https://git.kernel.org/netdev/net-next/c/04fb71cc5f18

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
index 1723e9912ae0..070711df612e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
@@ -545,6 +545,20 @@  otx2_qos_sw_create_leaf_node(struct otx2_nic *pfvf,
 	return node;
 }
 
+static struct otx2_qos_node
+*otx2_sw_node_find_by_qid(struct otx2_nic *pfvf, u16 qid)
+{
+	struct otx2_qos_node *node = NULL;
+	int bkt;
+
+	hash_for_each(pfvf->qos.qos_hlist, bkt, node, hlist) {
+		if (node->qid == qid)
+			break;
+	}
+
+	return node;
+}
+
 static struct otx2_qos_node *
 otx2_sw_node_find(struct otx2_nic *pfvf, u32 classid)
 {
@@ -917,6 +931,7 @@  static void otx2_qos_enadis_sq(struct otx2_nic *pfvf,
 		otx2_qos_disable_sq(pfvf, qid);
 
 	pfvf->qos.qid_to_sqmap[qid] = node->schq;
+	otx2_qos_txschq_config(pfvf, node);
 	otx2_qos_enable_sq(pfvf, qid);
 }
 
@@ -1475,13 +1490,45 @@  static int otx2_qos_leaf_to_inner(struct otx2_nic *pfvf, u16 classid,
 	return ret;
 }
 
+static int otx2_qos_cur_leaf_nodes(struct otx2_nic *pfvf)
+{
+	int last = find_last_bit(pfvf->qos.qos_sq_bmap, pfvf->hw.tc_tx_queues);
+
+	return last ==  pfvf->hw.tc_tx_queues ? 0 : last + 1;
+}
+
+static void otx2_reset_qdisc(struct net_device *dev, u16 qid)
+{
+	struct netdev_queue *dev_queue = netdev_get_tx_queue(dev, qid);
+	struct Qdisc *qdisc = rtnl_dereference(dev_queue->qdisc_sleeping);
+
+	if (!qdisc)
+		return;
+
+	spin_lock_bh(qdisc_lock(qdisc));
+	qdisc_reset(qdisc);
+	spin_unlock_bh(qdisc_lock(qdisc));
+}
+
+static void otx2_cfg_smq(struct otx2_nic *pfvf, struct otx2_qos_node *node,
+			 int qid)
+{
+	struct otx2_qos_node *tmp;
+
+	list_for_each_entry(tmp, &node->child_schq_list, list)
+		if (tmp->level == NIX_TXSCH_LVL_MDQ) {
+			otx2_qos_txschq_config(pfvf, tmp);
+			pfvf->qos.qid_to_sqmap[qid] = tmp->schq;
+		}
+}
+
 static int otx2_qos_leaf_del(struct otx2_nic *pfvf, u16 *classid,
 			     struct netlink_ext_ack *extack)
 {
 	struct otx2_qos_node *node, *parent;
 	int dwrr_del_node = false;
+	u16 qid, moved_qid;
 	u64 prio;
-	u16 qid;
 
 	netdev_dbg(pfvf->netdev, "TC_HTB_LEAF_DEL classid %04x\n", *classid);
 
@@ -1517,6 +1564,37 @@  static int otx2_qos_leaf_del(struct otx2_nic *pfvf, u16 *classid,
 	if (!parent->child_static_cnt)
 		parent->max_static_prio = 0;
 
+	moved_qid = otx2_qos_cur_leaf_nodes(pfvf);
+
+	/* last node just deleted */
+	if (moved_qid == 0 || moved_qid == qid)
+		return 0;
+
+	moved_qid--;
+
+	node = otx2_sw_node_find_by_qid(pfvf, moved_qid);
+	if (!node)
+		return 0;
+
+	/* stop traffic to the old queue and disable
+	 * SQ associated with it
+	 */
+	node->qid =  OTX2_QOS_QID_INNER;
+	__clear_bit(moved_qid, pfvf->qos.qos_sq_bmap);
+	otx2_qos_disable_sq(pfvf, moved_qid);
+
+	otx2_reset_qdisc(pfvf->netdev, pfvf->hw.tx_queues + moved_qid);
+
+	/* enable SQ associated with qid and
+	 * update the node
+	 */
+	otx2_cfg_smq(pfvf, node, qid);
+
+	otx2_qos_enable_sq(pfvf, qid);
+	__set_bit(qid, pfvf->qos.qos_sq_bmap);
+	node->qid = qid;
+
+	*classid = node->classid;
 	return 0;
 }