[v3.31] fix(QoS): Use QdiscReplace() instead of QdiscAdd()#11972
Merged
coutinhop merged 2 commits intoMar 4, 2026
Merged
Conversation
Use QdiscReplace() instead of QdiscAdd() so that adding the TBF qdiscs needed for QoS controls with tc does not error out when there is an existing non-default (handle != 0) qdisc on the interface for any reason. Add a test case to the felix FVs to cover this. Also, enable felix debug logging on the QoS felix FVs, and remove overzealous Skip() that was resulting in no test cases running on iptables/nftables modes.
coutinhop
added
release-note-required
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Felix’s Linux dataplane QoS qdisc programming so ingress/egress bandwidth limits can be applied even when the workload interface already has a non-default root qdisc handle, and extends FV coverage to exercise that scenario.
Changes:
- Switch ingress TBF programming to a create-or-update path that uses
netlink.QdiscReplace()(instead of add/update split paths). - Use
netlink.QdiscReplace()when creating the ingress qdisc used for IFB redirection (egress shaping path). - Update QoS FV tests to (a) always run with Felix debug logging, (b) remove a skip that was preventing coverage in iptables/nft modes, and (c) add a new case covering “pre-existing non-default qdisc handle”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
felix/fv/qos_controls_test.go |
Adjusts QoS FV setup/logging and adds an FV case for non-default existing qdisc handles. |
felix/dataplane/linux/qos_controls.go |
Routes ingress shaping add/update to a new unified create-or-update API. |
felix/dataplane/linux/qos/qos.go |
Implements create-or-update ingress qdisc and switches relevant qdisc operations to QdiscReplace(). |
nelljerram
approved these changes
Mar 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick history
Use QdiscReplace() instead of QdiscAdd() so that adding the TBF qdiscs needed for QoS controls with tc does not error out when there is an existing non-default (handle != 0) qdisc on the interface for any reason.
Add a test case to the felix FVs to cover this.
Also, enable felix debug logging on the QoS felix FVs, and remove overzealous Skip() that was resulting in no test cases running on iptables/nftables modes.
Description
Related issues/PRs
Release Note