Skip to content

Move bridged TC off the node NIC so the DUT can own its qdisc - #101

Open
Jafaral wants to merge 3 commits into
LabNConsulting:mainfrom
Jafaral:switch-side-tc
Open

Jafaral wants to merge 3 commits into
LabNConsulting:mainfrom
Jafaral:switch-side-tc

Conversation

@Jafaral

@Jafaral Jafaral commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Move bridged TC off the node NIC and add directional and per-peer constraints

Host-to-switch delay/rate/loss used to be installed as the node NIC's root qdisc, replacing whatever the DUT configured. The switch now shapes both directions on its port: netem/tbf on egress for traffic toward the node, and an IFB on ingress for traffic from the node. P2P links keep TC on the node interface.

Constraints can be wrapped in transmit and receive containers, relative to the owner of the config. On a network, transmit is traffic toward the node and receive is traffic from it. On a node connection, transmit is what the node sends and receive is what it receives. Unwrapped constraints still work, are deprecated, and mean transmit. Where network and node values overlap, the node's value wins. For P2P links, a node's receive is applied on the peer's egress.

A network connection's transmit can also list peers, giving traffic from a specific sender toward that node its own constraints. Each port marks incoming packets per sender, and the receiving port classifies on that mark into per-peer drr classes, so this works for VMs and broadcast too.

The YANG, README tree and JSON schema are updated, and there are tests for each case.

Example:

networks:
  - name: radio
    connections:
      - to: r1
        transmit: { delay: 2000 }    # radio -> r1
        receive:  { delay: 2000 }    # r1 -> radio
      - to: r2
        transmit:
          delay: 5000                # anyone -> r2
          peers:
            - peer: r1               # r1 -> r2 only
              delay: 30000
              loss: 5
nodes:
  - name: r1
    connections: [ { to: radio } ]
  - name: r2
    connections:
      - to: radio
        receive: { loss: 1 }         # r2 RX, applied on the switch port

@Jafaral
Jafaral force-pushed the switch-side-tc branch 2 times, most recently from 391de4a to 09e7ace Compare September 6, 2026 16:10
Host-to-switch netem/tbf was installed on the node NIC and replaced
the DUT's root qdisc. Keep the old TX meaning: node attachment
constraints shape that node's outbound via an IFB on the switch
veth ingress. Switch-attachment constraints still shape switch
outbound. P2P stays on the node; warn instead of inventing a switch.

Switch TC tests use a per-test munet so they do not share leftover
interfaces with the module munet.

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>

@liambrady liambrady left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your changes to TC with the usage of IFBs. I think we need to revisit the to/from config syntax before that portion gets merged, though.

Comment thread munet/native.py Outdated
# Both veth ends live inside the nodes. There is no outside
# device to own the "link" unless we insert a mid-netns.
if "physical" not in c1 and not node1.is_vm:
if Munet._tc_constraints(c1):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the usage of _tc_constraints() differ in the host-to-host and host-to-switch cases? Is merging required only in host-to-switch case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. I see that the behavior is corrected in the next commit. It is probably worth cleaning this up to ensure the history clean.

Comment thread README.org Outdated
| | | +--rw burst? number64
| | +--rw connections* [to]
| | +--rw to string
| | +--rw from? string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I like the idea of configuring egress/ingress link metrics from the same network, I don't think that the definition of a new from leaf is the way to go. Specifically, I would like to strictly adhere to standard YANG for new config, however, your usage of to and from as mutually exclusive key values cannot (to my knowledge) be modelled in YANG.

Furthermore, I don't see how the from can be extended to the config of ingress TC metrics for the typical node connections (which would be a suitable follow up feature). The presence of further config (e.g. hostintf, physical, ip, etc.) wouldn't quite make sense in the from context. (e.g. Why would I be configuring node A's interface from the context of node B?).

I think it makes more sense to wrap the TC metrics in new transmit/receive containers, to specify whether the metrics apply to local egress or ingress traffic. That would make both the direction of the rules clear and prevent conflicts with the usage of to as a key. (Note: the usage of the current TC metrics without the transmit/receive container would continue to have to be supported but should be labelled as deprecated under this approach.)

For example, my vision is:

|  |  +--rw connections* [to]
|  |     +--rw to                    string
|  |     +--rw name?                 string
|  |     +--rw remote-name?          string
|  |     +--rw transmit
|  |     | +--rw delay?                uint64
|  |     | +--rw jitter?               uint64
|  |     | +--rw jitter-correlation?   decimal64
|  |     | +--rw loss?                 uint64
|  |     | +--rw loss-correlation?     decimal64
|  |     | +--rw rate
|  |     |    +--rw rate?    number64
|  |     |    +--rw limit?   number64
|  |     |    +--rw burst?   number64
|  |     +--rw receive
|  |       +--rw delay?                uint64
|  |       +--rw jitter?               uint64
|  |       +--rw jitter-correlation?   decimal64
|  |       +--rw loss?                 uint64
|  |       +--rw loss-correlation?     decimal64
|  |       +--rw rate
|  |          +--rw rate?    number64
|  |          +--rw limit?   number64
|  |          +--rw burst?   number64

Wrap TC constraints in optional "transmit" and "receive" containers,
relative to the owner of the config: the network for network config,
the node for node connections. Unwrapped constraints are deprecated
and keep meaning transmit.

Host-to-switch links shape both directions on the switch port so the
DUT keeps its root qdisc: network transmit and node receive toward the
node (switch egress), network receive and node transmit from the node
(IFB). Node values win. For p2p, a node's receive is applied on the
peer's egress. Schema is generated from the YANG in README.org.

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
A network connection's transmit can now list peers, giving traffic
from a specific sending node toward that connection's node its own
delay/rate/loss. This allows different link quality for each ordered
pair of nodes on the same network.

Each port of a network that uses peers marks arriving packets with a
per-node skb mark. The receiving node's port uses a drr qdisc with one
class per peer, selected by fw filter on that mark, and a default class
for everything else. Classifying by ingress port rather than MAC keeps
this working for VMs, MAC changes, and broadcast/multicast.

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
@Jafaral
Jafaral force-pushed the switch-side-tc branch 2 times, most recently from e16b334 to 19bb492 Compare October 29, 2026 17:00
@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.97980% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.02%. Comparing base (e7ed4d5) to head (19bb492).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
munet/base.py 95.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   60.55%   61.02%   +0.47%     
==========================================
  Files          19       19              
  Lines        5990     6076      +86     
==========================================
+ Hits         3627     3708      +81     
- Misses       2363     2368       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants