Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[202111][counterpoll | wartmark] | The tables related to QUEUE_WATERMARK counter are not generated on WATERMARK enable #11219

Open
nazariig opened this issue Jun 22, 2022 · 3 comments

Comments

@nazariig
Copy link
Collaborator

Description

The issue appears to be in current approach: the queue watermark counters are only generated when a specific key is passed to FlexCounter orch.
This means that enabling watermark group won't actually enable the relevant counters but only when a stats for queue is requested.

root@sonic:/home/admin# counterpoll watermark enable

https://github.com/Azure/sonic-utilities/blob/202111/counterpoll/main.py#L223

@watermark.command()
def enable():
    """ Enable watermark counter query """
    configdb = ConfigDBConnector()
    configdb.connect()
    fc_info = {}
    fc_info['FLEX_COUNTER_STATUS'] = 'enable'
    configdb.mod_entry("FLEX_COUNTER_TABLE", "QUEUE_WATERMARK", fc_info)
    configdb.mod_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK", fc_info)
    configdb.mod_entry("FLEX_COUNTER_TABLE", BUFFER_POOL_WATERMARK, fc_info)

https://github.com/Azure/sonic-swss/blob/202111/orchagent/flexcounterorch.cpp#L118

                else if(field == FLEX_COUNTER_STATUS_FIELD)
                {
                    // Currently, the counters are disabled for polling by default
                    // The queue maps will be generated as soon as counters are enabled for polling
                    // Counter polling is enabled by pushing the COUNTER_ID_LIST/ATTR_ID_LIST, which contains
                    // the list of SAI stats/attributes of polling interest, to the FLEX_COUNTER_DB under the
                    // additional condition that the polling interval at that time is set nonzero positive,
                    // which is automatically satisfied upon the creation of the orch object that requires
                    // the syncd flex counter polling service
                    // This postponement is introduced by design to accelerate the initialization process
                    if(gPortsOrch && (value == "enable"))
                    {
                        if(key == PORT_KEY)
                        {
                            gPortsOrch->generatePortCounterMap();
                            m_port_counter_enabled = true;
                        }
                        else if(key == PORT_BUFFER_DROP_KEY)
                        {
                            gPortsOrch->generatePortBufferDropCounterMap();
                            m_port_buffer_drop_counter_enabled = true;
                        }
                        else if(key == QUEUE_KEY)
                        {
                            gPortsOrch->generateQueueMap();
                        }
                        else if(key == PG_WATERMARK_KEY)
                        {
                            gPortsOrch->generatePriorityGroupMap();
                        }
                    }
                    if(gIntfsOrch && (key == RIF_KEY) && (value == "enable"))
                    {
                        gIntfsOrch->generateInterfaceMap();
                    }
                    if (gBufferOrch && (key == BUFFER_POOL_WATERMARK_KEY) && (value == "enable"))
                    {
                        gBufferOrch->generateBufferPoolWatermarkCounterIdList();
                    }
                    if (gFabricPortsOrch)
                    {
                        gFabricPortsOrch->generateQueueStats();
                    }
                    if (vxlan_tunnel_orch && (key== TUNNEL_KEY) && (value == "enable"))
                    {
                        vxlan_tunnel_orch->generateTunnelCounterMap();
                    }
                    if (gCoppOrch && (key == FLOW_CNT_TRAP_KEY))
                    {
                        if (value == "enable")
                        {
                            m_hostif_trap_counter_enabled = true;
                            gCoppOrch->generateHostIfTrapCounterIdList();
                        }
                        else if (value == "disable")
                        {
                            gCoppOrch->clearHostIfTrapCounterIdList();
                            m_hostif_trap_counter_enabled = false;
                        }
                    }
                    vector<FieldValueTuple> fieldValues;
                    fieldValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, value);
                    m_flexCounterGroupTable->set(flexCounterGroupMap[key], fieldValues);
                }

Queue Watermark Counter map is generated only when user requests a stats for all queues:

root@sonic:/home/admin# counterpoll queue enable

https://github.com/Azure/sonic-swss/blob/202111/orchagent/portsorch.cpp#L5394

void PortsOrch::generateQueueMap()
{
    if (m_isQueueMapGenerated)
    {
        return;
    }

    for (const auto& it: m_portList)
    {
        if (it.second.m_type == Port::PHY)
        {
            generateQueueMapPerPort(it.second);
        }
    }

    m_isQueueMapGenerated = true;
}

