RDKCOM-5619: RDKBNETWOR-99 Implement Dynamic L2/L3 Packet Marking Framework

[sherik-sensin]

Dependency:
rdkcentral/wan-manager#223
rdkcentral/vlan-manager#41

[github-actions]

📋 PR Format Reminder

• Description missing:
  • Reason for change
  • Test Procedure
  • Risks (Low / Medium / High)
  • Priority (P0 / P1 / P2)

Expected:

TICKET-123 : brief description

Reason for change: why
Test Procedure: how to verify
Risks: Low / Medium / High
Priority: P0 / P1 / P2

[Copilot]

Pull request overview

This PR introduces WAN Manager–driven QoS marking rule generation in the firewall subsystem, emitting DSCP and CLASSIFY rules into the iptables mangle table for both IPv4 and IPv6 when the WAN service is ready.

Changes:

• Adds a new add_qos_skb_mark(FILE*, int family) API (guarded by FEATURE_WANMGR_L2_MARKING) to build DSCP/CLASSIFY rules based on WanManager TR-181 marking entries.
• Invokes the new QoS rule emission from the IPv4 mangle table setup and from the IPv6 filter path.
• Introduces WanManager TR-181 parameter name constants and protocol-specific rule generation (DNS/NTP/HTTP/VoIP/IPTV/IGMP/MLD/etc.).

Reviewed changes

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

File	Description
source/firewall/firewall.h	Declares the new add_qos_skb_mark API behind FEATURE_WANMGR_L2_MARKING.
source/firewall/firewall.c	Implements WanManager-based DSCP/CLASSIFY rule generation and hooks it into IPv4 mangle table preparation.
source/firewall/firewall_ipv6.c	Hooks the new rule generation into the IPv6 rule generation path.

Suppressed comments (4)

source/firewall/firewall.c:5504

• strncpy(..., sizeof(dest)) may leave SKBMark without a null terminator when the source is long. Prefer a guaranteed-terminated copy.

                strncpy(SKBMark, acTmpReturnValue, sizeof(SKBMark));

source/firewall/firewall.c:5517

• strncpy(..., sizeof(dest)) may leave DSCPMark without a null terminator when the source is long. Prefer a guaranteed-terminated copy.

                strncpy(DSCPMark, acTmpReturnValue, sizeof(DSCPMark));

source/firewall/firewall.c:5530

• strncpy(..., sizeof(dest)) may leave MarkingName without a null terminator when the source is long, which can break the subsequent strcmp chain. Prefer a guaranteed-terminated copy.

                strncpy(MarkingName, acTmpReturnValue, sizeof(MarkingName));

source/firewall/firewall.c:5545

• strncpy(..., sizeof(dest)) may leave WanVIfName without a null terminator when the source is long, which can result in malformed iptables rules. Prefer a guaranteed-terminated copy.

                    strncpy(WanVIfName, acTmpReturnValue, sizeof(WanVIfName));

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

Suppressed comments (16)

