feat(healthcheck/icmp): use the same id and increasing sequence number

- easier debug logs flow
- MAY cost less conntrack-ed slots on the VPN server
- resets id and sequence to 1 when reconnecting VPN
- resets id and sequence to 1 after 5 minutes
- resets id and sequence to 1 if sequence is complete (65535)

internal/healthcheck/checker.go

@@ -74,6 +74,7 @@ func (c *Checker) Start(ctx context.Context) (runError <-chan error, err error)
 		// restore forced check type to dns if icmp was found to be not permitted
 		c.smallCheckType = smallCheckDNS
 	}
+	c.echoer.Reset()
 
 	err = c.startupCheck(ctx)
 	if err != nil {

internal/healthcheck/icmp/echo.go

@@ -12,6 +12,7 @@ import (
 	"net"
 	"net/netip"
 	"strings"
+	"time"
 
 	"golang.org/x/net/icmp"
 	"golang.org/x/net/ipv4"
@@ -27,6 +28,9 @@ type Echoer struct {
 	buffer       []byte
 	randomSource io.Reader
 	logger       Logger
+	seqStart     time.Time
+	id           int
+	seq          int
 }
 
 func NewEchoer(logger Logger) *Echoer {
@@ -42,12 +46,26 @@ func NewEchoer(logger Logger) *Echoer {
 	}
 }
 
+// Reset resets the [Echoer] icmp echo parameters:
+// - ID is assigned a new random value
+// - sequence is reset to 1
+// - sequence start time is set to now
+// It is used when the sequence is complete or when the VPN reconnects.
+func (e *Echoer) Reset() {
+	const uint16Bytes = 2
+	idBytes := make([]byte, uint16Bytes)
+	_, _ = e.randomSource.Read(idBytes)
+	e.id = int(binary.BigEndian.Uint16(idBytes))
+	e.seq = 1
+	e.seqStart = time.Now()
+}
+
 var (
 	ErrTimedOut     = errors.New("timed out waiting for ICMP echo reply")
 	ErrNotPermitted = errors.New("not permitted")
 )
 
-func (i *Echoer) Echo(ctx context.Context, ip netip.Addr) (err error) {
+func (e *Echoer) Echo(ctx context.Context, ip netip.Addr) (err error) {
 	var ipVersion string
 	var conn net.PacketConn
 	if ip.Is4() {
@@ -69,8 +87,13 @@ func (i *Echoer) Echo(ctx context.Context, ip netip.Addr) (err error) {
 		conn.Close()
 	}()
 
-	const echoDataSize = 32
+	const maxSeq = 1<<16 - 1
-	id, message := buildMessageToSend(ipVersion, echoDataSize, i.randomSource)
+	const refreshIDInterval = 5 * time.Minute
+	if e.seq > maxSeq && time.Since(e.seqStart) >= refreshIDInterval {
+		e.Reset()
+	}
+
+	message := buildMessageToSend(ipVersion, e.id, e.seq, e.randomSource)
 
 	encodedMessage, err := message.Marshal(nil)
 	if err != nil {
@@ -84,8 +107,11 @@ func (i *Echoer) Echo(ctx context.Context, ip netip.Addr) (err error) {
 		}
 		return fmt.Errorf("writing ICMP message to %s: %w", ip, err)
 	}
+	defer func() {
+		e.seq++
+	}()
 
-	receivedData, err := receiveEchoReply(conn, id, i.buffer, ipVersion, i.logger)
+	receivedData, err := receiveEchoReply(conn, e.id, e.seq, e.buffer, ipVersion, e.logger)
 	if err != nil {
 		if errors.Is(err, net.ErrClosed) && ctx.Err() != nil {
 			return fmt.Errorf("%w from %s", ErrTimedOut, ip)
@@ -101,12 +127,7 @@ func (i *Echoer) Echo(ctx context.Context, ip netip.Addr) (err error) {
 	return nil
 }
 
-func buildMessageToSend(ipVersion string, size uint, randomSource io.Reader) (id int, message *icmp.Message) {
+func buildMessageToSend(ipVersion string, id, seq int, randomSource io.Reader) (message *icmp.Message) {
-	const uint16Bytes = 2
-	idBytes := make([]byte, uint16Bytes)
-	_, _ = randomSource.Read(idBytes)
-	id = int(binary.BigEndian.Uint16(idBytes))
-
 	var icmpType icmp.Type
 	switch ipVersion {
 	case "v4":
@@ -116,6 +137,7 @@ func buildMessageToSend(ipVersion string, size uint, randomSource io.Reader) (id
 	default:
 		panic(fmt.Sprintf("IP version %q not supported", ipVersion))
 	}
+	const size = 32
 	messageBodyData := make([]byte, size)
 	_, _ = randomSource.Read(messageBodyData)
 
@@ -126,14 +148,14 @@ func buildMessageToSend(ipVersion string, size uint, randomSource io.Reader) (id
 		Checksum: 0,        // calculated at encoding (ipv4) or sending (ipv6)
 		Body: &icmp.Echo{
 			ID:   id,
-			Seq:  0, // only one packet
+			Seq:  seq,
 			Data: messageBodyData,
 		},
 	}
-	return id, message
+	return message
 }
 
-func receiveEchoReply(conn net.PacketConn, id int, buffer []byte, ipVersion string, logger Logger,
+func receiveEchoReply(conn net.PacketConn, id, seq int, buffer []byte, ipVersion string, logger Logger,
 ) (data []byte, err error) {
 	var icmpProtocol int
 	const (
@@ -167,27 +189,35 @@ func receiveEchoReply(conn net.PacketConn, id int, buffer []byte, ipVersion stri
 
 		switch body := message.Body.(type) {
 		case *icmp.Echo:
-			if id != body.ID {
+			switch {
+			case id != body.ID:
 				logger.Warnf("ignoring ICMP echo reply mismatching expected id %d "+
-					"(id: %d, type: %d, code: %d, length: %d, return address %s)",
+					"(id: %d, seq: %d, type: %d, code: %d, length: %d, return address %s)",
-					id, body.ID, message.Type, message.Code, len(packetBytes), returnAddr)
+					id, body.Seq, body.ID, message.Type, message.Code, len(packetBytes), returnAddr)
 				continue // not the ID we are looking for
+			case seq != body.Seq:
+				logger.Warnf("ignoring ICMP echo reply mismatching expected sequence number %d "+
+					"(id: %d, seq: %d, type: %d, code: %d, length: %d, return address %s)",
+					seq, body.ID, body.Seq, message.Type, message.Code, len(packetBytes), returnAddr)
+				continue // not the seq we are looking for
 			}
 			return body.Data, nil
 		case *icmp.DstUnreach:
-			logger.Debugf("ignoring ICMP destination unreachable message (type: 3, code: %d, return address %s, expected-id %d)",
+			logger.Debugf("ignoring ICMP destination unreachable message "+
-				message.Code, returnAddr, id)
+				"(type: 3, code: %d, return address %s, expected id %d and seq %d)",
+				message.Code, returnAddr, id, seq)
 			// See https://github.com/qdm12/gluetun/pull/2923#issuecomment-3377532249
 			// on why we ignore this message. If it is actually unreachable, the timeout on waiting for
 			// the echo reply will do instead of returning an error error.
 			continue
 		case *icmp.TimeExceeded:
-			logger.Debugf("ignoring ICMP time exceeded message (type: 11, code: %d, return address %s, expected-id %d)",
+			logger.Debugf("ignoring ICMP time exceeded message "+
-				message.Code, returnAddr, id)
+				"(type: 11, code: %d, return address %s, expected id %d and seq %d)",
+				message.Code, returnAddr, id, seq)
 			continue
 		default:
-			return nil, fmt.Errorf("%w: %T (type %d, code %d, return address %s, expected-id %d)",
+			return nil, fmt.Errorf("%w: %T (type %d, code %d, return address %s, expected id %d and seq %d)",
-				ErrICMPBodyUnsupported, body, message.Type, message.Code, returnAddr, id)
+				ErrICMPBodyUnsupported, body, message.Type, message.Code, returnAddr, id, seq)
 		}
 	}
 }