Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 4 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,7 @@
## 2024-06-07 - [Fixed Worker Pools vs Semaphore Goroutines]
**Learning:** For distributing many tiny tasks (like port scans per IP, up to 65535 ports), launching a new goroutine for every single task and limiting concurrency via a channel semaphore introduces significant memory allocation and scheduling overhead. A benchmark showed that a fixed pool of workers iterating over an array using `atomic.AddInt32` is over 35x faster in raw scheduling overhead.
**Action:** When performing high-volume concurrent tasks over pre-known arrays, prefer spawning a fixed pool of `N` worker goroutines that pull tasks via an atomic index counter (`atomic.AddInt32`), rather than spinning up `M` goroutines constrained by an `N`-capacity semaphore channel.

## 2023-10-25 - Zero-allocation parsing of `/proc/net/arp`
**Learning:** In environments with heavily populated ARP tables, reading and converting `/proc/net/arp` into a string array using `strings.Split` and `strings.Fields` creates a significant O(N) memory allocation and garbage collection bottleneck. This degrades throughput heavily during highly concurrent per-IP discovery scans.
**Action:** Always prefer zero-allocation byte indexing functions (`bytes.Index`, `bytes.Fields`) when parsing structured Linux system files (`/proc/*`) sequentially, particularly inside highly concurrent routines or high-frequency loops. Avoid unnecessary `string(data)` type conversions that force complete memory duplication.
44 changes: 39 additions & 5 deletions pkg/discovery/os_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package discovery

import (
"bytes"
"context"
"fmt"
"net"
Expand Down Expand Up @@ -46,17 +47,50 @@ func osGetMAC(ip string) string {

// Fast path for Linux
if data, err := os.ReadFile("/proc/net/arp"); err == nil {
lines := strings.Split(string(data), "\n")
for _, line := range lines {
fields := strings.Fields(line)
if len(fields) >= 4 && fields[0] == ip {
mac := fields[3]
// ⚡ Bolt Optimization: Use zero-allocation bytes indexing to scan large ARP tables.
// Avoids massive strings.Split and strings.Fields memory overhead on systems with thousands of neighbors.
ipSpace := []byte("\n" + ip + " ")
ipTab := []byte("\n" + ip + "\t")

dataToSearch := data
for {
idx := bytes.Index(dataToSearch, ipSpace)
if idx == -1 {
idx = bytes.Index(dataToSearch, ipTab)
}

// Edge case: target is on the very first line without a preceding newline
if idx == -1 && (bytes.HasPrefix(dataToSearch, []byte(ip+" ")) || bytes.HasPrefix(dataToSearch, []byte(ip+"\t"))) {
idx = -1 // Indicates start of slice
} else if idx == -1 {
break
}

start := idx + 1
eol := bytes.IndexByte(dataToSearch[start:], '\n')
var line []byte
if eol == -1 {
line = dataToSearch[start:]
} else {
line = dataToSearch[start : start+eol]
}

fields := bytes.Fields(line)
if len(fields) >= 4 && string(fields[0]) == ip {
mac := string(fields[3])
// Ignore incomplete ARP entries
if mac != "00:00:00:00:00:00" {
return strings.ToUpper(strings.ReplaceAll(mac, ":", "-"))
}
break // Found the IP, but it's incomplete
}

if eol == -1 {
break
}
dataToSearch = dataToSearch[start+eol:]
}
Comment on lines +52 to 92

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are three critical issues with the current implementation of the zero-allocation ARP table parser:

  1. First-Line Override Bug (Correctness): If the target IP is on the very first line of /proc/net/arp and also appears later in the file, bytes.Index will find the later match (making idx != -1). This skips the idx == -1 check, completely ignoring the first line.
  2. Incomplete MAC Break Bug (Correctness): Breaking the loop when an incomplete MAC is found (break // Found the IP, but it's incomplete) prevents finding a valid, complete MAC entry for the same IP later in the file (e.g., on a different interface).
  3. Unnecessary Allocations (Efficiency): Converting fields[0] and fields[3] to strings inside the loop creates garbage collection overhead, defeating the "zero-allocation" goal. Using bytes.Equal with pre-allocated byte slices avoids this.

Here is a robust, fully optimized, and correct implementation that addresses all three issues.

		ipBytes := []byte(ip)
		ipSpace := []byte("\n" + ip + " ")
		ipTab := []byte("\n" + ip + "\t")
		incompleteMAC := []byte("00:00:00:00:00:00")

		dataToSearch := data
		for {
			var idx int
			matchedPrefix := false

			if bytes.HasPrefix(dataToSearch, []byte(ip+" ")) || bytes.HasPrefix(dataToSearch, []byte(ip+"\t")) {
				matchedPrefix = true
			} else {
				idxSpace := bytes.Index(dataToSearch, ipSpace)
				idxTab := bytes.Index(dataToSearch, ipTab)
				if idxSpace != -1 && idxTab != -1 {
					if idxSpace < idxTab {
						idx = idxSpace
					} else {
						idx = idxTab
					}
				} else if idxSpace != -1 {
					idx = idxSpace
				} else {
					idx = idxTab
				}

				if idxSpace == -1 && idxTab == -1 {
					break
				}
			}

			var start int
			if matchedPrefix {
				start = 0
			} else {
				start = idx + 1
			}

			eol := bytes.IndexByte(dataToSearch[start:], '\n')
			var line []byte
			if eol == -1 {
				line = dataToSearch[start:]
			} else {
				line = dataToSearch[start : start+eol]
			}

			fields := bytes.Fields(line)
			if len(fields) >= 4 && bytes.Equal(fields[0], ipBytes) {
				if !bytes.Equal(fields[3], incompleteMAC) {
					return strings.ToUpper(strings.ReplaceAll(string(fields[3]), ":", "-"))
				}
			}

			if eol == -1 {
				break
			}
			dataToSearch = dataToSearch[start+eol:]
		}


// If we successfully read /proc/net/arp but didn't find the IP (or it was incomplete),
// we know the MAC is unresolved. Do not fall back to `arp -an` which would just re-verify the same miss.
return ""
Expand Down
Loading