source/firewall/firewall.c:5515

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

                snprintf(acGetParamName, sizeof(acGetParamName), "%s.DSCPMark", WanVIfMarkingEntry);
                if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
                {
                    printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                    return -1;

source/firewall/firewall.c:5528

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

                snprintf(acGetParamName, sizeof(acGetParamName), "%s.Name", WanVIfMarkingEntry);
                if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
                {
                    printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                    return -1;

source/firewall/firewall.c:5407

• RdkBus_GetParamValues() is declared with a 5th returnValSize parameter (see firewall.h:845). This call passes only 4 arguments, which will not compile and also prevents safe truncation handling.

        memset(acTmpReturnValue, 0, sizeof(acTmpReturnValue));
        if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, WAN_IF_COUNT, acTmpReturnValue))
        {
            printf("[%s][%d]Failed to get WAN interface count\n", __FUNCTION__, __LINE__);
            return -1;

source/firewall/firewall.c:5422

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

            snprintf(acGetParamName, sizeof(acGetParamName), WAN_IF_ACTIVE_LINK, wanif_idx);
            if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
            {
                printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                return -1;

source/firewall/firewall.c:5452

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

            snprintf(acGetParamName, sizeof(acGetParamName), WAN_VIF_COUNT, activeLinkIdx);
            if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
            {
                printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                return -1;
            }

source/firewall/firewall.c:5470

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

            snprintf(acGetParamName, sizeof(acGetParamName), WAN_VIF_NOF_MARKINGS, activeLinkIdx, vlan_idx);
            if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
            {
                printf("[%s][%d]Failed to get [%s], skipping\n", __FUNCTION__, __LINE__, acGetParamName);
                continue;

source/firewall/firewall.c:5489

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

                snprintf(acGetParamName, sizeof(acGetParamName), WAN_VIF_MARKINGS_ENTRY, activeLinkIdx, vlan_idx, vIfMarkingIdx);
                if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
                {
                    printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                    return -1;

source/firewall/firewall.c:5502

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

                snprintf(acGetParamName, sizeof(acGetParamName), "%s.SKBMark", WanVIfMarkingEntry);
                if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
                {
                    printf("[%s][%d]Failed to get [%s]\n", __FUNCTION__, __LINE__, acGetParamName);
                    return -1;

source/firewall/firewall.c:5543

• RdkBus_GetParamValues() requires a returnValSize argument; this call currently passes only 4 parameters and will not compile.

                    snprintf(acGetParamName, sizeof(acGetParamName), WAN_VIF_NAME, activeLinkIdx, vlan_idx);
                    if (ANSC_STATUS_FAILURE == RdkBus_GetParamValues(WAN_COMPONENT_NAME, WAN_DBUS_PATH, acGetParamName, acTmpReturnValue))
                    {
                        printf("[%s][%d]Failed to get [%s], skipping\n", __FUNCTION__, __LINE__, acGetParamName);
                        continue;

source/firewall/firewall.c:5492

• strncpy(..., sizeof(buf)) does not guarantee NUL-termination when the source is the same size or longer. This value is later used as a C string (e.g., in snprintf with "%s"), which can cause out-of-bounds reads and potentially leak stack data into the generated iptables rules.

                strncpy(WanVIfMarkingEntry, acTmpReturnValue, sizeof(WanVIfMarkingEntry));

source/firewall/firewall.c:5505

• strncpy(..., sizeof(buf)) does not guarantee NUL-termination. Since SKBMark is later printed with %s, ensure it is always terminated to avoid out-of-bounds reads.

                strncpy(SKBMark, acTmpReturnValue, sizeof(SKBMark));

source/firewall/firewall.c:5518

• strncpy(..., sizeof(buf)) does not guarantee NUL-termination. Since DSCPMark is later printed with %s, ensure it is always terminated to avoid out-of-bounds reads.

                strncpy(DSCPMark, acTmpReturnValue, sizeof(DSCPMark));

source/firewall/firewall.c:5531

• strncpy(..., sizeof(buf)) does not guarantee NUL-termination. Since MarkingName is later used in strcmp(), ensure it is always terminated to avoid out-of-bounds reads.

                strncpy(MarkingName, acTmpReturnValue, sizeof(MarkingName));

source/firewall/firewall.c:5546

• strncpy(..., sizeof(buf)) does not guarantee NUL-termination. Since WanVIfName is used with %s in generated rules, ensure it is always terminated to avoid out-of-bounds reads.

                    strncpy(WanVIfName, acTmpReturnValue, sizeof(WanVIfName));

source/firewall/firewall.c:5562

• In iptables, match options like -p must appear before the jump target (-j). With -j DSCP placed before -p, -p udp/tcp becomes an invalid target option and the rule will fail to restore.

                    dest_port = 123; // for NTP
                    fprintf(mangle_fp, "-A POSTROUTING -j DSCP -p udp --dport %d -o %s --set-dscp-class %s\n", dest_port, current_wan_ifname, DSCPMark);
                    fprintf(mangle_fp, "-A POSTROUTING -j DSCP -p tcp --dport %d -o %s --set-dscp-class %s\n", dest_port, current_wan_ifname, DSCPMark);
                    fprintf(mangle_fp, "-A POSTROUTING -j CLASSIFY -p udp --dport %d -o %s --set-class 0:%s\n", dest_port, current_wan_ifname, SKBMark);
                    fprintf(mangle_fp, "-A POSTROUTING -j CLASSIFY -p tcp --dport %d -o %s --set-class 0:%s\n", dest_port, current_wan_ifname, SKBMark);

source/firewall/firewall.c:5575

• This HTTP CLASSIFY rule is missing -o %s (egress interface match). As written it can match traffic on any outgoing interface, not just the WAN interface, which is inconsistent with the adjacent DSCP rule.

                    fprintf(mangle_fp, "-A POSTROUTING -p tcp --dport %d -m dscp --dscp-class cs3 -o %s -j DSCP --set-dscp-class %s\n", HTTP_PORT, current_wan_ifname, DSCPMark);
                    fprintf(mangle_fp, "-A POSTROUTING -p tcp --dport %d -m dscp --dscp-class %s -j CLASSIFY --set-class 0:%s\n", HTTP_PORT, DSCPMark, SKBMark);