Skip to content

Implement KNE Packet Bridge daemon and CLI subcommand - #762

Merged
kraney merged 29 commits into
openconfig:mainfrom
kraney:feat/packet-bridge
Sep 24, 2026
Merged

kraney merged 29 commits into
openconfig:mainfrom
kraney:feat/packet-bridge

Conversation

@kraney

@kraney kraney commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds the KNE Packet Bridge: a daemon that carries raw Layer 2 Ethernet frames
between network segments over a gRPC stream, so that a KNE topology can be wired to
something that is not in the same cluster — another KNE cluster, a workstation veth,
or a process running outside Kubernetes entirely.

A FORWARD node declares the wires it terminates and KNE derives the daemon
invocations. The topology never names a flag, a port, or a Kubernetes object.

The contract

proto/forward.proto already described this model — a list of wires, each with an a
and a z endpoint — but nothing implemented it. This PR implements it, including the
RemoteNode endpoint that was declared and unreachable.

The endpoint written as an interface is always the declaring node:

Wire Meaning
a: { interface }, z: { local_node } this node dials a peer in the topology
a: { interface }, z: { remote_node } this node dials an address outside it
z: { interface }, no a this node listens; the client is external

So a node that a Borg job attaches to is written as:

nodes: {
  name: "bridge-server"
  vendor: FORWARD
  config: {
    vendor_data: {
      [type.googleapis.com/forward.ForwardConfig] {
        wires: { z: { interface: { name: "eth1" } } }
      }
    }
  }
  services: { key: 50058 value: { name: "wire" inside: 50058 type: NODE_PORT } }
}

No image, no CLI flags, no Service names. Two consequences fall out of the model:

  • Peer resolution. Naming a peer with local_node requires that name to resolve,
    and nothing created a Service under a node's own name. Every FORWARD node now gets
    a headless Service named after it. It declares no ports — a headless Service needs
    only a selector to publish A records, and ports would place it in the map kne show
    reports and collide with the wire port a topology may already expose. It carries the
    pod label so the node's existing teardown removes it.
  • Mode per process. The daemon runs either as a server or as a client, so a node
    that both serves and dials gets one container per role, named <node>-<role>. A node
    needing only one keeps the plain node name.

Changes

Bridge daemon and client (bridge/, cmd/bridge/)
Bidirectional streaming of raw frames over the existing Wire service, with a
per-interface demux so one server can serve many wires on one port. kne bridge server
and kne bridge client, with interface binding, reconnect, and optional ALTS transport
credentials for DirectPath on GCE.

Wire planning (topo/node/forward/)
ForwardConfig is translated into daemon invocations and containers, plus the headless
peer Service described above. The default image points at the published bridge image, so
topologies no longer carry an image: line.

Service exposure (proto/topo.proto, topo/node/, topo/topo.go)
Service.Type gains LOAD_BALANCER, NODE_PORT and CLUSTER_IP, with one Kubernetes
Service created per requested type. Service.external_ipv6 states that a service should
be reachable over IPv6 from outside the cluster; on a single-stack IPv4 cluster KNE
satisfies that with a host-network socat DaemonSet, and on a dual-stack cluster it may
become a no-op. outside_ip is now documented as populated only for LOAD_BALANCER.

Build and release (Makefile, deploy/bridge/, cloudbuild/, cmd/release/)
A Dockerfile whose entrypoint is kne bridge, make targets for local kind testing, and
kne release bridge driving a Cloud Build pipeline. The bridge image is the kne binary,
so it has no version of its own: releasing it tags KNE as a whole and labels the image
with that version, rather than minting a separate series.

Testing

Unit tests for bridge/, cmd/bridge/, the wire planner and peer Service in
topo/node/forward/, and the service-type handling in topo/. The planner tests cover
each endpoint shape, both malformed-wire cases, and the mixed server-plus-client node.

Verified on kind: kne deploy, then kne create of examples/bridge/paired-bridge.pb.txt,
then ping between the two hosts, which have no meshnet link and reach each other only
across the bridge. That exercises wire planning, peer resolution through the headless
Service, stream establishment, and bidirectional forwarding.

Also verified on kind with external_ipv6: true set on the service: the socat DaemonSet
is admitted, which exercises the apps/daemonsets grant added to the podrunner
ClusterRole, and its pod reaches Running, which exercises the explicit non-root UID the
container needs in order for the kubelet to accept it.

Not yet exercised on a cluster: multi-container FORWARD nodes and remote_node
endpoints. Both are covered by unit tests only.

Notes for reviewers

  • Service.v6_host_proxy was renamed to Service.external_ipv6. This is a breaking proto
    change, made deliberately while nothing depends on the field, because the old name
    described the mechanism rather than the intent.
  • deploy/ubuntu/serviceaccount.yaml is reindented throughout. super-linter runs prettier
    over every file a PR touches rather than only the changed lines, and this file did not
    previously indent block sequences under their mapping key. The parsed documents are
    unchanged.
  • x/wire/ contains an earlier, unfinished implementation of the same Wire service. It
    is untouched here and nothing imports it; retiring it is left to a follow-up.

@kraney
kraney force-pushed the feat/packet-bridge branch from b16dcb0 to e6328da Compare September 8, 2026 21:19
@kraney
kraney requested review from bstoll and manizzzz September 11, 2026 17:02
@kraney
kraney force-pushed the feat/packet-bridge branch 3 times, most recently from a042c7b to 79bdea3 Compare September 14, 2026 21:20
Comment thread bridge/bridge.go
Comment thread bridge/client.go Outdated
Comment thread bridge/bridge.go
Comment thread bridge/bridge.go
Comment thread bridge/bridge.go

@manizzzz manizzzz 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.

Thanks for the changes.

Comment thread bridge/bridge.go
Comment thread bridge/bridge.go
Comment thread bridge/bridge.go Outdated
Comment thread topo/node/node.go Outdated
Comment thread topo/node/node.go Outdated
Comment thread bridge/bridge_test.go
Comment thread bridge/bridge_test.go
Comment thread bridge/bridge_test.go
Comment thread bridge/bridge_test.go
Comment thread bridge/bridge_test.go
@kraney
kraney force-pushed the feat/packet-bridge branch 2 times, most recently from ab68daf to f5a190b Compare September 22, 2026 21:11

@manizzzz manizzzz 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.

Thanks for the super thorough fixes on the and new test suite all look great!

Holding LGTM for three small issues in the new diff before we merge:

  1. bridge/bridge.go (ReadPacket / WritePacket): retry unix.EINTR inside the s.rc.Read / s.rc.Write callbacks (RawConn doesn't ignore EINTR, so
    Go's SIGURG preemption will cause readLoop to tear down the demux).
  2. bridge/bridge.go (Server.Transmit): wait for the egress goroutine after demux.unsubscribe(pktChan) in the defer so stream.Send cannot race
    with Transmit returning.
  3. topo/node/forward/forward.go (CreatePod): preserve pb.Config.Args when ForwardConfig.wires is used so flags like --alts aren't dropped.

Left a few optional/non-blocking comments on forward.go and node.go as well. Once (1)–(3) are in, this is good to LGTM!

Comment thread bridge/bridge.go
Comment thread bridge/bridge.go
Comment thread topo/node/forward/forward.go
Comment thread topo/node/forward/forward.go Outdated
Comment thread topo/node/forward/forward.go
Comment thread topo/node/forward/forward.go
Comment thread topo/node/node.go
Comment thread topo/node/forward/forward.go
The bridge subcommand sets up a daemon process that will forward
packets into / out of the KNE cluster over a gRPC streaming service.
An external client can use this service to inject or receive packets
to/from a particular interface defined in the topology.
This makes bridge usable in KNE-only situations, to link two separate
clusters (like if one cluster has access another doesn't.) More
importantly, it facilitates easy testing of the feature.
* use a pool for packet buffers
* use cancel so waiting goroutines are freed if readLoop exits on a
  socket error
* use signal.NotifyContext()
* Make sure kne bridge runs root handlers
* distinguish between normal and unexpected stream closure in the client
* Better test coverage
  1. Netpoller Integration & Interruptible ReadPacket:
      • Refactored bridge.go:59 in bridge.go:69 to set the raw socket to non-blocking via unix.
      SetNonblock, wrap the file descriptor in os.NewFile, and use syscall.RawConn (s.rc.Read and s.rc.
      Write).
      • Calling s.Close() (s.f.Close()) now immediately interrupts any pending or concurrent ReadPacket()
      / WritePacket() calls via the Go runtime netpoller, preventing socket pinning and kernel memory
      leaks.
      • Documented on bridge.go:54 that implementations MUST ensure Close() interrupts pending
      ReadPacket() calls.
  2. Removed Goroutine & Socket Leaks in Client runStream:
      • In client.go:135, tracked child egress and ingress goroutines with a sync.WaitGroup.
      • Deferred teardown cancels streamCancel(), calls handler.Close(), and waits on wg.Wait() so
      reconnect loops cannot leave orphaned goroutines or open promiscuous sockets.
  3. Demux and Socket Concurrency Guarding:
      • Netpoller RawConn prevents raw file descriptor reuse races between concurrent WritePacket and
      Close.
      • Added bridge.go:183 which checks d.ctx.Err() prior to writing to the underlying handler. Transmit
      now calls demux.write() rather than accessing demux.handler directly.
  4. Explicit Demux Closed State & Safe Cache Teardown:
      • Added an explicit closed bool flag to bridge.go:141. bridge.go:229 returns an error if the demux
      is closed or cancelled, preventing orphaned subscriber channels.
      • Updated bridge.go:281 to purge dead demux instances before returning and added a pointer identity
      check (cur == d) in onClose to prevent late callbacks from evicting replacement demuxers.
  5. Server Teardown Synchronization (Server.Close):
      • Tracked demux read loops using a sync.WaitGroup in InterfaceDemux via bridge.go:251.
      • bridge.go:404 now invokes d.close(), awaits completion with d.wait(), and aggregates any close
      errors with errors.Join.
      • Added an early s.ctx.Err() check in getOrCreateDemux returning codes.Unavailable status errors if
      called during or after server shutdown.
  6. Distinguished Fatal vs. Per-Packet Write Errors & Length Checks:
      • Added bridge.go:423 and explicit MTU length validation against maxFrameSize.
      • Non-fatal write errors (such as EMSGSIZE or ENOBUFS) log and bump a droppedWriteFrames counter
      without tearing down the gRPC stream.
  7. Buffer & Subscriber Management:
      • Removed sync.Pool (packetPool) in favor of a dedicated per-SocketHandler 64 KB scratch buffer (s.
      rbuf).
      • Capped concurrent subscribers per interface to maxSubscribersPerDemux (32) and documented slice
      immutability on subscribe().
  8. Tests:
      • Added unit tests in bridge_test.go and client_test.go testing closed demux subscribe rejection,
      max subscriber limits, server shutdown waiting, dead demux cache replacement, and non-fatal error
      continuation.
  1. Explicit DeleteService Error Handling & DaemonSet Cleanup:
      • In node.go:772, DeleteService no longer ignores all errors with _ =. It explicitly checks and
      ignores only apierrors.IsNotFound(err) while collecting and returning all actionable failures (such
      as RBAC Forbidden, API timeouts, or context cancellation) using errors.Join.
      • Updated node.go:730 to propagate errors from DeleteService and DeleteResource using errors.Join
      rather than discarding them with log.Warningf.
      • Updated node.go:744 and node.go:811 to cleanly ignore NotFound and NotExist errors on already-
      deleted resources.
  2. Per-Type Service Grouping (Mixed-Type Support):
      • In node.go:578, eliminated service type collapsing across ports. Ports are now partitioned by
      service type:
          • LoadBalancer ports are grouped into service-<node> (with AllocateLoadBalancerNodePorts:
          false).
          • NodePort ports are grouped into service-<node>-nodeport.
          • ClusterIP ports are grouped into service-<node>-clusterip.
      • Updated node.go:895 to list all services matching the pod=<node> label selector, with backward-
      compatibility fallback.
      • Added mixed-type service test cases and DeleteService RBAC error propagation tests in
      node_test.go:424.
  3. Demux Fan-out, Multi-Interface, and Stress Tests:
      • Added bridge_test.go:574 in bridge/bridge_test.go verifying that concurrent Wire.Transmit streams
      on the same interface both receive the broadcast frame.
      • Added bridge_test.go:644 asserting socketOpener is called once per distinct interface across
      concurrent/sequential streams with per-interface fakes.
      • Added bridge_test.go:707 verifying subscription failure on a dead demux.
      • Added bridge_test.go:723 testing concurrent subscriptions/unsubscriptions under packet flow.
      • Verified that go test -race -count=10 ./bridge/... ./cmd/bridge/... ./topo/node passes cleanly.
  ### 1. Socat DaemonSet Hardening & Security (topo/node/node.go)

  • Eliminated False-Positive Proxy Creation (Comment 11 / ID 4030589333):
      • Finding: The fallback iteration over n.Proto.Services by service name caused services without
      explicit names to match ("" == ""), opening unintended host IPv6 socat proxies.
      • Fix: Removed the fallback search; only keyed services with NodePort > 0 create proxy containers,
      and added a warning when v6_host_proxy: true is configured on non-NodePort services.
  • Pinned & Configurable Image (Comment 14 / ID 4030627360):
      • Finding: alpine/socat:latest was hardcoded with a floating tag, creating non-deterministic builds
      and breaking offline/air-gapped clusters.
      • Fix: Replaced with a pinned, configurable package variable node.go:575-580 ("alpine/socat:1.8.0.
      0").
  • Connection Flooding & Container Security (Comment 15 / ID 4030639450):
      • Finding: Unbounded socat forks could trigger cgroup OOM kills during connection bursts, and
      containers lacked explicit security contexts and resource requests.
      • Fix: Added max-children=64 to TCP6-LISTEN, configured container SecurityContext (RunAsNonRoot:
      true, AllowPrivilegeEscalation: false, Capabilities: {Drop: ["ALL"]}), and added explicit
      CPU/memory Requests (50m/32Mi) alongside Limits (100m/64Mi).

  ### 2. Kubernetes Lifecycle & Resilience (topo/node/node.go)

  • Atomic Service Creation Rollback (Comment 12 / ID 4030609747):
      • Finding: If DaemonSets().Create() failed, previously created Service objects were left orphaned
      in the cluster, blocking retries.
      • Fix: Added a named-return deferred rollback in node.go:610-620 to delete partially created
      services on failure.
  • Garbage Collection & Scheduling (Comment 13 / ID 4030617630):
      • Finding: The proxy DaemonSet lacked an OwnerReference (preventing k8s GC on service deletion),
      lacked Tolerations (preventing scheduling on control-plane/master nodes in kind clusters), and
      lacked DNSPolicy.
      • Fix: Set OwnerReferences to the primary Service, added control-plane Tolerations (Operator:
      Exists), and configured DNSPolicy: DNSClusterFirstWithHostNet.

  ──────
  ### 3. Topology Proto Legacy Semantics (topo/topo.go)

  • Legacy Empty Type Handling (Comment 16 / ID 4030648120):
      • Finding: An empty spec.type in Kubernetes defaults to ClusterIP, but topo.go:940-950 was treating
      "" as LoadBalancer without explanation.
      • Fix: Documented that spec.Type == "" is handled as LoadBalancer strictly for backward
      compatibility with legacy test fixtures.

  ### 4. Test Suite Idempotency & New Unit Tests

  • Multi-run Idempotency under -count=10:
      • Finding: In-test calls to node.Vendor(...) in topo/topo_test.go caused panics due to duplicate
      registration when executed with -count=10.
      • Fix: Centralized vendor registrations in init() in topo_test.go:161-168.
  • Added Unit Tests:
      • Added node_test.go:607-675 verifying DaemonSet attributes (security context, requests/limits,
      tolerations, owner references, and max-children).
      • Added node_test.go:677-715 verifying automatic rollback on DaemonSet creation failure.
None of these are caught by the unit tests, since kfake neither runs a
kubelet nor enforces RBAC.

1. v6 proxy DaemonSet could never start. The socat image declares no
   USER, so its configured UID is 0 and RunAsNonRoot alone makes the
   kubelet reject the container with "container has runAsNonRoot and
   image will run as root". Set an explicit non-root UID/GID; socat only
   binds a NodePort (>1024) and dials loopback, so it needs no
   privileges.

2. Missing RBAC for the v6 proxy. The podrunner ClusterRole grants only
   core pods/services/logs, so creating or deleting the DaemonSet is
   Forbidden for in-cluster KNE. Now that DeleteService correctly
   surfaces non-NotFound errors, this turned into a teardown failure
   rather than a silent leak. Grant apps/daemonsets.

3. Example topology dialed a service that no longer exists. Grouping
   ports into one Service per type renamed the NODE_PORT service to
   service-<node>-nodeport, but bridge-client still pointed at
   service-bridge-server. Fix the peer address and the two stale README
   references, and document the naming rule where users will hit it.

Also gofmt the two new bridge test files (import ordering).
The bridge daemon was reachable only by hand-writing its command line into
a topology, which meant a topology had to know the daemon's flag names, the
wire port, and the name KNE happens to give a node's Service. None of that
is a contract worth asking callers to depend on, and the Service name in
particular is an implementation detail that changed within this branch.

FORWARD nodes already had a declarative model in forward.proto, added in
2024 but never implemented: a list of wires, each with an "a" and a "z"
endpoint, where an endpoint is either a local interface, another node in
the topology, or an address outside it. This implements it. The endpoint
written as an interface is the declaring node; if that is "a" the node
dials out, and if it is "z" the node listens. A wire with no "a" at all is
a server whose client lives outside the topology, which is how an external
process attaches. The remote_node case, declared but implemented nowhere,
now works too.

The daemon runs one mode per process, so a node that both serves and dials
gets a container per role, named <node>-<role>; a node needing only one
keeps the plain node name.

Naming a peer with local_node needs that name to resolve, and nothing
created a Service under a node's own name. Every FORWARD node now gets a
headless Service named after it. It declares no ports, because a headless
Service needs only a selector to publish A records, and ports would put it
in the map `kne show` reports and collide with the wire port a topology may
already expose. It carries the "pod" label so the node's teardown removes
it.

Alongside:

  - Rename Service.v6_host_proxy to external_ipv6. The old name described
    the mechanism KNE currently uses; the field states an intent that a
    dual-stack cluster could satisfy natively. Breaking, and deliberately
    done now while nothing depends on it.
  - Document outside_ip as populated only for LOAD_BALANCER services.
  - Point the FORWARD default image at the published bridge image, tagged
    :ga to match the rest of KNE, instead of the never-published
    forward:latest.
  - Fix the peer name typos in examples/forward, which named "fw1"/"fw2".
super-linter runs prettier over every file a PR changes, not just the
changed lines, so adding the daemonsets rule to the podrunner ClusterRole
pulled the whole of serviceaccount.yaml into scope. Prettier indents block
sequences under their mapping key, which this file never did.

The reindent is mechanical and the parsed documents are unchanged; verified
by comparing yaml.safe_load_all before and after.

Also drops a stray blank line from the bridge example README.
`git describe --tags --abbrev=0` returns whichever component was tagged
most recently, which in this repo is third_party/meshnet/vX.Y.Z. So
`make bridge-docker` was labelling the bridge image v0.5.1, meshnet's
version, and `make bridge-release` would have published it under that.

Matching v* picks KNE's own release tags and skips the prefixed component
ones. The bridge image is just the kne binary with `kne bridge` as its
entrypoint, so KNE's version is the right one for it to carry; it does not
need a series of its own. This also makes the $(notdir ...) wrapper dead,
since the tags that needed stripping are now excluded.

Also point bridge-release at :ga. bridge-docker builds :$(TAG) and :ga and
no longer builds :latest, so pushing :latest would have shipped whatever
stale image happened to still carry that tag.
`kne release bridge` minted a bridge/vX.Y.Z tag, copying the meshnet
pattern. Meshnet earns a series of its own: it is a separate vendored
component with its own source tree, Dockerfile and build. The bridge is not
that. Its image is the kne binary with `kne bridge` as the entrypoint, so a
bridge version distinct from KNE's describes nothing, and two series for one
artifact is a second thing to bump and a way for the image to disagree with
the binary inside it.

Tag KNE as a whole instead. Together with the Makefile now matching v*, both
release paths land on the same version for the same commit, where before
cloudbuild used whatever was passed on the command line and make used
meshnet's latest tag.

Nothing else in the repo creates tags: createAndPushTag has exactly two
callers, and no workflow under .github or cloudbuild tags anything. KNE's
v* tags are cut by hand, so this does not race an automated tagger, and a
collision with a hand-cut tag fails closed because `git tag` refuses to
overwrite.
staticcheck QF1008: corev1.Service embeds ObjectMeta, so the selector in
got.ObjectMeta.Labels is redundant. Only the lines this PR adds are checked,
because .github/linters/.golangci.yml sets new-from-rev to origin/main.

Prettier wanted the trailing document separator in serviceaccount.yaml not to
be followed by a blank line. The earlier reformat added one because it was
produced with prettier 3.6.2, which asks for the opposite; super-linter v8.6.0
pins ^3.8.1, and the two disagree on this one byte. `prettier --check` reports
only that a file has issues and never a diff, so the failure had nothing in it
to act on.

Verified with the versions CI uses: prettier 3.8.1 clean on every yaml, md and
json file the branch touches, and golangci-lint 2.12.2 reporting no issues
under the repo's own config.

@bstoll bstoll 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 didn't verify this but if GSO/TSO are enabled on the veth do TCP and UDP packets get forwarded with valid checksums? I think the way this reads packets they will forward with invalid checksums unless hardware offloads are disabled.

@kraney
kraney merged commit 961d4e5 into openconfig:main Sep 24, 2026
14 checks passed
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.

3 participants