internal/lldp/lldp.go

@@ -30,17 +30,15 @@ type LLDP struct {
 	advertiseOptions *AdvertiseOptions
 	onChange         func(neighbors []Neighbor)
 
-	neighbors *ttlcache.Cache[neighborCacheKey, Neighbor]
+	neighbors *ttlcache.Cache[string, Neighbor]
 
 	// State tracking
+	rxRunning bool
 	txRunning bool
 	txCtx     context.Context
 	txCancel  context.CancelFunc
-
-	rxRunning   bool
-	rxWaitGroup *sync.WaitGroup
-	rxCtx       context.Context
-	rxCancel    context.CancelFunc
+	rxCtx     context.Context
+	rxCancel  context.CancelFunc
 }
 
 type AdvertiseOptions struct {
@@ -74,9 +72,8 @@ func NewLLDP(opts *Options) *LLDP {
 		advertiseOptions: opts.AdvertiseOptions,
 		enableRx:         opts.EnableRx,
 		enableTx:         opts.EnableTx,
-		rxWaitGroup:      &sync.WaitGroup{},
 		l:                opts.Logger,
-		neighbors:        ttlcache.New(ttlcache.WithTTL[neighborCacheKey, Neighbor](1 * time.Hour)),
+		neighbors:        ttlcache.New(ttlcache.WithTTL[string, Neighbor](1 * time.Hour)),
 		onChange:         opts.OnChange,
 	}
 }

internal/lldp/neigh.go

@@ -1,6 +1,7 @@
 package lldp
 
 import (
+	"fmt"
 	"time"
 )
 
@@ -26,28 +27,22 @@ type Neighbor struct {
 	Values            map[string]string  `json:"values"`
 }
 
-type neighborCacheKey struct {
-	mac    string
-	source string
-}
-
-func (n *Neighbor) cacheKey() neighborCacheKey {
-	return neighborCacheKey{mac: n.Mac, source: n.Source}
+func (n *Neighbor) cacheKey() string {
+	return fmt.Sprintf("%s-%s", n.Mac, n.Source)
 }
 
 func (l *LLDP) addNeighbor(neighbor *Neighbor, ttl time.Duration) {
 	logger := l.l.With().
-		Str("source", neighbor.Source).
 		Str("mac", neighbor.Mac).
 		Interface("neighbor", neighbor).
 		Logger()
 
 	key := neighbor.cacheKey()
 
-	currentNeighbor := l.neighbors.Get(key)
-	if currentNeighbor != nil {
-		currentSource := currentNeighbor.Value().Source
-		if currentSource == "lldp" && neighbor.Source != "lldp" {
+	current_neigh := l.neighbors.Get(key)
+	if current_neigh != nil {
+		current_source := current_neigh.Value().Source
+		if current_source == "lldp" && neighbor.Source != "lldp" {
 			logger.Info().Msg("skip updating neighbor, as LLDP has higher priority")
 			return
 		}
@@ -61,7 +56,6 @@ func (l *LLDP) addNeighbor(neighbor *Neighbor, ttl time.Duration) {
 
 func (l *LLDP) deleteNeighbor(neighbor *Neighbor) {
 	logger := l.l.With().
-		Str("source", neighbor.Source).
 		Str("mac", neighbor.Mac).
 		Logger()
 

internal/lldp/rx.go

@@ -87,47 +87,51 @@ func (l *LLDP) setUpCapture() error {
 	return nil
 }
 
-func (l *LLDP) doCapture(logger *zerolog.Logger) {
-	l.rxWaitGroup.Add(1)
-	defer l.rxWaitGroup.Done()
+func (l *LLDP) doCapture(logger *zerolog.Logger, rxCtx context.Context) {
+	defer func() {
+		l.mu.Lock()
+		l.rxRunning = false
+		l.mu.Unlock()
+	}()
 
 	// TODO: use a channel to handle the packets
 	// PacketSource.Packets() is not reliable and can cause panics and the upstream hasn't fixed it yet
-	for {
-		// check if the context is done before blocking call
-		select {
-		case <-l.rxCtx.Done():
-			logger.Info().Msg("RX context cancelled")
-			return
-		default:
+	for rxCtx.Err() == nil {
+		if l.pktSourceRx == nil || l.tPacketRx == nil {
+			logger.Error().Msg("packet source or TPacketRx not initialized")
+			break
 		}
 
-		logger.Trace().Msg("waiting for next packet")
 		packet, err := l.pktSourceRx.NextPacket()
-		logger.Trace().Interface("packet", packet).Err(err).Msg("got next packet")
-
-		if err != nil {
-			logger.Error().
-				Err(err).
-				Msg("error getting next packet")
-
-			// Immediately break for known unrecoverable errors
-			if err == io.EOF || err == io.ErrUnexpectedEOF ||
-				err == io.ErrNoProgress || err == io.ErrClosedPipe || err == io.ErrShortBuffer ||
-				err == syscall.EBADF ||
-				strings.Contains(err.Error(), "use of closed file") {
-				return
+		if err == nil {
+			if handleErr := l.handlePacket(packet, logger); handleErr != nil {
+				logger.Error().
+					Err(handleErr).
+					Msg("error handling packet")
 			}
-
 			continue
 		}
 
-		if err := l.handlePacket(packet, logger); err != nil {
-			logger.Error().
-				Err(err).
-				Msg("error handling packet")
+		// Immediately retry for temporary network errors and EAGAIN
+		// temporary has been deprecated and most cases are timeouts
+		if nerr, ok := err.(net.Error); ok && nerr.Timeout() {
 			continue
 		}
+		if err == syscall.EAGAIN {
+			continue
+		}
+
+		// Immediately break for known unrecoverable errors
+		if err == io.EOF || err == io.ErrUnexpectedEOF ||
+			err == io.ErrNoProgress || err == io.ErrClosedPipe || err == io.ErrShortBuffer ||
+			err == syscall.EBADF ||
+			strings.Contains(err.Error(), "use of closed file") {
+			break
+		}
+
+		logger.Error().
+			Err(err).
+			Msg("error receiving LLDP packet")
 	}
 }
 
@@ -154,7 +158,9 @@ func (l *LLDP) startCapture() error {
 	l.rxCtx, l.rxCancel = context.WithCancel(context.Background())
 	l.rxRunning = true
 
-	go l.doCapture(&logger)
+	// Capture context in closure
+	rxCtx := l.rxCtx
+	go l.doCapture(&logger, rxCtx)
 
 	return nil
 }
@@ -363,24 +369,8 @@ func (l *LLDP) stopCapture() error {
 		logger.Info().Msg("cancelled RX context, waiting for goroutine to finish")
 	}
 
-	// stop the TPacketRx
-	go func() {
-		if l.tPacketRx == nil {
-			return
-		}
-
-		// write an empty packet to the TPacketRx to interrupt the blocking read
-		// it's a shitty workaround until https://github.com/google/gopacket/pull/777 is merged,
-		// or we have a better solution, see https://github.com/google/gopacket/issues/1064
-		l.tPacketRx.WritePacketData([]byte{})
-	}()
-
-	// wait for the goroutine to finish
-	start := time.Now()
-	l.rxWaitGroup.Wait()
-	logger.Info().Dur("duration", time.Since(start)).Msg("RX goroutine finished")
-
-	l.rxRunning = false
+	// Wait a bit for goroutine to finish
+	time.Sleep(500 * time.Millisecond)
 
 	if l.tPacketRx != nil {
 		logger.Info().Msg("closing TPacketRx")

internal/lldp/tx.go

@@ -60,10 +60,10 @@ var (
 func toLLDPCapabilitiesBytes(capabilities []string) uint16 {
 	r := uint16(0)
 	for _, capability := range capabilities {
-		mask, ok := capabilityMap[capability]
-		if ok {
-			r |= mask
+		if _, ok := capabilityMap[capability]; !ok {
+			continue
 		}
+		r |= capabilityMap[capability]
 	}
 	return r
 }