Configuration used: defaults

Review profile: CHILL

Plan: Pro

nl/nl_linux.go (1)

616-623: ⚠️ Potential issue | 🔴 Critical

Retries currently leak attempt-local callback state and can emit duplicate data.

At Line 622, control jumps to retry after earlier messages in the same attempt may already have been sent to f, so a later successful attempt can re-emit data (notably affecting Execute() aggregation). Also, if f was swapped to dummyMsgIterFunc earlier, that mutated callback is reused on retry, suppressing the original callback. This contradicts the intent in Lines 618-619.

🐛 Proposed fix direction

 func (req *NetlinkRequest) ExecuteIter(sockType int, resType uint16, f func(msg []byte) bool) error {
 	const maxRetries = 10
+	origF := f
+	var staged [][]byte

 retry:
+	f = origF
+	staged = staged[:0]

 	if err := s.Send(req); err != nil {
 		return err
 	}

 ...
-			if cont := f(m.Data); !cont {
-				// Drain the rest of the messages from the kernel but don't
-				// pass them to the iterator func.
-				f = dummyMsgIterFunc
-			}
+			// Stage per-attempt data; only flush after a successful, non-interrupted dump.
+			staged = append(staged, append([]byte(nil), m.Data...))
 ...
 	}
+	for _, data := range staged {
+		if cont := origF(data); !cont {
+			break
+		}
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nl/nl_linux.go` around lines 616 - 623, The retry logic leaks attempt-local
callback state and can re-emit or suppress messages; to fix, capture the
original callback (save f to origF) and ensure each attempt uses a fresh local
iterator/callback variable that you reinitialize before jumping to the retry
label, and restore origF when deciding to retry so dummyMsgIterFunc or any
mutated f isn't reused; additionally ensure partial messages from a failed
attempt are not forwarded to origF (either buffer per-attempt messages and only
forward after a successful attempt, or suppress forwarding until success),
updating the code paths around the retry label, the i/maxRetries loop, and where
f is set so Execute() aggregation sees each message exactly once.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

nl/nl_linux.go (2)

614-620: ⚠️ Potential issue | 🔴 Critical

Critical: Retry path can emit corrupted iteration semantics (stale/duplicate data and lost callback).

At Line 619, jumping to retry from inside the receive loop retries before fully draining the current multipart response. Combined with callback invocation before interruption is known (see Line 674) and callback mutation at Line 677, this can (a) leak partial data into Execute() accumulations, (b) duplicate entries across attempts, and (c) keep f as dummyMsgIterFunc on subsequent retries.

🔧 Proposed fix sketch (drain first, preserve callback, then retry)

 func (req *NetlinkRequest) ExecuteIter(sockType int, resType uint16, f func(msg []byte) bool) error {
 	var (
 		s        *NetlinkSocket
 		err      error
-		attempts = 10
+		maxRetries = 10
+		attempts   = maxRetries
 	)
+	origF := f

 retry:
 	if err := s.Send(req); err != nil {
 		return err
 	}
@@
 			if m.Header.Flags&unix.NLM_F_DUMP_INTR != 0 {
-				if attempts--; attempts > 0 {
-					goto retry
-				}
 				dumpIntr = true
+				// Finish draining this multipart response first.
+				f = dummyMsgIterFunc
 			}
@@
 	if dumpIntr {
-		return fmt.Errorf("dump interrupted after multiple attempts: %w", ErrDumpInterrupted)
+		if attempts--; attempts > 0 {
+			f = origF
+			goto retry
+		}
+		return fmt.Errorf("dump interrupted after %d retries: %w", maxRetries, ErrDumpInterrupted)
 	}

Also applies to: 674-678, 581-581

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nl/nl_linux.go` around lines 614 - 620, The retry jump from inside the
receive loop (the attempts-- then goto retry path) can leave the current
multipart Netlink response undrained and can corrupt iteration semantics
(leaking partial data, duplicating entries and clobbering the callback f to
dummyMsgIterFunc); fix by draining the rest of the multipart response before
performing the retry, preserve the original callback by using a local saved
callback variable instead of overwriting f (avoid assigning dummyMsgIterFunc
until after the entire exchange is known to be finished), and ensure no samples
are emitted to Execute() accumulations until the request fully succeeds; locate
the receive loop and retry label (retry), the attempts decrement, the callback
variable f, dummyMsgIterFunc, and Execute() accumulation code to implement these
changes.

---

547-547: ⚠️ Potential issue | 🟠 Major

Retry budget is off-by-one vs “retry up to 10 times”, and exhaustion context is vague.

With attempts = 10 and if attempts--; attempts > 0 at Line 618, the code performs at most 9 retries after the first interrupted attempt. Also, Line 685 should include the concrete retry count for clearer diagnostics.

Also applies to: 618-620, 685-685

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nl/nl_linux.go` at line 547, The retry loop uses "attempts" and the
conditional "if attempts--; attempts > 0" which causes one fewer retry than
intended; change the logic to perform exactly 10 retries by either initializing
a separate const retries = 10 and iterating with a clear loop (e.g. for i := 0;
i < retries; i++) or adjust the decrement/check to use >= 0 so the count matches
the intended number, and update the exhaustion error/log where the code reports
failure (the final error path that currently uses the "attempts" variable) to
include the concrete retry count (e.g., the configured retries constant or an
explicit attemptsPerformed value) for clearer diagnostics.

nl/nl_linux.go (1)

523-538: Add explicit retry-exhaustion context for ErrDumpInterrupted.

At Line 538, exhausted dump interruptions return a generic wrapped error without attempt count. Including attempt metadata would make CI/debugging much clearer.

Suggested diff

 func (req *NetlinkRequest) Execute(sockType int, resType uint16) ([][]byte, error) {
+	const maxAttempts = 10
 	var err error
-	for range 10 {
+	for range maxAttempts {
 		var res [][]byte
 		err = req.ExecuteIter(sockType, resType, func(msg []byte) bool {
 			res = append(res, msg)
 			return true
 		})
 		if errors.Is(err, ErrDumpInterrupted) {
 			continue
 		}
 		if err == nil {
 			return res, nil
 		}
 		break
 	}
 
+	if errors.Is(err, ErrDumpInterrupted) {
+		return nil, fmt.Errorf("execute netlink request interrupted after %d attempts: %w", maxAttempts, err)
+	}
 	return nil, fmt.Errorf("execute netlink request: %w", err)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nl/nl_linux.go` around lines 523 - 538, The loop swallows multiple
ErrDumpInterrupted attempts and returns a generic wrapped error; add a retry
counter and when ErrDumpInterrupted keeps occurring for all retries return an
error that includes the exhausted-attempts metadata. Modify the block that calls
req.ExecuteIter (the loop around req.ExecuteIter, ErrDumpInterrupted, res and
final return) to increment an attempts variable each iteration, and on final
failure format/Wrap the returned error to include the attempt count (e.g.,
"execute netlink request: %w (attempts=%d)") so consumers can see how many
retries occurred before giving up.