void PortsOrch::generateQueueMapPerPort(const Port& port)
{
    /* Create the Queue map in the Counter DB */
    /* Add stat counters to flex_counter */
    vector<FieldValueTuple> queueVector;
    vector<FieldValueTuple> queuePortVector;
    vector<FieldValueTuple> queueIndexVector;
    vector<FieldValueTuple> queueTypeVector;

    for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex)
    {
        std::ostringstream name;
        name << port.m_alias << ":" << queueIndex;

        const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]);

        queueVector.emplace_back(name.str(), id);
        queuePortVector.emplace_back(id, sai_serialize_object_id(port.m_port_id));

        string queueType;
        uint8_t queueRealIndex = 0;
        if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex))
        {
            queueTypeVector.emplace_back(id, queueType);
            queueIndexVector.emplace_back(id, to_string(queueRealIndex));
        }

        // Install a flex counter for this queue to track stats
        std::unordered_set<string> counter_stats;
        for (const auto& it: queue_stat_ids)
        {
            counter_stats.emplace(sai_serialize_queue_stat(it));
        }
        queue_stat_manager.setCounterIdList(port.m_queue_ids[queueIndex], CounterType::QUEUE, counter_stats);

        /* add watermark queue counters */
        string key = getQueueWatermarkFlexCounterTableKey(id);

        string delimiter("");
        std::ostringstream counters_stream;
        for (const auto& it: queueWatermarkStatIds)
        {
            counters_stream << delimiter << sai_serialize_queue_stat(it);
            delimiter = comma;
        }

        vector<FieldValueTuple> fieldValues;
        fieldValues.emplace_back(QUEUE_COUNTER_ID_LIST, counters_stream.str());

        m_flexCounterTable->set(key, fieldValues);
    }

    m_queueTable->set("", queueVector);
    m_queuePortTable->set("", queuePortVector);
    m_queueIndexTable->set("", queueIndexVector);
    m_queueTypeTable->set("", queueTypeVector);

    CounterCheckOrch::getInstance().addPort(port);
}

Steps to reproduce the issue:

  1. Disable all counterpoll
admin@sonic:~$ sudo counterpoll config-db disable
  1. Reboot DUT
admin@sonic:~$ sudo reboot
  1. Enable watermark
admin@sonic:~$ sudo counterpoll watermark enable
  1. Check tables
admin@sonic:~$ redis-cli -n 5
127.0.0.1:6379[5]> keys *QUEUE_WATERMARK*

The table related to QUEUE_WATERMARK is not generated. However, once we enable queue (sudo counterpoll queue enable), the tables related to QUEUE_WATERMARK can be created.

Describe the results you received:

The tables related to QUEUE_WATERMARK counter are not generated on WATERMARK enable

Describe the results you expected:

The tables related to QUEUE_WATERMARK counter should be generated on WATERMARK enable

Output of show version:

(paste your output here)

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):

@yxieca yxieca added Triaged this issue has been triaged Help Wanted 🆘 and removed Triaged this issue has been triaged labels Jun 22, 2022
@yxieca
Copy link
Contributor

yxieca commented Jun 22, 2022

Help from community is wanted.

@stephenxs
Copy link
Collaborator

Also observed in 202205.

@stephenxs
Copy link
Collaborator

IMO the correct logic should be:

  1. Flexcounter to generate the queue maps when queue or watermark counter is enabled for the 1st time
  2. Flexcounter to generate PG maps when pg-drop or watermark counter is enabled for the 1st time.

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Sep 14, 2022
liat-grozovik pushed a commit to sonic-net/sonic-swss that referenced this issue Feb 23, 2023
This PR fixes the following issue:
sonic-net/sonic-buildimage#11219
It's a cherry-pick (with conflicts resolution) for 202205 of the following PR: #2525

- What I did
Align watermark flow with port configuration
correct the queue, watermark and pg-drop counterpoll functionality according to cli commands

- Why I did it
the flow before this commit was wrong, when watermark counterpoll was enabled, stats wasn't created in FLEX_COUNTERS_DB
and COUNTERS_DB unless enabling queue counterpoll as well.
in the same way when pg-drop counterpoll was enabled stats weren't added until watermark was enabled.
This is due to the wrong flow where queue and pg maps were generated only when queue or watermark (pg-watermark)
key was detected in flexcountersorch.cpp

- How I verified it
manual testing
VS test (updated to reflect current flow) - before this commit only queue was tested
regression test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants