[dhcpmon] Validate configured DHCP server fan-out

[Xichen96]

Description of PR

Validate DHCP relay forward-path fan-out against the number of configured DHCP
servers.

Work item tracking

• Microsoft ADO (number only): 38615655

Type of change

• Bug fix

Approach

Read the current server count from CONFIG_DB on each health check:

• DHCPv4 native: DHCPV4_RELAY|<VLAN> / dhcpv4_servers
• DHCPv4 legacy fallback: VLAN|<VLAN> / dhcp_servers
• DHCPv6: DHCP_RELAY|<VLAN> / dhcpv6_servers
• DHCPv6 compatibility fallback: VLAN|<VLAN> / dhcpv6_servers

Both raw leaf-list fields with an @ suffix and unsuffixed fields are accepted.

This PR adds two separate group-3 magnitude states on Agg-<VLAN>. It does
not change the group-1 relay-flow checks or group-2 management checks.

The new checks require:

• DHCPv4 Discover or Request TX delta equals its RX delta multiplied by the
  configured DHCPv4 server count.
• DHCPv6 Relay-Forward TX delta equals the selected forward-input RX delta
  multiplied by the configured DHCPv6 server count.

If no supported server list is found or there is no relevant activity, the
server-fan-out state is INDETERMINATE.

Stack

Depends on #107. Review the final commit only:

12257b7 [dhcpmon]: Validate configured server fan-out

Verification

No local compilation was used. 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).

[Copilot]

Pull request overview

This PR strengthens sonic-dhcpmon’s relay forward-path health validation by factoring in the number of DHCP servers configured in CONFIG_DB, so counter-based fan-out checks can detect partial relay forwarding (e.g., forwarding to only a subset of configured servers).

Changes:

• Add a utility to read the configured DHCP server count (v4/v6) from CONFIG_DB with table/field fallbacks.
• Update DHCPv4 positive health checks to require expected TX fan-out (RX * server_count) for Discover/Request when server configuration is available.
• Update DHCPv6 positive health checks to apply configured-server fan-out validation to Relay-Forward output while keeping return-path checks activity-based.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/util.h	Declares helper to retrieve configured DHCP server count from CONFIG_DB.
src/util.cpp	Implements CONFIG_DB server-list counting and selection logic for v4/v6 (with fallbacks).
src/dhcp_device.cpp	Applies configured-server fan-out validation to DHCPv4 and DHCPv6 positive health checks using counter deltas.

[mssonicbld]

/azp run

[azure-pipelines]

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

src/util.cpp:45

• get_config_list_count() assumes the CONFIG_DB value is a comma-separated string and returns 1 for any non-empty value, which miscounts YANG leaf-list raw fields like [] (empty JSON array) as 1 server. That can cause false unhealthy results by enforcing fan-out against a non-existent server list. Consider parsing JSON arrays when the value starts with '[' and treating an empty array as 0 entries, falling back to comma-counting only for legacy comma-separated strings.

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) {

src/dhcp_device.cpp:134

• get_counter_delta() subtracts snapshot from current using unsigned arithmetic. If a counter resets or wraps between snapshots (current < snapshot), this underflows and produces a huge delta, which can incorrectly mark the relay unhealthy. Clamp the delta to 0 when the counter did not increase.

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);
}

src/health_check.cpp:65

• The aggregate error log message says "packets received but none transmitted", but the new v6 relay-flow check can fail for other reasons (e.g., fan-out mismatch or reply transformation missing). This can mislead operators when v6 health fails. Consider making the message generic so it remains accurate for both v4 and v6 checks.

static dhcp_mon_status_t check_relay_flow_health_v6()
{
    return dhcp_device_get_status(agg_dev_all, DHCP_DEVICE_CHECK_RELAY_FLOW_V6);
}

[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

• get_config_list_count() counts commas to infer list length, but CONFIG_DB leaf-list fields with an '@' suffix are typically stored as JSON arrays (e.g., "[]", "['1.2.3.4']"). For an empty list "[]" this currently returns 1 (not 0), which will incorrectly force fan-out validation and can create false UNHEALTHY results. Consider parsing the '@' value as JSON and using the array size, then falling back to comma-counting only for legacy non-JSON fields.

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) {
        value = mConfigDbPtr->hget(key, field);
    }
    if (value == NULL || value->empty()) {
        return 0;
    }

    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).

[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

• get_config_list_count() counts DHCP servers by counting commas and assuming the field is non-empty. This overcounts when the CONFIG_DB list contains empty entries (e.g., trailing commas) and can mis-handle bracket/quote wrappers, causing a wrong server_count and false health-check failures.

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) {
        value = mConfigDbPtr->hget(key, field);
    }
    if (value == NULL || value->empty()) {
        return 0;
    }

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

[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:56

• get_config_list_count() counts servers by counting commas and assuming any non-empty string contains at least one entry. This miscounts empty list encodings like "[]" (returns 1) and can overcount when there are trailing/extra separators. That would make fan-out validation expect the wrong multiplier and can cause false health alerts.

    size_t count = 1;
    for (const char ch : *value) {
        if (ch == ',') {
            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/health_check.cpp:31

• The relay disparity log message is now misleading: the updated positive-health checks can mark the aggregate unhealthy due to fan-out mismatch (TX != RX * configured server count), not only when there is “none transmitted”. This will make alerts harder to interpret during the new validation behavior.

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).

[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 (3)

src/dhcp_device.cpp:228

• Same as the IPv4 fan-out check: returning INDETERMINATE when the DHCPv6 server list is absent can allow a previous unhealthy count to continue increasing and alerting even though the strict fan-out validation is now unsupported. Returning HEALTHY when server_count==0 disables this check cleanly and restores the intended fallback behavior.

    const size_t server_count = get_configured_dhcp_server_count(true);
    if (server_count == 0) {
        return DHCP_MON_STATUS_INDETERMINATE;
    }

src/dhcp_device.cpp:203

• When no supported DHCP server list is found, this check returns INDETERMINATE. In health_check.cpp, INDETERMINATE does not reset an existing unhealthy count, so a previous fan-out failure can keep counting/alerting even after the server list disappears, which contradicts the PR intent to fall back to the loose forward-path check when no list is present. Consider treating “no list present” as a disabled check that returns HEALTHY (so the state resets) rather than INDETERMINATE.

This issue also appears on line 225 of the same file.

    const size_t server_count = get_configured_dhcp_server_count(false);
    if (server_count == 0) {
        return DHCP_MON_STATUS_INDETERMINATE;
    }

src/dhcp_device.cpp:233

• The PR description says DHCPv6 fan-out should use a selected forward-input RX delta, but the current implementation sums Solicit/Request/Relay-Forward RX deltas. If Relay-Forward packets can be received in addition to client-originated Solicit/Request, summing can over-count the forward input and falsely flag a disparity. Consider selecting one input signal (e.g., the larger of client-originated vs relay-forward RX deltas) instead of summing them.

    uint64_t rx_delta = 0;
    for (const auto msg_type : monitored_v6_forward_rx_msgs) {
        rx_delta += get_counter_delta(ifname, rx_sock_v6, msg_type);
    }

[Xichen96]

Closing because configured DHCP server membership can change independently of the sampled packet window, so exact server-count fan-out is not a reliable health invariant.