fix: crash when stopping LLDP receiver

This commit is contained in:
Siyuan 2025-11-06 15:27:36 +00:00
parent 3e39361aa7
commit 9f4acce279
5 changed files with 52 additions and 29 deletions

View File

@ -26,7 +26,6 @@ type LLDP struct {
enableRx bool enableRx bool
enableTx bool enableTx bool
packets chan gopacket.Packet
interfaceName string interfaceName string
advertiseOptions *AdvertiseOptions advertiseOptions *AdvertiseOptions
onChange func(neighbors []Neighbor) onChange func(neighbors []Neighbor)

View File

@ -3,7 +3,10 @@ package lldp
import ( import (
"context" "context"
"fmt" "fmt"
"io"
"net" "net"
"strings"
"syscall"
"time" "time"
"github.com/google/gopacket" "github.com/google/gopacket"
@ -42,13 +45,6 @@ func (l *LLDP) setUpCapture() error {
} }
logger.Info().Msg("created TPacketRx") 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 // set up multicast addresses
// otherwise the kernel might discard the packets // otherwise the kernel might discard the packets
// another workaround would be to enable promiscuous mode but that's too tricky // 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() l.mu.Unlock()
}() }()
packetChan := l.pktSourceRx.Packets() // TODO: use a channel to handle the packets
for { // PacketSource.Packets() is not reliable and can cause panics and the upstream hasn't fixed it yet
select { for rxCtx.Err() == nil {
case packet, ok := <-packetChan: if l.pktSourceRx == nil || l.tPacketRx == nil {
if !ok { logger.Error().Msg("packet source or TPacketRx not initialized")
logger.Info().Msg("packet source closed") break
return }
}
if err := l.handlePacket(packet, logger); err != nil { packet, err := l.pktSourceRx.NextPacket()
if err == nil {
if handleErr := l.handlePacket(packet, logger); handleErr != nil {
logger.Error(). logger.Error().
Err(err). Err(handleErr).
Msg("error handling packet") Msg("error handling packet")
} }
case <-rxCtx.Done(): continue
logger.Info().Msg("LLDP receiver stopped")
return
} }
// 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 { if rxCancel != nil {
rxCancel() rxCancel()
l.rxCancel = nil l.rxCancel = nil
logger.Info().Msg("cancelled RX context, waiting for goroutine to finish")
} }
// Wait a bit 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 { if l.tPacketRx != nil {
logger.Info().Msg("closing TPacketRx")
l.tPacketRx.Close() l.tPacketRx.Close()
l.tPacketRx = nil l.tPacketRx = nil
} }
if l.pktSourceRx != nil { if l.pktSourceRx != nil {
logger.Info().Msg("closing packet source")
l.pktSourceRx = nil l.pktSourceRx = nil
} }

View File

@ -54,7 +54,8 @@ type NetworkConfig struct {
} }
func (c *NetworkConfig) ShouldEnableLLDPTransmit() bool { 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 { func (c *NetworkConfig) ShouldEnableLLDPReceive() bool {

View File

@ -179,6 +179,7 @@ func initNetwork() error {
EnableTx: nc.ShouldEnableLLDPTransmit(), EnableTx: nc.ShouldEnableLLDPTransmit(),
AdvertiseOptions: advertiseOptions, AdvertiseOptions: advertiseOptions,
OnChange: func(neighbors []lldp.Neighbor) { OnChange: func(neighbors []lldp.Neighbor) {
// TODO: send deltas instead of the whole list
writeJSONRPCEvent("lldpNeighbors", neighbors, currentSession) writeJSONRPCEvent("lldpNeighbors", neighbors, currentSession)
}, },
Logger: networkLogger, Logger: networkLogger,

View File

@ -6,7 +6,7 @@ import dayjs from "dayjs";
import relativeTime from "dayjs/plugin/relativeTime"; import relativeTime from "dayjs/plugin/relativeTime";
import validator from "validator"; 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 { useJsonRpc } from "@hooks/useJsonRpc";
import AutoHeight from "@components/AutoHeight"; import AutoHeight from "@components/AutoHeight";
import { Button } from "@components/Button"; import { Button } from "@components/Button";
@ -98,11 +98,12 @@ export default function SettingsNetworkRoute() {
{ label: string; from: string; to: string }[] { label: string; from: string; to: string }[]
>([]); >([]);
const [lldpNeighbors, setLldpNeighbors] = useState<LLDPNeighbor[]>([]); const setLLDPNeighbors = useLLDPNeighborsStore(state => state.setNeighbors);
const lldpNeighbors = useLLDPNeighborsStore(state => state.neighbors);
const fetchLLDPNeighbors = useCallback(async () => { const fetchLLDPNeighbors = useCallback(async () => {
const neighbors = await getLLDPNeighbors(); const neighbors = await getLLDPNeighbors();
setLldpNeighbors(neighbors); setLLDPNeighbors(neighbors);
}, [setLldpNeighbors]); }, [setLLDPNeighbors]);
useEffect(() => { useEffect(() => {
fetchLLDPNeighbors(); fetchLLDPNeighbors();
@ -572,9 +573,9 @@ export default function SettingsNetworkRoute() {
/> />
</SettingsItem> </SettingsItem>
</div> </div>
<AutoHeight> {lldpNeighbors.length > 0 && <AutoHeight>
<LLDPNeighborsCard neighbors={lldpNeighbors} /> <LLDPNeighborsCard neighbors={lldpNeighbors} />
</AutoHeight> </AutoHeight>}
</div> </div>
) )
} }