Skip to content

Commit bcb8ada

Browse files
committed
procnet: align /proc/net docs and tighten startup validation
The package, type, and scanListeners doc comments said the scanner reads /proc/net/{tcp,udp} only. In fact procnettcp.ParseFiles reads all four files; the IPv6 entries are dropped one layer deeper, in addValidProtoEntryToPortMap's Kind switch. Update the three comments to match the code path and name the actual filter site. Move the tap-interface IP validation from inside the procnet goroutine to the top of runAgent. The same flag drives both NewAPITracker and NewProcNetScanner; failing fast lets both consumers surface the malformed value at the same point instead of running through tracker setup before the lazy procnet check fires. Document the rollback invariant at the publish loop: every binding under a nat.Port carries the same HostPort (addEntryToPortMap derives both from entry.Port), so forwarder.Add sees one key and the rollback unwinds at most one listener. Add EADDRINUSE coverage to the loopback forwarder by binding bindIP:port from outside before calling forwarder.Add, for both TCP and UDP. Signed-off-by: Jan Dubois <jan.dubois@suse.com>
1 parent ca3fd2f commit bcb8ada

4 files changed

Lines changed: 75 additions & 20 deletions

File tree

src/go/guestagent/main.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ func runAgent(
115115
adminInstall bool,
116116
k8sAPIPort, tapIfaceIP string,
117117
) error {
118+
bindIP := net.ParseIP(tapIfaceIP)
119+
if bindIP == nil {
120+
return fmt.Errorf("invalid tap interface IP %q", tapIfaceIP)
121+
}
122+
118123
groupCtx, cancel := context.WithCancel(context.Background())
119124
defer cancel()
120125
group, ctx := errgroup.WithContext(groupCtx)
@@ -238,10 +243,6 @@ func runAgent(
238243
}
239244

240245
group.Go(func() error {
241-
bindIP := net.ParseIP(tapIfaceIP)
242-
if bindIP == nil {
243-
return fmt.Errorf("invalid tap interface IP %q", tapIfaceIP)
244-
}
245246
procScanner, err := procnet.NewProcNetScanner(ctx, portTracker, bindIP, procNetScanInterval)
246247
if err != nil {
247248
return fmt.Errorf("scanning /proc/net/{tcp, udp} failed: %w", err)

src/go/guestagent/pkg/procnet/loopback_forwarder_linux.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ import (
4040
"github.com/containers/gvisor-tap-vsock/pkg/services/forwarder"
4141
)
4242

43+
const (
44+
protoTCP = "tcp"
45+
protoUDP = "udp"
46+
)
47+
4348
type loopbackForwarder struct {
4449
bindIP net.IP
4550
dialer net.Dialer
@@ -80,21 +85,21 @@ func (f *loopbackForwarder) Add(ctx context.Context, proto string, port uint16)
8085
defer f.mu.Unlock()
8186

8287
switch proto {
83-
case "tcp":
88+
case protoTCP:
8489
if _, ok := f.tcp[k]; ok {
8590
return nil
8691
}
87-
lis, err := net.ListenTCP("tcp", &net.TCPAddr{IP: f.bindIP, Port: int(port)})
92+
lis, err := net.ListenTCP(protoTCP, &net.TCPAddr{IP: f.bindIP, Port: int(port)})
8893
if err != nil {
8994
return fmt.Errorf("listen %s: %w", k, err)
9095
}
9196
f.tcp[k] = lis
9297
go f.acceptTCP(ctx, lis, port)
93-
case "udp":
98+
case protoUDP:
9499
if _, ok := f.udp[k]; ok {
95100
return nil
96101
}
97-
pc, err := net.ListenUDP("udp", &net.UDPAddr{IP: f.bindIP, Port: int(port)})
102+
pc, err := net.ListenUDP(protoUDP, &net.UDPAddr{IP: f.bindIP, Port: int(port)})
98103
if err != nil {
99104
return fmt.Errorf("listen %s: %w", k, err)
100105
}
@@ -106,7 +111,7 @@ func (f *loopbackForwarder) Add(ctx context.Context, proto string, port uint16)
106111
// scanner's lifetime context); a request-scoped or per-tick
107112
// ctx would silently break new-flow dialing once cancelled.
108113
proxy, err := forwarder.NewUDPProxy(pc, func() (net.Conn, error) {
109-
return f.dialer.DialContext(ctx, "udp", target)
114+
return f.dialer.DialContext(ctx, protoUDP, target)
110115
})
111116
if err != nil {
112117
_ = pc.Close()
@@ -125,12 +130,12 @@ func (f *loopbackForwarder) Remove(proto string, port uint16) error {
125130
f.mu.Lock()
126131
defer f.mu.Unlock()
127132
switch proto {
128-
case "tcp":
133+
case protoTCP:
129134
if lis, ok := f.tcp[k]; ok {
130135
delete(f.tcp, k)
131136
return lis.Close()
132137
}
133-
case "udp":
138+
case protoUDP:
134139
if proxy, ok := f.udp[k]; ok {
135140
delete(f.udp, k)
136141
return proxy.Close()
@@ -212,7 +217,7 @@ func (f *loopbackForwarder) acceptTCP(ctx context.Context, lis net.Listener, por
212217
func (f *loopbackForwarder) pipeTCP(ctx context.Context, in net.Conn, port uint16) {
213218
defer in.Close()
214219
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(int(port)))
215-
out, err := f.dialer.DialContext(ctx, "tcp", addr)
220+
out, err := f.dialer.DialContext(ctx, protoTCP, addr)
216221
if err != nil {
217222
log.Debugf("loopback forwarder dial tcp/%d: %s", port, err)
218223
return

src/go/guestagent/pkg/procnet/loopback_forwarder_linux_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,48 @@ func TestForwarderAddIsIdempotent(t *testing.T) {
128128
}
129129
}
130130

131+
// Hold bindIP:port from outside the forwarder so the forwarder's
132+
// own Listen returns EADDRINUSE on Add.
133+
func TestForwarderAddFailsWhenPortInUse(t *testing.T) {
134+
bindIP := net.ParseIP("127.0.0.99")
135+
cases := []struct {
136+
proto string
137+
grab func(*testing.T) (uint16, func())
138+
}{
139+
{"tcp", func(t *testing.T) (uint16, func()) {
140+
t.Helper()
141+
var lc net.ListenConfig
142+
ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.99:0")
143+
if err != nil {
144+
t.Skipf("listen via 127.0.0.99 failed (loopback aliases unavailable): %v", err)
145+
}
146+
return uint16(ln.Addr().(*net.TCPAddr).Port), func() { _ = ln.Close() }
147+
}},
148+
{"udp", func(t *testing.T) (uint16, func()) {
149+
t.Helper()
150+
var lc net.ListenConfig
151+
pc, err := lc.ListenPacket(context.Background(), "udp", "127.0.0.99:0")
152+
if err != nil {
153+
t.Skipf("listen via 127.0.0.99 failed: %v", err)
154+
}
155+
return uint16(pc.LocalAddr().(*net.UDPAddr).Port), func() { _ = pc.Close() }
156+
}},
157+
}
158+
for _, tc := range cases {
159+
t.Run(tc.proto, func(t *testing.T) {
160+
port, stop := tc.grab(t)
161+
defer stop()
162+
163+
fwd := newLoopbackForwarder(bindIP)
164+
defer fwd.Close()
165+
166+
if err := fwd.Add(context.Background(), tc.proto, port); err == nil {
167+
t.Fatalf("forwarder.Add succeeded on busy port; expected EADDRINUSE")
168+
}
169+
})
170+
}
171+
}
172+
131173
// startUDPEcho binds a UDP listener on 127.0.0.1:0 that echoes every
132174
// received datagram back to the sender. Returns the chosen port and a
133175
// stop function.

src/go/guestagent/pkg/procnet/scanner_linux.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ limitations under the License.
1212
*/
1313

1414
/*
15-
Package procnet scans /proc/net/{tcp,udp} for listeners the
15+
Package procnet scans /proc/net for IPv4 TCP and UDP listeners the
1616
container-engine events handler does not publish -- mainly
1717
--network=host containers binding 127.0.0.1 -- and exposes them to
1818
host-switch via the API tracker. For loopback listeners it also opens
@@ -22,8 +22,10 @@ two-scan stability gate filters out the transient reservation socket
2222
nerdctl's OCI createRuntime hook opens before CNI installs its
2323
iptables rules.
2424
25-
IPv6 limitation: the scanner reads /proc/net/{tcp,udp} only, not the
26-
tcp6/udp6 variants. A --network=host container that listens
25+
IPv6 limitation: procnettcp.ParseFiles returns entries from
26+
/proc/net/{tcp,tcp6,udp,udp6}, but addValidProtoEntryToPortMap
27+
dispatches only on the TCP and UDP Kinds and silently drops the
28+
TCP6 and UDP6 entries. A --network=host container that listens
2729
exclusively on [::1]:port is invisible to the scanner and is not
2830
reachable from Windows. Containers that bind dual-stack
2931
(e.g. [::]:port with IPV6_V6ONLY=0, the Go and Python default) are
@@ -63,9 +65,10 @@ type loopbackController interface {
6365
Close() error
6466
}
6567

66-
// ProcNetScanner polls /proc/net/{tcp,udp} and reconciles the
67-
// observed listener set against the API tracker and a userspace
68-
// loopback forwarder. See the package comment for the design.
68+
// ProcNetScanner polls /proc/net for IPv4 TCP and UDP listeners
69+
// and reconciles the observed set against the API tracker and a
70+
// userspace loopback forwarder. See the package comment for the
71+
// design, including the IPv6 limitation.
6972
type ProcNetScanner struct {
7073
ctx context.Context
7174
tracker tracker.Tracker
@@ -206,6 +209,9 @@ func (p *ProcNetScanner) publish(port nat.Port, bindings []nat.PortBinding) erro
206209
// forwarder; the tracker entry alone keeps the port reachable from
207210
// Windows.
208211
if !hasWildcardBinding(bindings) {
212+
// Every binding under a given nat.Port carries the same HostPort
213+
// (addEntryToPortMap derives both from entry.Port), so forwarder.Add
214+
// sees one key and rollback unwinds at most one listener.
209215
for _, b := range bindings {
210216
if b.HostIP != loopbackIP {
211217
continue
@@ -275,8 +281,9 @@ func (p *ProcNetScanner) unpublish(port nat.Port, bindings []nat.PortBinding) {
275281
}
276282
}
277283

278-
// scanListeners parses /proc/net/{tcp,udp} into a snapshot suitable
279-
// for Tick. See entriesToPortMap for the filter that drops the
284+
// scanListeners parses /proc/net/{tcp,tcp6,udp,udp6} via
285+
// procnettcp.ParseFiles. See addValidProtoEntryToPortMap for the
286+
// IPv6 drop and entriesToPortMap for the filter that drops the
280287
// forwarder's own sockets.
281288
func (p *ProcNetScanner) scanListeners() (nat.PortMap, error) {
282289
entries, err := procnettcp.ParseFiles()

0 commit comments

Comments
 (0)