Commit a22b7388 authored by Rahul Rameshbabu's avatar Rahul Rameshbabu Committed by Jakub Kicinski
Browse files

sch_htb: Avoid grafting on htb_destroy_class_offload when destroying htb



Peek at old qdisc and graft only when deleting a leaf class in the htb,
rather than when deleting the htb itself. Do not peek at the qdisc of the
netdev queue when destroying the htb. The caller may already have grafted a
new qdisc that is not part of the htb structure being destroyed.

This fix resolves two use cases.

  1. Using tc to destroy the htb.
    - Netdev was being prematurely activated before the htb was fully
      destroyed.
  2. Using tc to replace the htb with another qdisc (which also leads to
     the htb being destroyed).
    - Premature netdev activation like previous case. Newly grafted qdisc
      was also getting accidentally overwritten when destroying the htb.

Fixes: d03b195b ("sch_htb: Hierarchical QoS hardware offload")
Signed-off-by: default avatarRahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
Reviewed-by: default avatarMaxim Mikityanskiy <maxtram95@gmail.com>
Reviewed-by: default avatarJiri Pirko <jiri@nvidia.com>
Link: https://lore.kernel.org/r/20230113005528.302625-1-rrameshbabu@nvidia.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent da263fcb
Loading
Loading
Loading
Loading
+16 −11
Original line number Diff line number Diff line
@@ -1549,7 +1549,7 @@ static int htb_destroy_class_offload(struct Qdisc *sch, struct htb_class *cl,
	struct tc_htb_qopt_offload offload_opt;
	struct netdev_queue *dev_queue;
	struct Qdisc *q = cl->leaf.q;
	struct Qdisc *old = NULL;
	struct Qdisc *old;
	int err;

	if (cl->level)
@@ -1557,14 +1557,17 @@ static int htb_destroy_class_offload(struct Qdisc *sch, struct htb_class *cl,

	WARN_ON(!q);
	dev_queue = htb_offload_get_queue(cl);
	/* When destroying, caller qdisc_graft grafts the new qdisc and invokes
	 * qdisc_put for the qdisc being destroyed. htb_destroy_class_offload
	 * does not need to graft or qdisc_put the qdisc being destroyed.
	 */
	if (!destroying) {
		old = htb_graft_helper(dev_queue, NULL);
	if (destroying)
		/* Before HTB is destroyed, the kernel grafts noop_qdisc to
		 * all queues.
		/* Last qdisc grafted should be the same as cl->leaf.q when
		 * calling htb_delete.
		 */
		WARN_ON(!(old->flags & TCQ_F_BUILTIN));
	else
		WARN_ON(old != q);
	}

	if (cl->parent) {
		_bstats_update(&cl->parent->bstats_bias,
@@ -1581,10 +1584,12 @@ static int htb_destroy_class_offload(struct Qdisc *sch, struct htb_class *cl,
	};
	err = htb_offload(qdisc_dev(sch), &offload_opt);

	if (!err || destroying)
	if (!destroying) {
		if (!err)
			qdisc_put(old);
		else
			htb_graft_helper(dev_queue, old);
	}

	if (last_child)
		return err;