From b5306fe31c0b3703cd7c6b662f62ee1de9fb4c2c Mon Sep 17 00:00:00 2001 From: Robin Jarry Date: Tue, 13 Jan 2026 16:46:40 +0100 Subject: [PATCH] devtools: add node complexity analysis script Introduce a tool to measure and track the code complexity of datapath node processing functions. The script parses objdump disassembly output and reports instruction count, conditional branches, jumps, and calls for each node. Node functions are identified by scanning source files for `.process = function_name` patterns in struct rte_node_register definitions. The script merges compiler-split `.cold` functions back into their parents and recursively traverses the call graph to compute total instruction counts. Logging, tracing, and DPDK node infrastructure functions are excluded from the recursive count to focus on actual packet processing logic: rte_log, rte_vlog, gr_mbuf_trace*, trace_log_packet, and __rte_node_stream_alloc*. Three output modes are supported: - Table format (default): human-readable, sorted by instruction count - JSON format (--json): for generating baseline files - Compare mode (--compare): shows deltas against a baseline and exits with code 1 if any function exceeds the regression threshold Add a CI job that builds grout and compares the current complexity metrics against the checked-in baseline. Force cpu_instruction_set to x86-64-v3 to ensure consistent results regardless of the CI runner CPU features. Signed-off-by: Robin Jarry --- .github/workflows/check.yml | 52 +++++ GNUmakefile | 2 +- devtools/complexity.json | 344 +++++++++++++++++++++++++++++++ devtools/node-complexity | 396 ++++++++++++++++++++++++++++++++++++ 4 files changed, 793 insertions(+), 1 deletion(-) create mode 100644 devtools/complexity.json create mode 100755 devtools/node-complexity diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index d7f28342a..681ddd58a 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -129,6 +129,58 @@ jobs: path: ${{ env.CCACHE_DIR }} key: ${{ steps.cache.outputs.cache-primary-key }} + node-complexity: + if: ${{ github.actor != 'grout-bot' }} + permissions: + actions: write + runs-on: ubuntu-24.04 + container: fedora:latest + env: + DEBIAN_FRONTEND: noninteractive + NEEDRESTART_MODE: l + CC: ccache gcc + BUILDTYPE: debugoptimized + SANITIZE: none + MESON_EXTRA_OPTS: -Ddpdk:cpu_instruction_set=x86-64-v3 + steps: + - name: install system dependencies + run: | + set -xe + dnf install -y ccache git gcc make meson ninja-build \ + pkgconf python3-pyelftools scdoc libmnl-devel binutils \ + libcmocka-devel libedit-devel libevent-devel numactl-devel \ + libsmartcols-devel libarchive-devel rdma-core-devel + - uses: actions/checkout@v4 + - run: echo "CCACHE_DIR=$(ccache -k cache_dir)" >> $GITHUB_ENV + - uses: actions/cache/restore@v4 + id: cache + with: + path: ${{ env.CCACHE_DIR }} + key: ccache-x86_64-gcc-node-complexity + - run: ccache -z + - run: make + - run: devtools/node-complexity --compare devtools/complexity.json + - run: ccache -sv + - name: delete old cache + if: ${{ ! github.event.pull_request.commits }} + uses: actions/github-script@v7 + with: + script: | + try { + await github.rest.actions.deleteActionsCacheByKey({ + owner: context.repo.owner, + repo: context.repo.repo, + key: "${{ steps.cache.outputs.cache-primary-key }}" + }); + } catch (error) { + if (error.status !== 404) throw error; + } + - uses: actions/cache/save@v4 + if: ${{ ! github.event.pull_request.commits }} + with: + path: ${{ env.CCACHE_DIR }} + key: ${{ steps.cache.outputs.cache-primary-key }} + build-cross-aarch64: if: ${{ github.actor != 'grout-bot' }} permissions: diff --git a/GNUmakefile b/GNUmakefile index 5a95b376a..064c71593 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -105,7 +105,7 @@ rpm: CLANG_FORMAT ?= clang-format c_src = git ls-files '*.[ch]' ':!:subprojects' all_files = git ls-files ':!:subprojects' -licensed_files = git ls-files ':!:*.svg' ':!:licenses' ':!:*.md' ':!:*.asc' ':!:subprojects' ':!:debian' ':!:.*' ':!:*.scdoc' +licensed_files = git ls-files ':!:*.svg' ':!:licenses' ':!:*.md' ':!:*.asc' ':!:subprojects' ':!:debian' ':!:.*' ':!:*.scdoc' ':!:*.json' .PHONY: lint lint: diff --git a/devtools/complexity.json b/devtools/complexity.json new file mode 100644 index 000000000..634795271 --- /dev/null +++ b/devtools/complexity.json @@ -0,0 +1,344 @@ +{ + "arp_input_process": { + "branches": 8, + "calls": 2, + "instructions": 85, + "jumps": 1 + }, + "arp_input_reply_process": { + "branches": 9, + "calls": 4, + "instructions": 178, + "jumps": 3 + }, + "arp_input_request_process": { + "branches": 8, + "calls": 4, + "instructions": 159, + "jumps": 3 + }, + "arp_output_reply_process": { + "branches": 13, + "calls": 5, + "instructions": 195, + "jumps": 3 + }, + "arp_output_request_process": { + "branches": 20, + "calls": 7, + "instructions": 351, + "jumps": 7 + }, + "bond_output_process": { + "branches": 28, + "calls": 3, + "instructions": 242, + "jumps": 9 + }, + "control_input_process": { + "branches": 66, + "calls": 9, + "instructions": 670, + "jumps": 31 + }, + "control_output_process": { + "branches": 8, + "calls": 5, + "instructions": 255, + "jumps": 3 + }, + "dhcp_input_process": { + "branches": 13, + "calls": 2, + "instructions": 121, + "jumps": 5 + }, + "dnat44_dynamic_process": { + "branches": 12, + "calls": 4, + "instructions": 325, + "jumps": 5 + }, + "dnat44_static_process": { + "branches": 14, + "calls": 4, + "instructions": 237, + "jumps": 4 + }, + "eth_input_process": { + "branches": 22, + "calls": 4, + "instructions": 307, + "jumps": 8 + }, + "eth_output_process": { + "branches": 25, + "calls": 6, + "instructions": 323, + "jumps": 7 + }, + "icmp6_input_process": { + "branches": 20, + "calls": 6, + "instructions": 273, + "jumps": 10 + }, + "icmp6_local_send_process": { + "branches": 7, + "calls": 5, + "instructions": 148, + "jumps": 1 + }, + "icmp6_output_process": { + "branches": 28, + "calls": 5, + "instructions": 453, + "jumps": 9 + }, + "icmp_input_process": { + "branches": 17, + "calls": 3, + "instructions": 183, + "jumps": 5 + }, + "icmp_local_send_process": { + "branches": 7, + "calls": 5, + "instructions": 145, + "jumps": 1 + }, + "icmp_output_process": { + "branches": 22, + "calls": 4, + "instructions": 311, + "jumps": 6 + }, + "ip6_error_process": { + "branches": 32, + "calls": 8, + "instructions": 392, + "jumps": 7 + }, + "ip6_forward_process": { + "branches": 8, + "calls": 2, + "instructions": 82, + "jumps": 2 + }, + "ip6_hold_process": { + "branches": 5, + "calls": 2, + "instructions": 66, + "jumps": 1 + }, + "ip6_input_local_process": { + "branches": 31, + "calls": 3, + "instructions": 519, + "jumps": 11 + }, + "ip6_input_process": { + "branches": 20, + "calls": 4, + "instructions": 268, + "jumps": 8 + }, + "ip6_loadbalance_process": { + "branches": 6, + "calls": 2, + "instructions": 85, + "jumps": 2 + }, + "ip6_output_process": { + "branches": 17, + "calls": 4, + "instructions": 177, + "jumps": 8 + }, + "ip_error_process": { + "branches": 19, + "calls": 6, + "instructions": 326, + "jumps": 3 + }, + "ip_forward_process": { + "branches": 6, + "calls": 2, + "instructions": 85, + "jumps": 1 + }, + "ip_fragment_process": { + "branches": 43, + "calls": 10, + "instructions": 1651, + "jumps": 18 + }, + "ip_hold_process": { + "branches": 5, + "calls": 2, + "instructions": 66, + "jumps": 1 + }, + "ip_input_local_process": { + "branches": 7, + "calls": 2, + "instructions": 96, + "jumps": 1 + }, + "ip_input_process": { + "branches": 35, + "calls": 5, + "instructions": 383, + "jumps": 8 + }, + "ip_loadbalance_process": { + "branches": 6, + "calls": 2, + "instructions": 80, + "jumps": 2 + }, + "ip_output_process": { + "branches": 19, + "calls": 6, + "instructions": 7723, + "jumps": 6 + }, + "ipip_input_process": { + "branches": 13, + "calls": 6, + "instructions": 194, + "jumps": 6 + }, + "ipip_output_process": { + "branches": 16, + "calls": 6, + "instructions": 266, + "jumps": 4 + }, + "l1_xconnect_process": { + "branches": 6, + "calls": 3, + "instructions": 115, + "jumps": 3 + }, + "l2_redirect_process": { + "branches": 5, + "calls": 2, + "instructions": 65, + "jumps": 1 + }, + "l4_input_local_process": { + "branches": 7, + "calls": 1, + "instructions": 81, + "jumps": 3 + }, + "l4_loopback_output_process": { + "branches": 22, + "calls": 5, + "instructions": 275, + "jumps": 7 + }, + "lacp_input_process": { + "branches": 14, + "calls": 2, + "instructions": 108, + "jumps": 2 + }, + "lacp_output_process": { + "branches": 8, + "calls": 3, + "instructions": 120, + "jumps": 2 + }, + "loop_xvrf_process": { + "branches": 6, + "calls": 2, + "instructions": 93, + "jumps": 3 + }, + "loopback_input_process": { + "branches": 7, + "calls": 2, + "instructions": 84, + "jumps": 3 + }, + "loopback_output_process": { + "branches": 4, + "calls": 1, + "instructions": 63, + "jumps": 1 + }, + "ndp_na_input_process": { + "branches": 17, + "calls": 4, + "instructions": 404, + "jumps": 3 + }, + "ndp_na_output_process": { + "branches": 12, + "calls": 4, + "instructions": 167, + "jumps": 3 + }, + "ndp_ns_input_process": { + "branches": 21, + "calls": 5, + "instructions": 473, + "jumps": 8 + }, + "ndp_ns_output_process": { + "branches": 14, + "calls": 6, + "instructions": 312, + "jumps": 3 + }, + "ndp_rs_input_process": { + "branches": 8, + "calls": 2, + "instructions": 75, + "jumps": 1 + }, + "ospf_redirect_process": { + "branches": 40, + "calls": 8, + "instructions": 591, + "jumps": 9 + }, + "port_output_process": { + "branches": 7, + "calls": 4, + "instructions": 92, + "jumps": 1 + }, + "rx_process": { + "branches": 34, + "calls": 8, + "instructions": 378, + "jumps": 13 + }, + "snap_input_process": { + "branches": 15, + "calls": 2, + "instructions": 223, + "jumps": 8 + }, + "srv6_local_process": { + "branches": 33, + "calls": 9, + "instructions": 742, + "jumps": 9 + }, + "srv6_output_process": { + "branches": 28, + "calls": 7, + "instructions": 477, + "jumps": 7 + }, + "tx_process": { + "branches": 14, + "calls": 9, + "instructions": 542, + "jumps": 4 + } +} diff --git a/devtools/node-complexity b/devtools/node-complexity new file mode 100755 index 000000000..9c4387e5c --- /dev/null +++ b/devtools/node-complexity @@ -0,0 +1,396 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2025 Robin Jarry + +""" +Analyze datapath node complexity by parsing objdump disassembly. + +Reports instruction count, conditional branches, jumps, and calls for each +node processing function. +""" + +import argparse +import json +import pathlib +import re +import subprocess +import sys + + +def run_objdump(binary_path): + """ + Run objdump (or llvm-objdump) and return stdout. + + Tries objdump first, falls back to llvm-objdump. + """ + for cmd in [ + ["objdump", "-d", "--no-show-raw-insn", binary_path], + ["llvm-objdump", "-d", "--no-show-raw-insn", binary_path], + ]: + try: + result = subprocess.run(cmd, capture_output=True, text=True, check=True) + return result.stdout + except FileNotFoundError: + continue + except subprocess.CalledProcessError as e: + raise RuntimeError(f"{cmd[0]} failed: {e.stderr}") from e + raise FileNotFoundError("neither objdump nor llvm-objdump found") + + +def parse_objdump(binary_path): + """ + Run objdump and parse the disassembly output. + + Returns a tuple of (functions, call_graph) where: + - functions: dict mapping function names to their direct metrics + - call_graph: dict mapping function names to list of called function names + """ + output = run_objdump(binary_path) + + functions = {} + call_graph = {} + current_func = None + metrics = None + calls_list = None + + # x86_64 conditional branch instructions + cond_branches = frozenset( + [ + "je", + "jne", + "jz", + "jnz", + "jl", + "jle", + "jg", + "jge", + "jb", + "jbe", + "ja", + "jae", + "js", + "jns", + "jo", + "jno", + "jp", + "jnp", + "jcxz", + "jecxz", + "jrcxz", + ] + ) + # Unconditional jumps + jumps = frozenset(["jmp", "jmpq"]) + # Call instructions + calls = frozenset(["call", "callq"]) + + # Pattern to match function headers: "0000000000001234 :" + func_pattern = re.compile(r"^[0-9a-f]+\s+<([^>]+)>:$") + # Pattern to match instructions: " 1234: mnemonic operands" + insn_pattern = re.compile(r"^\s+[0-9a-f]+:\s+(\S+)") + # Pattern to extract call target: "call ... " + call_target_pattern = re.compile(r"<([^>@]+)(?:@plt)?>") + + for line in output.splitlines(): + func_match = func_pattern.match(line) + if func_match: + if current_func and metrics: + functions[current_func] = metrics + call_graph[current_func] = calls_list + current_func = func_match.group(1) + metrics = { + "instructions": 0, + "branches": 0, + "jumps": 0, + "calls": 0, + } + calls_list = [] + continue + + if metrics is None: + continue + + insn_match = insn_pattern.match(line) + if insn_match: + mnemonic = insn_match.group(1).lower() + metrics["instructions"] += 1 + if mnemonic in cond_branches: + metrics["branches"] += 1 + elif mnemonic in jumps: + metrics["jumps"] += 1 + elif mnemonic in calls: + metrics["calls"] += 1 + # Extract call target if it's a direct call + target_match = call_target_pattern.search(line) + if target_match: + calls_list.append(target_match.group(1)) + + if current_func and metrics: + functions[current_func] = metrics + call_graph[current_func] = calls_list + + return functions, call_graph + + +def merge_cold_functions(functions, call_graph): + """ + Merge .cold function metrics into their parent functions. + """ + cold_funcs = [name for name in functions if ".cold" in name] + for cold_name in cold_funcs: + # Extract parent name (e.g., "rx_process.cold" -> "rx_process") + parent_name = cold_name.split(".cold")[0] + if parent_name in functions: + parent = functions[parent_name] + cold = functions[cold_name] + for key in parent: + parent[key] += cold[key] + # Merge call graph too + if parent_name in call_graph and cold_name in call_graph: + call_graph[parent_name].extend(call_graph[cold_name]) + del functions[cold_name] + if cold_name in call_graph: + del call_graph[cold_name] + + return functions, call_graph + + +# ignore debugging/tracing/infrastructure +EXCLUDED_FUNCS = ( + "rte_log", + "rte_vlog", + "gr_mbuf_trace", + "trace_log_packet", + "__rte_node_", +) + + +def compute_total_instructions(functions, call_graph, target_funcs) -> dict[str, dict]: + """ + Compute total instructions for each target function including called functions. + + Uses memoization and handles cycles. Excludes infrastructure functions. + """ + cache = {} + + def func_excluded(name: str) -> bool: + # Check PLT functions (libc) + if "@plt" in name: + return True + # Handle .cold, .part, etc. + base = name.split(".")[0] + for pattern in EXCLUDED_FUNCS: + if pattern in base: + return True + return False + + def total_insns(func_name: str, visited: set[str]): + if func_name in cache: + return cache[func_name] + if func_name not in functions: + return 0 + if func_name in visited: + return 0 # Cycle detected, don't double count + if func_excluded(func_name): + return 0 + + visited = visited | {func_name} + total = functions[func_name]["instructions"] + + for called in call_graph.get(func_name, []): + total += total_insns(called, visited) + + cache[func_name] = total + + return total + + result = {} + for func_name in target_funcs: + if func_name in functions: + result[func_name] = { + "instructions": total_insns(func_name, set()), + "branches": functions[func_name]["branches"], + "jumps": functions[func_name]["jumps"], + "calls": functions[func_name]["calls"], + } + + return result + + +def extract_source_functions(source_dir): + """ + Extract node process function names from source code. + + Scans for '.process = function_name' patterns in C files. + """ + process_pattern = re.compile(r"\.process\s*=\s*(\w+)") + functions = set() + + for filepath in pathlib.Path(source_dir).rglob("*.c"): + text = filepath.read_text(encoding="utf-8", errors="ignore") + for match in process_pattern.finditer(text): + functions.add(match.group(1)) + + return functions + + +def filter_by_source(functions, source_functions): + """Filter binary functions to only those registered in source.""" + return { + name: metrics for name, metrics in functions.items() if name in source_functions + } + + +def format_col(current, baseline=None): + """Format column value with optional delta.""" + if baseline is None or current == baseline: + return str(current) + return f"{current}({current - baseline:+d})" + + +def print_table(functions, baseline=None): + """Print metrics in a human-readable table format with optional baseline diff.""" + if not functions: + print("No matching functions found.") + return + + name_width = max(len(name) for name in functions) + name_width = max(name_width, len("FUNCTION")) + + print( + f"{'FUNCTION':<{name_width}} " + f"{'INSTRUCTIONS':>16} {'BRANCHES':>12} {'JUMPS':>9} {'CALLS':>9}" + ) + + sorted_funcs = sorted( + functions.items(), key=lambda x: x[1]["instructions"], reverse=True + ) + + for name, m in sorted_funcs: + b = (baseline or {}).get(name, {}) + print( + f"{name:<{name_width}} " + f"{format_col(m['instructions'], b.get('instructions')):>16} " + f"{format_col(m['branches'], b.get('branches')):>12} " + f"{format_col(m['jumps'], b.get('jumps')):>9} " + f"{format_col(m['calls'], b.get('calls')):>9}" + ) + + +def print_json(functions): + """Print metrics in JSON format for CI consumption.""" + print(json.dumps(functions, indent=2, sort_keys=True)) + + +def compare_baseline(functions, baseline_path, threshold): + """ + Compare current metrics against a baseline file. + + Returns exit code: 0 if within threshold, 1 if regressions detected. + """ + with open(baseline_path) as f: + baseline = json.load(f) + + print_table(functions, baseline) + + regressions = [] + for name, metrics in functions.items(): + base = baseline.get(name, {}) + for key in ["instructions", "branches"]: + base_val = base.get(key, 0) + if base_val == 0: + continue + pct = ((metrics[key] - base_val) / base_val) * 100 + if pct > threshold: + regressions.append(f"{name}: {key} +{pct:.1f}%") + + print() + if regressions: + print(f"Regressions detected (threshold: {threshold}%):") + for r in regressions: + print(f" {r}") + return 1 + + print(f"No regressions detected (threshold: {threshold}%).") + return 0 + + +def main(): + root_dir = pathlib.Path(__file__).resolve().parent.parent + + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.ArgumentDefaultsHelpFormatter, + ) + parser.add_argument( + "-b", + "--binary", + type=pathlib.Path, + default=root_dir / "build" / "grout", + help="pathlib.Path to the compiled binary.", + ) + parser.add_argument( + "-s", + "--source-dir", + type=pathlib.Path, + default=root_dir / "modules", + help="Source directory for .process= extraction.", + ) + parser.add_argument( + "-j", + "--json", + action="store_true", + help="Output in JSON format instead of table.", + ) + parser.add_argument( + "-c", + "--compare", + metavar="BASELINE", + type=pathlib.Path, + help="Compare against baseline JSON file.", + ) + parser.add_argument( + "-t", + "--threshold", + type=float, + default=5.0, + help="Regression threshold percentage.", + ) + + args = parser.parse_args() + + try: + all_functions, call_graph = parse_objdump(args.binary) + except RuntimeError as e: + print(f"error: {e}", file=sys.stderr) + sys.exit(1) + except FileNotFoundError as e: + print(f"error: {e}", file=sys.stderr) + sys.exit(1) + + # Merge .cold functions into their parents + all_functions, call_graph = merge_cold_functions(all_functions, call_graph) + + source_funcs = extract_source_functions(args.source_dir) + missing = source_funcs - set(all_functions.keys()) + if missing: + print( + f"warning: {len(missing)} source functions not found in binary:", + file=sys.stderr, + ) + for name in sorted(missing): + print(f" {name}", file=sys.stderr) + + # Compute total instructions including called functions + functions = compute_total_instructions(all_functions, call_graph, source_funcs) + + if args.compare: + sys.exit(compare_baseline(functions, args.compare, args.threshold)) + elif args.json: + print_json(functions) + else: + print_table(functions) + + +if __name__ == "__main__": + main()