arp fixes for #70 (#71)

arp/handler.go

@@ -73,6 +73,9 @@ type queryResult struct {
 	hwaddr    []byte
 	dstHw     []byte
 	querysent bool
+	// inc tracks amount of times the query survived compaction.
+	// The queries with higher inc will be discarded first.
+	inc uint16
 }
 
 func (qr *queryResult) destroy() {
@@ -126,8 +129,16 @@ func (h *Handler) DiscardQuery(protoAddr []byte) error {
 
 func (h *Handler) compactQueries() {
 	validOff := 0
+	maxIdx := -1
+	maxInc := uint16(0)
 	for i := 0; i < len(h.queries); i++ {
-		if !h.queries[i].isInvalid() {
+		discard := h.queries[i].isInvalid()
+		if !discard {
+			h.queries[i].inc++
+			if h.queries[i].inc > maxInc {
+				maxInc = h.queries[i].inc
+				maxIdx = i
+			}
 			if i != validOff {
 				// We swap the queries here so that when `StartQuery` extends
 				// queries slice, we don't have sharing of the internal structures.
@@ -138,6 +149,9 @@ func (h *Handler) compactQueries() {
 			validOff++
 		}
 	}
+	if validOff == len(h.queries) && maxIdx >= 0 {
+		h.queries[maxIdx].destroy() // Destroy oldest query if unable to compact.
+	}
 	h.queries = h.queries[:validOff]
 }
 
@@ -149,7 +163,7 @@ func (h *Handler) StartQuery(dstHWAddr, proto []byte) error {
 	if len(h.queries) == cap(h.queries) {
 		h.compactQueries()
 		if len(h.queries) == cap(h.queries) {
-			return lneto.ErrExhausted
+			return lneto.ErrExhausted // Should never fail.
 		}
 	}
 	if len(proto) != len(h.ourProtoAddr) {
@@ -159,8 +173,7 @@ func (h *Handler) StartQuery(dstHWAddr, proto []byte) error {
 	} else if dstHWAddr != nil && !internal.IsZeroed(dstHWAddr...) {
 		return lneto.ErrInvalidConfig
 	}
-	h.queries = h.queries[:len(h.queries)+1]
-	q := &h.queries[len(h.queries)-1]
+	q := internal.SliceReclaim(&h.queries)
 	*q = queryResult{
 		protoaddr: append(q.protoaddr[:0], proto...),
 		hwaddr:    q.hwaddr[:0],
@@ -255,7 +268,10 @@ func (h *Handler) Demux(ethFrame []byte, frameOffset int) error {
 					if !internal.IsZeroed(q.dstHw...) {
 						internal.LogAttrs(nil, slog.LevelError, "race-condition:ARP-reused-buffer")
 					}
-					copy(q.dstHw, hwaddr) // External write to user buffer.
+					// External write to user buffer.
+					// Copy data and free up this memory.
+					copy(q.dstHw, hwaddr)
+					q.inc = 10000
 				}
 				return nil
 			}

dhcpv4/dhcp_test.go

@@ -1,9 +1,10 @@
 package dhcpv4
 
 import (
-	"bytes"
 	"net/netip"
 	"testing"
+
+	"github.com/soypat/lneto/internal"
 )
 
 func TestClientServer(t *testing.T) {
@@ -516,7 +517,7 @@ func TestRequestedIPNotSentWhenInvalid(t *testing.T) {
 	// it should be included. This test documents expected behavior.
 	if foundRequestedIP {
 		// Verify the value matches what was requested
-		if !bytes.Equal(ipValue[:], []byte{0, 0, 0, 0}) {
+		if !internal.BytesEqual(ipValue[:], []byte{0, 0, 0, 0}) {
 			t.Errorf("unexpected requested IP value: %v", ipValue)
 		}
 	}

lneto_test.go

@@ -1,12 +1,12 @@
 package lneto_test
 
 import (
-	"bytes"
 	"math/rand"
 	"testing"
 
 	"github.com/soypat/lneto"
 	"github.com/soypat/lneto/ethernet"
+	"github.com/soypat/lneto/internal"
 	"github.com/soypat/lneto/internal/ltesto"
 	"github.com/soypat/lneto/ipv4"
 	"github.com/soypat/lneto/tcp"
@@ -29,7 +29,7 @@ func TestTCPMarshalUnmarshal(t *testing.T) {
 		})
 		dst = dst[:len(src)]
 		testMoveTCPPacket(t, src, dst)
-		if !bytes.Equal(src, dst) {
+		if !internal.BytesEqual(src, dst) {
 			t.Fatal("mismatching data")
 		}
 	}
@@ -88,15 +88,15 @@ func testMoveTCPPacket(t *testing.T, src, dst []byte) {
 	copy(tfrm2.Payload(), tfrm.Payload())
 
 	elen := efrm.HeaderLength()
-	if !bytes.Equal(src[:elen], dst[:elen]) {
+	if !internal.BytesEqual(src[:elen], dst[:elen]) {
 		t.Fatalf("Ethernet header mismatch\n%x\n%x", src[:elen], dst[:elen])
 	}
 	ilen := ifrm.HeaderLength()
-	if !bytes.Equal(src[elen:elen+20], dst[elen:elen+20]) {
+	if !internal.BytesEqual(src[elen:elen+20], dst[elen:elen+20]) {
 		t.Fatalf("IPv4 header mismatch\n%x\n%x", src[elen:elen+20], dst[elen:elen+20])
 	}
 	ipoptLen := len(ifrm.Options())
-	if !bytes.Equal(ifrm.Options(), ifrm2.Options()) {
+	if !internal.BytesEqual(ifrm.Options(), ifrm2.Options()) {
 		t.Fatalf("IPv4 options mismatch\n%x\n%x", ifrm.Options(), ifrm2.Options())
 	} else if ipoptLen > 0 && &ifrm.Options()[0] != &src[elen+20] {
 		t.Fatal("IPv4 options start pointer mismatch")
@@ -104,12 +104,12 @@ func testMoveTCPPacket(t *testing.T, src, dst []byte) {
 
 	tlen := tfrm.HeaderLength()
 	toff := elen + ilen + ipoptLen
-	if !bytes.Equal(src[toff:toff+tlen], dst[toff:toff+tlen]) {
+	if !internal.BytesEqual(src[toff:toff+tlen], dst[toff:toff+tlen]) {
 		t.Fatalf("TCP header mismatch\n%x\n%x", src[toff:toff+tlen], dst[toff:toff+tlen])
 	}
 	payload := tfrm.Payload()
 
-	if !bytes.Equal(payload, tfrm2.Payload()) {
+	if !internal.BytesEqual(payload, tfrm2.Payload()) {
 		t.Fatalf("payload mismatch %d %d", len(payload), len(tfrm2.Payload()))
 	}
 }

x/xnet/stack-async.go

@@ -358,14 +358,20 @@ func (s *StackAsync) DialTCP(conn *tcp.Conn, localPort uint16, addrp netip.AddrP
 	defer s.mu.Unlock()
 	var mac []byte
 	if s.subnet.Contains(addrp.Addr()) {
-		mac = make([]byte, 6)
 		ip := addrp.Addr().As4()
-		// StartQuery starts an ARP query for addresses in this network.
-		// On finishing query MAC is set and thus the StackPort will allow encapsulating
-		// data on that connection.
-		err = s.arp.StartQuery(mac, ip[:])
-		if err != nil {
-			return err
+		hw, err := s.arp.QueryResult(ip[:])
+		mac = make([]byte, 6)
+		if err == nil {
+			// Query exists, use pre-existing result.
+			copy(mac, hw)
+		} else {
+			// StartQuery starts an ARP query for addresses in this network.
+			// On finishing query MAC is set and thus the StackPort will allow encapsulating
+			// data on that connection.
+			err = s.arp.StartQuery(mac, ip[:])
+			if err != nil {
+				return err
+			}
 		}
 	}
 	err = conn.OpenActive(localPort, addrp, tcp.Value(s.prand32()))

x/xnet/xnet_test.go

@@ -587,9 +587,8 @@ func (tst *tester) ARPExchangeOnly(querying, target *StackAsync) {
 	// === PHASE 3: Verify querying stack learned target's MAC ===
 	resolvedHw, err := querying.ResultResolveHardwareAddress6(tgtIP)
 	if err != nil {
-		t.Fatalf("ARP query result failed: %v", err)
-	}
-	if resolvedHw != tgtHw {
+		t.Errorf("ARP query result failed: %v", err)
+	} else if resolvedHw != tgtHw {
 		t.Errorf("ARP resolved wrong MAC: got %x, want %x", resolvedHw, tgtHw)
 	}
 }