[dhcpmon] Tolerate one packet across health windows

[Xichen96]

Description of PR

Ignore a relay-health discrepancy caused by one logical packet crossing either
side of the health snapshot boundary.

Work item tracking

• Microsoft ADO (number only): 38615655

Type of change

• Bug fix

Approach

This PR is stateless. It evaluates only the current counter window and does not
carry an old discrepancy into later windows.

The internal constant defaults to:

INFLIGHT_PACKET_TOLERANCE = 1

Changing it to 0 restores strict comparisons. Increasing it adjusts every
snapshot allowance without changing an external interface.

For a fixed output ratio R, tolerance T, and current input delta N, the
accepted output range is:

[max(0, N - T) × R, (N + T) × R]

This permits one current packet to be partially processed or one packet from
the prior window to finish during the current window.

The bounded comparison is applied to:

• Exact parent/member RX and TX relationships.
• IPv4 downstream VLAN TX unicast-or-broadcast fan-out. With tolerance zero,
  every packet must contribute exactly one or all member observations. With
  tolerance enabled, aggregate observations remain within the corresponding
  lower and upper member-count bounds.

Group-1 relay-flow checks retain their existing loose activity semantics:

• A DHCPv4 DORA message type may have up to the configured RX tolerance with
  no same-type TX activity.
• DHCPv6 forward and reply transformations may each have up to the configured
  RX tolerance with no corresponding TX activity.

Group-2 management-interface checks remain strict: any monitored TX activity
is unhealthy.

Configured-server-count validation is intentionally excluded. It should be
introduced only after dhcpmon discovers and reconciles live configuration
changes consistently.

This replaces closed PRs #103, #104, and #105.

Stack

Depends on #107. Review the final commit only:

ed3a5f8 [dhcpmon]: Tolerate one packet across health windows

Isolated review

• Only the changes added after #107
• Single commit

Verification

• git diff --check
• Independent adversarial logic review
• No local compilation; Azure PR CI will validate the branch

Back port request

None. This targets master only.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Xichen96]

Isolated stacked diff (only #109 changes after #108): Xichen96/sonic-dhcpmon@dev/xichenlin/validate-configured-server-fanout...dev/xichenlin/tolerate-one-inflight-packet

[Copilot]

Pull request overview

This PR updates sonic-dhcpmon’s relay health evaluation to tolerate a ±1 “logical packet” mismatch across health snapshot windows, reducing false DHCP relay discrepancy alerts caused by packets that straddle window boundaries. It also refactors how interface health checks are enumerated and strengthens expected RX/TX and parent/member aggregate relationships using bounded ratios (including configured-server fan-out).

Changes:

• Implement “within one packet” tolerance for DHCPv4 DORA and DHCPv6 forward/reply relationships, including configured-server fan-out where available.
• Add CONFIG_DB-based DHCP server list counting to derive expected fan-out ratios during health checks.
• Refactor health-check state from fixed function-pointer table to an initialized per-interface state vector, and introduce device-manager helpers for parent/aggregate naming.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/util.h	Declares configured DHCP server-count helper and simplifies aggregate-counter naming helper.
src/util.cpp	Implements CONFIG_DB server list counting and exposes configured DHCP server-count query.
src/packet_handler.cpp	Switches aggregate counter attribution to device-manager aggregation resolution.
src/health_check.h	Refactors health state representation and adds initializer API for health states.
src/health_check.cpp	Builds per-interface health-check state list at init and logs per-check threshold errors uniformly.
src/dhcp_mon.cpp	Uses new aggregate-counter naming and initializes relay health state after counter initialization.
src/dhcp_devman.h	Adds APIs for parent resolution and aggregate-counter resolution for tracked interfaces.
src/dhcp_devman.cpp	Implements parent/context traversal with depth guard; refines PortChannel membership handling.
src/dhcp_device.h	Replaces legacy aggregate check enum variants with consolidated AGG_RX/TX (v4/v6) check types.
src/dhcp_device.cpp	Implements ratio-based “±1 packet” tolerance for relay and aggregate health relationships.
src/dhcp_check_profile_relay.cpp	Updates DHCPv4 reply TX profile to allow unicast Offer/Ack while keeping NAK broadcast-only.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/util.cpp:58

• Counting list entries by starting at 1 and incrementing on every comma will over-count when the CONFIG_DB field contains empty elements (e.g., trailing comma, consecutive commas) or whitespace-only elements. That can produce an incorrect configured server count, which directly affects the new health-check ratio/tolerance logic and can cause false health results.

    size_t count = 1;
    for (const char ch : *value) {
        if (ch == ',') {
            count++;
        }
    }
    return count;

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/util.cpp:59

• The server-count logic counts commas and assumes any non-empty string represents at least one entry. This miscounts cases like an empty leaf-list representation (e.g. "[]"), trailing commas, or whitespace-only values, which can cause an incorrect server_count and therefore incorrect health decisions. Parse the list and count only non-empty tokens (optionally handling both "a,b" and JSON-ish "["a","b"]" forms).

static size_t get_config_list_count(const std::string &table, const std::string &field)
{
    const std::string key = table + "|" + downstream_ifname;
    auto value = mConfigDbPtr->hget(key, field + "@");
    if (value == NULL) {

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/health_check.cpp:31

• relay_disparity_error is now used for the positive v4/v6 health checks that validate RX/TX ratios (including configured-server fan-out and other mismatch cases), not only the "received but none transmitted" scenario. This makes the syslog message misleading when the check fails for reasons other than zero TX.

static const char relay_disparity_error[] =
    "dhcpmon detected DHCPv4/v6 packets received but none transmitted for intf: %s. Duration: %d (sec)";

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/dhcp_device.cpp:96

• get_counter_delta() subtracts unsigned counters directly; if a counter is cleared or otherwise decreases below the snapshot value, this underflows and produces a huge delta, which can spuriously trigger health-check failures (e.g., via check_counter_increased and aggregate comparisons). Clamp decreases to 0 (or treat them as a reset) before subtracting.

static uint64_t get_counter_delta(const std::string &ifname, int sock, int msg_type)
{
    const sock_info_t &sock_info = sock_mgr_get_sock_info(sock);
    return sock_info.all_counters.at(ifname).at(msg_type) -
           sock_info.all_counters_snapshot.at(ifname).at(msg_type);
}

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/dhcp_device.cpp:96

• get_counter_delta() subtracts snapshot from current using unsigned arithmetic. If cache counters are resynced/reset (e.g., via DB sync paths) such that current < snapshot, this underflows and produces a huge delta, which can incorrectly mark health checks unhealthy/healthy and skew tolerance calculations. Clamp the delta to 0 when current < snapshot (or otherwise handle resets explicitly).

{
    const sock_info_t &sock_info = sock_mgr_get_sock_info(sock);
    return sock_info.all_counters.at(ifname).at(msg_type) -
           sock_info.all_counters_snapshot.at(ifname).at(msg_type);
}

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.