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

Add support for voq counters in portsorch. #1913

Closed
wants to merge 16 commits into from
Closed

Add support for voq counters in portsorch. #1913

wants to merge 16 commits into from

Conversation

skbarista
Copy link
Contributor

@skbarista skbarista commented Sep 17, 2021

What I did

  1. Add m_voq_ids to SystemPortInfo to maintain the list of queue ids.
  2. Add two new tables COUNTERS_SYSTEM_PORT_NAME_MAP and
    COUNTERS_VOQ_NAME_MAP to enable queuestat to differentiate between
    Port Tx queues and Voqs.
  3. Add a new function initializeVoqs that retrieves the number of voqs
    for a system port and stores the voq object ids in m_voq_ids
  4. Add code to handle queue type SAI_QUEUE_TYPE_UNICAST_VOQ.
  5. Initialize voqs and populate COUNTERS_SYSTEM_PORT_NAME_MAP in
    addSystemPorts function.
  6. Update generateQueueMap to generate queue maps for both Txqs and Voq.
    For PHY ports in a voq system both Txqs and Voqs are instantiated. For
    Voqs of remote system port, only Voq counters are initialized.

Why I did it

Add support for voq counters.

How I verified it

Modified queuestat locally and confirming the counters are populated for Voqs.

Details if related

1) Add m_voq_ids to SystemPortInfo to maintain the list of queue ids.
2) Add two new tables COUNTERS_SYSTEM_PORT_NAME_MAP and
   COUNTERS_VOQ_NAME_MAP to enable queuestat to differentiate between
   Port Tx queues and Voqs.
3) Add a new function initializeVoqs that retrieves the number of voqs
   for a system port and stores the voq object ids in m_voq_ids
4) Add code to handle queue type SAI_QUEUE_TYPE_UNICAST_VOQ.
5) Initialize voqs and populate COUNTERS_SYSTEM_PORT_NAME_MAP in
   addSystemPorts function.
6) Update generateQueueMap to generate queue maps for both Txqs and Voq.
For PHY ports in a voq system both Txqs and Voqs are instantiated. For
Voqs of remote system port, only Voq counters are initialized.
@skbarista skbarista requested a review from prsunny as a code owner September 17, 2021 00:42
@ghost
Copy link

ghost commented Sep 17, 2021

CLA assistant check
All CLA requirements met.

@skbarista
Copy link
Contributor Author

Build dependency on sonic-swss-common/pull/530

@abdosi abdosi self-requested a review September 22, 2021 16:02
abdosi
abdosi previously approved these changes Sep 22, 2021
}

m_isQueueMapGenerated = true;
}

void PortsOrch::generateQueueMapPerPort(const Port& port)
void PortsOrch::generateQueueMapPerPort(const Port& port, bool voq)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of passing argument, can we generalize this function generateQueueMapPerPort to call initializeVoqs if global switch type is voq

@rlhui rlhui added the chassis label Nov 1, 2021
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
* Added the Route Module to the Debug Dump Utility
Signed-off-by: Vivek Reddy Karri <[email protected]>
@arlakshm
Copy link
Contributor

arlakshm commented May 4, 2022

@skbarista, can you fix the conflicts ?

1 similar comment
@arlakshm
Copy link
Contributor

@skbarista, can you fix the conflicts ?

@skbarista
Copy link
Contributor Author

@arlakshm sorry missed this message. Will do it next week and refresh the pr. Thanks, Sambath

@skbarista
Copy link
Contributor Author

@rlhui @abdosi sorry was not able to get to resolve the conflict. Let me do this tomorrow.

@arlakshm
Copy link
Contributor

Hi @skbarista, are conflicts resolved

@skbarista
Copy link
Contributor Author

@arlakshm I resolved the conflict and I am looking at some test failures from the build. I see there are some new conflicts. Will resolve them once I fix the test failures.

@skbarista
Copy link
Contributor Author

@arlakshm I resolved the conflict and I am looking at some test failures from the build. I see there are some new conflicts. Will resolve them once I fix the test failures.

@arlakshm sonic-net/sonic-sairedis#1061 fixes the build failure for this pull request.

@vmittal-msft
Copy link
Contributor

@skbarista Can you please resolve conflicts and take care of build errors ?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

arlakshm
arlakshm previously approved these changes Sep 23, 2022
@skbarista
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1913 in repo sonic-net/sonic-swss

@skbarista
Copy link
Contributor Author

/azpw run sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run sonic-buildimage

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@vmittal-msft
Copy link
Contributor

/azp run sonic-net/sonic-swss

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@vmittal-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmittal-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmittal-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmittal-msft
Copy link
Contributor

@skbarista Can you please update your branch to latest and retry ?

@vmittal-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rlhui
Copy link
Contributor

rlhui commented Dec 22, 2022

@skbarista is there another PR to replace this one ? Please add comment why it's closed and also link to the other PR? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants