diff --git a/internal/lldp/lldp.go b/internal/lldp/lldp.go index 450e06d2..e9c24986 100644 --- a/internal/lldp/lldp.go +++ b/internal/lldp/lldp.go @@ -26,7 +26,6 @@ type LLDP struct { enableRx bool enableTx bool - packets chan gopacket.Packet interfaceName string advertiseOptions *AdvertiseOptions onChange func(neighbors []Neighbor) diff --git a/internal/lldp/rx.go b/internal/lldp/rx.go index 1ea35156..fbb38b4e 100644 --- a/internal/lldp/rx.go +++ b/internal/lldp/rx.go @@ -3,7 +3,10 @@ package lldp import ( "context" "fmt" + "io" "net" + "strings" + "syscall" "time" "github.com/google/gopacket" @@ -42,13 +45,6 @@ func (l *LLDP) setUpCapture() error { } logger.Info().Msg("created TPacketRx") - // Double-check: another goroutine might have set it up while we were creating - if l.tPacketRx != nil { - // Another goroutine already set it up, close our instance - tPacketRx.Close() - return nil - } - // set up multicast addresses // otherwise the kernel might discard the packets // another workaround would be to enable promiscuous mode but that's too tricky @@ -98,23 +94,44 @@ func (l *LLDP) doCapture(logger *zerolog.Logger, rxCtx context.Context) { l.mu.Unlock() }() - packetChan := l.pktSourceRx.Packets() - for { - select { - case packet, ok := <-packetChan: - if !ok { - logger.Info().Msg("packet source closed") - return - } - if err := l.handlePacket(packet, logger); err != nil { + // 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 rxCtx.Err() == nil { + if l.pktSourceRx == nil || l.tPacketRx == nil { + logger.Error().Msg("packet source or TPacketRx not initialized") + break + } + + packet, err := l.pktSourceRx.NextPacket() + if err == nil { + if handleErr := l.handlePacket(packet, logger); handleErr != nil { logger.Error(). - Err(err). + Err(handleErr). Msg("error handling packet") } - case <-rxCtx.Done(): - logger.Info().Msg("LLDP receiver stopped") - return + continue } + + // 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") } } @@ -348,17 +365,21 @@ func (l *LLDP) stopCapture() error { if rxCancel != nil { rxCancel() l.rxCancel = nil + + logger.Info().Msg("cancelled RX context, waiting for goroutine to finish") } // Wait a bit for goroutine to finish - time.Sleep(1000 * time.Millisecond) + time.Sleep(500 * time.Millisecond) if l.tPacketRx != nil { + logger.Info().Msg("closing TPacketRx") l.tPacketRx.Close() l.tPacketRx = nil } if l.pktSourceRx != nil { + logger.Info().Msg("closing packet source") l.pktSourceRx = nil } diff --git a/internal/network/types/config.go b/internal/network/types/config.go index bc6a6900..841eaafa 100644 --- a/internal/network/types/config.go +++ b/internal/network/types/config.go @@ -54,7 +54,8 @@ type NetworkConfig struct { } func (c *NetworkConfig) ShouldEnableLLDPTransmit() bool { - return c.LLDPMode.String != "rx_only" && c.LLDPMode.String != "disabled" + // backwards compatibility: `basic` mode will be `rx_only` due to privacy concerns + return c.LLDPMode.String != "rx_only" && c.LLDPMode.String != "disabled" && c.LLDPMode.String != "basic" } func (c *NetworkConfig) ShouldEnableLLDPReceive() bool { diff --git a/network.go b/network.go index 30c76f53..4213b486 100644 --- a/network.go +++ b/network.go @@ -179,6 +179,7 @@ func initNetwork() error { EnableTx: nc.ShouldEnableLLDPTransmit(), AdvertiseOptions: advertiseOptions, OnChange: func(neighbors []lldp.Neighbor) { + // TODO: send deltas instead of the whole list writeJSONRPCEvent("lldpNeighbors", neighbors, currentSession) }, Logger: networkLogger, diff --git a/ui/src/routes/devices.$id.settings.network.tsx b/ui/src/routes/devices.$id.settings.network.tsx index d1b4a541..fda192b8 100644 --- a/ui/src/routes/devices.$id.settings.network.tsx +++ b/ui/src/routes/devices.$id.settings.network.tsx @@ -6,7 +6,7 @@ import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; import validator from "validator"; -import { LLDPNeighbor, NetworkSettings, NetworkState, useNetworkStateStore, useRTCStore } from "@hooks/stores"; +import { NetworkSettings, NetworkState, useLLDPNeighborsStore, useNetworkStateStore, useRTCStore } from "@hooks/stores"; import { useJsonRpc } from "@hooks/useJsonRpc"; import AutoHeight from "@components/AutoHeight"; import { Button } from "@components/Button"; @@ -98,11 +98,12 @@ export default function SettingsNetworkRoute() { { label: string; from: string; to: string }[] >([]); - const [lldpNeighbors, setLldpNeighbors] = useState([]); + const setLLDPNeighbors = useLLDPNeighborsStore(state => state.setNeighbors); + const lldpNeighbors = useLLDPNeighborsStore(state => state.neighbors); const fetchLLDPNeighbors = useCallback(async () => { const neighbors = await getLLDPNeighbors(); - setLldpNeighbors(neighbors); - }, [setLldpNeighbors]); + setLLDPNeighbors(neighbors); + }, [setLLDPNeighbors]); useEffect(() => { fetchLLDPNeighbors(); @@ -572,9 +573,9 @@ export default function SettingsNetworkRoute() { /> - + {lldpNeighbors.length > 0 && - + } ) }