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
  • Host-to-switch delay / rate / loss are applied on the switch, not the node NIC, so the DUT keeps its root qdisc.
  • Switch to: N is traffic toward that node (node RX, switch egress). Switch from: N is traffic from that node (node TX, IFB on switch ingress). Use two entries for both directions.
  • A node’s own delay still means that node’s TX (same pipe as from:). Network-root constraints still default to to: (RX). P2P is unchanged; warn if TC stays on the node.

Example Config

networks:
  - name: wan1
    connections:
      - to: r1
        delay: 2500
        rate: { rate: "15M", burst: 16384, limit: 131072 }
      - from: r1
        delay: 2500
        rate: { rate: "15M", burst: 16384, limit: 131072 }

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.

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Keep TX and RX on the switch so the DUT keeps its qdisc. Switch
to: is node RX; from: is node TX (IFB). Schema is generated from
the YANG in README.org.

Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Switch tests were sharing the module munet with leftover sw1-e0/eth0.
Node delay/rate/loss now live on the switch IFB, not the node NIC.

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
| | | +--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

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