Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jafaral wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
999f9ad
munet: apply bridged node delay/rate/loss on a switch IFB
Jafaral Sep 6, 2026
d2257f6
munet: shape both directions with switch to:/from:
Jafaral Sep 6, 2026
758b1e0
test: isolate switch TC cases and expect node delay on IFB
Jafaral Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions README.org
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ module: labn-munet-config
| | | +--rw burst? number64
| | +--rw connections* [to]
| | +--rw to string
| | +--rw from? string

@liambrady liambrady Sep 15, 2026

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

| | +--rw name? string
| | +--rw remote-name? string
| | +--rw delay? uint64
Expand Down Expand Up @@ -1154,14 +1155,19 @@ munet>
list connections {
key to;
description
"Overriding network side configuration for connections to nodes from
this network. If the default network tc parameters are not being
overriden for a specific node connection then this configuration is
not required.";
"Per-node one-way constraints on this switch. to: N is traffic
toward that node (node RX). from: N is traffic from that node
(node TX). Use two entries for both directions.";

leaf to {
type string;
description "The target of this connection.";
description
"Shape traffic toward this node (switch egress, node RX).";
}
leaf from {
type string;
description
"Shape traffic from this node (switch ingress, node TX).";
}
leaf name {
type string;
Expand Down
55 changes: 42 additions & 13 deletions munet/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,24 @@ def get_number(c, v, d=None):

return netem_args, tbf_args

def _add_tc_qdiscs(self, nsifname, constraints):
netem_args, tbf_args = self.get_linux_tc_args(nsifname, constraints)
if not netem_args and not tbf_args:
return False
count = 1
selector = f"root handle {count}:"
if netem_args:
self.cmd_raises(
f"tc qdisc add dev {nsifname} {selector} netem {netem_args}"
)
count += 1
selector = f"parent {count-1}: handle {count}"
# Place rate limit after delay otherwise limit/burst too complex
if tbf_args:
self.cmd_raises(f"tc qdisc add dev {nsifname} {selector} tbf {tbf_args}")
self.cmd_raises(f"tc qdisc show dev {nsifname}")
return True

def set_intf_constraints(self, ifname, **constraints):
"""Set interface outbound constraints.

Expand All @@ -1801,21 +1819,32 @@ def set_intf_constraints(self, ifname, **constraints):
rate (int): bits per second, string allows for use of
{KMGTKiMiGiTi} prefixes "i" means K == 1024 otherwise K == 1000.
"""
self._add_tc_qdiscs(self.get_ns_ifname(ifname), constraints)

def set_intf_ingress_constraints(self, ifname, **constraints):
"""Shape packets arriving on ifname via an IFB in this namespace.

Used on a switch port so a node's delay/rate/loss still apply to that
node's outbound traffic (same as a qdisc on the node NIC) without
taking the node's root qdisc.
"""
nsifname = self.get_ns_ifname(ifname)
netem_args, tbf_args = self.get_linux_tc_args(nsifname, constraints)
count = 1
selector = f"root handle {count}:"
if netem_args:
self.cmd_raises(
f"tc qdisc add dev {nsifname} {selector} netem {netem_args}"
)
count += 1
selector = f"parent {count-1}: handle {count}"
# Place rate limit after delay otherwise limit/burst too complex
if tbf_args:
self.cmd_raises(f"tc qdisc add dev {nsifname} {selector} tbf {tbf_args}")

self.cmd_raises(f"tc qdisc show dev {nsifname}")
if not netem_args and not tbf_args:
return
ifb = "ifb" + re.sub(r"[^A-Za-z0-9]", "", nsifname)
ifb = ifb[:15] or "ifb0"
# Modules are host-global. The munet mount ns often has no
# /lib/modules, so modprobe inside it fails even when ifb is loaded.
commander.cmd_status("modprobe ifb", warn=False)
self.cmd_raises(f"ip link add {ifb} type ifb")
self.cmd_raises(f"ip link set {ifb} up")
self.cmd_raises(f"tc qdisc add dev {nsifname} handle ffff: ingress")
self.cmd_raises(
f"tc filter add dev {nsifname} parent ffff: protocol all "
f"u32 match u32 0 0 action mirred egress redirect dev {ifb}"
)
self._add_tc_qdiscs(ifb, constraints)


class LinuxNamespace(Commander, InterfaceMixin):
Expand Down
8 changes: 6 additions & 2 deletions munet/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ def find_all_with_kv(lst, k, v):
return rv


def find_matching_net_config(name, cconf, oconf):
p = find_all_with_kv(oconf.get("connections", {}), "to", name)
def find_matching_net_config(name, cconf, oconf, direction="to"):
"""Return the peer's connection entry facing this node.

direction is "to" (toward the node) or "from" (from the node).
"""
p = find_all_with_kv(oconf.get("connections", {}), direction, name)
if not p:
return {}

Expand Down
3 changes: 3 additions & 0 deletions munet/munet-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,9 @@
"to": {
"type": "string"
},
"from": {
"type": "string"
},
"name": {
"type": "string"
},
Expand Down
102 changes: 89 additions & 13 deletions munet/native.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3282,15 +3282,26 @@ async def _async_build(self, logger=None):
# default tc values for interfaces added to the bridge which aren't
# present in `connections`.
switch = self.switches[to]
swconf = find_matching_net_config(name, cconf, switch.config)
if not swconf:
sw_to = find_matching_net_config(
name, cconf, switch.config, "to"
)
sw_from = find_matching_net_config(
name, cconf, switch.config, "from"
)
if not sw_to:
# "name" most important key to leave out, so it gets generated
nontc = ("connections", "external", "ip", "ipv6", "name")
swconf = {
k: v for k, v in switch.config.items() if k not in nontc
nontc = (
"connections", "external", "ip", "ipv6", "name",
)
sw_to = {
k: v
for k, v in switch.config.items()
if k not in nontc
}
swconf = deepcopy(swconf)
await self.add_native_link(switch, node, swconf, cconf)
sw_to = deepcopy(sw_to)
await self.add_native_link(
switch, node, sw_to, cconf, c1_from=sw_from
)
elif cconf["name"] not in node.intfs:
# Only add the p2p interface if not already there.
other = self.hosts[to]
Expand Down Expand Up @@ -3332,8 +3343,40 @@ async def add_dummy_link(self, node1, c1=None):
super().add_dummy(node1, if1, **c1)
node1.set_dummy_addr(c1)

async def add_native_link(self, node1, node2, c1=None, c2=None):
"""Add a link between switch and node or 2 nodes."""
_TC_KEYS = (
"delay",
"jitter",
"jitter-correlation",
"loss",
"loss-correlation",
"rate",
)

@staticmethod
def _tc_pick(config):
"""Copy linux TC keys from config."""
if not config:
return {}
out = {}
for key in Munet._TC_KEYS:
if config.get(key) is not None:
out[key] = config[key]
return out

@staticmethod
def _tc_constraints(*configs):
"""Merge unprefixed TC keys; later configs override earlier ones."""
out = {}
for config in configs:
out.update(Munet._tc_pick(config))
return out

async def add_native_link(self, node1, node2, c1=None, c2=None, c1_from=None):
"""Add a link between switch and node or 2 nodes.

c1_from is switch ``from:`` (host TX / IFB). It is always the host
node's outbound constraints, even if the node is passed first.
"""
isp2p = False

c1 = {} if c1 is None else c1
Expand Down Expand Up @@ -3392,10 +3435,43 @@ async def add_native_link(self, node1, node2, c1=None, c2=None):

if isinstance(node1, ExternalNetwork):
pass
elif "physical" not in c1 and not node1.is_vm:
node1.set_intf_constraints(if1, **c1)
if "physical" not in c2 and not node2.is_vm:
node2.set_intf_constraints(if2, **c2)
elif isp2p:
# 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:
tx = Munet._tc_constraints(c1)
if tx:
self.logger.warning(
"%s: p2p constraints on %s:%s stay inside the node",
self,
node1.name,
if1,
)
node1.set_intf_constraints(if1, **tx)
if "physical" not in c2 and not node2.is_vm:
tx = Munet._tc_constraints(c2)
if tx:
self.logger.warning(
"%s: p2p constraints on %s:%s stay inside the node",
self,
node2.name,
if2,
)
node2.set_intf_constraints(if2, **tx)
elif "physical" not in c2:
# Host-to-switch (switch-centric to/from):
# to: r1 + delay → into r1 (node RX, switch egress)
# from: r1 + delay → from r1 (node TX, IFB on switch ingress)
# Node delay still shapes TX if set (same as from:).
node_tx = {
**Munet._tc_constraints(c1_from),
**Munet._tc_constraints(c2),
}
node_rx = Munet._tc_constraints(c1)
if node_rx:
node1.set_intf_constraints(if1, **node_rx)
if node_tx:
node1.set_intf_ingress_constraints(if1, **node_tx)

def add_l3_node(self, name, config=None, **kwargs):
"""Add a node to munet."""
Expand Down
Loading
Loading